A test with the command below gives this error:
mmc@fe310000: rockchip,txclk-tapnum: missing size tag in [[8]]
With this added to a dts file:
rockchip,txclk-tapnum = <0x8>;
A look at the driver shows that:
DLL_TXCLK_TAPNUM_DEFAULT 0x8
Adding the default value to the dts files is not needed.
Every clock is divided into 32 taps equally and
the max value is 31.
Fix rockchip,txclk-tapnum property in snps,dwcmshc-sdhci.yaml by
adding a minimum, maximum and default.
In the driver the function of_property_read_u8() is used,
but with dtbs_check the notifications only disappear in YAML
by changing uint8 to uint32.
The driver has no limit check for rockchip,txclk-tapnum.
make ARCH=arm64 dtbs_check
Signed-off-by: Johan Jonker <[email protected]>
---
Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
index e6c9a2f77..f43d8d829 100644
--- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
+++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
@@ -48,7 +48,10 @@ properties:
rockchip,txclk-tapnum:
description: Specify the number of delay for tx sampling.
- $ref: /schemas/types.yaml#/definitions/uint8
+ $ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 0
+ maximum: 31
+ default: 8
required:
--
2.11.0
On Fri, Apr 30, 2021 at 2:21 AM Johan Jonker <[email protected]> wrote:
>
> A test with the command below gives this error:
> mmc@fe310000: rockchip,txclk-tapnum: missing size tag in [[8]]
>
> With this added to a dts file:
> rockchip,txclk-tapnum = <0x8>;
>
> A look at the driver shows that:
> DLL_TXCLK_TAPNUM_DEFAULT 0x8
>
> Adding the default value to the dts files is not needed.
> Every clock is divided into 32 taps equally and
> the max value is 31.
>
> Fix rockchip,txclk-tapnum property in snps,dwcmshc-sdhci.yaml by
> adding a minimum, maximum and default.
>
> In the driver the function of_property_read_u8() is used,
> but with dtbs_check the notifications only disappear in YAML
> by changing uint8 to uint32.
Did you check what value the driver sees when you put 8 in the dts. I
expect it will be 0 because you are reading the 1st byte in big
endian.
> The driver has no limit check for rockchip,txclk-tapnum.
>
> make ARCH=arm64 dtbs_check
>
> Signed-off-by: Johan Jonker <[email protected]>
> ---
> Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> index e6c9a2f77..f43d8d829 100644
> --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
> @@ -48,7 +48,10 @@ properties:
>
> rockchip,txclk-tapnum:
> description: Specify the number of delay for tx sampling.
> - $ref: /schemas/types.yaml#/definitions/uint8
> + $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 0
> + maximum: 31
> + default: 8
>
>
> required:
> --
> 2.11.0
>
On 5/4/21 12:40 AM, Rob Herring wrote:
> On Fri, Apr 30, 2021 at 2:21 AM Johan Jonker <[email protected]> wrote:
>>
>> A test with the command below gives this error:
>> mmc@fe310000: rockchip,txclk-tapnum: missing size tag in [[8]]
>>
>> With this added to a dts file:
>> rockchip,txclk-tapnum = <0x8>;
>>
>> A look at the driver shows that:
>> DLL_TXCLK_TAPNUM_DEFAULT 0x8
>>
>> Adding the default value to the dts files is not needed.
>> Every clock is divided into 32 taps equally and
>> the max value is 31.
>>
>> Fix rockchip,txclk-tapnum property in snps,dwcmshc-sdhci.yaml by
>> adding a minimum, maximum and default.
>>
>> In the driver the function of_property_read_u8() is used,
>> but with dtbs_check the notifications only disappear in YAML
>> by changing uint8 to uint32.
>
> Did you check what value the driver sees when you put 8 in the dts. I
> expect it will be 0 because you are reading the 1st byte in big
> endian.
>
Hi,
Thanks for info.
A test with a dummy module shows that of_property_read_u8() read a 0
instead of 8.
There are approx 23 of_property_read_u8() functions in use in the
mainline kernel. Do they need a recheck?
Johan
=== other Rockchip recheck examples
drivers/mmc/host/sdhci-of-dwcmshc.c
if (of_property_read_u8(mmc_dev(host->mmc)->of_node,
"rockchip,txclk-tapnum",
&priv->txclk_tapnum))
drivers/pci/controller/pcie-rockchip-ep.c
err = of_property_read_u8(dev->of_node, "max-functions",
&ep->epc->max_functions);
=== dts test
dummy {
compatible = "dummy";
rockchip,dummy0x8 = <0x8>;
rockchip,dummy0x11223344 = <0x11223344>;
};
=== driver probe
of_property_read_u8(np, "rockchip,dummy0x8", &tmp8);
printk("u8 : 0x%08x\n", tmp8);
of_property_read_u32(np, "rockchip,dummy0x8", &tmp32);
printk("u32: 0x%08x\n", tmp32);
of_property_read_u8(np, "rockchip,dummy0x11223344", &tmp8);
printk("u8 : 0x%08x\n", tmp8);
of_property_read_u32(np, "rockchip,dummy0x11223344", &tmp32);
printk("u32: 0x%08x\n", tmp32);
=== dmesg
[ 18.089694] u8 : 0x00000000
[ 18.090123] u32: 0x00000008
[ 18.090494] u8 : 0x00000011
[ 18.090865] u32: 0x11223344
>> The driver has no limit check for rockchip,txclk-tapnum.
>>
>> make ARCH=arm64 dtbs_check
>>
>> Signed-off-by: Johan Jonker <[email protected]>
>> ---
>> Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>> index e6c9a2f77..f43d8d829 100644
>> --- a/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>> +++ b/Documentation/devicetree/bindings/mmc/snps,dwcmshc-sdhci.yaml
>> @@ -48,7 +48,10 @@ properties:
>>
>> rockchip,txclk-tapnum:
>> description: Specify the number of delay for tx sampling.
>> - $ref: /schemas/types.yaml#/definitions/uint8
>> + $ref: /schemas/types.yaml#/definitions/uint32
>> + minimum: 0
>> + maximum: 31
>> + default: 8
>>
>>
>> required:
>> --
>> 2.11.0
>>