Re: [PATCH v1 6/9] hwmon: Add Nuvoton NCT6694 HWMON support

[Jonathan Cameron <jic23@kernel.org>]

On Mon, 28 Oct 2024 15:58:00 +0800
Ming Yu <a0282524688@gmail.com> wrote:

> Dear Guenter,
> 
> The original plan was to use the IIO driver to access the temperature
> and voltage sensors, and the HWMON driver to access the tachometers.
> However, since the device is a hot-plug USB device, as far as I know,
> IIO-HWMON is not applicable. I will merge the IIO driver part into the
> HWMON driver in the next patch.
> In  other words, the driver will be used to access TIN, VIN and FIN.
See drivers/mfd/sun4i-gpadc.c
for an example of an mfd using the iio-hwmon bridge.

Jonathan

> 
> Best regards
> Ming
> 
> Guenter Roeck <linux@roeck-us.net> 於 2024年10月26日 週六 下午10:50寫道：
> >
> > On 10/25/24 08:44, Guenter Roeck wrote:  
> > > On 10/25/24 08:22, Ming Yu wrote:
> > > [ ... ]
> > >  
> > >>>>> +static int nct6694_fan_write(struct device *dev, u32 attr, int channel,
> > >>>>> +                            long val)
> > >>>>> +{
> > >>>>> +       struct nct6694_hwmon_data *data = dev_get_drvdata(dev);
> > >>>>> +       unsigned char enable_buf[REQUEST_HWMON_CMD0_LEN] = {0};  
> > >>>> [Kalesh] Please try to maintain RCT order for variable declaration  
> > >>>
> > >>> Ok, but that is already the case here ?  
> > >>
> > >> [Ming] Is there anything that needs to be changed?
> > >>  
> > >
> > > I don't think so, If two lines have the same length, the order is up
> > > to the developer to decide.
> > >
> > > Question though is if the buffer needs to be initialized. You should drop
> > > the initialization if it is not necessary. In that case the second line
> > > would be shorter anyway, and the order question would not arise.
> > >  
> >
> > Actually, I just noticed that you also submitted an IIO driver which
> > reports the same data again. If a chip has an IIO driver, there should
> > be no HWMON driver since the IIO -> HWMON bridge can then be used if
> > necessary. So please drop this driver.
> >
> > Thanks,
> > Guenter
> >
> >  
>