| Message ID | b181d867cb9523e1877a3dfd258bafde2988024f.1587742492.git-series.maxime@cerno.tech (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/vc4: Support BCM2711 Display Pipeline | expand |
Hi Maxime, On Fri, 2020-04-24 at 17:33 +0200, Maxime Ripard wrote: > The firmware clocks driver was previously probed through a platform_device > created by the firmware driver. > > Since we will now have a node for that clocks driver, we need to create the > device only in the case where there's no node for it already. > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > --- > drivers/firmware/raspberrypi.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c > index da26a584dca0..1874f41b007c 100644 > --- a/drivers/firmware/raspberrypi.c > +++ b/drivers/firmware/raspberrypi.c > @@ -210,6 +210,15 @@ rpi_register_hwmon_driver(struct device *dev, struct > rpi_firmware *fw) > > static void rpi_register_clk_driver(struct device *dev) > { > + /* > + * Earlier DTs don't have a node for the firmware clocks but > + * rely on us creating a platform device by hand. If we do > + * have a node for the firmware clocks, just bail out here. > + */ > + if (of_get_compatible_child(dev->of_node, > + "raspberrypi,firmware-clocks")) > + return; > + > rpi_clk = platform_device_register_data(dev, "raspberrypi-clk", > -1, NULL, 0); > } > @@ -262,8 +271,12 @@ static int rpi_firmware_remove(struct platform_device > *pdev) > > platform_device_unregister(rpi_hwmon); > rpi_hwmon = NULL; > - platform_device_unregister(rpi_clk); > - rpi_clk = NULL; > + > + if (rpi_clk) { rpi_clk, being a static global variable, will be zeroed by default. So, if you don't register the platform device, rpi_clk is going to be NULL, which you're allowed to feed platform_device_unregister(), so no need to be extra careful. Regards, Nicolas > + platform_device_unregister(rpi_clk); > + rpi_clk = NULL; > + } > + > mbox_free_channel(fw->chan); > > return 0;
Hi Nicolas, On Mon, Apr 27, 2020 at 01:24:14PM +0200, Nicolas Saenz Julienne wrote: > Hi Maxime, > > On Fri, 2020-04-24 at 17:33 +0200, Maxime Ripard wrote: > > The firmware clocks driver was previously probed through a platform_device > > created by the firmware driver. > > > > Since we will now have a node for that clocks driver, we need to create the > > device only in the case where there's no node for it already. > > > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > > --- > > drivers/firmware/raspberrypi.c | 17 +++++++++++++++-- > > 1 file changed, 15 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c > > index da26a584dca0..1874f41b007c 100644 > > --- a/drivers/firmware/raspberrypi.c > > +++ b/drivers/firmware/raspberrypi.c > > @@ -210,6 +210,15 @@ rpi_register_hwmon_driver(struct device *dev, struct > > rpi_firmware *fw) > > > > static void rpi_register_clk_driver(struct device *dev) > > { > > + /* > > + * Earlier DTs don't have a node for the firmware clocks but > > + * rely on us creating a platform device by hand. If we do > > + * have a node for the firmware clocks, just bail out here. > > + */ > > + if (of_get_compatible_child(dev->of_node, > > + "raspberrypi,firmware-clocks")) > > + return; > > + > > rpi_clk = platform_device_register_data(dev, "raspberrypi-clk", > > -1, NULL, 0); > > } > > @@ -262,8 +271,12 @@ static int rpi_firmware_remove(struct platform_device > > *pdev) > > > > platform_device_unregister(rpi_hwmon); > > rpi_hwmon = NULL; > > - platform_device_unregister(rpi_clk); > > - rpi_clk = NULL; > > + > > + if (rpi_clk) { > > rpi_clk, being a static global variable, will be zeroed by default. So, if you > don't register the platform device, rpi_clk is going to be NULL, which you're > allowed to feed platform_device_unregister(), so no need to be extra careful. Indeed, I'll fix that up. Thanks! Maxime
On Fri, 2020-04-24 at 17:33 +0200, Maxime Ripard wrote: > The firmware clocks driver was previously probed through a platform_device > created by the firmware driver. > > Since we will now have a node for that clocks driver, we need to create the > device only in the case where there's no node for it already. > > Signed-off-by: Maxime Ripard <maxime@cerno.tech> > --- > drivers/firmware/raspberrypi.c | 17 +++++++++++++++-- > 1 file changed, 15 insertions(+), 2 deletions(-) > > diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c > index da26a584dca0..1874f41b007c 100644 > --- a/drivers/firmware/raspberrypi.c > +++ b/drivers/firmware/raspberrypi.c > @@ -210,6 +210,15 @@ rpi_register_hwmon_driver(struct device *dev, struct > rpi_firmware *fw) > > static void rpi_register_clk_driver(struct device *dev) > { > + /* > + * Earlier DTs don't have a node for the firmware clocks but > + * rely on us creating a platform device by hand. If we do > + * have a node for the firmware clocks, just bail out here. > + */ > + if (of_get_compatible_child(dev->of_node, > + "raspberrypi,firmware-clocks")) In the case you find a compatible device node you have to decrement the refcount of_get_compatible_child() increased before leaving. Regards, Nicolas
diff --git a/drivers/firmware/raspberrypi.c b/drivers/firmware/raspberrypi.c index da26a584dca0..1874f41b007c 100644 --- a/drivers/firmware/raspberrypi.c +++ b/drivers/firmware/raspberrypi.c @@ -210,6 +210,15 @@ rpi_register_hwmon_driver(struct device *dev, struct rpi_firmware *fw) static void rpi_register_clk_driver(struct device *dev) { + /* + * Earlier DTs don't have a node for the firmware clocks but + * rely on us creating a platform device by hand. If we do + * have a node for the firmware clocks, just bail out here. + */ + if (of_get_compatible_child(dev->of_node, + "raspberrypi,firmware-clocks")) + return; + rpi_clk = platform_device_register_data(dev, "raspberrypi-clk", -1, NULL, 0); } @@ -262,8 +271,12 @@ static int rpi_firmware_remove(struct platform_device *pdev) platform_device_unregister(rpi_hwmon); rpi_hwmon = NULL; - platform_device_unregister(rpi_clk); - rpi_clk = NULL; + + if (rpi_clk) { + platform_device_unregister(rpi_clk); + rpi_clk = NULL; + } + mbox_free_channel(fw->chan); return 0;
The firmware clocks driver was previously probed through a platform_device created by the firmware driver. Since we will now have a node for that clocks driver, we need to create the device only in the case where there's no node for it already. Signed-off-by: Maxime Ripard <maxime@cerno.tech> --- drivers/firmware/raspberrypi.c | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-)