Hi Guenter, Thanks for your comments On Mon, 30 Oct 2023 at 17:57, Guenter Roeck <linux@xxxxxxxxxxxx> wrote: > > On 10/30/23 08:01, Tomer Maimon wrote: > > Adding Pulse Width Modulation (PWM) and fan tacho NPCM8xx support to > > NPCM PWM and fan tacho driver. > > NPCM8xx uses a different number of PWM devices. > > > > As part of adding NPCM8XX support: > > - Add NPCM8xx specific compatible string. > > - Add data to handle architecture-specific PWM and fan tacho parameters. > > > > Signed-off-by: Tomer Maimon <tmaimon77@xxxxxxxxx> > > --- > > drivers/hwmon/npcm750-pwm-fan.c | 34 +++++++++++++++++++++++++++++---- > > 1 file changed, 30 insertions(+), 4 deletions(-) > > > > diff --git a/drivers/hwmon/npcm750-pwm-fan.c b/drivers/hwmon/npcm750-pwm-fan.c > > index 10ed3f4335d4..765b08fa0396 100644 > > --- a/drivers/hwmon/npcm750-pwm-fan.c > > +++ b/drivers/hwmon/npcm750-pwm-fan.c > > @@ -46,9 +46,9 @@ > > #define NPCM7XX_PWM_CTRL_CH3_EN_BIT BIT(16) > > > > /* Define the maximum PWM channel number */ > > -#define NPCM7XX_PWM_MAX_CHN_NUM 8 > > +#define NPCM7XX_PWM_MAX_CHN_NUM 12 > > #define NPCM7XX_PWM_MAX_CHN_NUM_IN_A_MODULE 4 > > -#define NPCM7XX_PWM_MAX_MODULES 2 > > +#define NPCM7XX_PWM_MAX_MODULES 3 > > > > /* Define the Counter Register, value = 100 for match 100% */ > > #define NPCM7XX_PWM_COUNTER_DEFAULT_NUM 255 > > @@ -171,6 +171,10 @@ > > #define FAN_PREPARE_TO_GET_FIRST_CAPTURE 0x01 > > #define FAN_ENOUGH_SAMPLE 0x02 > > > > +struct npcm_hwmon_info { > > + u32 pwm_max_channel; > > +}; > > + > > struct npcm7xx_fan_dev { > > u8 fan_st_flg; > > u8 fan_pls_per_rev; > > @@ -204,6 +208,7 @@ struct npcm7xx_pwm_fan_data { > > struct timer_list fan_timer; > > struct npcm7xx_fan_dev fan_dev[NPCM7XX_FAN_MAX_CHN_NUM]; > > struct npcm7xx_cooling_device *cdev[NPCM7XX_PWM_MAX_CHN_NUM]; > > + const struct npcm_hwmon_info *info; > > u8 fan_select; > > }; > > > > @@ -619,9 +624,13 @@ static umode_t npcm7xx_is_visible(const void *data, > > enum hwmon_sensor_types type, > > u32 attr, int channel) > > { > > + const struct npcm7xx_pwm_fan_data *hwmon_data = data; > > + > > switch (type) { > > case hwmon_pwm: > > - return npcm7xx_pwm_is_visible(data, attr, channel); > > + if (channel < hwmon_data->info->pwm_max_channel) > > + return npcm7xx_pwm_is_visible(data, attr, channel); > > I would have expected this check to be handled in npcm7xx_pwm_is_visible(). I will change the handle in npcm7xx_pwm_is_visible > > Guenter > Thanks, Tomer