mbox series

[v2,0/5] support ipq5332 platform

Message ID 20231212115151.20016-1-quic_luoj@quicinc.com
Headers show
Series support ipq5332 platform | expand

Message

Luo Jie Dec. 12, 2023, 11:51 a.m. UTC
For IPQ5332 platform, there are two MAC PCSs, and qca8084 is
connected with one of them.

1. The Ethernet LDO needs to be enabled to make the PHY GPIO
   reset taking effect, which uses the MDIO bus level reset.

2. The SoC GCC uniphy AHB and SYS clocks need to be enabled
   to make the ethernet PHY device accessible.

3. To provide the clock to the ethernet, the CMN clock needs
   to be initialized for selecting reference clock and enabling
   the output clock.

4. Support optional MDIO clock frequency config.

5. Update dt-bindings doc for the new added properties.

Changes in v2:
	* remove the PHY related features such as PHY address
	  program and clock initialization.

	* leverage the MDIO level GPIO reset for qca8084 PHY.


Luo Jie (5):
  net: mdio: ipq4019: move eth_ldo_rdy before MDIO bus register
  net: mdio: ipq4019: enable the SoC uniphy clocks for ipq5332 platform
  net: mdio: ipq4019: configure CMN PLL clock for ipq5332
  net: mdio: ipq4019: support MDIO clock frequency divider
  dt-bindings: net: ipq4019-mdio: Document ipq5332 platform

 .../bindings/net/qcom,ipq4019-mdio.yaml       | 157 +++++++++-
 drivers/net/mdio/mdio-ipq4019.c               | 296 ++++++++++++++++--
 2 files changed, 424 insertions(+), 29 deletions(-)


base-commit: abb240f7a2bd14567ab53e602db562bb683391e6

Comments

Maxime Chevallier Dec. 12, 2023, 12:46 p.m. UTC | #1
Hello,

On Tue, 12 Dec 2023 19:51:47 +0800
Luo Jie <quic_luoj@quicinc.com> wrote:

> On the platform ipq5332, the related SoC uniphy GCC clocks need
> to be enabled for making the MDIO slave devices accessible.
> 
> These UNIPHY clocks are from the SoC platform GCC clock provider,
> which are enabled for the connected PHY devices working.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>

[...]

>  static int ipq4019_mdio_wait_busy(struct mii_bus *bus)
> @@ -209,14 +230,43 @@ static int ipq4019_mdio_write_c22(struct mii_bus *bus, int mii_id, int regnum,
>  static int ipq_mdio_reset(struct mii_bus *bus)
>  {
>  	struct ipq4019_mdio_data *priv = bus->priv;
> -	int ret;
> +	int ret, index;
> +	unsigned long rate;

Please remember to use reverse christmas-tree ordering, meaning longer
declaration lines go first :

	struct ipq4019_mdio_data *priv = bus->priv;
	unsigned long rate;
	int ret, index;

> +
> +	/* For the platform ipq5332, there are two SoC uniphies available
> +	 * for connecting with ethernet PHY, the SoC uniphy gcc clock
> +	 * should be enabled for resetting the connected device such
> +	 * as qca8386 switch, qca8081 PHY or other PHYs effectively.
> +	 *
> +	 * Configure MDIO/UNIPHY clock source frequency if clock instance
> +	 * is specified in the device tree.
> +	 */
> +	for (index = MDIO_CLK_MDIO_AHB; index < MDIO_CLK_CNT; index++) {
> +		switch (index) {
> +		case MDIO_CLK_MDIO_AHB:
> +			rate = IPQ_MDIO_CLK_RATE;
> +			break;
> +		case MDIO_CLK_UNIPHY0_AHB:
> +		case MDIO_CLK_UNIPHY1_AHB:
> +			rate = IPQ_UNIPHY_AHB_CLK_RATE;
> +			break;
> +		case MDIO_CLK_UNIPHY0_SYS:
> +		case MDIO_CLK_UNIPHY1_SYS:
> +			rate = IPQ_UNIPHY_SYS_CLK_RATE;
> +			break;
> +		default:
> +			break;
> +		}
>  
> -	/* Configure MDIO clock source frequency if clock is specified in the device tree */
> -	ret = clk_set_rate(priv->mdio_clk, IPQ_MDIO_CLK_RATE);
> -	if (ret)
> -		return ret;
> +		ret = clk_set_rate(priv->clk[index], rate);
> +		if (ret)
> +			return ret;
> +
> +		ret = clk_prepare_enable(priv->clk[index]);
> +		if (ret)
> +			return ret;
> +	}
>  
> -	ret = clk_prepare_enable(priv->mdio_clk);
>  	if (ret == 0)
>  		mdelay(10);
>  
> @@ -240,10 +290,6 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->membase))
>  		return PTR_ERR(priv->membase);
>  
> -	priv->mdio_clk = devm_clk_get_optional(&pdev->dev, "gcc_mdio_ahb_clk");
> -	if (IS_ERR(priv->mdio_clk))
> -		return PTR_ERR(priv->mdio_clk);
> -
>  	/* These platform resources are provided on the chipset IPQ5018 or
>  	 * IPQ5332.
>  	 */
> @@ -271,6 +317,13 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	for (index = 0; index < MDIO_CLK_CNT; index++) {
> +		priv->clk[index] = devm_clk_get_optional(&pdev->dev,
> +							 mdio_clk_name[index]);
> +		if (IS_ERR(priv->clk[index]))
> +			return PTR_ERR(priv->clk[index]);
> +	}

You should be able to use devm_clk_bulk_get_optional(), to avoid that
loop.

Thanks,

Maxime
Maxime Chevallier Dec. 12, 2023, 12:50 p.m. UTC | #2
Hello,

On Tue, 12 Dec 2023 19:51:46 +0800
Luo Jie <quic_luoj@quicinc.com> wrote:

> The ethernet LDO provides the clock for the ethernet PHY that
> is connected with PCS, each LDO enables the clock output to
> each PCS, after the clock output enablement, the PHY GPIO reset
> can take effect.
> 
> For the PHY taking the MDIO bus level GPIO reset, the ethernet
> LDO should be enabled before the MDIO bus register.
> 
> For example, the qca8084 PHY takes the MDIO bus level GPIO
> reset for quad PHYs, there is another reason for qca8084 PHY
> using MDIO bus level GPIO reset instead of PHY level GPIO
> reset as below.
> 
> The work sequence of qca8084:
> 1. enable ethernet LDO.
> 2. GPIO reset on quad PHYs.
> 3. register clock provider based on MDIO device of qca8084.
> 4. PHY probe function called for initializing common clocks.
> 5. PHY capabilities acquirement.
> 
> If qca8084 takes PHY level GPIO reset in the step 4, the clock
> provider of qca8084 can't be registered correctly, since the
> clock parent(reading the current qca8084 hardware registers in
> step 3) of the registered clocks is deserted after GPIO reset.
> 
> There are two PCS(UNIPHY) supported in SOC side on ipq5332,
> and three PCS(UNIPHY) supported on ipq9574.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> ---

[...]

> @@ -252,11 +244,32 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
>  	if (IS_ERR(priv->mdio_clk))
>  		return PTR_ERR(priv->mdio_clk);
>  
> -	/* The platform resource is provided on the chipset IPQ5018 */
> -	/* This resource is optional */
> -	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> -	if (res)
> -		priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res);
> +	/* These platform resources are provided on the chipset IPQ5018 or
> +	 * IPQ5332.
> +	 */
> +	/* This resource are optional */
> +	for (index = 0; index < ETH_LDO_RDY_CNT; index++) {
> +		res = platform_get_resource(pdev, IORESOURCE_MEM, index + 1);
> +		if (res) {
> +			priv->eth_ldo_rdy[index] = devm_ioremap(&pdev->dev,
> +								res->start,
> +								resource_size(res));

You can simplify that sequence by using
devm_platform_get_and_ioremap_resource(), which will do both the
platform_get_resource and the devm_ioremap at once for you.

Thanks,

Maxime
Maxime Chevallier Dec. 12, 2023, 12:54 p.m. UTC | #3
Hello,

I have some more minor comments for yoi :)

On Tue, 12 Dec 2023 19:51:48 +0800
Luo Jie <quic_luoj@quicinc.com> wrote:

> The reference clock of CMN PLL block is selectable, the internal
> 48MHZ is used by default.
> 
> The output clock of CMN PLL block is for providing the clock
> source of ethernet device(such as qca8084), there are 1 * 25MHZ
> and 3 * 50MHZ output clocks available for the ethernet devices.
> 
> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> ---

[...]

> +/* For the CMN PLL block, the reference clock can be configured according to
> + * the device tree property "cmn-reference-clock", the internal 48MHZ is used
> + * by default on the ipq533 platform.
> + *
> + * The output clock of CMN PLL block is provided to the ethernet devices,
> + * threre are 4 CMN PLL output clocks (1*25MHZ + 3*50MHZ) enabled by default.
> + *
> + * Such as the output 50M clock for the qca8084 ethernet PHY.
> + */
> +static int ipq_cmn_clock_config(struct mii_bus *bus)
> +{
> +	int ret;
> +	u32 reg_val, src_sel, ref_clk;
> +	struct ipq4019_mdio_data *priv;

Here you should also use reverse christmas-tree notation

[...]

> @@ -317,6 +441,17 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
>  		}
>  	}
>  
> +	/* The CMN block resource is for providing clock source to ethernet,
> +	 * which can be optionally configured on the platform ipq9574 and
> +	 * ipq5332.
> +	 */
> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cmn_blk");
> +	if (res) {
> +		priv->cmn_membase = devm_ioremap_resource(&pdev->dev, res);
> +		if (IS_ERR(priv->cmn_membase))
> +			return PTR_ERR(priv->cmn_membase);
> +	}
> +

And here you can simplify a bit by using
devm_platform_ioremap_resource_byname()

Thanks,

Maxime
Russell King (Oracle) Dec. 12, 2023, 7:11 p.m. UTC | #4
On Tue, Dec 12, 2023 at 01:50:01PM +0100, Maxime Chevallier wrote:
> Hello,
> 
> On Tue, 12 Dec 2023 19:51:46 +0800
> Luo Jie <quic_luoj@quicinc.com> wrote:
> > @@ -252,11 +244,32 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
> >  	if (IS_ERR(priv->mdio_clk))
> >  		return PTR_ERR(priv->mdio_clk);
> >  
> > -	/* The platform resource is provided on the chipset IPQ5018 */
> > -	/* This resource is optional */
> > -	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > -	if (res)
> > -		priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res);
> > +	/* These platform resources are provided on the chipset IPQ5018 or
> > +	 * IPQ5332.
> > +	 */
> > +	/* This resource are optional */
> > +	for (index = 0; index < ETH_LDO_RDY_CNT; index++) {
> > +		res = platform_get_resource(pdev, IORESOURCE_MEM, index + 1);
> > +		if (res) {
> > +			priv->eth_ldo_rdy[index] = devm_ioremap(&pdev->dev,
> > +								res->start,
> > +								resource_size(res));
> 
> You can simplify that sequence by using
> devm_platform_get_and_ioremap_resource(), which will do both the
> platform_get_resource and the devm_ioremap at once for you.

Sadly it can't if resources are optional. __devm_ioremap_resource()
which will be capped by devm_platform_get_and_ioremap_resource() will
be passed a NULL 'res', which will lead to:

        if (!res || resource_type(res) != IORESOURCE_MEM) {
                dev_err(dev, "invalid resource %pR\n", res);
                return IOMEM_ERR_PTR(-EINVAL);
        }

There isn't an "optional" version of
devm_platform_get_and_ioremap_resource().
Russell King (Oracle) Dec. 12, 2023, 7:12 p.m. UTC | #5
On Tue, Dec 12, 2023 at 01:54:17PM +0100, Maxime Chevallier wrote:
> Hello,
> 
> I have some more minor comments for yoi :)
> 
> On Tue, 12 Dec 2023 19:51:48 +0800
> Luo Jie <quic_luoj@quicinc.com> wrote:
> > +	/* The CMN block resource is for providing clock source to ethernet,
> > +	 * which can be optionally configured on the platform ipq9574 and
> > +	 * ipq5332.
> > +	 */
> > +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cmn_blk");
> > +	if (res) {
> > +		priv->cmn_membase = devm_ioremap_resource(&pdev->dev, res);
> > +		if (IS_ERR(priv->cmn_membase))
> > +			return PTR_ERR(priv->cmn_membase);
> > +	}
> > +
> 
> And here you can simplify a bit by using
> devm_platform_ioremap_resource_byname()

Not if the resource is optional.
Luo Jie Dec. 13, 2023, 8:05 a.m. UTC | #6
On 12/12/2023 8:46 PM, Maxime Chevallier wrote:
> Hello,
> 
> On Tue, 12 Dec 2023 19:51:47 +0800
> Luo Jie <quic_luoj@quicinc.com> wrote:
> 
>> On the platform ipq5332, the related SoC uniphy GCC clocks need
>> to be enabled for making the MDIO slave devices accessible.
>>
>> These UNIPHY clocks are from the SoC platform GCC clock provider,
>> which are enabled for the connected PHY devices working.
>>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> 
> [...]
> 
>>   static int ipq4019_mdio_wait_busy(struct mii_bus *bus)
>> @@ -209,14 +230,43 @@ static int ipq4019_mdio_write_c22(struct mii_bus *bus, int mii_id, int regnum,
>>   static int ipq_mdio_reset(struct mii_bus *bus)
>>   {
>>   	struct ipq4019_mdio_data *priv = bus->priv;
>> -	int ret;
>> +	int ret, index;
>> +	unsigned long rate;
> 
> Please remember to use reverse christmas-tree ordering, meaning longer
> declaration lines go first :
> 
> 	struct ipq4019_mdio_data *priv = bus->priv;
> 	unsigned long rate;
> 	int ret, index;

Thanks, i will update this.

> 
>> +
>> +	/* For the platform ipq5332, there are two SoC uniphies available
>> +	 * for connecting with ethernet PHY, the SoC uniphy gcc clock
>> +	 * should be enabled for resetting the connected device such
>> +	 * as qca8386 switch, qca8081 PHY or other PHYs effectively.
>> +	 *
>> +	 * Configure MDIO/UNIPHY clock source frequency if clock instance
>> +	 * is specified in the device tree.
>> +	 */
>> +	for (index = MDIO_CLK_MDIO_AHB; index < MDIO_CLK_CNT; index++) {
>> +		switch (index) {
>> +		case MDIO_CLK_MDIO_AHB:
>> +			rate = IPQ_MDIO_CLK_RATE;
>> +			break;
>> +		case MDIO_CLK_UNIPHY0_AHB:
>> +		case MDIO_CLK_UNIPHY1_AHB:
>> +			rate = IPQ_UNIPHY_AHB_CLK_RATE;
>> +			break;
>> +		case MDIO_CLK_UNIPHY0_SYS:
>> +		case MDIO_CLK_UNIPHY1_SYS:
>> +			rate = IPQ_UNIPHY_SYS_CLK_RATE;
>> +			break;
>> +		default:
>> +			break;
>> +		}
>>   
>> -	/* Configure MDIO clock source frequency if clock is specified in the device tree */
>> -	ret = clk_set_rate(priv->mdio_clk, IPQ_MDIO_CLK_RATE);
>> -	if (ret)
>> -		return ret;
>> +		ret = clk_set_rate(priv->clk[index], rate);
>> +		if (ret)
>> +			return ret;
>> +
>> +		ret = clk_prepare_enable(priv->clk[index]);
>> +		if (ret)
>> +			return ret;
>> +	}
>>   
>> -	ret = clk_prepare_enable(priv->mdio_clk);
>>   	if (ret == 0)
>>   		mdelay(10);
>>   
>> @@ -240,10 +290,6 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
>>   	if (IS_ERR(priv->membase))
>>   		return PTR_ERR(priv->membase);
>>   
>> -	priv->mdio_clk = devm_clk_get_optional(&pdev->dev, "gcc_mdio_ahb_clk");
>> -	if (IS_ERR(priv->mdio_clk))
>> -		return PTR_ERR(priv->mdio_clk);
>> -
>>   	/* These platform resources are provided on the chipset IPQ5018 or
>>   	 * IPQ5332.
>>   	 */
>> @@ -271,6 +317,13 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
>>   		}
>>   	}
>>   
>> +	for (index = 0; index < MDIO_CLK_CNT; index++) {
>> +		priv->clk[index] = devm_clk_get_optional(&pdev->dev,
>> +							 mdio_clk_name[index]);
>> +		if (IS_ERR(priv->clk[index]))
>> +			return PTR_ERR(priv->clk[index]);
>> +	}
> 
> You should be able to use devm_clk_bulk_get_optional(), to avoid that
> loop.
> 
> Thanks,
> 
> Maxime

Thanks Maxime for the suggestion.
These clocks need to be configured the different clock rate, MDIO system
clock works on 100MHZ, but UNIPHY system clock works on 24MHZ.

For the clock rate set, i still need the loop to configure the different
clock rate on the different clock instance.

So i use the devm_clk_get_optional to acquire the exact clock ID here.
Luo Jie Dec. 13, 2023, 8:09 a.m. UTC | #7
On 12/12/2023 8:54 PM, Maxime Chevallier wrote:
> Hello,
> 
> I have some more minor comments for yoi :)
> 
> On Tue, 12 Dec 2023 19:51:48 +0800
> Luo Jie <quic_luoj@quicinc.com> wrote:
> 
>> The reference clock of CMN PLL block is selectable, the internal
>> 48MHZ is used by default.
>>
>> The output clock of CMN PLL block is for providing the clock
>> source of ethernet device(such as qca8084), there are 1 * 25MHZ
>> and 3 * 50MHZ output clocks available for the ethernet devices.
>>
>> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
>> ---
> 
> [...]
> 
>> +/* For the CMN PLL block, the reference clock can be configured according to
>> + * the device tree property "cmn-reference-clock", the internal 48MHZ is used
>> + * by default on the ipq533 platform.
>> + *
>> + * The output clock of CMN PLL block is provided to the ethernet devices,
>> + * threre are 4 CMN PLL output clocks (1*25MHZ + 3*50MHZ) enabled by default.
>> + *
>> + * Such as the output 50M clock for the qca8084 ethernet PHY.
>> + */
>> +static int ipq_cmn_clock_config(struct mii_bus *bus)
>> +{
>> +	int ret;
>> +	u32 reg_val, src_sel, ref_clk;
>> +	struct ipq4019_mdio_data *priv;
> 
> Here you should also use reverse christmas-tree notation

Ok, will correct this, thanks.

> 
> [...]
> 
>> @@ -317,6 +441,17 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
>>   		}
>>   	}
>>   
>> +	/* The CMN block resource is for providing clock source to ethernet,
>> +	 * which can be optionally configured on the platform ipq9574 and
>> +	 * ipq5332.
>> +	 */
>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cmn_blk");
>> +	if (res) {
>> +		priv->cmn_membase = devm_ioremap_resource(&pdev->dev, res);
>> +		if (IS_ERR(priv->cmn_membase))
>> +			return PTR_ERR(priv->cmn_membase);
>> +	}
>> +
> 
> And here you can simplify a bit by using
> devm_platform_ioremap_resource_byname()
> 
> Thanks,
> 
> Maxime
> 
As Russell mentioned, since this resource is optional,
so devm_platform_ioremap_resource_byname can't be used here.
Maxime Chevallier Dec. 13, 2023, 10:07 a.m. UTC | #8
Hello Russell,

On Tue, 12 Dec 2023 19:11:15 +0000
"Russell King (Oracle)" <linux@armlinux.org.uk> wrote:

> On Tue, Dec 12, 2023 at 01:50:01PM +0100, Maxime Chevallier wrote:
> > Hello,
> > 
> > On Tue, 12 Dec 2023 19:51:46 +0800
> > Luo Jie <quic_luoj@quicinc.com> wrote:  
> > > @@ -252,11 +244,32 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
> > >  	if (IS_ERR(priv->mdio_clk))
> > >  		return PTR_ERR(priv->mdio_clk);
> > >  
> > > -	/* The platform resource is provided on the chipset IPQ5018 */
> > > -	/* This resource is optional */
> > > -	res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> > > -	if (res)
> > > -		priv->eth_ldo_rdy = devm_ioremap_resource(&pdev->dev, res);
> > > +	/* These platform resources are provided on the chipset IPQ5018 or
> > > +	 * IPQ5332.
> > > +	 */
> > > +	/* This resource are optional */
> > > +	for (index = 0; index < ETH_LDO_RDY_CNT; index++) {
> > > +		res = platform_get_resource(pdev, IORESOURCE_MEM, index + 1);
> > > +		if (res) {
> > > +			priv->eth_ldo_rdy[index] = devm_ioremap(&pdev->dev,
> > > +								res->start,
> > > +								resource_size(res));  
> > 
> > You can simplify that sequence by using
> > devm_platform_get_and_ioremap_resource(), which will do both the
> > platform_get_resource and the devm_ioremap at once for you.  
> 
> Sadly it can't if resources are optional. __devm_ioremap_resource()
> which will be capped by devm_platform_get_and_ioremap_resource() will
> be passed a NULL 'res', which will lead to:
> 
>         if (!res || resource_type(res) != IORESOURCE_MEM) {
>                 dev_err(dev, "invalid resource %pR\n", res);
>                 return IOMEM_ERR_PTR(-EINVAL);
>         }
> 
> There isn't an "optional" version of
> devm_platform_get_and_ioremap_resource().
> 

Ah right, I missed that part indeed. Sorry for the noise then, and
thanks for double-checking :)

Best regards,

Maxime
Maxime Chevallier Dec. 13, 2023, 10:08 a.m. UTC | #9
On Wed, 13 Dec 2023 16:09:53 +0800
Jie Luo <quic_luoj@quicinc.com> wrote:

> On 12/12/2023 8:54 PM, Maxime Chevallier wrote:
> > Hello,
> > 
> > I have some more minor comments for yoi :)
> > 
> > On Tue, 12 Dec 2023 19:51:48 +0800
> > Luo Jie <quic_luoj@quicinc.com> wrote:
> >   
> >> The reference clock of CMN PLL block is selectable, the internal
> >> 48MHZ is used by default.
> >>
> >> The output clock of CMN PLL block is for providing the clock
> >> source of ethernet device(such as qca8084), there are 1 * 25MHZ
> >> and 3 * 50MHZ output clocks available for the ethernet devices.
> >>
> >> Signed-off-by: Luo Jie <quic_luoj@quicinc.com>
> >> ---  
> > 
> > [...]
> >   
> >> +/* For the CMN PLL block, the reference clock can be configured according to
> >> + * the device tree property "cmn-reference-clock", the internal 48MHZ is used
> >> + * by default on the ipq533 platform.
> >> + *
> >> + * The output clock of CMN PLL block is provided to the ethernet devices,
> >> + * threre are 4 CMN PLL output clocks (1*25MHZ + 3*50MHZ) enabled by default.
> >> + *
> >> + * Such as the output 50M clock for the qca8084 ethernet PHY.
> >> + */
> >> +static int ipq_cmn_clock_config(struct mii_bus *bus)
> >> +{
> >> +	int ret;
> >> +	u32 reg_val, src_sel, ref_clk;
> >> +	struct ipq4019_mdio_data *priv;  
> > 
> > Here you should also use reverse christmas-tree notation  
> 
> Ok, will correct this, thanks.
> 
> > 
> > [...]
> >   
> >> @@ -317,6 +441,17 @@ static int ipq4019_mdio_probe(struct platform_device *pdev)
> >>   		}
> >>   	}
> >>   
> >> +	/* The CMN block resource is for providing clock source to ethernet,
> >> +	 * which can be optionally configured on the platform ipq9574 and
> >> +	 * ipq5332.
> >> +	 */
> >> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "cmn_blk");
> >> +	if (res) {
> >> +		priv->cmn_membase = devm_ioremap_resource(&pdev->dev, res);
> >> +		if (IS_ERR(priv->cmn_membase))
> >> +			return PTR_ERR(priv->cmn_membase);
> >> +	}
> >> +  
> > 
> > And here you can simplify a bit by using
> > devm_platform_ioremap_resource_byname()
> > 
> > Thanks,
> > 
> > Maxime
> >   
> As Russell mentioned, since this resource is optional,
> so devm_platform_ioremap_resource_byname can't be used here.
> 

Indeed, my bad I missed that point. Sorry for the noise :/

Thanks,

Maxime