[PATCH linux dev-4.10 v3 1/5] drivers/hwmon/pmbus: Add IBM power supply hwmon driver

[Andrew Jeffery]

On Wed, 2017-08-09 at 17:43 -0500, Brandon Wyman wrote:
> > On Mon, Aug 7, 2017 at 9:34 PM, Andrew Jeffery <andrew at aj.id.au> wrote:
> > On Mon, 2017-08-07 at 15:11 +0930, Andrew Jeffery wrote:
> > > > 
> > > > > 
> > > > > > +
> > > > > > +static const struct i2c_device_id ibmps_id[] = {
> > > > > > > > > > > +       { "witherspoon", witherspoon },
> > > > > > > 
> > > > > > > +       { }
> > > > > > 
> > > > > > +};
> > > > > > +MODULE_DEVICE_TABLE(i2c, ibmps_id);
> > > > > > +
> > > > > > +static const struct of_device_id ibmps_of_match[] = {
> > > > > > > > > > > +       { .compatible = "ibm,ibmps" },
> > > > > > > 
> > > > > > > +       {}
> > > > > > 
> > > > > > +};
> > > > > > +MODULE_DEVICE_TABLE(of, p8_i2c_occ_of_match);
> > > > > 
> > > > > p8_i2c_occ_of_match isn't right. Do we need both device tables?
> > > > > "witherspoon" as a device ID also seems unusual. What's going on there?
> > > > 
> > > > Uhh, not sure how that got in there... How does this compile?? I'll fix
> > > > it. The power supply doesn't have any device id or anything, so I just
> > > > went with witherspoon...
> > > 
> > > My issue is "withersoon" identifies a class of machine, not a power
> > > supply, even if the powersupply is specific to Witherspoon.
> > 
> > Further to this, I've just got hold of the datasheet. My reading of it
> > says the part number is "01KL054". Maybe the driver should include that
> > in its name and compatible.
> 
> I don't know that this part number value always remains the same, for
> the part used in a particular system. There was a similar 2000W PMBus
> power supply that had one part number, and it stuck. However, a
> previous 1720W power supply had at least four different part numbers
> before it finally stuck on the final one.
> 
> That being said, there quite probably are some differences between
> various part numbers, but most of what this device driver is doing
> would still be compatible. I can see have 01KL054 in the compatible
> list, but trying to shoe-horn it into the device driver name doesn't
> feel like it makes sense to me.

Right; without the context you've provided above I wouldn't consider it
shoe-horning. However, since having a diversity of part numbers is a
thing, I agree with your assessment. It should be part of a compatible,
but maybe we do need something generic for the driver name.

Andrew

> 
> > 
> > Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20170810/5ea6778f/attachment.sig>