Update the name of property 'clk_csr' as 'snps,clk-csr' to align with
the property name in the binding file.
Signed-off-by: Jianguo Zhang <[email protected]>
---
drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
index 9f5cac4000da..18f9952d667f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
@@ -444,7 +444,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
* or get clk_csr from device tree.
*/
plat->clk_csr = -1;
- of_property_read_u32(np, "clk_csr", &plat->clk_csr);
+ of_property_read_u32(np, "snps,clk-csr", &plat->clk_csr);
/* "snps,phy-addr" is not a standard property. Mark it as deprecated
* and warn of its use. Remove this when phy node support is added.
--
2.25.1
Il 23/09/22 07:28, Jianguo Zhang ha scritto:
> Update the name of property 'clk_csr' as 'snps,clk-csr' to align with
> the property name in the binding file.
>
> Signed-off-by: Jianguo Zhang <[email protected]>
> ---
> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> index 9f5cac4000da..18f9952d667f 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
> @@ -444,7 +444,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
> * or get clk_csr from device tree.
> */
> plat->clk_csr = -1;
> - of_property_read_u32(np, "clk_csr", &plat->clk_csr);
> + of_property_read_u32(np, "snps,clk-csr", &plat->clk_csr);
This is going to break MT2712e on old devicetrees.
The right way of doing that is to check the return value of of_property_read_u32()
for "snps,clk-csr": if the property is not found, fall back to the old "clk_csr".
Regards,
Angelo
>
> /* "snps,phy-addr" is not a standard property. Mark it as deprecated
> * and warn of its use. Remove this when phy node support is added.
On 23/09/2022 20:14, Krzysztof Kozlowski wrote:
>> This is going to break MT2712e on old devicetrees.
>>
>> The right way of doing that is to check the return value of of_property_read_u32()
>> for "snps,clk-csr": if the property is not found, fall back to the old "clk_csr".
>
> I must admit - I don't care. That's the effect when submitter bypasses
> DT bindings review (81311c03ab4d ("net: ethernet: stmmac: add management
> of clk_csr property")).
>
> If anyone wants ABI, please document the properties.
>
> If out-of-tree users complain, please upstream your DTS or do not use
> undocumented features...
>
OTOH, as Angelo pointed out, handling old and new properties is quite
easy to achieve, so... :)
Best regards,
Krzysztof
On 23/09/2022 11:10, AngeloGioacchino Del Regno wrote:
> Il 23/09/22 07:28, Jianguo Zhang ha scritto:
>> Update the name of property 'clk_csr' as 'snps,clk-csr' to align with
>> the property name in the binding file.
>>
>> Signed-off-by: Jianguo Zhang <[email protected]>
>> ---
>> drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index 9f5cac4000da..18f9952d667f 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -444,7 +444,7 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>> * or get clk_csr from device tree.
>> */
>> plat->clk_csr = -1;
>> - of_property_read_u32(np, "clk_csr", &plat->clk_csr);
>> + of_property_read_u32(np, "snps,clk-csr", &plat->clk_csr);
>
> This is going to break MT2712e on old devicetrees.
>
> The right way of doing that is to check the return value of of_property_read_u32()
> for "snps,clk-csr": if the property is not found, fall back to the old "clk_csr".
I must admit - I don't care. That's the effect when submitter bypasses
DT bindings review (81311c03ab4d ("net: ethernet: stmmac: add management
of clk_csr property")).
If anyone wants ABI, please document the properties.
If out-of-tree users complain, please upstream your DTS or do not use
undocumented features...
Best regards,
Krzysztof
Dear Krzysztof,
Thanks for your comment.
On Fri, 2022-09-23 at 20:15 +0200, Krzysztof Kozlowski wrote:
> On 23/09/2022 20:14, Krzysztof Kozlowski wrote:
> > > This is going to break MT2712e on old devicetrees.
> > >
> > > The right way of doing that is to check the return value of
> > > of_property_read_u32()
> > > for "snps,clk-csr": if the property is not found, fall back to
> > > the old "clk_csr".
> >
> > I must admit - I don't care. That's the effect when submitter
> > bypasses
> > DT bindings review (81311c03ab4d ("net: ethernet: stmmac: add
> > management
> > of clk_csr property")).
> >
> > If anyone wants ABI, please document the properties.
> >
> > If out-of-tree users complain, please upstream your DTS or do not
> > use
> > undocumented features...
> >
>
> OTOH, as Angelo pointed out, handling old and new properties is quite
> easy to achieve, so... :)
>
So, the conclusion is as following:
1. add new property 'snps,clk-csr' and document it in binding file.
2. parse new property 'snps,clk-csr' firstly, if failed, fall back to
old property 'clk_csr' in driver.
Is my understanding correct?
> Best regards,
> Krzysztof
>
BRS
Jianguo
Dear Krzysztof,
Thanks for your comment.
On Fri, 2022-09-23 at 20:15 +0200, Krzysztof Kozlowski wrote:
> On 23/09/2022 20:14, Krzysztof Kozlowski wrote:
> > > This is going to break MT2712e on old devicetrees.
> > >
> > > The right way of doing that is to check the return value of
> > > of_property_read_u32()
> > > for "snps,clk-csr": if the property is not found, fall back to
> > > the old "clk_csr".
> >
> > I must admit - I don't care. That's the effect when submitter
> > bypasses
> > DT bindings review (81311c03ab4d ("net: ethernet: stmmac: add
> > management
> > of clk_csr property")).
> >
> > If anyone wants ABI, please document the properties.
> >
> > If out-of-tree users complain, please upstream your DTS or do not
> > use
> > undocumented features...
> >
>
> OTOH, as Angelo pointed out, handling old and new properties is quite
> easy to achieve, so... :)
>
So, the conclusion is as following:
1. add new property 'snps,clk-csr' and document it in binding file.
2. parse new property 'snps,clk-csr' firstly, if failed, fall back to
old property 'clk_csr' in driver.
Is my understanding correct?
> Best regards,
> Krzysztof
>
BRS
Jianguo
Il 27/09/22 10:44, Jianguo Zhang ha scritto:
> Dear Krzysztof,
> Thanks for your comment.
>
> On Fri, 2022-09-23 at 20:15 +0200, Krzysztof Kozlowski wrote:
>> On 23/09/2022 20:14, Krzysztof Kozlowski wrote:
>>>> This is going to break MT2712e on old devicetrees.
>>>>
>>>> The right way of doing that is to check the return value of
>>>> of_property_read_u32()
>>>> for "snps,clk-csr": if the property is not found, fall back to
>>>> the old "clk_csr".
>>>
>>> I must admit - I don't care. That's the effect when submitter
>>> bypasses
>>> DT bindings review (81311c03ab4d ("net: ethernet: stmmac: add
>>> management
>>> of clk_csr property")).
>>>
>>> If anyone wants ABI, please document the properties.
>>>
>>> If out-of-tree users complain, please upstream your DTS or do not
>>> use
>>> undocumented features...
>>>
>>
>> OTOH, as Angelo pointed out, handling old and new properties is quite
>> easy to achieve, so... :)
>>
> So, the conclusion is as following:
>
> 1. add new property 'snps,clk-csr' and document it in binding file.
> 2. parse new property 'snps,clk-csr' firstly, if failed, fall back to
> old property 'clk_csr' in driver.
>
> Is my understanding correct?
Yes, please.
I think that bindings should also get a 'clk_csr' with deprecated: true,
but that's Krzysztof's call.
Regards,
Angelo
>
>> Best regards,
>> Krzysztof
>>
> BRS
> Jianguo
>
On 27/09/2022 12:44, AngeloGioacchino Del Regno wrote:
>>> OTOH, as Angelo pointed out, handling old and new properties is quite
>>> easy to achieve, so... :)
>>>
>> So, the conclusion is as following:
>>
>> 1. add new property 'snps,clk-csr' and document it in binding file.
>> 2. parse new property 'snps,clk-csr' firstly, if failed, fall back to
>> old property 'clk_csr' in driver.
>>
>> Is my understanding correct?
>
> Yes, please.
>
> I think that bindings should also get a 'clk_csr' with deprecated: true,
> but that's Krzysztof's call.
The property was never documented, so I think we can skip it as deprecated.
Best regards,
Krzysztof
Dear Krzysztof,
Thanks for your comment.
On Wed, 2022-09-28 at 09:17 +0200, Krzysztof Kozlowski wrote:
> On 27/09/2022 12:44, AngeloGioacchino Del Regno wrote:
>
> > > > OTOH, as Angelo pointed out, handling old and new properties is
> > > > quite
> > > > easy to achieve, so... :)
> > > >
> > >
> > > So, the conclusion is as following:
> > >
> > > 1. add new property 'snps,clk-csr' and document it in binding
> > > file.
> > > 2. parse new property 'snps,clk-csr' firstly, if failed, fall
> > > back to
> > > old property 'clk_csr' in driver.
> > >
> > > Is my understanding correct?
> >
> > Yes, please.
> >
> > I think that bindings should also get a 'clk_csr' with deprecated:
> > true,
> > but that's Krzysztof's call.
>
> The property was never documented, so I think we can skip it as
> deprecated.
>
We will send next version patches according to the conclusion.
> Best regards,
> Krzysztof
>
BRS
Jianguo