Re: [PATCH linux dev-4.10 6/6] drivers/hwmon: Add a driver for a generic PECI hwmon

[Jae Hyun Yoo]

On 1/11/2018 5:22 AM, Arnd Bergmann wrote:
> On Thu, Jan 11, 2018 at 12:45 AM, Jae Hyun Yoo
> <jae.hyun.yoo@xxxxxxxxxxxxxxx> wrote:
> > On 1/10/2018 4:29 AM, Arnd Bergmann wrote:
> > > On Tue, Jan 9, 2018 at 11:31 PM, Jae Hyun Yoo
> > > <jae.hyun.yoo@xxxxxxxxxxxxxxx> wrote:
> > > > This commit adds driver implementation for a generic PECI hwmon.
> > > >
> > > > Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@xxxxxxxxxxxxxxx>
> > >
> > >
> > > > +static int xfer_peci_msg(int cmd, void *pmsg)
> > > > +{
> > > > +       int rc;
> > > > +
> > > > +       mutex_lock(&peci_hwmon_lock);
> > > > +       rc = peci_ioctl(NULL, cmd, (unsigned long)pmsg);
> > > > +       mutex_unlock(&peci_hwmon_lock);
> > > > +
> > > > +       return rc;
> > > > +}
> > >
> > >
> > > I said earlier that peci_ioctl() looked unused, that was obviously
> > > wrong, but what you have here
> > > is not a proper way to abstract a bus.
> > >
> > > Maybe this can be done more like an i2c bus: make the peci controller
> > > a bus device
> > > and register all known target/index pairs as devices with the peci bus
> > > type, and have
> > > them probed from DT. The driver can then bind to each of those
> > > individually.
> > > Not sure if that is getting to granular at that point, I'd have to
> > > understand better
> > > how it is expected to get used, and what the variances are between
> > > implementations.
> >
> > Thanks for sharing your opinion. In fact, this was also suggested by openbmc
> > community so I should consider of redesigning it. I'm currently thinking
> > about adding a new PECI device class as an abstract layer and any BMC
> > chipset specific driver could be attached to the PECI class driver. Then,
> > each CPU client could be registered as an individual device as you
> > suggested. Will consider your suggestion.
>
> Another idea might be to pretend that PECI was I2C. We already have a few
> drivers for hardware that is not I2C but whose software interface looks
> similar enough that it just works. No idea if that is the case for PECI, but
> xfer_peci_msg might be close enough to i2c_xfer to make it work. If you
> are able to do that, then the PECI controller would just register itself
> as an i2c controller and it can be accessed using /dev/i2c from user space
> or a high-level i2c_driver.
>
>        Arnd

Thanks for the good idea. It looks like one of possible options. I'll 
check this idea as well. :)

Thanks,
Jae