diff mbox series

[1/1] phy: rockchip: naneng-combphy: compatible with old DT for RK3568

Message ID 20250106070000.605284-1-amadeus@jmu.edu.cn (mailing list archive)
State New
Headers show
Series [1/1] phy: rockchip: naneng-combphy: compatible with old DT for RK3568 | expand

Commit Message

Chukun Pan Jan. 6, 2025, 7 a.m. UTC
The device tree of RK3568 did not specify reset-names before.
So add fallback to old behaviour to be compatible with old DT.

Fixes: fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset")
Cc: Jianfeng Liu <liujianfeng1994@gmail.com>
Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
---
 drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Philipp Zabel Jan. 6, 2025, 7:33 a.m. UTC | #1
On Mo, 2025-01-06 at 15:00 +0800, Chukun Pan wrote:
> The device tree of RK3568 did not specify reset-names before.
> So add fallback to old behaviour to be compatible with old DT.
> 
> Fixes: fbcbffbac994 ("phy: rockchip: naneng-combphy: fix phy reset")
> Cc: Jianfeng Liu <liujianfeng1994@gmail.com>
> Signed-off-by: Chukun Pan <amadeus@jmu.edu.cn>
> ---
>  drivers/phy/rockchip/phy-rockchip-naneng-combphy.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> index a1532ef8bbe9..372f5c07b5bd 100644
> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> @@ -325,6 +325,10 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
>  	priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
>  
>  	priv->phy_rst = devm_reset_control_get(dev, "phy");

This should be devm_reset_control_get_exclusive().

> +	/* fallback to old behaviour */
> +	if (IS_ERR(ERR_PTR(priv->phy_rst)))

Drop superfluous ERR_PTR().

> +		priv->phy_rst = devm_reset_control_array_get_exclusive(dev);
> +

I'd drop the empty line, keep error handling close.

>  	if (IS_ERR(priv->phy_rst))
>  		return dev_err_probe(dev, PTR_ERR(priv->phy_rst), "failed to get phy reset\n");
>  

regards
Philipp
Chukun Pan Jan. 6, 2025, 7:50 a.m. UTC | #2
Hi,
>> diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
>> index a1532ef8bbe9..372f5c07b5bd 100644
>> --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
>> +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
>> @@ -325,6 +325,10 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
>>  	priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
>>  
>>  	priv->phy_rst = devm_reset_control_get(dev, "phy");
>
> This should be devm_reset_control_get_exclusive().

I will change this.

>> +	/* fallback to old behaviour */
>> +	if (IS_ERR(ERR_PTR(priv->phy_rst)))
>
> Drop superfluous ERR_PTR().

I think it would be better if I changed it to this?

if (PTR_ERR(priv->phy_rst) == -ENOENT)
	priv->phy_rst = devm_reset_control_array_get_exclusive(dev);

Thanks,
Chukun
Philipp Zabel Jan. 6, 2025, 9:19 a.m. UTC | #3
On Mo, 2025-01-06 at 15:50 +0800, Chukun Pan wrote:
> Hi,
> > > diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > index a1532ef8bbe9..372f5c07b5bd 100644
> > > --- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > +++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
> > > @@ -325,6 +325,10 @@ static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
> > >  	priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
> > >  
> > >  	priv->phy_rst = devm_reset_control_get(dev, "phy");
> > 
> > This should be devm_reset_control_get_exclusive().
> 
> I will change this.
> 
> > > +	/* fallback to old behaviour */
> > > +	if (IS_ERR(ERR_PTR(priv->phy_rst)))
> > 
> > Drop superfluous ERR_PTR().
> 
> I think it would be better if I changed it to this?
> 
> if (PTR_ERR(priv->phy_rst) == -ENOENT)
> 	priv->phy_rst = devm_reset_control_array_get_exclusive(dev);

I agree, otherwise this would ignore a broken reset-names property in
the device tree. When the resets property is broken, the fallback would
fail anyway.

regards
Philipp
kernel test robot Jan. 6, 2025, 12:03 p.m. UTC | #4
Hi Chukun,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.13-rc6 next-20250106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chukun-Pan/phy-rockchip-naneng-combphy-compatible-with-old-DT-for-RK3568/20250106-150311
base:   linus/master
patch link:    https://lore.kernel.org/r/20250106070000.605284-1-amadeus%40jmu.edu.cn
patch subject: [PATCH 1/1] phy: rockchip: naneng-combphy: compatible with old DT for RK3568
config: arm64-randconfig-004-20250106 (https://download.01.org/0day-ci/archive/20250106/202501061921.ToV59CNC-lkp@intel.com/config)
compiler: clang version 20.0.0git (https://github.com/llvm/llvm-project 096551537b2a747a3387726ca618ceeb3950e9bc)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250106/202501061921.ToV59CNC-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501061921.ToV59CNC-lkp@intel.com/

All errors (new ones prefixed by >>):

   In file included from drivers/phy/rockchip/phy-rockchip-naneng-combphy.c:12:
   In file included from include/linux/phy/phy.h:17:
   In file included from include/linux/regulator/consumer.h:35:
   In file included from include/linux/suspend.h:5:
   In file included from include/linux/swap.h:9:
   In file included from include/linux/memcontrol.h:21:
   In file included from include/linux/mm.h:2224:
   include/linux/vmstat.h:504:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     504 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     505 |                            item];
         |                            ~~~~
   include/linux/vmstat.h:511:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     511 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     512 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
   include/linux/vmstat.h:524:43: warning: arithmetic between different enumeration types ('enum zone_stat_item' and 'enum numa_stat_item') [-Wenum-enum-conversion]
     524 |         return vmstat_text[NR_VM_ZONE_STAT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~ ^
     525 |                            NR_VM_NUMA_EVENT_ITEMS +
         |                            ~~~~~~~~~~~~~~~~~~~~~~
>> drivers/phy/rockchip/phy-rockchip-naneng-combphy.c:314:21: error: incompatible pointer to integer conversion passing 'struct reset_control *' to parameter of type 'long' [-Wint-conversion]
     314 |         if (IS_ERR(ERR_PTR(priv->phy_rst)))
         |                            ^~~~~~~~~~~~~
   include/linux/err.h:39:48: note: passing argument to parameter 'error' here
      39 | static inline void * __must_check ERR_PTR(long error)
         |                                                ^
   3 warnings and 1 error generated.


vim +314 drivers/phy/rockchip/phy-rockchip-naneng-combphy.c

   274	
   275	static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy_priv *priv)
   276	{
   277		int i;
   278	
   279		priv->num_clks = devm_clk_bulk_get_all(dev, &priv->clks);
   280		if (priv->num_clks < 1)
   281			return -EINVAL;
   282	
   283		priv->refclk = NULL;
   284		for (i = 0; i < priv->num_clks; i++) {
   285			if (!strncmp(priv->clks[i].id, "ref", 3)) {
   286				priv->refclk = priv->clks[i].clk;
   287				break;
   288			}
   289		}
   290	
   291		if (!priv->refclk) {
   292			dev_err(dev, "no refclk found\n");
   293			return -EINVAL;
   294		}
   295	
   296		priv->pipe_grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,pipe-grf");
   297		if (IS_ERR(priv->pipe_grf)) {
   298			dev_err(dev, "failed to find peri_ctrl pipe-grf regmap\n");
   299			return PTR_ERR(priv->pipe_grf);
   300		}
   301	
   302		priv->phy_grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,pipe-phy-grf");
   303		if (IS_ERR(priv->phy_grf)) {
   304			dev_err(dev, "failed to find peri_ctrl pipe-phy-grf regmap\n");
   305			return PTR_ERR(priv->phy_grf);
   306		}
   307	
   308		priv->enable_ssc = device_property_present(dev, "rockchip,enable-ssc");
   309	
   310		priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
   311	
   312		priv->phy_rst = devm_reset_control_get(dev, "phy");
   313		/* fallback to old behaviour */
 > 314		if (IS_ERR(ERR_PTR(priv->phy_rst)))
   315			priv->phy_rst = devm_reset_control_array_get_exclusive(dev);
   316	
   317		if (IS_ERR(priv->phy_rst))
   318			return dev_err_probe(dev, PTR_ERR(priv->phy_rst), "failed to get phy reset\n");
   319	
   320		return 0;
   321	}
   322
kernel test robot Jan. 6, 2025, 9:37 p.m. UTC | #5
Hi Chukun,

kernel test robot noticed the following build errors:

[auto build test ERROR on linus/master]
[also build test ERROR on v6.13-rc6 next-20250106]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Chukun-Pan/phy-rockchip-naneng-combphy-compatible-with-old-DT-for-RK3568/20250106-150311
base:   linus/master
patch link:    https://lore.kernel.org/r/20250106070000.605284-1-amadeus%40jmu.edu.cn
patch subject: [PATCH 1/1] phy: rockchip: naneng-combphy: compatible with old DT for RK3568
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20250107/202501070458.qKEuL2HH-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 14.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250107/202501070458.qKEuL2HH-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202501070458.qKEuL2HH-lkp@intel.com/

All errors (new ones prefixed by >>):

   drivers/phy/rockchip/phy-rockchip-naneng-combphy.c: In function 'rockchip_combphy_parse_dt':
>> drivers/phy/rockchip/phy-rockchip-naneng-combphy.c:314:32: error: passing argument 1 of 'ERR_PTR' makes integer from pointer without a cast [-Wint-conversion]
     314 |         if (IS_ERR(ERR_PTR(priv->phy_rst)))
         |                            ~~~~^~~~~~~~~
         |                                |
         |                                struct reset_control *
   In file included from include/linux/clk.h:12,
                    from drivers/phy/rockchip/phy-rockchip-naneng-combphy.c:9:
   include/linux/err.h:39:48: note: expected 'long int' but argument is of type 'struct reset_control *'
      39 | static inline void * __must_check ERR_PTR(long error)
         |                                           ~~~~~^~~~~


vim +/ERR_PTR +314 drivers/phy/rockchip/phy-rockchip-naneng-combphy.c

   274	
   275	static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy_priv *priv)
   276	{
   277		int i;
   278	
   279		priv->num_clks = devm_clk_bulk_get_all(dev, &priv->clks);
   280		if (priv->num_clks < 1)
   281			return -EINVAL;
   282	
   283		priv->refclk = NULL;
   284		for (i = 0; i < priv->num_clks; i++) {
   285			if (!strncmp(priv->clks[i].id, "ref", 3)) {
   286				priv->refclk = priv->clks[i].clk;
   287				break;
   288			}
   289		}
   290	
   291		if (!priv->refclk) {
   292			dev_err(dev, "no refclk found\n");
   293			return -EINVAL;
   294		}
   295	
   296		priv->pipe_grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,pipe-grf");
   297		if (IS_ERR(priv->pipe_grf)) {
   298			dev_err(dev, "failed to find peri_ctrl pipe-grf regmap\n");
   299			return PTR_ERR(priv->pipe_grf);
   300		}
   301	
   302		priv->phy_grf = syscon_regmap_lookup_by_phandle(dev->of_node, "rockchip,pipe-phy-grf");
   303		if (IS_ERR(priv->phy_grf)) {
   304			dev_err(dev, "failed to find peri_ctrl pipe-phy-grf regmap\n");
   305			return PTR_ERR(priv->phy_grf);
   306		}
   307	
   308		priv->enable_ssc = device_property_present(dev, "rockchip,enable-ssc");
   309	
   310		priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
   311	
   312		priv->phy_rst = devm_reset_control_get(dev, "phy");
   313		/* fallback to old behaviour */
 > 314		if (IS_ERR(ERR_PTR(priv->phy_rst)))
   315			priv->phy_rst = devm_reset_control_array_get_exclusive(dev);
   316	
   317		if (IS_ERR(priv->phy_rst))
   318			return dev_err_probe(dev, PTR_ERR(priv->phy_rst), "failed to get phy reset\n");
   319	
   320		return 0;
   321	}
   322
diff mbox series

Patch

diff --git a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
index a1532ef8bbe9..372f5c07b5bd 100644
--- a/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
+++ b/drivers/phy/rockchip/phy-rockchip-naneng-combphy.c
@@ -325,6 +325,10 @@  static int rockchip_combphy_parse_dt(struct device *dev, struct rockchip_combphy
 	priv->ext_refclk = device_property_present(dev, "rockchip,ext-refclk");
 
 	priv->phy_rst = devm_reset_control_get(dev, "phy");
+	/* fallback to old behaviour */
+	if (IS_ERR(ERR_PTR(priv->phy_rst)))
+		priv->phy_rst = devm_reset_control_array_get_exclusive(dev);
+
 	if (IS_ERR(priv->phy_rst))
 		return dev_err_probe(dev, PTR_ERR(priv->phy_rst), "failed to get phy reset\n");