| Message ID | 20220710212423.681301-26-daniel.lezcano@linexp.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | New thermal OF code | expand |
On 7/11/22 06:24, Daniel Lezcano wrote: > The thermal OF code has a new API allowing to migrate the OF > initialization to a simpler approach. > > Use this new API. > > Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org> > --- > drivers/ata/ahci_imx.c | 15 ++++++++++----- > 1 file changed, 10 insertions(+), 5 deletions(-) > > diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c > index 79aa9f285312..5ad9a890e71a 100644 > --- a/drivers/ata/ahci_imx.c > +++ b/drivers/ata/ahci_imx.c > @@ -327,7 +327,7 @@ static int read_adc_sum(void *dev, u16 rtune_ctl_reg, void __iomem * mmio) > } > > /* SATA AHCI temperature monitor */ > -static int sata_ahci_read_temperature(void *dev, int *temp) > +static int __sata_ahci_read_temperature(void *dev, int *temp) > { > u16 mpll_test_reg, rtune_ctl_reg, dac_ctl_reg, read_sum; > u32 str1, str2, str3, str4; > @@ -416,6 +416,11 @@ static int sata_ahci_read_temperature(void *dev, int *temp) > return 0; > } > > +static int sata_ahci_read_temperature(struct thermal_zone_device *tz, int *temp) > +{ > + return __sata_ahci_read_temperature(tz->devdata, temp); > +} > + > static ssize_t sata_ahci_show_temp(struct device *dev, > struct device_attribute *da, > char *buf) > @@ -423,14 +428,14 @@ static ssize_t sata_ahci_show_temp(struct device *dev, > unsigned int temp = 0; > int err; > > - err = sata_ahci_read_temperature(dev, &temp); > + err = __sata_ahci_read_temperature(dev, &temp); > if (err < 0) > return err; > > return sprintf(buf, "%u\n", temp); > } I do not see why the above changes are necessary. Please explain. > > -static const struct thermal_zone_of_device_ops fsl_sata_ahci_of_thermal_ops = { > +static struct thermal_zone_device_ops fsl_sata_ahci_of_thermal_ops = { Why remove the const ? > .get_temp = sata_ahci_read_temperature, > }; > > @@ -1131,8 +1136,8 @@ static int imx_ahci_probe(struct platform_device *pdev) > ret = PTR_ERR(hwmon_dev); > goto disable_clk; > } > - devm_thermal_zone_of_sensor_register(hwmon_dev, 0, hwmon_dev, > - &fsl_sata_ahci_of_thermal_ops); > + devm_thermal_of_zone_register(hwmon_dev, 0, hwmon_dev, > + &fsl_sata_ahci_of_thermal_ops); This is the only change that seems necessary. > dev_info(dev, "%s: sensor 'sata_ahci'\n", dev_name(hwmon_dev)); > } > And it is hard to review a patch without the full series for context. Please send all patches next time.
On 11/07/2022 01:18, Damien Le Moal wrote: > On 7/11/22 06:24, Daniel Lezcano wrote: >> The thermal OF code has a new API allowing to migrate the OF >> initialization to a simpler approach. >> >> Use this new API. >> >> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org> >> --- >> drivers/ata/ahci_imx.c | 15 ++++++++++----- >> 1 file changed, 10 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c >> index 79aa9f285312..5ad9a890e71a 100644 >> --- a/drivers/ata/ahci_imx.c >> +++ b/drivers/ata/ahci_imx.c >> @@ -327,7 +327,7 @@ static int read_adc_sum(void *dev, u16 rtune_ctl_reg, void __iomem * mmio) >> } >> >> /* SATA AHCI temperature monitor */ >> -static int sata_ahci_read_temperature(void *dev, int *temp) >> +static int __sata_ahci_read_temperature(void *dev, int *temp) >> { >> u16 mpll_test_reg, rtune_ctl_reg, dac_ctl_reg, read_sum; >> u32 str1, str2, str3, str4; >> @@ -416,6 +416,11 @@ static int sata_ahci_read_temperature(void *dev, int *temp) >> return 0; >> } >> >> +static int sata_ahci_read_temperature(struct thermal_zone_device *tz, int *temp) >> +{ >> + return __sata_ahci_read_temperature(tz->devdata, temp); >> +} >> + >> static ssize_t sata_ahci_show_temp(struct device *dev, >> struct device_attribute *da, >> char *buf) >> @@ -423,14 +428,14 @@ static ssize_t sata_ahci_show_temp(struct device *dev, >> unsigned int temp = 0; >> int err; >> >> - err = sata_ahci_read_temperature(dev, &temp); >> + err = __sata_ahci_read_temperature(dev, &temp); >> if (err < 0) >> return err; >> >> return sprintf(buf, "%u\n", temp); >> } > > I do not see why the above changes are necessary. Please explain. sata_ahci_read_temperature() is used by sata_ahci_show_temp() also. So in order to change the function prototype for the get_temp ops which does not take a void* but a thermal_zone_device* structure, this function wraps the call. Do you prefer sata_ahci_read_temperature() becomes sata_ahci_get_temp() and keep __sata_ahci_read_temperature() name untouched ? >> >> -static const struct thermal_zone_of_device_ops fsl_sata_ahci_of_thermal_ops = { >> +static struct thermal_zone_device_ops fsl_sata_ahci_of_thermal_ops = { > > Why remove the const ? > >> .get_temp = sata_ahci_read_temperature, >> }; >> >> @@ -1131,8 +1136,8 @@ static int imx_ahci_probe(struct platform_device *pdev) >> ret = PTR_ERR(hwmon_dev); >> goto disable_clk; >> } >> - devm_thermal_zone_of_sensor_register(hwmon_dev, 0, hwmon_dev, >> - &fsl_sata_ahci_of_thermal_ops); >> + devm_thermal_of_zone_register(hwmon_dev, 0, hwmon_dev, >> + &fsl_sata_ahci_of_thermal_ops); > > This is the only change that seems necessary. > >> dev_info(dev, "%s: sensor 'sata_ahci'\n", dev_name(hwmon_dev)); >> } >> > > And it is hard to review a patch without the full series for context. > Please send all patches next time. > >
On 7/11/22 18:14, Daniel Lezcano wrote: > On 11/07/2022 01:18, Damien Le Moal wrote: >> On 7/11/22 06:24, Daniel Lezcano wrote: >>> The thermal OF code has a new API allowing to migrate the OF >>> initialization to a simpler approach. >>> >>> Use this new API. >>> >>> Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org> >>> --- >>> drivers/ata/ahci_imx.c | 15 ++++++++++----- >>> 1 file changed, 10 insertions(+), 5 deletions(-) >>> >>> diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c >>> index 79aa9f285312..5ad9a890e71a 100644 >>> --- a/drivers/ata/ahci_imx.c >>> +++ b/drivers/ata/ahci_imx.c >>> @@ -327,7 +327,7 @@ static int read_adc_sum(void *dev, u16 rtune_ctl_reg, void __iomem * mmio) >>> } >>> >>> /* SATA AHCI temperature monitor */ >>> -static int sata_ahci_read_temperature(void *dev, int *temp) >>> +static int __sata_ahci_read_temperature(void *dev, int *temp) >>> { >>> u16 mpll_test_reg, rtune_ctl_reg, dac_ctl_reg, read_sum; >>> u32 str1, str2, str3, str4; >>> @@ -416,6 +416,11 @@ static int sata_ahci_read_temperature(void *dev, int *temp) >>> return 0; >>> } >>> >>> +static int sata_ahci_read_temperature(struct thermal_zone_device *tz, int *temp) >>> +{ >>> + return __sata_ahci_read_temperature(tz->devdata, temp); >>> +} >>> + >>> static ssize_t sata_ahci_show_temp(struct device *dev, >>> struct device_attribute *da, >>> char *buf) >>> @@ -423,14 +428,14 @@ static ssize_t sata_ahci_show_temp(struct device *dev, >>> unsigned int temp = 0; >>> int err; >>> >>> - err = sata_ahci_read_temperature(dev, &temp); >>> + err = __sata_ahci_read_temperature(dev, &temp); >>> if (err < 0) >>> return err; >>> >>> return sprintf(buf, "%u\n", temp); >>> } >> >> I do not see why the above changes are necessary. Please explain. > > sata_ahci_read_temperature() is used by sata_ahci_show_temp() also. > > So in order to change the function prototype for the get_temp ops which > does not take a void* but a thermal_zone_device* structure, this > function wraps the call. > > Do you prefer sata_ahci_read_temperature() becomes sata_ahci_get_temp() > and keep __sata_ahci_read_temperature() name untouched ? Understood. You can keep the name changes you have, but please add this explanation to the commit message. > >>> >>> -static const struct thermal_zone_of_device_ops fsl_sata_ahci_of_thermal_ops = { >>> +static struct thermal_zone_device_ops fsl_sata_ahci_of_thermal_ops = { >> >> Why remove the const ? >> >>> .get_temp = sata_ahci_read_temperature, >>> }; >>> >>> @@ -1131,8 +1136,8 @@ static int imx_ahci_probe(struct platform_device *pdev) >>> ret = PTR_ERR(hwmon_dev); >>> goto disable_clk; >>> } >>> - devm_thermal_zone_of_sensor_register(hwmon_dev, 0, hwmon_dev, >>> - &fsl_sata_ahci_of_thermal_ops); >>> + devm_thermal_of_zone_register(hwmon_dev, 0, hwmon_dev, >>> + &fsl_sata_ahci_of_thermal_ops); >> >> This is the only change that seems necessary. >> >>> dev_info(dev, "%s: sensor 'sata_ahci'\n", dev_name(hwmon_dev)); >>> } >>> >> >> And it is hard to review a patch without the full series for context. >> Please send all patches next time. >> >> > >
diff --git a/drivers/ata/ahci_imx.c b/drivers/ata/ahci_imx.c index 79aa9f285312..5ad9a890e71a 100644 --- a/drivers/ata/ahci_imx.c +++ b/drivers/ata/ahci_imx.c @@ -327,7 +327,7 @@ static int read_adc_sum(void *dev, u16 rtune_ctl_reg, void __iomem * mmio) } /* SATA AHCI temperature monitor */ -static int sata_ahci_read_temperature(void *dev, int *temp) +static int __sata_ahci_read_temperature(void *dev, int *temp) { u16 mpll_test_reg, rtune_ctl_reg, dac_ctl_reg, read_sum; u32 str1, str2, str3, str4; @@ -416,6 +416,11 @@ static int sata_ahci_read_temperature(void *dev, int *temp) return 0; } +static int sata_ahci_read_temperature(struct thermal_zone_device *tz, int *temp) +{ + return __sata_ahci_read_temperature(tz->devdata, temp); +} + static ssize_t sata_ahci_show_temp(struct device *dev, struct device_attribute *da, char *buf) @@ -423,14 +428,14 @@ static ssize_t sata_ahci_show_temp(struct device *dev, unsigned int temp = 0; int err; - err = sata_ahci_read_temperature(dev, &temp); + err = __sata_ahci_read_temperature(dev, &temp); if (err < 0) return err; return sprintf(buf, "%u\n", temp); } -static const struct thermal_zone_of_device_ops fsl_sata_ahci_of_thermal_ops = { +static struct thermal_zone_device_ops fsl_sata_ahci_of_thermal_ops = { .get_temp = sata_ahci_read_temperature, }; @@ -1131,8 +1136,8 @@ static int imx_ahci_probe(struct platform_device *pdev) ret = PTR_ERR(hwmon_dev); goto disable_clk; } - devm_thermal_zone_of_sensor_register(hwmon_dev, 0, hwmon_dev, - &fsl_sata_ahci_of_thermal_ops); + devm_thermal_of_zone_register(hwmon_dev, 0, hwmon_dev, + &fsl_sata_ahci_of_thermal_ops); dev_info(dev, "%s: sensor 'sata_ahci'\n", dev_name(hwmon_dev)); }
The thermal OF code has a new API allowing to migrate the OF initialization to a simpler approach. Use this new API. Signed-off-by: Daniel Lezcano <daniel.lezcano@linexp.org> --- drivers/ata/ahci_imx.c | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-)