Message ID | 20190606142255.29454-6-nsaenzjulienne@suse.de (mailing list archive) |
---|---|
State | Changes Requested, archived |
Headers | show |
Series | cpufreq support for Raspberry Pi | expand |
Quoting Nicolas Saenz Julienne (2019-06-06 07:22:58) > diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c > index b1365cf19f3a..052296b5fbe4 100644 > --- a/drivers/clk/bcm/clk-raspberrypi.c > +++ b/drivers/clk/bcm/clk-raspberrypi.c > @@ -63,6 +63,8 @@ struct raspberrypi_firmware_prop { > __le32 disable_turbo; > } __packed; > > +static struct platform_device *rpi_cpufreq; Why can't this be stored in platform driver data? > + > static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag, > u32 clk, u32 *val) > { > @@ -285,6 +287,17 @@ static int raspberrypi_clk_probe(struct platform_device *pdev) > return ret; > } > > + rpi_cpufreq = platform_device_register_data(dev, "raspberrypi-cpufreq", > + -1, NULL, 0); > + > + return 0; > +} > + > +static int raspberrypi_clk_remove(struct platform_device *pdev) > +{ > + platform_device_unregister(rpi_cpufreq); > + rpi_cpufreq = NULL; This assignment to NULL looks unnecessary. > + > return 0; > } >
On Thu, 2019-06-06 at 10:05 -0700, Stephen Boyd wrote: > Quoting Nicolas Saenz Julienne (2019-06-06 07:22:58) > > diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk- > > raspberrypi.c > > index b1365cf19f3a..052296b5fbe4 100644 > > --- a/drivers/clk/bcm/clk-raspberrypi.c > > +++ b/drivers/clk/bcm/clk-raspberrypi.c > > @@ -63,6 +63,8 @@ struct raspberrypi_firmware_prop { > > __le32 disable_turbo; > > } __packed; > > > > +static struct platform_device *rpi_cpufreq; > > Why can't this be stored in platform driver data? It actually could, I just followed the same pattern as the code found in patch #3. I'll update it in the next version if you prefer it. > > > + > > static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 > > tag, > > u32 clk, u32 *val) > > { > > @@ -285,6 +287,17 @@ static int raspberrypi_clk_probe(struct platform_device > > *pdev) > > return ret; > > } > > > > + rpi_cpufreq = platform_device_register_data(dev, "raspberrypi- > > cpufreq", > > + -1, NULL, 0); > > + > > + return 0; > > +} > > + > > +static int raspberrypi_clk_remove(struct platform_device *pdev) > > +{ > > + platform_device_unregister(rpi_cpufreq); > > + rpi_cpufreq = NULL; > > This assignment to NULL looks unnecessary. > Same as above, but now that you pointed out, it's true it doesn't have any effect > > + > > return 0; > > } > > > > _______________________________________________ > linux-arm-kernel mailing list > linux-arm-kernel@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Quoting Nicolas Saenz Julienne (2019-06-06 10:16:56) > On Thu, 2019-06-06 at 10:05 -0700, Stephen Boyd wrote: > > Quoting Nicolas Saenz Julienne (2019-06-06 07:22:58) > > > diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk- > > > raspberrypi.c > > > index b1365cf19f3a..052296b5fbe4 100644 > > > --- a/drivers/clk/bcm/clk-raspberrypi.c > > > +++ b/drivers/clk/bcm/clk-raspberrypi.c > > > @@ -63,6 +63,8 @@ struct raspberrypi_firmware_prop { > > > __le32 disable_turbo; > > > } __packed; > > > > > > +static struct platform_device *rpi_cpufreq; > > > > Why can't this be stored in platform driver data? > > It actually could, I just followed the same pattern as the code found in patch > #3. I'll update it in the next version if you prefer it. > Yes please. It reduces global state.
diff --git a/drivers/clk/bcm/clk-raspberrypi.c b/drivers/clk/bcm/clk-raspberrypi.c index b1365cf19f3a..052296b5fbe4 100644 --- a/drivers/clk/bcm/clk-raspberrypi.c +++ b/drivers/clk/bcm/clk-raspberrypi.c @@ -63,6 +63,8 @@ struct raspberrypi_firmware_prop { __le32 disable_turbo; } __packed; +static struct platform_device *rpi_cpufreq; + static int raspberrypi_clock_property(struct rpi_firmware *firmware, u32 tag, u32 clk, u32 *val) { @@ -285,6 +287,17 @@ static int raspberrypi_clk_probe(struct platform_device *pdev) return ret; } + rpi_cpufreq = platform_device_register_data(dev, "raspberrypi-cpufreq", + -1, NULL, 0); + + return 0; +} + +static int raspberrypi_clk_remove(struct platform_device *pdev) +{ + platform_device_unregister(rpi_cpufreq); + rpi_cpufreq = NULL; + return 0; } @@ -293,6 +306,7 @@ static struct platform_driver raspberrypi_clk_driver = { .name = "raspberrypi-clk", }, .probe = raspberrypi_clk_probe, + .remove = raspberrypi_clk_remove, }; module_platform_driver(raspberrypi_clk_driver);