diff mbox series

[v4,1/4] dt-bindings: soc: imx8mp-media-blk-ctrl: Align block controller example name

Message ID 20230223151043.41548-1-marex@denx.de
State Not Applicable, archived
Headers show
Series [v4,1/4] dt-bindings: soc: imx8mp-media-blk-ctrl: Align block controller example name | expand

Checks

Context Check Description
robh/checkpatch success
robh/patch-applied success
robh/dtbs-check warning build log
robh/dt-meta-schema success

Commit Message

Marek Vasut Feb. 23, 2023, 3:10 p.m. UTC 
Align the block controller example node name with Linux imx8mp.dtsi .
No functional change.

Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Liu Ying <victor.liu@nxp.com>
Signed-off-by: Marek Vasut <marex@denx.de>
---
Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
Cc: Fabio Estevam <festevam@gmail.com>
Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
Cc: Lucas Stach <l.stach@pengutronix.de>
Cc: NXP Linux Team <linux-imx@nxp.com>
Cc: Paul Elder <paul.elder@ideasonboard.com>
Cc: Peng Fan <peng.fan@nxp.com>
Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
Cc: Richard Cochran <richardcochran@gmail.com>
Cc: Richard Zhu <hongxing.zhu@nxp.com>
Cc: Rob Herring <robh+dt@kernel.org>
Cc: Sascha Hauer <s.hauer@pengutronix.de>
Cc: Shawn Guo <shawnguo@kernel.org>
Cc: devicetree@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
---
V2: Adjust the label too
V3: - Add RB from Liu
    - Drop the label
V4: Add AB from Krzysztof
---
 .../devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Lucas Stach Feb. 24, 2023, 9:08 a.m. UTC | #1 
Am Donnerstag, dem 23.02.2023 um 16:10 +0100 schrieb Marek Vasut:
> This particular block can have DT subnodes describing the LVDS LDB
> bridge. Instead of misusing simple-bus to scan for those nodes, do
> the scan within the driver.
> 
> Fixes: 94e6197dadc9 ("arm64: dts: imx8mp: Add LCDIF2 & LDB nodes")
> Signed-off-by: Marek Vasut <marex@denx.de>
> ---
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Paul Elder <paul.elder@ideasonboard.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> V2: - Turn this into 3/4
>     - Warn and continue in case of error
> V3: Add RB from Liu
> V4: - Use devm_of_platform_populate()
>     - Remove RB from Liu
> ---
>  drivers/soc/imx/imx8m-blk-ctrl.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> index 399cb85105a18..77e7dc4eb8cff 100644
> --- a/drivers/soc/imx/imx8m-blk-ctrl.c
> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> @@ -310,7 +310,7 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
>  
>  	dev_set_drvdata(dev, bc);
>  
> -	return 0;
> +	return devm_of_platform_populate(dev);

You need to handle the return value, not simply pass it through as the
return value of the probe function. When devm_of_platform_populate
fails you miss to clean up the genpd provider and detach from the power
domains.

Regards,
Lucas

>  
>  cleanup_provider:
>  	of_genpd_del_provider(dev->of_node);
Marek Vasut Feb. 24, 2023, 12:08 p.m. UTC | #2 
On 2/24/23 10:08, Lucas Stach wrote:

Hi,

>> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
>> index 399cb85105a18..77e7dc4eb8cff 100644
>> --- a/drivers/soc/imx/imx8m-blk-ctrl.c
>> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
>> @@ -310,7 +310,7 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
>>   
>>   	dev_set_drvdata(dev, bc);
>>   
>> -	return 0;
>> +	return devm_of_platform_populate(dev);
> 
> You need to handle the return value, not simply pass it through as the
> return value of the probe function. When devm_of_platform_populate
> fails you miss to clean up the genpd provider and detach from the power
> domains.

Hmmm, but then I cannot use the devm_ variant, can I ?
Lucas Stach Feb. 24, 2023, 1:09 p.m. UTC | #3 
Am Freitag, dem 24.02.2023 um 13:08 +0100 schrieb Marek Vasut:
> On 2/24/23 10:08, Lucas Stach wrote:
> 
> Hi,
> 
> > > diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
> > > index 399cb85105a18..77e7dc4eb8cff 100644
> > > --- a/drivers/soc/imx/imx8m-blk-ctrl.c
> > > +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
> > > @@ -310,7 +310,7 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
> > >   
> > >   	dev_set_drvdata(dev, bc);
> > >   
> > > -	return 0;
> > > +	return devm_of_platform_populate(dev);
> > 
> > You need to handle the return value, not simply pass it through as the
> > return value of the probe function. When devm_of_platform_populate
> > fails you miss to clean up the genpd provider and detach from the power
> > domains.
> 
> Hmmm, but then I cannot use the devm_ variant, can I ?

Why not? If everything works okay the devm will just take care of
cleaning up the platform devices when the blk-ctrl is removed. If it
fails you just need to roll back the non-devm parts of the blk-ctrl
probe routine.

Regards,
Lucas
Alexander Stein Feb. 27, 2023, 7:28 a.m. UTC | #4 
Hi Marek,

Am Donnerstag, 23. Februar 2023, 16:10:40 CET schrieb Marek Vasut:
> Align the block controller example node name with Linux imx8mp.dtsi .
> No functional change.
> 
> Acked-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> Reviewed-by: Liu Ying <victor.liu@nxp.com>
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> ---
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Paul Elder <paul.elder@ideasonboard.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> V2: Adjust the label too
> V3: - Add RB from Liu
>     - Drop the label
> V4: Add AB from Krzysztof
> ---
>  .../devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml  | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git
> a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> index dadb6108e3213..a0f7c73510d04 100644
> ---
> a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> +++
> b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
> @@ -94,7 +94,7 @@ examples:
>      #include <dt-bindings/clock/imx8mp-clock.h>
>      #include <dt-bindings/power/imx8mp-power.h>
> 
> -    media_blk_ctl: blk-ctl@32ec0000 {
> +    blk-ctrl@32ec0000 {
>          compatible = "fsl,imx8mp-media-blk-ctrl", "syscon";
>          reg = <0x32ec0000 0x138>;
>          power-domains = <&mediamix_pd>, <&mipi_phy1_pd>, <&mipi_phy1_pd>,
Alexander Stein Feb. 27, 2023, 7:41 a.m. UTC | #5 
Hi Marek,

thanks for the cleanup.

Am Donnerstag, 23. Februar 2023, 16:10:43 CET schrieb Marek Vasut:
> This block should not be compatible with simple-bus and misuse it that way.
> Instead, the driver should scan its subnodes and bind drivers to them.
> 
> Reviewed-by: Liu Ying <victor.liu@nxp.com>
> Fixes: 94e6197dadc9 ("arm64: dts: imx8mp: Add LCDIF2 & LDB nodes")
> Signed-off-by: Marek Vasut <marex@denx.de>

Reviewed-by: Alexander Stein <alexander.stein@ew.tq-group.com>

Also the whole series tested on imx8mp and imx93.
Tested-by: Alexander Stein <alexander.stein@ew.tq-group.com>

> ---
> Cc: Alexander Stein <alexander.stein@ew.tq-group.com>
> Cc: Fabio Estevam <festevam@gmail.com>
> Cc: Krzysztof Kozlowski <krzysztof.kozlowski+dt@linaro.org>
> Cc: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> Cc: Lucas Stach <l.stach@pengutronix.de>
> Cc: NXP Linux Team <linux-imx@nxp.com>
> Cc: Paul Elder <paul.elder@ideasonboard.com>
> Cc: Peng Fan <peng.fan@nxp.com>
> Cc: Pengutronix Kernel Team <kernel@pengutronix.de>
> Cc: Richard Cochran <richardcochran@gmail.com>
> Cc: Richard Zhu <hongxing.zhu@nxp.com>
> Cc: Rob Herring <robh+dt@kernel.org>
> Cc: Sascha Hauer <s.hauer@pengutronix.de>
> Cc: Shawn Guo <shawnguo@kernel.org>
> Cc: devicetree@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> ---
> V2: Turn this into 4/4
> V3: Add RB from Liu
> V4: No change
> ---
>  arch/arm64/boot/dts/freescale/imx8mp.dtsi | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> b/arch/arm64/boot/dts/freescale/imx8mp.dtsi index
> 2dd60e3252f35..ca8093ee4d0e4 100644
> --- a/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> +++ b/arch/arm64/boot/dts/freescale/imx8mp.dtsi
> @@ -1151,7 +1151,7 @@ lcdif2_to_ldb: endpoint {
> 
>  			media_blk_ctrl: blk-ctrl@32ec0000 {
>  				compatible = "fsl,imx8mp-media-blk-
ctrl",
> -					     "simple-bus", "syscon";
> +					     "syscon";
>  				reg = <0x32ec0000 0x10000>;
>  				#address-cells = <1>;
>  				#size-cells = <1>;
Marek Vasut Feb. 27, 2023, 3:55 p.m. UTC | #6 
On 2/24/23 14:09, Lucas Stach wrote:
> Am Freitag, dem 24.02.2023 um 13:08 +0100 schrieb Marek Vasut:
>> On 2/24/23 10:08, Lucas Stach wrote:
>>
>> Hi,
>>
>>>> diff --git a/drivers/soc/imx/imx8m-blk-ctrl.c b/drivers/soc/imx/imx8m-blk-ctrl.c
>>>> index 399cb85105a18..77e7dc4eb8cff 100644
>>>> --- a/drivers/soc/imx/imx8m-blk-ctrl.c
>>>> +++ b/drivers/soc/imx/imx8m-blk-ctrl.c
>>>> @@ -310,7 +310,7 @@ static int imx8m_blk_ctrl_probe(struct platform_device *pdev)
>>>>    
>>>>    	dev_set_drvdata(dev, bc);
>>>>    
>>>> -	return 0;
>>>> +	return devm_of_platform_populate(dev);
>>>
>>> You need to handle the return value, not simply pass it through as the
>>> return value of the probe function. When devm_of_platform_populate
>>> fails you miss to clean up the genpd provider and detach from the power
>>> domains.
>>
>> Hmmm, but then I cannot use the devm_ variant, can I ?
> 
> Why not? If everything works okay the devm will just take care of
> cleaning up the platform devices when the blk-ctrl is removed. If it
> fails you just need to roll back the non-devm parts of the blk-ctrl
> probe routine.

All right, I just sent a V5. Thanks for the clarification.
diff mbox series

Patch

diff --git a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
index dadb6108e3213..a0f7c73510d04 100644
--- a/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
+++ b/Documentation/devicetree/bindings/soc/imx/fsl,imx8mp-media-blk-ctrl.yaml
@@ -94,7 +94,7 @@  examples:
     #include <dt-bindings/clock/imx8mp-clock.h>
     #include <dt-bindings/power/imx8mp-power.h>
 
-    media_blk_ctl: blk-ctl@32ec0000 {
+    blk-ctrl@32ec0000 {
         compatible = "fsl,imx8mp-media-blk-ctrl", "syscon";
         reg = <0x32ec0000 0x138>;
         power-domains = <&mediamix_pd>, <&mipi_phy1_pd>, <&mipi_phy1_pd>,