[net-next,PATCH v6 7/8] net: stmmac: dwmac-stm32: Mask support for PMCR configuration

Christophe Roullier posted 8 patches 1 year, 8 months ago
There is a newer version of this series
[net-next,PATCH v6 7/8] net: stmmac: dwmac-stm32: Mask support for PMCR configuration
Posted by Christophe Roullier 1 year, 8 months ago
Add possibility to have second argument in syscon property to manage
mask. This mask will be used to address right BITFIELDS of PMCR register.

Signed-off-by: Christophe Roullier <christophe.roullier@foss.st.com>
---
 .../net/ethernet/stmicro/stmmac/dwmac-stm32.c | 28 +++++++++++++------
 1 file changed, 19 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
index bed2be129b2d2..09ff0be0bdcdc 100644
--- a/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
+++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-stm32.c
@@ -90,6 +90,7 @@ struct stm32_dwmac {
 	int eth_ref_clk_sel_reg;
 	int irq_pwr_wakeup;
 	u32 mode_reg;		 /* MAC glue-logic mode register */
+	u32 mode_mask;
 	struct regmap *regmap;
 	u32 speed;
 	const struct stm32_ops *ops;
@@ -102,8 +103,8 @@ struct stm32_ops {
 	void (*resume)(struct stm32_dwmac *dwmac);
 	int (*parse_data)(struct stm32_dwmac *dwmac,
 			  struct device *dev);
-	u32 syscfg_eth_mask;
 	bool clk_rx_enable_in_suspend;
+	u32 syscfg_clr_off;
 };
 
 static int stm32_dwmac_clk_enable(struct stm32_dwmac *dwmac, bool resume)
@@ -256,13 +257,16 @@ static int stm32mp1_configure_pmcr(struct plat_stmmacenet_data *plat_dat)
 
 	dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface));
 
+	/* Shift value at correct ethernet MAC offset in SYSCFG_PMCSETR */
+	val <<= ffs(dwmac->mode_mask) - ffs(SYSCFG_MP1_ETH_MASK);
+
 	/* Need to update PMCCLRR (clear register) */
-	regmap_write(dwmac->regmap, reg + SYSCFG_PMCCLRR_OFFSET,
-		     dwmac->ops->syscfg_eth_mask);
+	regmap_write(dwmac->regmap, dwmac->ops->syscfg_clr_off,
+		     dwmac->mode_mask);
 
 	/* Update PMCSETR (set register) */
 	return regmap_update_bits(dwmac->regmap, reg,
-				 dwmac->ops->syscfg_eth_mask, val);
+				 dwmac->mode_mask, val);
 }
 
 static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
@@ -303,7 +307,7 @@ static int stm32mcu_set_mode(struct plat_stmmacenet_data *plat_dat)
 	dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface));
 
 	return regmap_update_bits(dwmac->regmap, reg,
-				 dwmac->ops->syscfg_eth_mask, val << 23);
+				 SYSCFG_MCU_ETH_MASK, val << 23);
 }
 
 static void stm32_dwmac_clk_disable(struct stm32_dwmac *dwmac, bool suspend)
@@ -348,8 +352,15 @@ static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
 		return PTR_ERR(dwmac->regmap);
 
 	err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
-	if (err)
+	if (err) {
 		dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
+		return err;
+	}
+
+	dwmac->mode_mask = SYSCFG_MP1_ETH_MASK;
+	err = of_property_read_u32_index(np, "st,syscon", 2, &dwmac->mode_mask);
+	if (err)
+		dev_dbg(dev, "Warning sysconfig register mask not set\n");
 
 	return err;
 }
@@ -540,8 +551,7 @@ static SIMPLE_DEV_PM_OPS(stm32_dwmac_pm_ops,
 	stm32_dwmac_suspend, stm32_dwmac_resume);
 
 static struct stm32_ops stm32mcu_dwmac_data = {
-	.set_mode = stm32mcu_set_mode,
-	.syscfg_eth_mask = SYSCFG_MCU_ETH_MASK
+	.set_mode = stm32mcu_set_mode
 };
 
 static struct stm32_ops stm32mp1_dwmac_data = {
@@ -549,7 +559,7 @@ static struct stm32_ops stm32mp1_dwmac_data = {
 	.suspend = stm32mp1_suspend,
 	.resume = stm32mp1_resume,
 	.parse_data = stm32mp1_parse_data,
-	.syscfg_eth_mask = SYSCFG_MP1_ETH_MASK,
+	.syscfg_clr_off = 0x44,
 	.clk_rx_enable_in_suspend = true
 };
 
-- 
2.25.1
Re: [net-next,PATCH v6 7/8] net: stmmac: dwmac-stm32: Mask support for PMCR configuration
Posted by Marek Vasut 1 year, 8 months ago
On 6/10/24 9:14 AM, Christophe Roullier wrote:

[...]

>   static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
> @@ -303,7 +307,7 @@ static int stm32mcu_set_mode(struct plat_stmmacenet_data *plat_dat)
>   	dev_dbg(dwmac->dev, "Mode %s", phy_modes(plat_dat->mac_interface));
>   
>   	return regmap_update_bits(dwmac->regmap, reg,
> -				 dwmac->ops->syscfg_eth_mask, val << 23);
> +				 SYSCFG_MCU_ETH_MASK, val << 23);
>   }
>   
>   static void stm32_dwmac_clk_disable(struct stm32_dwmac *dwmac, bool suspend)
> @@ -348,8 +352,15 @@ static int stm32_dwmac_parse_data(struct stm32_dwmac *dwmac,
>   		return PTR_ERR(dwmac->regmap);
>   
>   	err = of_property_read_u32_index(np, "st,syscon", 1, &dwmac->mode_reg);
> -	if (err)
> +	if (err) {
>   		dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
> +		return err;
> +	}
> +
> +	dwmac->mode_mask = SYSCFG_MP1_ETH_MASK;
> +	err = of_property_read_u32_index(np, "st,syscon", 2, &dwmac->mode_mask);
> +	if (err)
> +		dev_dbg(dev, "Warning sysconfig register mask not set\n");

Isn't this an error , so dev_err() ?

Include the err variable in the error message, see the dev_err() above 
for an example. That way the log already contains useful information 
(the error code) that can be used to narrow down the problem.
Re: [net-next,PATCH v6 7/8] net: stmmac: dwmac-stm32: Mask support for PMCR configuration
Posted by Christophe ROULLIER 1 year, 8 months ago
On 6/10/24 12:39, Marek Vasut wrote:
> On 6/10/24 9:14 AM, Christophe Roullier wrote:
>
> [...]
>
>>   static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>> @@ -303,7 +307,7 @@ static int stm32mcu_set_mode(struct 
>> plat_stmmacenet_data *plat_dat)
>>       dev_dbg(dwmac->dev, "Mode %s", 
>> phy_modes(plat_dat->mac_interface));
>>         return regmap_update_bits(dwmac->regmap, reg,
>> -                 dwmac->ops->syscfg_eth_mask, val << 23);
>> +                 SYSCFG_MCU_ETH_MASK, val << 23);
>>   }
>>     static void stm32_dwmac_clk_disable(struct stm32_dwmac *dwmac, 
>> bool suspend)
>> @@ -348,8 +352,15 @@ static int stm32_dwmac_parse_data(struct 
>> stm32_dwmac *dwmac,
>>           return PTR_ERR(dwmac->regmap);
>>         err = of_property_read_u32_index(np, "st,syscon", 1, 
>> &dwmac->mode_reg);
>> -    if (err)
>> +    if (err) {
>>           dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
>> +        return err;
>> +    }
>> +
>> +    dwmac->mode_mask = SYSCFG_MP1_ETH_MASK;
>> +    err = of_property_read_u32_index(np, "st,syscon", 2, 
>> &dwmac->mode_mask);
>> +    if (err)
>> +        dev_dbg(dev, "Warning sysconfig register mask not set\n");
>
> Isn't this an error , so dev_err() ?
No, it is only "warning" information, for MP1 the mask is not needed 
(and for backward compatibility is not planned to put mask parameter 
mandatory)
>
> Include the err variable in the error message, see the dev_err() above 
> for an example. That way the log already contains useful information 
> (the error code) that can be used to narrow down the problem.
Re: [net-next,PATCH v6 7/8] net: stmmac: dwmac-stm32: Mask support for PMCR configuration
Posted by Marek Vasut 1 year, 8 months ago
On 6/10/24 1:45 PM, Christophe ROULLIER wrote:
> 
> On 6/10/24 12:39, Marek Vasut wrote:
>> On 6/10/24 9:14 AM, Christophe Roullier wrote:
>>
>> [...]
>>
>>>   static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>>> @@ -303,7 +307,7 @@ static int stm32mcu_set_mode(struct 
>>> plat_stmmacenet_data *plat_dat)
>>>       dev_dbg(dwmac->dev, "Mode %s", 
>>> phy_modes(plat_dat->mac_interface));
>>>         return regmap_update_bits(dwmac->regmap, reg,
>>> -                 dwmac->ops->syscfg_eth_mask, val << 23);
>>> +                 SYSCFG_MCU_ETH_MASK, val << 23);
>>>   }
>>>     static void stm32_dwmac_clk_disable(struct stm32_dwmac *dwmac, 
>>> bool suspend)
>>> @@ -348,8 +352,15 @@ static int stm32_dwmac_parse_data(struct 
>>> stm32_dwmac *dwmac,
>>>           return PTR_ERR(dwmac->regmap);
>>>         err = of_property_read_u32_index(np, "st,syscon", 1, 
>>> &dwmac->mode_reg);
>>> -    if (err)
>>> +    if (err) {
>>>           dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
>>> +        return err;
>>> +    }
>>> +
>>> +    dwmac->mode_mask = SYSCFG_MP1_ETH_MASK;
>>> +    err = of_property_read_u32_index(np, "st,syscon", 2, 
>>> &dwmac->mode_mask);
>>> +    if (err)
>>> +        dev_dbg(dev, "Warning sysconfig register mask not set\n");
>>
>> Isn't this an error , so dev_err() ?
> No, it is only "warning" information, for MP1 the mask is not needed 
> (and for backward compatibility is not planned to put mask parameter 
> mandatory)

Should this be an error for anything newer than MP15 then ?
Re: [net-next,PATCH v6 7/8] net: stmmac: dwmac-stm32: Mask support for PMCR configuration
Posted by Christophe ROULLIER 1 year, 8 months ago
On 6/10/24 15:43, Marek Vasut wrote:
> On 6/10/24 1:45 PM, Christophe ROULLIER wrote:
>>
>> On 6/10/24 12:39, Marek Vasut wrote:
>>> On 6/10/24 9:14 AM, Christophe Roullier wrote:
>>>
>>> [...]
>>>
>>>>   static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>>>> @@ -303,7 +307,7 @@ static int stm32mcu_set_mode(struct 
>>>> plat_stmmacenet_data *plat_dat)
>>>>       dev_dbg(dwmac->dev, "Mode %s", 
>>>> phy_modes(plat_dat->mac_interface));
>>>>         return regmap_update_bits(dwmac->regmap, reg,
>>>> -                 dwmac->ops->syscfg_eth_mask, val << 23);
>>>> +                 SYSCFG_MCU_ETH_MASK, val << 23);
>>>>   }
>>>>     static void stm32_dwmac_clk_disable(struct stm32_dwmac *dwmac, 
>>>> bool suspend)
>>>> @@ -348,8 +352,15 @@ static int stm32_dwmac_parse_data(struct 
>>>> stm32_dwmac *dwmac,
>>>>           return PTR_ERR(dwmac->regmap);
>>>>         err = of_property_read_u32_index(np, "st,syscon", 1, 
>>>> &dwmac->mode_reg);
>>>> -    if (err)
>>>> +    if (err) {
>>>>           dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
>>>> +        return err;
>>>> +    }
>>>> +
>>>> +    dwmac->mode_mask = SYSCFG_MP1_ETH_MASK;
>>>> +    err = of_property_read_u32_index(np, "st,syscon", 2, 
>>>> &dwmac->mode_mask);
>>>> +    if (err)
>>>> +        dev_dbg(dev, "Warning sysconfig register mask not set\n");
>>>
>>> Isn't this an error , so dev_err() ?
>> No, it is only "warning" information, for MP1 the mask is not needed 
>> (and for backward compatibility is not planned to put mask parameter 
>> mandatory)
>
> Should this be an error for anything newer than MP15 then ?
For MP25, no need of mask, so for moment it is specific to MP13.
Re: [net-next,PATCH v6 7/8] net: stmmac: dwmac-stm32: Mask support for PMCR configuration
Posted by Marek Vasut 1 year, 8 months ago
On 6/10/24 3:49 PM, Christophe ROULLIER wrote:
> 
> On 6/10/24 15:43, Marek Vasut wrote:
>> On 6/10/24 1:45 PM, Christophe ROULLIER wrote:
>>>
>>> On 6/10/24 12:39, Marek Vasut wrote:
>>>> On 6/10/24 9:14 AM, Christophe Roullier wrote:
>>>>
>>>> [...]
>>>>
>>>>>   static int stm32mp1_set_mode(struct plat_stmmacenet_data *plat_dat)
>>>>> @@ -303,7 +307,7 @@ static int stm32mcu_set_mode(struct 
>>>>> plat_stmmacenet_data *plat_dat)
>>>>>       dev_dbg(dwmac->dev, "Mode %s", 
>>>>> phy_modes(plat_dat->mac_interface));
>>>>>         return regmap_update_bits(dwmac->regmap, reg,
>>>>> -                 dwmac->ops->syscfg_eth_mask, val << 23);
>>>>> +                 SYSCFG_MCU_ETH_MASK, val << 23);
>>>>>   }
>>>>>     static void stm32_dwmac_clk_disable(struct stm32_dwmac *dwmac, 
>>>>> bool suspend)
>>>>> @@ -348,8 +352,15 @@ static int stm32_dwmac_parse_data(struct 
>>>>> stm32_dwmac *dwmac,
>>>>>           return PTR_ERR(dwmac->regmap);
>>>>>         err = of_property_read_u32_index(np, "st,syscon", 1, 
>>>>> &dwmac->mode_reg);
>>>>> -    if (err)
>>>>> +    if (err) {
>>>>>           dev_err(dev, "Can't get sysconfig mode offset (%d)\n", err);
>>>>> +        return err;
>>>>> +    }
>>>>> +
>>>>> +    dwmac->mode_mask = SYSCFG_MP1_ETH_MASK;
>>>>> +    err = of_property_read_u32_index(np, "st,syscon", 2, 
>>>>> &dwmac->mode_mask);
>>>>> +    if (err)
>>>>> +        dev_dbg(dev, "Warning sysconfig register mask not set\n");
>>>>
>>>> Isn't this an error , so dev_err() ?
>>> No, it is only "warning" information, for MP1 the mask is not needed 
>>> (and for backward compatibility is not planned to put mask parameter 
>>> mandatory)
>>
>> Should this be an error for anything newer than MP15 then ?
> For MP25, no need of mask, so for moment it is specific to MP13.

Make this a warning for MP15, error for MP13, do not check st,syscon 
presence for MP2 at all. Would that work ?
Re: [net-next,PATCH v6 7/8] net: stmmac: dwmac-stm32: Mask support for PMCR configuration
Posted by Christophe ROULLIER 1 year, 8 months ago
On 6/10/24 19:29, Marek Vasut wrote:
> On 6/10/24 3:49 PM, Christophe ROULLIER wrote:
>>
>> On 6/10/24 15:43, Marek Vasut wrote:
>>> On 6/10/24 1:45 PM, Christophe ROULLIER wrote:
>>>>
>>>> On 6/10/24 12:39, Marek Vasut wrote:
>>>>> On 6/10/24 9:14 AM, Christophe Roullier wrote:
>>>>>
>>>>> [...]
>>>>>
>>>>>>   static int stm32mp1_set_mode(struct plat_stmmacenet_data 
>>>>>> *plat_dat)
>>>>>> @@ -303,7 +307,7 @@ static int stm32mcu_set_mode(struct 
>>>>>> plat_stmmacenet_data *plat_dat)
>>>>>>       dev_dbg(dwmac->dev, "Mode %s", 
>>>>>> phy_modes(plat_dat->mac_interface));
>>>>>>         return regmap_update_bits(dwmac->regmap, reg,
>>>>>> -                 dwmac->ops->syscfg_eth_mask, val << 23);
>>>>>> +                 SYSCFG_MCU_ETH_MASK, val << 23);
>>>>>>   }
>>>>>>     static void stm32_dwmac_clk_disable(struct stm32_dwmac 
>>>>>> *dwmac, bool suspend)
>>>>>> @@ -348,8 +352,15 @@ static int stm32_dwmac_parse_data(struct 
>>>>>> stm32_dwmac *dwmac,
>>>>>>           return PTR_ERR(dwmac->regmap);
>>>>>>         err = of_property_read_u32_index(np, "st,syscon", 1, 
>>>>>> &dwmac->mode_reg);
>>>>>> -    if (err)
>>>>>> +    if (err) {
>>>>>>           dev_err(dev, "Can't get sysconfig mode offset (%d)\n", 
>>>>>> err);
>>>>>> +        return err;
>>>>>> +    }
>>>>>> +
>>>>>> +    dwmac->mode_mask = SYSCFG_MP1_ETH_MASK;
>>>>>> +    err = of_property_read_u32_index(np, "st,syscon", 2, 
>>>>>> &dwmac->mode_mask);
>>>>>> +    if (err)
>>>>>> +        dev_dbg(dev, "Warning sysconfig register mask not set\n");
>>>>>
>>>>> Isn't this an error , so dev_err() ?
>>>> No, it is only "warning" information, for MP1 the mask is not 
>>>> needed (and for backward compatibility is not planned to put mask 
>>>> parameter mandatory)
>>>
>>> Should this be an error for anything newer than MP15 then ?
>> For MP25, no need of mask, so for moment it is specific to MP13.
>
> Make this a warning for MP15, error for MP13, do not check st,syscon 
> presence for MP2 at all. Would that work ?
Ok I will make a warning for MP15 and MP25 and error for MP13.