Re: [PATCH v2] mx6cuboxi: Fix Ethernet after DT sync with Linux

2024-04-02 Thread Josua Mayer
Hi Fabio,

Am 01.04.24 um 12:13 schrieb Christian Gmeiner:
> Hi Fabio
>
>> From: Josua Mayer 
>>
>> The i.MX6 Cubox-i and HummingBoards can have different PHYs at varying
>> addresses. U-Boot needs to auto-detect which phy is actually present,
>> and at which address it is responding.
>>
>> Auto-detection from multiple phy nodes specified in device-tree does not
>> currently work correct. As a work-around merge all three possible phys
>> into one node with the special address 0x which indicates to the
>> generic phy driver to probe all addresses.
>>
>> Signed-off-by: Josua Mayer 
>> [fabio: Added the changes to imx6qdl-sr-som-u-boot.dtsi.]
>> Signed-off-by: Fabio Estevam 
>> Tested-by: Christian Gmeiner 
>> ---
>> Changes since v1:
>> - Disable ethernet-phy at addresses 0, 1 and 4.
>> - Remove the fixup of the fake 0xff address before booting Linux.
>>
>> Josua and Christian,
>>
>> I got access to a imx6 humming board and I was able to test it.
>>
>> This is the minimal fix I came up based on your suggestions.
>>
>> There is no need to fixup of the fake 0xff address before booting Linux,
>> as this fake address does not exist in Linux.
>>
>> Successfully tested Ethernet in U-Boot and in the kernel.
>>
>> Given that Ethernet is currently broken, I suggest we go with this
>> version to restore Ethernet for 2024.04.
>>
>> What do you think?
>>
> I am happy with the patch and love the idea to fix Ethernet for 2024.04.
>
> Tested-by: Christian Gmeiner 
>
Patch looks good to me, too (however I don't have the hardware handy
for testing version with new phy)!


Re: [PATCH] mx6cuboxi: Fix Ethernet after DT sync with Linux

2024-03-28 Thread Josua Mayer
Am 28.03.24 um 13:51 schrieb Josua Mayer:
> Am 28.03.24 um 13:21 schrieb Fabio Estevam:
>> From: Josua Mayer 
>>
>> The i.MX6 Cubox-i and HummingBoards can have different PHYs at varying
>> addresses. U-Boot needs to auto-detect which phy is actually present,
>> and at which address it is responding.
>>
>> Auto-detection from multiple phy nodes specified in device-tree does not
>> currently work correct. As a work-around merge all three possible phys
>> into one node with the special address 0x which indicates to the
>> generic phy driver to probe all addresses.
>> Also fixup this fake address before booting Linux, *if* booting with
>> U-Boot's internal dtb.
>>
>> Signed-off-by: Josua Mayer 
>> [fabio: Added the changes to imx6qdl-sr-som-u-boot.dtsi.]
>> Signed-off-by: Fabio Estevam 
>> Tested-by: Christian Gmeiner 
>> ---
>>  ...qdl-hummingboard2-emmc-som-v15-u-boot.dtsi |  1 +
>>  arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi   | 40 +++
>>  board/solidrun/mx6cuboxi/mx6cuboxi.c  |  8 +++-
>>  3 files changed, 48 insertions(+), 1 deletion(-)
>>  create mode 100644 arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi
>>
>> diff --git a/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi 
>> b/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi
>> index e9b188ed6587..358cf8abc4ff 100644
>> --- a/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi
>> +++ b/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi
>> @@ -1,6 +1,7 @@
>>  // SPDX-License-Identifier: GPL-2.0+
>>  
>>  #include "imx6qdl-u-boot.dtsi"
>> +#include "imx6qdl-sr-som-u-boot.dtsi"
>>  
>>  / {
>>  board-detect {
>> diff --git a/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi 
>> b/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi
>> new file mode 100644
>> index ..4c5f043ea92a
>> --- /dev/null
>> +++ b/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi
>> @@ -0,0 +1,40 @@
>> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
>> +
>> +#include 
>> +
>> + {
>> +pinctrl-names = "default";
>> +pinctrl-0 = <_microsom_enet_ar8035>;
>> +phy-handle = <>;
>> +phy-mode = "rgmii-id";
>> +
>> +/*
>> + * The PHY seems to require a long-enough reset duration to avoid
>> + * some rare issues where the PHY gets stuck in an inconsistent and
>> + * non-functional state at boot-up. 10ms proved to be fine .
>> + */
>> +phy-reset-duration = <10>;
>> +phy-reset-gpios = < 15 GPIO_ACTIVE_LOW>;
>> +status = "okay";
>> +
>> +mdio {
>> +#address-cells = <1>;
>> +#size-cells = <0>;
>> +
>> +/delete-node/ ethernet-phy@1;
>> +/delete-node/ ethernet-phy@4;
I suggest changing their status to disabled, and keeping the nodes.
>> +
>> +phy: ethernet-phy@0 {
> This node name is shared with upstream imx6qdl-sr-som.dtsi
Give this one a u-boot-only internal name, maybe ethernet-phy@ff
>> +/*
>> + * The PHY can appear either:
>> + * - AR8035: at address 0 or 4
>> + * - ADIN1300: at address 1
>> + * Actual address being detected at runtime.
>> + */
>> +reg = <0x>;
>> +qca,clk-out-frequency = <12500>;
>> +qca,smarteee-tw-us-1g = <24>;
>> +adi,phy-output-clock = "125mhz-free-running";
>> +};
>> +};
>> +};
>> diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c 
>> b/board/solidrun/mx6cuboxi/mx6cuboxi.c
>> index 8edabf4404c2..fbab39e800a6 100644
>> --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
>> +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
>> @@ -447,7 +447,7 @@ static int find_ethernet_phy(void)
>>   */
>>  int ft_board_setup(void *fdt, struct bd_info *bd)
>>  {
>> -int node_phy0, node_phy1, node_phy4;
>> +int node_phy, node_phy0, node_phy1, node_phy4;
>>  int ret, phy;
>>  bool enable_phy0 = false, enable_phy1 = false, enable_phy4 = false;
>>  enum board_type board;
>> @@ -479,6 +479,12 @@ int ft_board_setup(void *fdt, struct bd_info *bd)
>>  return 0;
>>  }
>>  
>> +// update U-Boot's own unified phy node phy address, if present
>> +node_phy = fd

Re: [PATCH] mx6cuboxi: Fix Ethernet after DT sync with Linux

2024-03-28 Thread Josua Mayer

Am 28.03.24 um 13:21 schrieb Fabio Estevam:
> From: Josua Mayer 
>
> The i.MX6 Cubox-i and HummingBoards can have different PHYs at varying
> addresses. U-Boot needs to auto-detect which phy is actually present,
> and at which address it is responding.
>
> Auto-detection from multiple phy nodes specified in device-tree does not
> currently work correct. As a work-around merge all three possible phys
> into one node with the special address 0x which indicates to the
> generic phy driver to probe all addresses.
> Also fixup this fake address before booting Linux, *if* booting with
> U-Boot's internal dtb.
>
> Signed-off-by: Josua Mayer 
> [fabio: Added the changes to imx6qdl-sr-som-u-boot.dtsi.]
> Signed-off-by: Fabio Estevam 
> Tested-by: Christian Gmeiner 
> ---
>  ...qdl-hummingboard2-emmc-som-v15-u-boot.dtsi |  1 +
>  arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi   | 40 +++
>  board/solidrun/mx6cuboxi/mx6cuboxi.c  |  8 +++-
>  3 files changed, 48 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi
>
> diff --git a/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi 
> b/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi
> index e9b188ed6587..358cf8abc4ff 100644
> --- a/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi
> +++ b/arch/arm/dts/imx6qdl-hummingboard2-emmc-som-v15-u-boot.dtsi
> @@ -1,6 +1,7 @@
>  // SPDX-License-Identifier: GPL-2.0+
>  
>  #include "imx6qdl-u-boot.dtsi"
> +#include "imx6qdl-sr-som-u-boot.dtsi"
>  
>  / {
>   board-detect {
> diff --git a/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi 
> b/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi
> new file mode 100644
> index ..4c5f043ea92a
> --- /dev/null
> +++ b/arch/arm/dts/imx6qdl-sr-som-u-boot.dtsi
> @@ -0,0 +1,40 @@
> +// SPDX-License-Identifier: (GPL-2.0 OR MIT)
> +
> +#include 
> +
> + {
> + pinctrl-names = "default";
> + pinctrl-0 = <_microsom_enet_ar8035>;
> + phy-handle = <>;
> + phy-mode = "rgmii-id";
> +
> + /*
> +  * The PHY seems to require a long-enough reset duration to avoid
> +  * some rare issues where the PHY gets stuck in an inconsistent and
> +  * non-functional state at boot-up. 10ms proved to be fine .
> +  */
> + phy-reset-duration = <10>;
> + phy-reset-gpios = < 15 GPIO_ACTIVE_LOW>;
> + status = "okay";
> +
> + mdio {
> + #address-cells = <1>;
> + #size-cells = <0>;
> +
> + /delete-node/ ethernet-phy@1;
> + /delete-node/ ethernet-phy@4;
> +
> + phy: ethernet-phy@0 {
This node name is shared with upstream imx6qdl-sr-som.dtsi
> + /*
> +  * The PHY can appear either:
> +  * - AR8035: at address 0 or 4
> +  * - ADIN1300: at address 1
> +  * Actual address being detected at runtime.
> +  */
> + reg = <0x>;
> + qca,clk-out-frequency = <12500>;
> + qca,smarteee-tw-us-1g = <24>;
> + adi,phy-output-clock = "125mhz-free-running";
> + };
> + };
> +};
> diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c 
> b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> index 8edabf4404c2..fbab39e800a6 100644
> --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
> +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> @@ -447,7 +447,7 @@ static int find_ethernet_phy(void)
>   */
>  int ft_board_setup(void *fdt, struct bd_info *bd)
>  {
> - int node_phy0, node_phy1, node_phy4;
> + int node_phy, node_phy0, node_phy1, node_phy4;
>   int ret, phy;
>   bool enable_phy0 = false, enable_phy1 = false, enable_phy4 = false;
>   enum board_type board;
> @@ -479,6 +479,12 @@ int ft_board_setup(void *fdt, struct bd_info *bd)
>   return 0;
>   }
>  
> + // update U-Boot's own unified phy node phy address, if present
> + node_phy = fdt_path_offset(fdt, 
> "/soc/bus@210/ethernet@2188000/mdio/phy");
This node no longer exists, unless you rename the u-boot-specific one.
The u-boot node should probably have its own separate name, to ensure
we do not find the upstream linux dtb ethernet-phy@0 node by mistake.
> + ret = fdt_setprop_u32(fdt, node_phy, "reg", phy);
> + if (ret < 0)
> + pr_err("%s: failed to update unified PHY node address\n", 
> __func__);
> +
>   // update all phy nodes status
>   node_phy0 = fdt_path_offset(fdt, 
> "/soc/bus@210/ethernet@2188000/mdio/ethernet-phy@0");
>   ret = fdt_setprop_string(fdt, node_phy0, "status", enable_phy0 ? "okay" 
> : "disabled");


Re: [PATCH] mx6cuboxi: fix ethernet after synchronise device-tree

2024-03-27 Thread Josua Mayer
Am 27.03.24 um 16:55 schrieb Christian Gmeiner:
> Hi Josua
>
>>> Hi Josua
>>>
>>> My bisect showed me that after a device-tree sync the ethernet broke.
>>>
>>>> please take a look at this patch, I suspect it will (hack-)fix your
>>>> ethernet issue.
>>>>
>>> Yes.. it fixes the problem I am seeing.
>>>
>>>> Unfortunately I had no time to revisit this yet and implement a correct 
>>>> solution.
>>>>
>>> Would it be okay for you if I look into a proper solution?
>> Sure. I am swamped by other products at the moment.
>>
>> However I will provide a rough overview what needs to be done:
>>
>> Background: i.MX6 SoMs originally had a an atheros phy at unstable address,
>> either 0 or 4 depending on electrical noise on floating configuration 
>> signals.
>>
> Would it be possible to set those configuration signals from the MX6
> to a defined state and
> then toggle the PHY reset , to force correct PHY address?
There is no cpu control over the signals responsible for phy address.
>
>> Linux had solved this by placing 2 phy nodes in device-tree.
>> During boot the kernel would attempt in order to probe the phys,
>> then link the successful one to the ethernet netdev.
>> As a side-effect there is always an error in the kernel log for one of the
>> addresses.
>>
>> U-Boot had something similar in that with a special address (I think 0xff)
>> in device-tree, the code will probe mdio bus for all addresses, but only
>> for a single phy node in dts.
>>
>> With release of SoM 2.0 we changed to an analog devices phy at address 1,
>> which most importantly uses a different driver, and requires a different 
>> description
>> in device-tree.
>>
>> When adding this new phy, as a third node in device-tree, kernel maintainers
>> requested a better solution, and we got u-boot to runtime patch dtb to update
>> status properties of the dtb for linux, after probing mdio bus for phys.
>>
> Got it.
>
>> Now - what u-boot needs to do is probe the mdio bus, and then runtime-patch 
>> its own DTB.
>> Either with status properties, or for adding the phy-handle property (not 
>> sure which method will work).
>>
> We could patch DTB in SPL for U-Boot proper but doing a mdio scan in
> SPL looks like a lot of work.
In SPL I agree this would be quite painful.
Maybe it can be done in u-boot proper, at the right moment.
We basically need to intersept the place where ethernet (controller?) driver
is looking for the phy-handle property or probing a phy.

Maybe there is already a call-back for board-specific code,
or one could be added.

> Do
> you know if there is a pull up/down resistor etc. that I could use to
> detect pre SoM 2.0 and SoM 2.0?
>
> Is this what board_type() does?
> Is HummingBoard2 == SoM 2.0?
No, this is carrier-board detection.
For the SoM we have nothing.
PHY mdio address is actually the best indication we have.
>
>> This somehow has to happen after probing mdio driver, but before probing 
>> ethernet driver.
>>
>>> I have a
>>> handful of such devices here
>>> that are already or will be used in a CI farm so I am interested in
>>> using the latest U-Boot for them.
>>>
>>>> sincerely
>>>> Josua Mayer
>>>>
>>>> Am 28.07.22 um 09:08 schrieb Josua Mayer:
>>>>> Please hold off merging this patch until someone tested it, I can not do 
>>>>> so this week.
>>>>> @Tom Can you confirm if this fixes the networking on your Cubox?
>>>>> Also note that the phy-handle property may or may not be required, I am 
>>>>> not sure.
>>>>>
>>>>> sincerely
>>>>> Josua Mayer
>>>>>
>>>>> On Thu, Jul 28, 2022 at 7:05 AM Josua Mayer  wrote:
>>>>>
>>>>> The i.MX6 Cubox-i and HummingBoards can have different PHYs at varying
>>>>> addresses. U-Boot needs to auto-detect which phy is actually present,
>>>>> and at which address it is responding.
>>>>>
>>>>> Auto-detection from multiple phy nodes specified in device-tree does 
>>>>> not
>>>>> currently work correct. As a work-around merge all three possible phys
>>>>> into one node with the special address 0x which indicates to 
>>>>> the
>>>>> generic phy driver to probe all addresses.
>>>>> Also fixup this fake address before booting L

Re: [PATCH] mx6cuboxi: fix ethernet after synchronise device-tree

2024-03-27 Thread Josua Mayer
Am 27.03.24 um 13:17 schrieb Christian Gmeiner:
> Hi Josua
>
> My bisect showed me that after a device-tree sync the ethernet broke.
>
>> please take a look at this patch, I suspect it will (hack-)fix your
>> ethernet issue.
>>
> Yes.. it fixes the problem I am seeing.
>
>> Unfortunately I had no time to revisit this yet and implement a correct 
>> solution.
>>
> Would it be okay for you if I look into a proper solution?

Sure. I am swamped by other products at the moment.

However I will provide a rough overview what needs to be done:

Background: i.MX6 SoMs originally had a an atheros phy at unstable address,
either 0 or 4 depending on electrical noise on floating configuration signals.

Linux had solved this by placing 2 phy nodes in device-tree.
During boot the kernel would attempt in order to probe the phys,
then link the successful one to the ethernet netdev.
As a side-effect there is always an error in the kernel log for one of the
addresses.

U-Boot had something similar in that with a special address (I think 0xff)
in device-tree, the code will probe mdio bus for all addresses, but only
for a single phy node in dts.

With release of SoM 2.0 we changed to an analog devices phy at address 1,
which most importantly uses a different driver, and requires a different 
description
in device-tree.

When adding this new phy, as a third node in device-tree, kernel maintainers
requested a better solution, and we got u-boot to runtime patch dtb to update
status properties of the dtb for linux, after probing mdio bus for phys.


Now - what u-boot needs to do is probe the mdio bus, and then runtime-patch its 
own DTB.
Either with status properties, or for adding the phy-handle property (not sure 
which method will work).

This somehow has to happen after probing mdio driver, but before probing 
ethernet driver.

> I have a
> handful of such devices here
> that are already or will be used in a CI farm so I am interested in
> using the latest U-Boot for them.
>
>> sincerely
>> Josua Mayer
>>
>> Am 28.07.22 um 09:08 schrieb Josua Mayer:
>>> Please hold off merging this patch until someone tested it, I can not do so 
>>> this week.
>>> @Tom Can you confirm if this fixes the networking on your Cubox?
>>> Also note that the phy-handle property may or may not be required, I am not 
>>> sure.
>>>
>>> sincerely
>>> Josua Mayer
>>>
>>> On Thu, Jul 28, 2022 at 7:05 AM Josua Mayer  wrote:
>>>
>>> The i.MX6 Cubox-i and HummingBoards can have different PHYs at varying
>>> addresses. U-Boot needs to auto-detect which phy is actually present,
>>> and at which address it is responding.
>>>
>>> Auto-detection from multiple phy nodes specified in device-tree does not
>>> currently work correct. As a work-around merge all three possible phys
>>> into one node with the special address 0x which indicates to the
>>> generic phy driver to probe all addresses.
>>> Also fixup this fake address before booting Linux, *if* booting with
>>> U-Boot's internal dtb.
>>>
>>> Signed-off-by: Josua Mayer 
>>> Fixes: d0399a46e7cd
>>> ---
>>>  arch/arm/dts/imx6qdl-sr-som.dtsi | 30 +---
>>>  board/solidrun/mx6cuboxi/mx6cuboxi.c |  6 +-
>>>  2 files changed, 14 insertions(+), 22 deletions(-)
>>>
>>> diff --git a/arch/arm/dts/imx6qdl-sr-som.dtsi 
>>> b/arch/arm/dts/imx6qdl-sr-som.dtsi
>>> index ce543e325c..2d7cbc26b3 100644
>>> ---_a/arch/arm/dts/imx6qdl-sr-som.dtsi
>>> +++ b/arch/arm/dts/imx6qdl-sr-som.dtsi
>>> @@ -53,6 +53,7 @@
>>>   {
>>> pinctrl-names = "default";
>>> pinctrl-0 = <_microsom_enet_ar8035>;
>>> +   phy-handle = <>;
>>> phy-mode = "rgmii-id";
>>>
>>> /*
>>> @@ -68,30 +69,17 @@
>>> #address-cells = <1>;
>>> #size-cells = <0>;
>>>
>>> -   /*
>>> -* The PHY can appear at either address 0 or 4 due to 
>>> the
>>> -* configuration (LED) pin not being pulled 
>>> sufficiently.
>>> -*/
>>> -   ethernet-phy@0 {
>>> -   reg = <0>;
>>> +   phy: ethernet-phy@0 {
>>> +   /*
>>> +* The PHY can 

Re: [PATCH] mx6cuboxi: fix ethernet after synchronise device-tree

2024-03-27 Thread Josua Mayer
Cc: christian.gmei...@gmail.com

Hi Christian,

please take a look at this patch, I suspect it will (hack-)fix your
ethernet issue.

Unfortunately I had no time to revisit this yet and implement a correct 
solution.

sincerely
Josua Mayer

Am 28.07.22 um 09:08 schrieb Josua Mayer:
> Please hold off merging this patch until someone tested it, I can not do so 
> this week.
> @Tom Can you confirm if this fixes the networking on your Cubox?
> Also note that the phy-handle property may or may not be required, I am not 
> sure.
>
> sincerely
> Josua Mayer
>
> On Thu, Jul 28, 2022 at 7:05 AM Josua Mayer  wrote:
>
> The i.MX6 Cubox-i and HummingBoards can have different PHYs at varying
> addresses. U-Boot needs to auto-detect which phy is actually present,
> and at which address it is responding.
>
> Auto-detection from multiple phy nodes specified in device-tree does not
> currently work correct. As a work-around merge all three possible phys
> into one node with the special address 0x which indicates to the
> generic phy driver to probe all addresses.
> Also fixup this fake address before booting Linux, *if* booting with
> U-Boot's internal dtb.
>
> Signed-off-by: Josua Mayer 
> Fixes: d0399a46e7cd
> ---
>  arch/arm/dts/imx6qdl-sr-som.dtsi     | 30 +---
>  board/solidrun/mx6cuboxi/mx6cuboxi.c |  6 +-
>  2 files changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm/dts/imx6qdl-sr-som.dtsi 
> b/arch/arm/dts/imx6qdl-sr-som.dtsi
> index ce543e325c..2d7cbc26b3 100644
> ---_a/arch/arm/dts/imx6qdl-sr-som.dtsi
> +++ b/arch/arm/dts/imx6qdl-sr-som.dtsi
> @@ -53,6 +53,7 @@
>   {
>         pinctrl-names = "default";
>         pinctrl-0 = <_microsom_enet_ar8035>;
> +       phy-handle = <>;
>         phy-mode = "rgmii-id";
>
>         /*
> @@ -68,30 +69,17 @@
>                 #address-cells = <1>;
>                 #size-cells = <0>;
>
> -               /*
> -                * The PHY can appear at either address 0 or 4 due to the
> -                * configuration (LED) pin not being pulled sufficiently.
> -                */
> -               ethernet-phy@0 {
> -                       reg = <0>;
> +               phy: ethernet-phy@0 {
> +                       /*
> +                        * The PHY can appear either:
> +                        * - AR8035: at address 0 or 4
> +                        * - ADIN1300: at address 1
> +                        * Actual address being detected at runtime.
> +                        */
> +                       reg = <0x>;
>                         qca,clk-out-frequency = <12500>;
>                         qca,smarteee-tw-us-1g = <24>;
> -               };
> -
> -               ethernet-phy@4 {
> -                       reg = <4>;
> -                       qca,clk-out-frequency = <12500>;
> -                       qca,smarteee-tw-us-1g = <24>;
> -               };
> -
> -               /*
> -                * ADIN1300 (som rev 1.9 or later) is always at address 
> 1. It
> -                * will be enabled automatically by U-Boot if detected.
> -                */
> -               ethernet-phy@1 {
> -                       reg = <1>;
>                         adi,phy-output-clock = "125mhz-free-running";
> -                       status = "disabled";
>                 };
>         };
>  };
> diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c 
> b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> index debf4f6a3b..52172a03b1 100644
> ---_a/board/solidrun/mx6cuboxi/mx6cuboxi.c
> +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> @@ -446,7 +446,7 @@ static int find_ethernet_phy(void)
>   */
>  int ft_board_setup(void *fdt, struct bd_info *bd)
>  {
> -       int node_phy0, node_phy1, node_phy4;
> +       int node_phy, node_phy0, node_phy1, node_phy4;
>         int ret, phy;
>         bool enable_phy0 = false, enable_phy1 = false, enable_phy4 = 
> false;
>         enum board_type board;
> @@ -478,6 +478,10 @@ int ft_board_setup(void *fdt, struct bd_info *bd)
>                 return 0;
>         }
>
> +       // update U-Boot's own unified phy node phy address, if present
> +       node_phy = fdt_path_offset(fdt, 
> "/soc/bus@210/ethernet@2188000/mdio/phy");
> +       ret = fdt_setprop_u32(fdt, node_phy, "reg", phy);
> +
>         // update all phy nodes status
>         node_phy0 = fdt_path_offset(fdt, 
> "/soc/bus@210/ethernet@2188000/mdio/ethernet-phy@0");
>         ret = fdt_setprop_string(fdt, node_phy0, "status", enable_phy0 ? 
> "okay" : "disabled");
> --_
> 2.37.1
>


Re: [PATCH v2 2/3] arm: mvebu: helios4_defconfig: enable setexpr command

2024-02-13 Thread Josua Mayer
Hi Dennis,

Use-case is with separate boot and rootfs partitions to calculate
next partition number as argument for further commands:

setexpr openwrt_rootpart ${distro_bootpart} + 1
part uuid ${devtype} ${devnum}:${openwrt_rootpart} uuid
setenv bootargs ${bootargs} root=PARTUUID=${uuid} rootfstype=auto rootwait

- Josua Mayer

Am 12.02.24 um 21:40 schrieb Dennis Gilmore:
> I am curious about your use cases for this. as ideally all systems use the 
> generic distro boot paths and it is not needed for them.
>
> Dennis
>
> On Fri, Feb 2, 2024 at 9:13 AM Josua Mayer  wrote:
>
> Update the helios4 defconfig to enable the 'setexpr' command, which is a
> default and useful for various complex boot-scripts.
>
> Signed-off-by: Josua Mayer 
> ---
>  configs/helios4_defconfig | 1 -
>  1 file changed, 1 deletion(-)
>
> diff --git a/configs/helios4_defconfig b/configs/helios4_defconfig
> index 04194004f0..73f0638344 100644
> --- a/configs/helios4_defconfig
> +++ b/configs/helios4_defconfig
> @@ -44,7 +44,6 @@ CONFIG_CMD_MMC=y
>  CONFIG_CMD_PCI=y
>  CONFIG_CMD_SPI=y
>  CONFIG_CMD_USB=y
> -# CONFIG_CMD_SETEXPR is not set
>  CONFIG_CMD_TFTPPUT=y
>  CONFIG_CMD_CACHE=y
>  CONFIG_CMD_TIME=y
>
> -- 
> 2.35.3
>
>
>
> -- 
> Dennis Gilmore
> Senior Manager Systems Enablement OpenShift Development


[PATCH v2 3/3] board: helios-4: add config fragment for spi booting

2024-02-02 Thread Josua Mayer
Add a config fragment with required differences for booting from spi
flash instead of sd-card (default).

Settings for environment location are based on vendor u-boot:
https://github.com/kobol-io/u-boot/blob/helios4/include/configs/helios4.h#L59

The fragment can be applied on top of helios4_defconfig by make:
make helios4_defconfig spiboot.config

Signed-off-by: Josua Mayer 
---
 board/kobol/helios4/spiboot.config | 4 
 1 file changed, 4 insertions(+)

diff --git a/board/kobol/helios4/spiboot.config 
b/board/kobol/helios4/spiboot.config
new file mode 100644
index 00..5ffb7d2e3b
--- /dev/null
+++ b/board/kobol/helios4/spiboot.config
@@ -0,0 +1,4 @@
+CONFIG_MVEBU_SPL_BOOT_DEVICE_SPI=y
+# CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC is not set
+CONFIG_ENV_OFFSET=0x10
+CONFIG_ENV_SECT_SIZE=0x1

-- 
2.35.3



[PATCH v2 2/3] arm: mvebu: helios4_defconfig: enable setexpr command

2024-02-02 Thread Josua Mayer
Update the helios4 defconfig to enable the 'setexpr' command, which is a
default and useful for various complex boot-scripts.

Signed-off-by: Josua Mayer 
---
 configs/helios4_defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configs/helios4_defconfig b/configs/helios4_defconfig
index 04194004f0..73f0638344 100644
--- a/configs/helios4_defconfig
+++ b/configs/helios4_defconfig
@@ -44,7 +44,6 @@ CONFIG_CMD_MMC=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_SPI=y
 CONFIG_CMD_USB=y
-# CONFIG_CMD_SETEXPR is not set
 CONFIG_CMD_TFTPPUT=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y

-- 
2.35.3



[PATCH v2 0/3] arm: mvebu: helios-4: add config fragment for spi booting et al

2024-02-02 Thread Josua Mayer
This patch-set introduces a new config fragment for spi booting on kobol
helios-4.

Additionally add minor update for default defconfig to enable setexpr
command, and fix u-boot command access to i2c0 bus.

Signed-off-by: Josua Mayer 
---
Changes in v2:
- replaced new defconfig with config fragment in board directory
  (suggested by Tom Rini )
- Link to v1: 
https://lore.kernel.org/r/20240113-helios4-spi-v1-0-2a5663a10...@solid-run.com

---
Josua Mayer (3):
  arm: dts: armada-38x-solidrun-microsom: configure i2c0 bus
  arm: mvebu: helios4_defconfig: enable setexpr command
  board: helios-4: add config fragment for spi booting

 arch/arm/dts/armada-38x-solidrun-microsom.dtsi | 5 +
 board/kobol/helios4/spiboot.config | 4 
 configs/helios4_defconfig  | 1 -
 3 files changed, 9 insertions(+), 1 deletion(-)
---
base-commit: 050a9b981d6a835133521b599be3ae189ce70f41
change-id: 20240113-helios4-spi-144085c0bbc7

Sincerely,
-- 
Josua Mayer 



[PATCH v2 1/3] arm: dts: armada-38x-solidrun-microsom: configure i2c0 bus

2024-02-02 Thread Josua Mayer
SolidRun Armada-388 SoM has an i2c bus supporting on-som eeprom, and
peripherals on a carrier.
armada-38x.dtsi disables this bus by default, it should be enabled by
som or carrier dts.

Linux has moved i2c0 from helios-4 board dts to som dtsi, including
status, pinctrl and clock speed.
Copy these settings from mainline.

This fixes accessing i2c bus from u-boot commandline.

Signed-off-by: Josua Mayer 
---
 arch/arm/dts/armada-38x-solidrun-microsom.dtsi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/dts/armada-38x-solidrun-microsom.dtsi 
b/arch/arm/dts/armada-38x-solidrun-microsom.dtsi
index f6ae784bed..1540162e03 100644
--- a/arch/arm/dts/armada-38x-solidrun-microsom.dtsi
+++ b/arch/arm/dts/armada-38x-solidrun-microsom.dtsi
@@ -99,6 +99,11 @@
 };
 
  {
+   clock-frequency = <40>;
+   pinctrl-0 = <_pins>;
+   pinctrl-names = "default";
+   status = "okay";
+
eeprom@53 {
compatible = "atmel,24c02";
reg = <0x53>;

-- 
2.35.3



Re: [PATCH 3/3] arm: mvebu: helios-4: add defconfig for spi booting

2024-02-02 Thread Josua Mayer
Am 14.01.24 um 15:37 schrieb Tom Rini:
> On Sun, Jan 14, 2024 at 09:47:31AM +0000, Josua Mayer wrote:
>> Hi Tim,
>>
>> Am 13.01.24 um 18:22 schrieb Tom Rini:
>>> On Sat, Jan 13, 2024 at 05:36:57PM +0100, Josua Mayer wrote:
>>>
>>>> Add a new defconfig based on existing helios4_config file to support
>>>> booting from spi flash.
>>>>
>>>> Settings for environment location are based on vendor u-boot:
>>>> https://github.com/kobol-io/u-boot/blob/helios4/include/configs/helios4.h#L59
>>>>
>>>> A separate defconfig is required because the options are not
>>>> intuitive from menuconfig, numeric values in particular.
>>>>
>>>> Signed-off-by: Josua Mayer 
>>>> ---
>>>>  configs/helios4_spi_defconfig | 81 
>>>> +++
>>>>  1 file changed, 81 insertions(+)
>>> So, a super new thing that might be of interest, but please try this
>>> since I'd like to prove/disprove the use-case.  This could instead be:
>>> configs/helios4_spi_defconfig:
>>> #include "configs/helios4_defconfig"
>>> ... enable/disable options specific to the SPI boot use case ...
>> I will try it, I like the idea conceptually.
> OK.
>
>>> And so now both boards will otherwise be kept in-sync for general config
>>> changes.  It could further be done as:
>>> configs/helios4_spi_defconfig:
>>> #include "configs/helios4_defconfig"
>>> #include "board/kobol/helios4/spiboot.config"
>> Here is potential for making it infinitely complex.
>> E.g. one might argue there should be an armada38x_spi.config
> Yes, and it depends a little on the use cases of the hardware platforms
> what makes the most sense. Today we have some Tegra platforms that
> started out using config fragments, but thinking about it right now
> should perhaps change to this style of defconfig.
Using the fragments avoids adding more defconfig files,
and adding to MAINTAINERS file.
So I attempt this method for v2.

My reasoning would be that boot media related options
are mostly maintainers choices to be made once per product,
and then kept constant. So it fits better in board/ directory than configs/.

> On the other hand, TI
> is intentionally adding feature.config fragments as the "feature.config"
> doesn't change between parts of the K3 family and should also be re-used
> on custom designs to enable that feature. Things are new enough here
> that I want people to experiment and see what works best for their use
> cases rather than providing strict rules just yet.
>


Re: [PATCH 3/3] arm: mvebu: helios-4: add defconfig for spi booting

2024-01-14 Thread Josua Mayer
Hi Tim,

Am 13.01.24 um 18:22 schrieb Tom Rini:
> On Sat, Jan 13, 2024 at 05:36:57PM +0100, Josua Mayer wrote:
>
>> Add a new defconfig based on existing helios4_config file to support
>> booting from spi flash.
>>
>> Settings for environment location are based on vendor u-boot:
>> https://github.com/kobol-io/u-boot/blob/helios4/include/configs/helios4.h#L59
>>
>> A separate defconfig is required because the options are not
>> intuitive from menuconfig, numeric values in particular.
>>
>> Signed-off-by: Josua Mayer 
>> ---
>>  configs/helios4_spi_defconfig | 81 
>> +++
>>  1 file changed, 81 insertions(+)
> So, a super new thing that might be of interest, but please try this
> since I'd like to prove/disprove the use-case.  This could instead be:
> configs/helios4_spi_defconfig:
> #include "configs/helios4_defconfig"
> ... enable/disable options specific to the SPI boot use case ...
I will try it, I like the idea conceptually.
>
> And so now both boards will otherwise be kept in-sync for general config
> changes.  It could further be done as:
> configs/helios4_spi_defconfig:
> #include "configs/helios4_defconfig"
> #include "board/kobol/helios4/spiboot.config"
Here is potential for making it infinitely complex.
E.g. one might argue there should be an armada38x_spi.config
>
> And have board/kobol/helios4/spiboot.config contain the SPI-boot
> specific options.  This path would allow for:
> make helios4_defconfig spiboot.config
> to be a valid alternative way of configuring a build for spiboot.
>


[PATCH 3/3] arm: mvebu: helios-4: add defconfig for spi booting

2024-01-13 Thread Josua Mayer
Add a new defconfig based on existing helios4_config file to support
booting from spi flash.

Settings for environment location are based on vendor u-boot:
https://github.com/kobol-io/u-boot/blob/helios4/include/configs/helios4.h#L59

A separate defconfig is required because the options are not
intuitive from menuconfig, numeric values in particular.

Signed-off-by: Josua Mayer 
---
 configs/helios4_spi_defconfig | 81 +++
 1 file changed, 81 insertions(+)

diff --git a/configs/helios4_spi_defconfig b/configs/helios4_spi_defconfig
new file mode 100644
index 00..6b8c6dfc56
--- /dev/null
+++ b/configs/helios4_spi_defconfig
@@ -0,0 +1,81 @@
+CONFIG_ARM=y
+CONFIG_ARCH_CPU_INIT=y
+CONFIG_SYS_THUMB_BUILD=y
+CONFIG_ARCH_MVEBU=y
+CONFIG_TEXT_BASE=0x0080
+CONFIG_SPL_LIBCOMMON_SUPPORT=y
+CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff
+CONFIG_TARGET_HELIOS4=y
+CONFIG_ENV_OFFSET=0x10
+CONFIG_ENV_SECT_SIZE=0x1
+CONFIG_DEFAULT_DEVICE_TREE="armada-388-helios4"
+CONFIG_SPL_TEXT_BASE=0x4030
+CONFIG_SPL_SERIAL=y
+CONFIG_SPL_STACK=0x4002c000
+CONFIG_SPL=y
+CONFIG_DEBUG_UART_BASE=0xf1012000
+CONFIG_DEBUG_UART_CLOCK=25000
+CONFIG_SYS_LOAD_ADDR=0x80
+CONFIG_PCI=y
+CONFIG_DEBUG_UART=y
+CONFIG_AHCI=y
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_BOOTDELAY=3
+CONFIG_USE_PREBOOT=y
+CONFIG_SYS_CONSOLE_INFO_QUIET=y
+# CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_SPL_MAX_SIZE=0x22fd0
+CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
+CONFIG_SPL_BSS_START_ADDR=0x40023000
+CONFIG_SPL_BSS_MAX_SIZE=0x4000
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
+CONFIG_SPL_I2C=y
+CONFIG_SYS_MAXARGS=32
+CONFIG_CMD_TLV_EEPROM=y
+CONFIG_SPL_CMD_TLV_EEPROM=y
+# CONFIG_CMD_FLASH is not set
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_PCI=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_TFTPPUT=y
+CONFIG_CMD_CACHE=y
+CONFIG_CMD_TIME=y
+CONFIG_ENV_OVERWRITE=y
+CONFIG_ENV_MIN_ENTRIES=128
+CONFIG_ARP_TIMEOUT=200
+CONFIG_NET_RETRY_COUNT=50
+CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_SPL_OF_TRANSLATE=y
+CONFIG_AHCI_MVEBU=y
+CONFIG_DM_PCA953X=y
+CONFIG_DM_I2C=y
+CONFIG_SYS_I2C_MVTWSI=y
+CONFIG_I2C_EEPROM=y
+CONFIG_SPL_I2C_EEPROM=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
+CONFIG_MMC_SDHCI_MV=y
+CONFIG_MTD=y
+CONFIG_SF_DEFAULT_BUS=1
+CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_SPI_FLASH_MTD=y
+CONFIG_PHY_MARVELL=y
+CONFIG_PHY_GIGE=y
+CONFIG_MVNETA=y
+CONFIG_MII=y
+CONFIG_MVMDIO=y
+CONFIG_PCI_MVEBU=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_SYS_NS16550=y
+CONFIG_KIRKWOOD_SPI=y
+CONFIG_USB=y
+CONFIG_USB_XHCI_HCD=y

-- 
2.35.3



[PATCH 0/3] arm: mvebu: helios-4: add defconfig forspi booting et al

2024-01-13 Thread Josua Mayer
This patch-set introduces a new defconfig for spi booting on kobol
helios-4.

Additionally add minor update for default defconfig to enable setexpr
command, and fix u-boot command access to i2c0 bus.

Signed-off-by: Josua Mayer 
---
Josua Mayer (3):
  arm: dts: armada-38x-solidrun-microsom: configure i2c0 bus
  arm: mvebu: helios4_defconfig: enable setexpr command
  arm: mvebu: helios-4: add defconfig for spi booting

 arch/arm/dts/armada-38x-solidrun-microsom.dtsi |  5 ++
 configs/helios4_defconfig  |  1 -
 configs/helios4_spi_defconfig  | 81 ++
 3 files changed, 86 insertions(+), 1 deletion(-)
---
base-commit: 547d3dd28a46a18d59e00a153c8becca8d4e8cf9
change-id: 20240113-helios4-spi-144085c0bbc7

Sincerely,
-- 
Josua Mayer 



[PATCH 2/3] arm: mvebu: helios4_defconfig: enable setexpr command

2024-01-13 Thread Josua Mayer
Update the helios4 defconfig to enable the 'setexpr' command, which is a
default and useful for various complex boot-scripts.

Signed-off-by: Josua Mayer 
---
 configs/helios4_defconfig | 1 -
 1 file changed, 1 deletion(-)

diff --git a/configs/helios4_defconfig b/configs/helios4_defconfig
index 1d8d374fe2..5f568d1695 100644
--- a/configs/helios4_defconfig
+++ b/configs/helios4_defconfig
@@ -44,7 +44,6 @@ CONFIG_CMD_MMC=y
 CONFIG_CMD_PCI=y
 CONFIG_CMD_SPI=y
 CONFIG_CMD_USB=y
-# CONFIG_CMD_SETEXPR is not set
 CONFIG_CMD_TFTPPUT=y
 CONFIG_CMD_CACHE=y
 CONFIG_CMD_TIME=y

-- 
2.35.3



[PATCH 1/3] arm: dts: armada-38x-solidrun-microsom: configure i2c0 bus

2024-01-13 Thread Josua Mayer
SolidRun Armada-388 SoM has an i2c bus supporting on-som eeprom, and
peripherals on a carrier.
armada-38x.dtsi disables this bus by default, it should be enabled by
som or carrier dts.

Linux has moved i2c0 from helios-4 board dts to som dtsi, including
status, pinctrl and clock speed.
Copy these settings from mainline.

This fixes accessing i2c bus from u-boot commandline.

Signed-off-by: Josua Mayer 
---
 arch/arm/dts/armada-38x-solidrun-microsom.dtsi | 5 +
 1 file changed, 5 insertions(+)

diff --git a/arch/arm/dts/armada-38x-solidrun-microsom.dtsi 
b/arch/arm/dts/armada-38x-solidrun-microsom.dtsi
index f6ae784bed..1540162e03 100644
--- a/arch/arm/dts/armada-38x-solidrun-microsom.dtsi
+++ b/arch/arm/dts/armada-38x-solidrun-microsom.dtsi
@@ -99,6 +99,11 @@
 };
 
  {
+   clock-frequency = <40>;
+   pinctrl-0 = <_pins>;
+   pinctrl-names = "default";
+   status = "okay";
+
eeprom@53 {
compatible = "atmel,24c02";
reg = <0x53>;

-- 
2.35.3



[PATCH v2 1/2] arm: mvebu: clearfog gtr: add config option to select serdes0 interface

2024-01-12 Thread Josua Mayer
Clearfog GTR has an assembly option for a SATA connector, CON18.
It shares the serdes with mini-pcie connector CON3.

Add new kconfig option to select betweenata and pci, defaulting to pci
as it was previously configured in board-file.

Clearfog GTR connects eth2 / serdes 1 to a 2.5Gbps capable ethernet
switch port. Linux already configures a fixed-link at speed 2500 from
device-tree.
Upgrade serdes 1 rate to 3.125Gbps to support a 2.5Gbps network link on
Clearfog GTR.

Signed-off-by: Josua Mayer 
---
 board/solidrun/clearfog/Kconfig| 19 +++
 board/solidrun/clearfog/clearfog.c | 11 ---
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/board/solidrun/clearfog/Kconfig b/board/solidrun/clearfog/Kconfig
index 60d3921307..b1623038d0 100644
--- a/board/solidrun/clearfog/Kconfig
+++ b/board/solidrun/clearfog/Kconfig
@@ -39,6 +39,25 @@ config CLEARFOG_SFP_25GB
  SGMII connection (requires a supporting SFP). By default, transfer 
speed
  of 1.25 Gbps is used, suitable for a more common 1 Gbps SFP module.
 
+choice CLEARFOG_GTR_SERDES0
+   prompt "Select Clearfog GTR SerDes 0 Function"
+   default CLEARFOG_GTR_SERDES0_PCIE
+   help
+ Select function for SerDes 0 which is shared between CON3 and CON18
+ for either pci-e or sata.
+
+config CLEARFOG_GTR_SERDES0_PCIE
+   bool "PCI-E on CON3"
+   help
+ Configure SerDes 0 for PCI-E to enable CON3 mini-PCI-E connector.
+
+config CLEARFOG_GTR_SERDES0_SATA
+   bool "SATA on CON18"
+   help
+ Configure SerDes 0 for SATA to enable CON18 SATA connector.
+
+endchoice
+
 config ENV_SIZE
hex "Environment Size"
default 0x1
diff --git a/board/solidrun/clearfog/clearfog.c 
b/board/solidrun/clearfog/clearfog.c
index 6fa2fe5fe3..51c5be518a 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -90,9 +90,14 @@ int hws_board_topology_load(struct serdes_map 
**serdes_map_array, u8 *count)
 
/* Apply runtime detection changes */
if (sr_product_is(_tlv_data, "Clearfog GTR")) {
-   board_serdes_map[0].serdes_type = PEX0;
-   board_serdes_map[0].serdes_speed = SERDES_SPEED_5_GBPS;
-   board_serdes_map[0].serdes_mode = PEX_ROOT_COMPLEX_X1;
+   if (IS_ENABLED(CONFIG_CLEARFOG_GTR_SERDES0_SATA)) {
+   /* serdes 0 is sata (like clearfog pro) */
+   } else if (IS_ENABLED(CONFIG_CLEARFOG_GTR_SERDES0_PCIE)) {
+   /* serdes 0 is pci */
+   board_serdes_map[0].serdes_type = PEX0;
+   board_serdes_map[0].serdes_speed = SERDES_SPEED_5_GBPS;
+   board_serdes_map[0].serdes_mode = PEX_ROOT_COMPLEX_X1;
+   }
} else if (sr_product_is(_tlv_data, "Clearfog Pro")) {
/* handle recognized product as noop, no adjustment required */
} else if (sr_product_is(_tlv_data, "Clearfog Base")) {

-- 
2.35.3



[PATCH v2 2/2] board: solidrun: clearfog: fix serdes 1 / eth2 speed for clearfog gtr

2024-01-12 Thread Josua Mayer
Clearfog GTR connects eth2 / serdes 1 to a 2.5Gbps capable ethernet
switch port. Linux already configures a fixed-link at speed 2500 from
device-tree.

Upgrade serdes 1 rate to 3.125Gbps to support a 2.5Gbps link.

Additionally add comments documenting each serdes' function of clearfog
gtr, which are shared with clearfog pro.

Signed-off-by: Josua Mayer 
---
 board/solidrun/clearfog/clearfog.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/board/solidrun/clearfog/clearfog.c 
b/board/solidrun/clearfog/clearfog.c
index 51c5be518a..6977db0a9e 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -98,6 +98,14 @@ int hws_board_topology_load(struct serdes_map 
**serdes_map_array, u8 *count)
board_serdes_map[0].serdes_speed = SERDES_SPEED_5_GBPS;
board_serdes_map[0].serdes_mode = PEX_ROOT_COMPLEX_X1;
}
+   /* serdes 1 is 2.5Gbps fixed link to ethernet switch */
+   board_serdes_map[1].serdes_type = SGMII1;
+   board_serdes_map[1].serdes_speed = SERDES_SPEED_3_125_GBPS;
+   board_serdes_map[1].serdes_mode = SERDES_DEFAULT_MODE;
+   /* serdes 2 is pci (like clearfog pro) */
+   /* serdes 3 is usb-3 (like clearfog pro) */
+   /* serdes 4 is pci (like clearfog pro) */
+   /* serdes 5 is sfp connector (like clearfog pro) */
} else if (sr_product_is(_tlv_data, "Clearfog Pro")) {
/* handle recognized product as noop, no adjustment required */
} else if (sr_product_is(_tlv_data, "Clearfog Base")) {

-- 
2.35.3



[PATCH v2 0/2] board: solidrun: clearfog gtr: add serdes configuration

2024-01-12 Thread Josua Mayer
Add missing configuration options for clearog gtr serdes:

1. select between sata and pci-e for serdes 0
2. configure serdes 2 for 2.5Gbps link with managed switch

Signed-off-by: Josua Mayer 
---
Changes in v2:
- change choice default logic to remove kconfig warning
- Link to v1: 
https://lore.kernel.org/r/20240106-clearfog-gtr-serdes-v1-0-66cbc350c...@solid-run.com

---
Josua Mayer (2):
  arm: mvebu: clearfog gtr: add config option to select serdes0 interface
  board: solidrun: clearfog: fix serdes 1 / eth2 speed for clearfog gtr

 board/solidrun/clearfog/Kconfig| 19 +++
 board/solidrun/clearfog/clearfog.c | 19 ---
 2 files changed, 35 insertions(+), 3 deletions(-)
---
base-commit: 2ee7a8ec6f1711abe9619fd8765edc16742be9de
change-id: 20240106-clearfog-gtr-serdes-8a927f496bda

Sincerely,
-- 
Josua Mayer 



Re: [PATCH 1/2] arm: mvebu: clearfog gtr: add config option to select serdes0 interface

2024-01-12 Thread Josua Mayer
Hi Stefan,

Am 09.01.24 um 12:45 schrieb Stefan Roese:
>>   +choice CLEARFOG_GTR_SERDES0
>> +    prompt "Select Clearfog GTR SerDes 0 Function"
>> +    help
>> +  Select function for SerDes 0 which is shared between CON3 and CON18
>> +  for either pci-e or sata.
>> +
>> +config CLEARFOG_GTR_SERDES0_PCIE
>> +    bool "PCI-E on CON3"
>> +    default y
>
> After applying this patch I get this warning:
>
> board/solidrun/clearfog/Kconfig:50:warning: defaults for choice values not 
> supported
>
> Could you please take a look? 
Thank you, I was mistaken about where the default setting belongs,  apparently 
it belongs to the "choice" - not its elements.
Will send v2 soon.


[PATCH 2/2] board: solidrun: clearfog: fix serdes 1 / eth2 speed for clearfog gtr

2024-01-06 Thread Josua Mayer
Clearfog GTR connects eth2 / serdes 1 to a 2.5Gbps capable ethernet
switch port. Linux already configures a fixed-link at speed 2500 from
device-tree.

Upgrade serdes 1 rate to 3.125Gbps to support a 2.5Gbps link.

Additionally add comments documenting each serdes' function of clearfog
gtr, which are shared with clearfog pro.

Signed-off-by: Josua Mayer 
---
 board/solidrun/clearfog/clearfog.c | 8 
 1 file changed, 8 insertions(+)

diff --git a/board/solidrun/clearfog/clearfog.c 
b/board/solidrun/clearfog/clearfog.c
index 51c5be518a..6977db0a9e 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -98,6 +98,14 @@ int hws_board_topology_load(struct serdes_map 
**serdes_map_array, u8 *count)
board_serdes_map[0].serdes_speed = SERDES_SPEED_5_GBPS;
board_serdes_map[0].serdes_mode = PEX_ROOT_COMPLEX_X1;
}
+   /* serdes 1 is 2.5Gbps fixed link to ethernet switch */
+   board_serdes_map[1].serdes_type = SGMII1;
+   board_serdes_map[1].serdes_speed = SERDES_SPEED_3_125_GBPS;
+   board_serdes_map[1].serdes_mode = SERDES_DEFAULT_MODE;
+   /* serdes 2 is pci (like clearfog pro) */
+   /* serdes 3 is usb-3 (like clearfog pro) */
+   /* serdes 4 is pci (like clearfog pro) */
+   /* serdes 5 is sfp connector (like clearfog pro) */
} else if (sr_product_is(_tlv_data, "Clearfog Pro")) {
/* handle recognized product as noop, no adjustment required */
} else if (sr_product_is(_tlv_data, "Clearfog Base")) {

-- 
2.35.3



[PATCH 1/2] arm: mvebu: clearfog gtr: add config option to select serdes0 interface

2024-01-06 Thread Josua Mayer
Clearfog GTR has an assembly option for a SATA connector, CON18.
It shares the serdes with mini-pcie connector CON3.

Add new kconfig option to select betweenata and pci, defaulting to pci
as it was previously configured in board-file.

Clearfog GTR connects eth2 / serdes 1 to a 2.5Gbps capable ethernet
switch port. Linux already configures a fixed-link at speed 2500 from
device-tree.
Upgrade serdes 1 rate to 3.125Gbps to support a 2.5Gbps network link on
Clearfog GTR.

Signed-off-by: Josua Mayer 
---
 board/solidrun/clearfog/Kconfig| 19 +++
 board/solidrun/clearfog/clearfog.c | 11 ---
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/board/solidrun/clearfog/Kconfig b/board/solidrun/clearfog/Kconfig
index 60d3921307..765d8a6355 100644
--- a/board/solidrun/clearfog/Kconfig
+++ b/board/solidrun/clearfog/Kconfig
@@ -39,6 +39,25 @@ config CLEARFOG_SFP_25GB
  SGMII connection (requires a supporting SFP). By default, transfer 
speed
  of 1.25 Gbps is used, suitable for a more common 1 Gbps SFP module.
 
+choice CLEARFOG_GTR_SERDES0
+   prompt "Select Clearfog GTR SerDes 0 Function"
+   help
+ Select function for SerDes 0 which is shared between CON3 and CON18
+ for either pci-e or sata.
+
+config CLEARFOG_GTR_SERDES0_PCIE
+   bool "PCI-E on CON3"
+   default y
+   help
+ Configure SerDes 0 for PCI-E to enable CON3 mini-PCI-E connector.
+
+config CLEARFOG_GTR_SERDES0_SATA
+   bool "SATA on CON18"
+   help
+ Configure SerDes 0 for SATA to enable CON18 SATA connector.
+
+endchoice
+
 config ENV_SIZE
hex "Environment Size"
default 0x1
diff --git a/board/solidrun/clearfog/clearfog.c 
b/board/solidrun/clearfog/clearfog.c
index 6fa2fe5fe3..51c5be518a 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -90,9 +90,14 @@ int hws_board_topology_load(struct serdes_map 
**serdes_map_array, u8 *count)
 
/* Apply runtime detection changes */
if (sr_product_is(_tlv_data, "Clearfog GTR")) {
-   board_serdes_map[0].serdes_type = PEX0;
-   board_serdes_map[0].serdes_speed = SERDES_SPEED_5_GBPS;
-   board_serdes_map[0].serdes_mode = PEX_ROOT_COMPLEX_X1;
+   if (IS_ENABLED(CONFIG_CLEARFOG_GTR_SERDES0_SATA)) {
+   /* serdes 0 is sata (like clearfog pro) */
+   } else if (IS_ENABLED(CONFIG_CLEARFOG_GTR_SERDES0_PCIE)) {
+   /* serdes 0 is pci */
+   board_serdes_map[0].serdes_type = PEX0;
+   board_serdes_map[0].serdes_speed = SERDES_SPEED_5_GBPS;
+   board_serdes_map[0].serdes_mode = PEX_ROOT_COMPLEX_X1;
+   }
} else if (sr_product_is(_tlv_data, "Clearfog Pro")) {
/* handle recognized product as noop, no adjustment required */
} else if (sr_product_is(_tlv_data, "Clearfog Base")) {

-- 
2.35.3



[PATCH 0/2] board: solidrun: clearfog gtr: add serdes configuration

2024-01-06 Thread Josua Mayer
Add missing configuration options for clearog gtr serdes:

1. select between sata and pci-e for serdes 0
2. configure serdes 2 for 2.5Gbps link with managed switch

Signed-off-by: Josua Mayer 
---
Josua Mayer (2):
  arm: mvebu: clearfog gtr: add config option to select serdes0 interface
  board: solidrun: clearfog: fix serdes 1 / eth2 speed for clearfog gtr

 board/solidrun/clearfog/Kconfig| 19 +++
 board/solidrun/clearfog/clearfog.c | 19 ---
 2 files changed, 35 insertions(+), 3 deletions(-)
---
base-commit: 82750ce44226e5f2b3bbcd79cf7b3ba3dfd3de4d
change-id: 20240106-clearfog-gtr-serdes-8a927f496bda

Sincerely,
-- 
Josua Mayer 



[PATCH v2 2/2] cmd: mvebu/bubt: move eMMC data-partition uboot from LBA-0 to 4096

2023-10-25 Thread Josua Mayer
A38x bootrom only searches 2 sectors when booting from eMMC,
irregardless of data or boot partition: 0 & 4096.

For eMMC boot partitions sector 0 is fine, but on data partition it
conflicts with MBR.

Change bubt command default to 4096 for eMMC data partition only, to
allow using an MBR partition table on the eMMC data partition while also
booting from it.

Signed-off-by: Josua Mayer 
---
V1 -> V2: fixed build without CONFIG_SUPPORT_EMMC_BOOT

 cmd/mvebu/bubt.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
index ca24a5c1c4b..744b1c20aa8 100644
--- a/cmd/mvebu/bubt.c
+++ b/cmd/mvebu/bubt.c
@@ -240,9 +240,16 @@ static int mmc_burn_image(size_t image_size)
 #endif
 
/* SD reserves LBA-0 for MBR and boots from LBA-1,
-* MMC/eMMC boots from LBA-0
+* MMC/eMMC boots from LBA-0 and LBA-4096
 */
-   start_lba = IS_SD(mmc) ? 1 : 0;
+   if (IS_SD(mmc))
+   start_lba = 1;
+#ifdef CONFIG_SUPPORT_EMMC_BOOT
+   else if (part)
+   start_lba = 0;
+#endif
+   else
+   start_lba = 4096;
 #ifdef CONFIG_BLK
blk_count = image_size / mmc->write_bl_len;
if (image_size % mmc->write_bl_len)
-- 
2.35.3



[PATCH v2 1/2] arm: mvebu: allow additional 4096 offset for bootable mmc image

2023-10-25 Thread Josua Mayer
Disarm the error message forcing u-boot/spl image to be located at
sector 0 on eMMC data-partition and microSD.
Offset 0 makes sense on eMMC boot partitions only, data partition must
use 4096 to avoid conflicting with MBR.

Valid offsets when booting from microSD, reported by boot-rom v1.73:

BootROM: Bad header at offset 0200
BootROM: Bad header at offset 4400
BootROM: Bad header at offset 0020
BootROM: Bad header at offset 0040
BootROM: Bad header at offset 0060
BootROM: Bad header at offset 0080
BootROM: Bad header at offset 00A0
BootROM: Bad header at offset 00C0
BootROM: Bad header at offset 00E0
BootROM: Bad header at offset 0100
BootROM: Bad header at offset 0120
BootROM: Bad header at offset 0140
BootROM: Bad header at offset 0160
BootROM: Bad header at offset 0180
BootROM: Bad header at offset 01A0
BootROM: Bad header at offset 01C0
BootROM: Bad header at offset 01E0
BootROM: Bad header at offset 0200
BootROM: Bad header at offset 0220
BootROM: Bad header at offset 0240
BootROM: Bad header at offset 0260
BootROM: Bad header at offset 0280
BootROM: Bad header at offset 02A0
BootROM: Bad header at offset 02C0
BootROM: Bad header at offset 02E0

Valid offsets when booting from eMMC:

BootROM: Bad header at offset 
BootROM: Bad header at offset 0020
Switching BootPartitions.
BootROM: Bad header at offset 
BootROM: Bad header at offset 0020

Fixes: 2226ca17348 ("arm: mvebu: Load U-Boot proper binary in SPL code based on 
kwbimage header")

Signed-off-by: Josua Mayer 
---
 arch/arm/mach-mvebu/spl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index eaaa68a8564..79f8877745b 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -71,8 +71,9 @@
 #error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR must be set to 0
 #endif
 #if !defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET) || \
-CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0
-#error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET must be set to 0
+(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0 && \
+ CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 4096)
+#error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET must be set to either 
0 or 4096
 #endif
 #endif
 
-- 
2.35.3



[PATCH v2 0/2] mvebu: clearfog: add support for emmc boot

2023-10-25 Thread Josua Mayer
On Armada 388 booting from eMMC is different to SD-Card in two major ways:

- Environment location
- Sectors scanned by Boot-ROM

This patchset first makes it possible to select offset 4096 for
eMMC partition. Here U-Boot can be placed to avoid conflict
conflict with MBR.

Secondly the bubt command is updated to use LBA-4096 for eMMC data
partition only, keeping previous values for SD and boot0/1 unchanged

Changes since v1:

- New defconfigs for environment location are skipped in this version,
  pending further research if it can be auto-detected.

- invert logic of if statement allowing it to compile both with,
  and without CONFIG_SUPPORT_EMMC_BOOT defined.
  Reported by Stefan Roese with turris_mox_defconfig, thanks!

Josua Mayer (2):
  arm: mvebu: allow additional 4096 offset for bootable mmc image
  cmd: mvebu/bubt: move eMMC data-partition uboot from LBA-0 to 4096

 arch/arm/mach-mvebu/spl.c |  5 +++--
 cmd/mvebu/bubt.c  | 11 +--
 2 files changed, 12 insertions(+), 4 deletions(-)

-- 
2.35.3



Re: [PATCH 2/3] cmd: mvebu/bubt: move eMMC data-partition uboot from LBA-0 to 4096

2023-10-25 Thread Josua Mayer

Hi Stefan,

Am 20.10.23 um 11:30 schrieb Stefan Roese:

On 10/8/23 14:46, Josua Mayer wrote:

+    else if (!part)
+    start_lba = 4096;


This patch leads to this error, e.g. for turris_mox_defconfig:

cmd/mvebu/bubt.c: In function 'mmc_burn_image':
cmd/mvebu/bubt.c:247:19: error: 'part' undeclared (first use in this 
function)

  247 | else if (!part)
  |   ^~~~
cmd/mvebu/bubt.c:247:19: note: each undeclared identifier is reported 
only once for each function it appears in


Could you please check and fix?
I will fix guard it the same way the declaration of part variable is 
guarded, and submit v2.


Thanks,
Stefan


Re: [PATCH 3/3] arm: mvebu: clearfog: Add defconfigs for eMMC booting

2023-10-17 Thread Josua Mayer

Hi Stefan,

Am 16.10.23 um 10:37 schrieb Stefan Roese:

Hi Josua,

On 10/8/23 14:46, Josua Mayer wrote:

Armada 388 can boot from either eMMC data, boot0 or boot1 partitions.

data partition requires booting from LBA-4096 to avoid conflict with
MBR. When booting from boot0/boot1 environment should be stored there
respectively, too.

Add 3 new defconfigs, for each eMMC partition:
- clearfog_emmcboot0_defconfig
- clearfog_emmcboot1_defconfig
- clearfog_emmcdata_defconfig


Do we really need new, separate defconfig files for those very similar
configurations? Or is there a way to detect this at runtime

I do not know, Pali Rohár might know.
We can skip patch 3 until some research has been done.

or if not
possible to include a common defconfig file?
So, if the boot partition used to load u-boot by the boot-rom can be 
detected at runtime,

the specialisation of boot0/boot1 configs could be dropped.
That would still leave special case of u-boot offset on eMMC,
which is different between boot and data partitions, and also different 
to SD configuration.


IMO it could be unified by using LBA-4096 as the general default, for 
every place.
But that changes everyones default, and there is no good way to update 
an existing system

(see overlap with MBR partitions).



Just checking...

Thanks,
Stefan

br
Josua



Signed-off-by: Josua Mayer 
---
  configs/clearfog_emmcboot0_defconfig | 82 
  configs/clearfog_emmcboot1_defconfig | 82 
  configs/clearfog_emmcdata_defconfig  | 82 
  3 files changed, 246 insertions(+)
  create mode 100644 configs/clearfog_emmcboot0_defconfig
  create mode 100644 configs/clearfog_emmcboot1_defconfig
  create mode 100644 configs/clearfog_emmcdata_defconfig

diff --git a/configs/clearfog_emmcboot0_defconfig 
b/configs/clearfog_emmcboot0_defconfig

new file mode 100644
index 000..215b7708a0d
--- /dev/null
+++ b/configs/clearfog_emmcboot0_defconfig
@@ -0,0 +1,82 @@
+CONFIG_ARM=y
+CONFIG_ARCH_CPU_INIT=y
+CONFIG_SYS_THUMB_BUILD=y
+CONFIG_ARCH_MVEBU=y
+CONFIG_TEXT_BASE=0x0080
+CONFIG_SPL_LIBCOMMON_SUPPORT=y
+CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff
+CONFIG_TARGET_CLEARFOG=y
+CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC=y
+CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
+CONFIG_SPL_TEXT_BASE=0x4030
+CONFIG_SPL_SERIAL=y
+CONFIG_SPL_STACK=0x4002c000
+CONFIG_SPL=y
+CONFIG_DEBUG_UART_BASE=0xf1012000
+CONFIG_DEBUG_UART_CLOCK=25000
+CONFIG_SYS_LOAD_ADDR=0x80
+CONFIG_PCI=y
+CONFIG_DEBUG_UART=y
+CONFIG_AHCI=y
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_BOOTDELAY=3
+CONFIG_USE_PREBOOT=y
+CONFIG_SYS_CONSOLE_INFO_QUIET=y
+# CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_SPL_MAX_SIZE=0x22fd0
+CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
+CONFIG_SPL_BSS_START_ADDR=0x40023000
+CONFIG_SPL_BSS_MAX_SIZE=0x4000
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
+CONFIG_SPL_I2C=y
+CONFIG_SYS_MAXARGS=32
+CONFIG_CMD_TLV_EEPROM=y
+CONFIG_SPL_CMD_TLV_EEPROM=y
+# CONFIG_CMD_FLASH is not set
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_PCI=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_TFTPPUT=y
+CONFIG_CMD_CACHE=y
+CONFIG_CMD_TIME=y
+CONFIG_ENV_OVERWRITE=y
+CONFIG_ENV_MIN_ENTRIES=128
+CONFIG_SYS_MMC_ENV_PART=1
+CONFIG_ARP_TIMEOUT=200
+CONFIG_NET_RETRY_COUNT=50
+CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_SPL_OF_TRANSLATE=y
+CONFIG_AHCI_MVEBU=y
+CONFIG_DM_PCA953X=y
+CONFIG_DM_I2C=y
+CONFIG_SYS_I2C_MVTWSI=y
+CONFIG_I2C_EEPROM=y
+CONFIG_SPL_I2C_EEPROM=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
+CONFIG_MMC_SDHCI_MV=y
+CONFIG_MTD=y
+CONFIG_SF_DEFAULT_BUS=1
+CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_SPI_FLASH_MTD=y
+CONFIG_PHY_MARVELL=y
+CONFIG_PHY_GIGE=y
+CONFIG_MVNETA=y
+CONFIG_MII=y
+CONFIG_MVMDIO=y
+CONFIG_PCI_MVEBU=y
+CONFIG_SCSI=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_SYS_NS16550=y
+CONFIG_KIRKWOOD_SPI=y
+CONFIG_USB=y
+CONFIG_USB_XHCI_HCD=y
diff --git a/configs/clearfog_emmcboot1_defconfig 
b/configs/clearfog_emmcboot1_defconfig

new file mode 100644
index 000..59f88c64a2e
--- /dev/null
+++ b/configs/clearfog_emmcboot1_defconfig
@@ -0,0 +1,82 @@
+CONFIG_ARM=y
+CONFIG_ARCH_CPU_INIT=y
+CONFIG_SYS_THUMB_BUILD=y
+CONFIG_ARCH_MVEBU=y
+CONFIG_TEXT_BASE=0x0080
+CONFIG_SPL_LIBCOMMON_SUPPORT=y
+CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff
+CONFIG_TARGET_CLEARFOG=y
+CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC=y
+CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
+CONFIG_SPL_TEXT_BASE=0x4030
+CONFIG_SPL_SERIAL=y
+CONFIG_SPL_STACK=0x4002c000
+CONFIG_SPL=y
+CONFIG_DEBUG_UART_BASE=0xf1012000
+CONFIG_DEBUG_UART_CLOCK=25000
+CONFIG_SYS_LOAD_ADDR=0x80
+CONFIG_PCI=y
+CONFIG_DEBUG_UART=y
+CONFIG_AHCI=y
+CONFIG_DISTRO_DEFAULTS=y

[PATCH 2/2] arm: mvebu: clearfog: support 512MB memory size from tlv eeprom

2023-10-08 Thread Josua Mayer
Handle 2GBit memory size value "2" from tlv eeprom on ddr
initialisation, to support SoMs with 512MB ddr memory.

Signed-off-by: Josua Mayer 
---
 board/solidrun/clearfog/clearfog.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/board/solidrun/clearfog/clearfog.c 
b/board/solidrun/clearfog/clearfog.c
index 4f4532b537e..6fa2fe5fe3e 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -159,6 +159,9 @@ struct mv_ddr_topology_map *mv_ddr_topology_map_get(void)
cf_read_tlv_data();
 
switch (cf_tlv_data.ram_size) {
+   case 2:
+   ifp->memory_size = MV_DDR_DIE_CAP_2GBIT;
+   break;
case 4:
default:
ifp->memory_size = MV_DDR_DIE_CAP_4GBIT;
-- 
2.35.3



[PATCH 1/2] arm: mvebu: clearfog: read number of ddr channels from tlv data

2023-10-08 Thread Josua Mayer
Extend the existing tlv vendor extension used for ram size by one byte to
also store the number of ddr channels.
The length of the tlv entry can indicate whether the new information is
present. If not default to single channel.

Signed-off-by: Josua Mayer 
---
 board/solidrun/clearfog/clearfog.c | 14 +-
 board/solidrun/common/tlv_data.c   |  7 ++-
 board/solidrun/common/tlv_data.h   |  1 +
 3 files changed, 20 insertions(+), 2 deletions(-)

diff --git a/board/solidrun/clearfog/clearfog.c 
b/board/solidrun/clearfog/clearfog.c
index 6edb4221551..4f4532b537e 100644
--- a/board/solidrun/clearfog/clearfog.c
+++ b/board/solidrun/clearfog/clearfog.c
@@ -36,7 +36,7 @@ DECLARE_GLOBAL_DATA_PTR;
 #define BOARD_GPP_POL_LOW  0x0
 #define BOARD_GPP_POL_MID  0x0
 
-static struct tlv_data cf_tlv_data;
+static struct tlv_data cf_tlv_data = { 0 };
 
 static void cf_read_tlv_data(void)
 {
@@ -168,6 +168,18 @@ struct mv_ddr_topology_map *mv_ddr_topology_map_get(void)
break;
}
 
+   switch (cf_tlv_data.ram_channels) {
+   default:
+   case 1:
+   for (uint8_t i = 0; i < 5; i++)
+   ifp->as_bus_params[i].cs_bitmask = 0x1;
+   break;
+   case 2:
+   for (uint8_t i = 0; i < 5; i++)
+   ifp->as_bus_params[i].cs_bitmask = 0x3;
+   break;
+   }
+
/* Return the board topology as defined in the board code */
return _topology_map;
 }
diff --git a/board/solidrun/common/tlv_data.c b/board/solidrun/common/tlv_data.c
index 11d6e4a1380..cf5824886c3 100644
--- a/board/solidrun/common/tlv_data.c
+++ b/board/solidrun/common/tlv_data.c
@@ -45,9 +45,14 @@ static void parse_tlv_vendor_ext(struct tlvinfo_tlv 
*tlv_entry,
 
if (val[4] != SR_TLV_CODE_RAM_SIZE)
return;
-   if (tlv_entry->length != 6)
+   if (tlv_entry->length < 6)
return;
td->ram_size = val[5];
+
+   /* extension with additional data field for number of ddr channels */
+   if (tlv_entry->length >= 7) {
+   td->ram_channels = val[6];
+   }
 }
 
 static void parse_tlv_data(u8 *eeprom, struct tlvinfo_header *hdr,
diff --git a/board/solidrun/common/tlv_data.h b/board/solidrun/common/tlv_data.h
index a1432e4b8e1..be3f782ac4a 100644
--- a/board/solidrun/common/tlv_data.h
+++ b/board/solidrun/common/tlv_data.h
@@ -10,6 +10,7 @@ struct tlv_data {
/* Store product name of both SOM and carrier */
char tlv_product_name[2][32];
unsigned int ram_size;
+   uint8_t ram_channels;
 };
 
 void read_tlv_data(struct tlv_data *td);
-- 
2.35.3



[PATCH 0/2] arm: mvebu: clearfog: support additional ddr

2023-10-08 Thread Josua Mayer
This series adds support for additional memory configuration that
were been produced in the past - most notably
a dual-channel configuration with two 512MB modules.

Josua Mayer (2):
  arm: mvebu: clearfog: read number of ddr channels from tlv data
  arm: mvebu: clearfog: support 512MB memory size from tlv eeprom

 board/solidrun/clearfog/clearfog.c | 17 -
 board/solidrun/common/tlv_data.c   |  7 ++-
 board/solidrun/common/tlv_data.h   |  1 +
 3 files changed, 23 insertions(+), 2 deletions(-)

-- 
2.35.3



[PATCH 3/3] arm: mvebu: clearfog: Add defconfigs for eMMC booting

2023-10-08 Thread Josua Mayer
Armada 388 can boot from either eMMC data, boot0 or boot1 partitions.

data partition requires booting from LBA-4096 to avoid conflict with
MBR. When booting from boot0/boot1 environment should be stored there
respectively, too.

Add 3 new defconfigs, for each eMMC partition:
- clearfog_emmcboot0_defconfig
- clearfog_emmcboot1_defconfig
- clearfog_emmcdata_defconfig

Signed-off-by: Josua Mayer 
---
 configs/clearfog_emmcboot0_defconfig | 82 
 configs/clearfog_emmcboot1_defconfig | 82 
 configs/clearfog_emmcdata_defconfig  | 82 
 3 files changed, 246 insertions(+)
 create mode 100644 configs/clearfog_emmcboot0_defconfig
 create mode 100644 configs/clearfog_emmcboot1_defconfig
 create mode 100644 configs/clearfog_emmcdata_defconfig

diff --git a/configs/clearfog_emmcboot0_defconfig 
b/configs/clearfog_emmcboot0_defconfig
new file mode 100644
index 000..215b7708a0d
--- /dev/null
+++ b/configs/clearfog_emmcboot0_defconfig
@@ -0,0 +1,82 @@
+CONFIG_ARM=y
+CONFIG_ARCH_CPU_INIT=y
+CONFIG_SYS_THUMB_BUILD=y
+CONFIG_ARCH_MVEBU=y
+CONFIG_TEXT_BASE=0x0080
+CONFIG_SPL_LIBCOMMON_SUPPORT=y
+CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff
+CONFIG_TARGET_CLEARFOG=y
+CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC=y
+CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
+CONFIG_SPL_TEXT_BASE=0x4030
+CONFIG_SPL_SERIAL=y
+CONFIG_SPL_STACK=0x4002c000
+CONFIG_SPL=y
+CONFIG_DEBUG_UART_BASE=0xf1012000
+CONFIG_DEBUG_UART_CLOCK=25000
+CONFIG_SYS_LOAD_ADDR=0x80
+CONFIG_PCI=y
+CONFIG_DEBUG_UART=y
+CONFIG_AHCI=y
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_BOOTDELAY=3
+CONFIG_USE_PREBOOT=y
+CONFIG_SYS_CONSOLE_INFO_QUIET=y
+# CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_SPL_MAX_SIZE=0x22fd0
+CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
+CONFIG_SPL_BSS_START_ADDR=0x40023000
+CONFIG_SPL_BSS_MAX_SIZE=0x4000
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
+CONFIG_SPL_I2C=y
+CONFIG_SYS_MAXARGS=32
+CONFIG_CMD_TLV_EEPROM=y
+CONFIG_SPL_CMD_TLV_EEPROM=y
+# CONFIG_CMD_FLASH is not set
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_PCI=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_TFTPPUT=y
+CONFIG_CMD_CACHE=y
+CONFIG_CMD_TIME=y
+CONFIG_ENV_OVERWRITE=y
+CONFIG_ENV_MIN_ENTRIES=128
+CONFIG_SYS_MMC_ENV_PART=1
+CONFIG_ARP_TIMEOUT=200
+CONFIG_NET_RETRY_COUNT=50
+CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_SPL_OF_TRANSLATE=y
+CONFIG_AHCI_MVEBU=y
+CONFIG_DM_PCA953X=y
+CONFIG_DM_I2C=y
+CONFIG_SYS_I2C_MVTWSI=y
+CONFIG_I2C_EEPROM=y
+CONFIG_SPL_I2C_EEPROM=y
+CONFIG_MMC_SDHCI=y
+CONFIG_MMC_SDHCI_SDMA=y
+CONFIG_MMC_SDHCI_MV=y
+CONFIG_MTD=y
+CONFIG_SF_DEFAULT_BUS=1
+CONFIG_SPI_FLASH_WINBOND=y
+CONFIG_SPI_FLASH_MTD=y
+CONFIG_PHY_MARVELL=y
+CONFIG_PHY_GIGE=y
+CONFIG_MVNETA=y
+CONFIG_MII=y
+CONFIG_MVMDIO=y
+CONFIG_PCI_MVEBU=y
+CONFIG_SCSI=y
+CONFIG_SPL_DEBUG_UART_BASE=0xd0012000
+CONFIG_DEBUG_UART_SHIFT=2
+CONFIG_SYS_NS16550=y
+CONFIG_KIRKWOOD_SPI=y
+CONFIG_USB=y
+CONFIG_USB_XHCI_HCD=y
diff --git a/configs/clearfog_emmcboot1_defconfig 
b/configs/clearfog_emmcboot1_defconfig
new file mode 100644
index 000..59f88c64a2e
--- /dev/null
+++ b/configs/clearfog_emmcboot1_defconfig
@@ -0,0 +1,82 @@
+CONFIG_ARM=y
+CONFIG_ARCH_CPU_INIT=y
+CONFIG_SYS_THUMB_BUILD=y
+CONFIG_ARCH_MVEBU=y
+CONFIG_TEXT_BASE=0x0080
+CONFIG_SPL_LIBCOMMON_SUPPORT=y
+CONFIG_SPL_LIBGENERIC_SUPPORT=y
+CONFIG_NR_DRAM_BANKS=2
+CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
+CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff
+CONFIG_TARGET_CLEARFOG=y
+CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC=y
+CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
+CONFIG_SPL_TEXT_BASE=0x4030
+CONFIG_SPL_SERIAL=y
+CONFIG_SPL_STACK=0x4002c000
+CONFIG_SPL=y
+CONFIG_DEBUG_UART_BASE=0xf1012000
+CONFIG_DEBUG_UART_CLOCK=25000
+CONFIG_SYS_LOAD_ADDR=0x80
+CONFIG_PCI=y
+CONFIG_DEBUG_UART=y
+CONFIG_AHCI=y
+CONFIG_DISTRO_DEFAULTS=y
+CONFIG_BOOTDELAY=3
+CONFIG_USE_PREBOOT=y
+CONFIG_SYS_CONSOLE_INFO_QUIET=y
+# CONFIG_DISPLAY_BOARDINFO is not set
+CONFIG_DISPLAY_BOARDINFO_LATE=y
+CONFIG_SPL_MAX_SIZE=0x22fd0
+CONFIG_SPL_HAS_BSS_LINKER_SECTION=y
+CONFIG_SPL_BSS_START_ADDR=0x40023000
+CONFIG_SPL_BSS_MAX_SIZE=0x4000
+CONFIG_SPL_SYS_MALLOC_SIMPLE=y
+# CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
+CONFIG_SPL_I2C=y
+CONFIG_SYS_MAXARGS=32
+CONFIG_CMD_TLV_EEPROM=y
+CONFIG_SPL_CMD_TLV_EEPROM=y
+# CONFIG_CMD_FLASH is not set
+CONFIG_CMD_GPIO=y
+CONFIG_CMD_I2C=y
+CONFIG_CMD_MMC=y
+CONFIG_CMD_PCI=y
+CONFIG_CMD_SPI=y
+CONFIG_CMD_USB=y
+CONFIG_CMD_TFTPPUT=y
+CONFIG_CMD_CACHE=y
+CONFIG_CMD_TIME=y
+CONFIG_ENV_OVERWRITE=y
+CONFIG_ENV_MIN_ENTRIES=128
+CONFIG_SYS_MMC_ENV_PART=2
+CONFIG_ARP_TIMEOUT=200
+CONFIG_NET_RETRY_COUNT=50
+CONFIG_NET_RANDOM_ETHADDR=y
+CONFIG_SPL_OF_TRANSLATE=y
+CONFIG_AHCI_MVEBU=y
+CONFIG_DM_PCA953X=y
+CONFIG_DM_I2C=y
+CONFIG_SYS_I2C_MVTWSI=y
+CONFIG_I2C_EEPROM=y
+CONFIG_SPL_I2C_EEPROM=y

[PATCH 1/3] arm: mvebu: allow additional 4096 offset for bootable mmc image

2023-10-08 Thread Josua Mayer
Disarm the error message forcing u-boot/spl image to be located at
sector 0 on eMMC data-partition and microSD.
Offset 0 makes sense on eMMC boot partitions only, data partition must
use 4096 to avoid conflicting with MBR.

Valid offsets when booting from microSD, reported by boot-rom v1.73:

BootROM: Bad header at offset 0200
BootROM: Bad header at offset 4400
BootROM: Bad header at offset 0020
BootROM: Bad header at offset 0040
BootROM: Bad header at offset 0060
BootROM: Bad header at offset 0080
BootROM: Bad header at offset 00A0
BootROM: Bad header at offset 00C0
BootROM: Bad header at offset 00E0
BootROM: Bad header at offset 0100
BootROM: Bad header at offset 0120
BootROM: Bad header at offset 0140
BootROM: Bad header at offset 0160
BootROM: Bad header at offset 0180
BootROM: Bad header at offset 01A0
BootROM: Bad header at offset 01C0
BootROM: Bad header at offset 01E0
BootROM: Bad header at offset 0200
BootROM: Bad header at offset 0220
BootROM: Bad header at offset 0240
BootROM: Bad header at offset 0260
BootROM: Bad header at offset 0280
BootROM: Bad header at offset 02A0
BootROM: Bad header at offset 02C0
BootROM: Bad header at offset 02E0

Valid offsets when booting from eMMC:

BootROM: Bad header at offset 
BootROM: Bad header at offset 0020
Switching BootPartitions.
BootROM: Bad header at offset 
BootROM: Bad header at offset 0020

Fixes: 2226ca17348 ("arm: mvebu: Load U-Boot proper binary in SPL code based on 
kwbimage header")

Signed-off-by: Josua Mayer 
---
 arch/arm/mach-mvebu/spl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/arm/mach-mvebu/spl.c b/arch/arm/mach-mvebu/spl.c
index 379daa88a4d..0dbc12f66ea 100644
--- a/arch/arm/mach-mvebu/spl.c
+++ b/arch/arm/mach-mvebu/spl.c
@@ -71,8 +71,9 @@
 #error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR must be set to 0
 #endif
 #if !defined(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET) || \
-CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0
-#error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET must be set to 0
+(CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 0 && \
+ CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET != 4096)
+#error CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET must be set to either 
0 or 4096
 #endif
 #endif
 
-- 
2.35.3



[PATCH 2/3] cmd: mvebu/bubt: move eMMC data-partition uboot from LBA-0 to 4096

2023-10-08 Thread Josua Mayer
A38x bootrom only searches 2 sectors when booting from eMMC,
irregardless of data or boot partition: 0 & 4096.

For eMMC boot partitions sector 0 is fine, but on data partition it
conflicts with MBR.

Change bubt command default to 4096 for eMMC data partition only, to
allow using an MBR partition table on the eMMC data partition while also
booting from it.

Signed-off-by: Josua Mayer 
---
 cmd/mvebu/bubt.c | 9 +++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/cmd/mvebu/bubt.c b/cmd/mvebu/bubt.c
index ca24a5c1c4b..eb1fef9243d 100644
--- a/cmd/mvebu/bubt.c
+++ b/cmd/mvebu/bubt.c
@@ -240,9 +240,14 @@ static int mmc_burn_image(size_t image_size)
 #endif
 
/* SD reserves LBA-0 for MBR and boots from LBA-1,
-* MMC/eMMC boots from LBA-0
+* MMC/eMMC boots from LBA-0 and LBA-4096
 */
-   start_lba = IS_SD(mmc) ? 1 : 0;
+   if (IS_SD(mmc))
+   start_lba = 1;
+   else if (!part)
+   start_lba = 4096;
+   else
+   start_lba = 0;
 #ifdef CONFIG_BLK
blk_count = image_size / mmc->write_bl_len;
if (image_size % mmc->write_bl_len)
-- 
2.35.3



[PATCH 0/3] mvebu: clearfog: add support for emmc boot

2023-10-08 Thread Josua Mayer
On Armada 388 booting from eMMC is different to SD-Card in two major ways:

- Environment location
- Sectors scanned by Boot-ROM

This patchset first makes it possible to select offset 4096 for
eMMC partition. Here U-Boot can be placed to avoid conflict
conflict with MBR.

Secondly the bubt command is updated to use LBA-4096 for eMMC data
partition only, keeping previous values for SD and boot0/1 unchanged

Finally 3 new defconfigs are added - one for each bootable eMMC partition,
selecting correct offset, and environment partition.

Josua Mayer (3):
  arm: mvebu: allow additional 4096 offset for bootable mmc image
  cmd: mvebu/bubt: move eMMC data-partition uboot from LBA-0 to 4096
  arm: mvebu: clearfog: Add defconfigs for eMMC booting

 arch/arm/mach-mvebu/spl.c|  5 +-
 cmd/mvebu/bubt.c |  9 ++-
 configs/clearfog_emmcboot0_defconfig | 82 
 configs/clearfog_emmcboot1_defconfig | 82 
 configs/clearfog_emmcdata_defconfig  | 82 
 5 files changed, 256 insertions(+), 4 deletions(-)
 create mode 100644 configs/clearfog_emmcboot0_defconfig
 create mode 100644 configs/clearfog_emmcboot1_defconfig
 create mode 100644 configs/clearfog_emmcdata_defconfig

-- 
2.35.3



Re: [PATCH] cmd: tlv_eeprom: fix signature for populate_serial_number function

2023-05-16 Thread Josua Mayer

Hi Heinrich,

Am 16.05.23 um 14:21 schrieb Heinrich Schuchardt:

On 5/16/23 10:27, Josua Mayer wrote:

populate_serial_number is not used internally for the tlv_eeprom
command, but rather provided as a library function for external use..
Remove the devnum that had recently been added by mistake, returning the
function to its original signature.

Instead place a 0-initialised member variable inside the function to
same purpose, along with a node that it only supports reading from the


%s/node/note/

Good find!



first EEPROM in the system.

Fixes: dfda0c0 ("cmd: tlv_eeprom: remove use of global variable 
current_dev")

Signed-off-by: Josua Mayer 
Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 
---
  cmd/tlv_eeprom.c | 3 ++-
  1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 79796394c5c..0ca4d714645 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -1100,11 +1100,12 @@ int mac_read_from_eeprom(void)
   *
   *  This function must be called after relocation.
   */
-int populate_serial_number(int devnum)
+int populate_serial_number(void)


If populate_serial_number() is to be used as a library function, it
should live in lib/ or possibly in drivers/misc/. The definition needs
to be provided in an include file. Otherwise the function should be 
deleted.


Where will this library function be used?
I don't know for sure. GitHub search finds its use in board files on 
some old ONIE u-boot forks.


Main purpose of this patch right now is to undo my accidental change and 
restore the default behaviour:
The previous instance of devnum variable inside tlv_eeprom.c would have 
had value 0 by default.





Shouldn't the EEPROM with the serial number be identified via the
device-tree?

Something like that would be nice.
However we also want to use identical device-tree between Linux and U-Boot.
I am not sure if we have such indication.


Best regards

Heinrich


  {
  char serialstr[257];
  int eeprom_index;
  struct tlvinfo_tlv *eeprom_tlv;
+    int devnum = 0; // TODO: support multiple EEPROMs

  if (env_get("serial#"))
      return 0;




- Josua Mayer



[RFC v2 3/3] cmd: tlv_eeprom: port to new shared tlv library

2023-05-16 Thread Josua Mayer
Rewrite tlv_eeprom command for using the new tlv library, and drop all
unused functions. From this point on, tlv_eeprom command internal
functions shall not be reused externally.

mac_read_from_eeprom & populate_serial_number are kept in place for now
as is, however these probably deserve a new location.

Signed-off-by: Josua Mayer 
---
 cmd/Kconfig|   9 +-
 cmd/tlv_eeprom.c   | 507 +++--
 configs/clearfog_defconfig |   2 +-
 include/tlv_eeprom.h   | 148 ---
 4 files changed, 92 insertions(+), 574 deletions(-)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 9c2149dc881..4b7ea8cd358 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -190,19 +190,14 @@ config CMD_REGINFO
 
 config CMD_TLV_EEPROM
bool "tlv_eeprom"
-   depends on I2C_EEPROM
-   depends on !EEPROM_TLV_LIB
-   select CRC32
+   depends on EEPROM_TLV_LIB
help
  Display and program the system EEPROM data block in ONIE Tlvinfo
  format. TLV stands for Type-Length-Value.
 
 config SPL_CMD_TLV_EEPROM
bool "tlv_eeprom for SPL"
-   depends on SPL_I2C_EEPROM
-   depends on !SPL_EEPROM_TLV_LIB
-   select SPL_DRIVERS_MISC
-   select SPL_CRC32
+   depends on SPL_EEPROM_TLV_LIB
help
  Read system EEPROM data block in ONIE Tlvinfo format from SPL.
 
diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 0ca4d714645..02e1dd88d35 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -7,69 +7,36 @@
  * Copyright (C) 2014 Srideep 
  * Copyright (C) 2013 Miles Tseng 
  * Copyright (C) 2014,2016 david_yang 
+ * Copyright (C) 2023 Josua Mayer 
  */
 
-#include 
 #include 
-#include 
-#include 
-#include 
-#include 
-#include 
+#include 
 #include 
-#include 
-#include 
-#include 
-
-#include "tlv_eeprom.h"
+#include 
+#include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define MAX_TLV_DEVICES2
+#define DEBUG
 
-/* File scope function prototypes */
-static bool is_checksum_valid(u8 *eeprom);
-static int read_eeprom(int devnum, u8 *eeprom);
-static void show_eeprom(int devnum, u8 *eeprom);
 static void decode_tlv(struct tlvinfo_tlv *tlv);
-static void update_crc(u8 *eeprom);
-static int prog_eeprom(int devnum, u8 *eeprom);
-static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
-static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
-static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
+static bool tlvinfo_add_tlv(struct tlvinfo_priv *header, int tcode, char 
*strval);
 static int set_mac(char *buf, const char *string);
 static int set_date(char *buf, const char *string);
 static int set_bytes(char *buf, const char *string, int *converted_accum);
-static void show_tlv_devices(int current_dev);
 
-/* The EERPOM contents after being read into memory */
+/* The EEPROM contents after being read into memory */
 static u8 eeprom[TLV_INFO_MAX_LEN];
 
-static struct udevice *tlv_devices[MAX_TLV_DEVICES];
-
-#define to_header(p) ((struct tlvinfo_header *)p)
-#define to_entry(p) ((struct tlvinfo_tlv *)p)
-
-#define HDR_SIZE sizeof(struct tlvinfo_header)
-#define ENT_SIZE sizeof(struct tlvinfo_tlv)
+static void show_tlv_devices(int current_dev);
 
 static inline bool is_digit(char c)
 {
return (c >= '0' && c <= '9');
 }
 
-/**
- *  is_valid_tlv
- *
- *  Perform basic sanity checks on a TLV field. The TLV is pointed to
- *  by the parameter provided.
- *  1. The type code is not reserved (0x00 or 0xFF)
- */
-static inline bool is_valid_tlv(struct tlvinfo_tlv *tlv)
-{
-   return((tlv->type != 0x00) && (tlv->type != 0xFF));
-}
-
 /**
  *  is_hex
  *
@@ -78,79 +45,8 @@ static inline bool is_valid_tlv(struct tlvinfo_tlv *tlv)
 static inline u8 is_hex(char p)
 {
return (((p >= '0') && (p <= '9')) ||
-   ((p >= 'A') && (p <= 'F')) ||
-   ((p >= 'a') && (p <= 'f')));
-}
-
-/**
- *  is_checksum_valid
- *
- *  Validate the checksum in the provided TlvInfo EEPROM data. First,
- *  verify that the TlvInfo header is valid, then make sure the last
- *  TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
- *  and compare it to the value stored in the EEPROM CRC-32 TLV.
- */
-static bool is_checksum_valid(u8 *eeprom)
-{
-   struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
-   struct tlvinfo_tlv*eeprom_crc;
-   unsigned int   calc_crc;
-   unsigned int   stored_crc;
-
-   // Is the eeprom header valid?
-   if (!is_valid_tlvinfo_header(eeprom_hdr))
-   return false;
-
-   // Is the last TLV a CRC?
-   eeprom_crc = to_entry([HDR_SIZE +
-   be16_to_cpu(eeprom_hdr->totallen) - (ENT_SIZE + 4)]);
-   if (eeprom_crc->type != TLV_CODE_CRC_32 || eeprom_crc->length != 4)
-   return false;
-
-   // Calculate the checksum
-   calc_crc = crc32(

[RFC v2 1/3] lib: add tlv_eeprom library

2023-05-16 Thread Josua Mayer
Create a new tlv library by rewriting parts, and reusing other parts of
the tlv_eeprom command. This library is intended to simplify reading tlv
data during system initialisation from board files, as well as increase
maintainability by defining a clear API and functionally decouple from
the command implementation.

Signed-off-by: Josua Mayer 
Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 
---
 cmd/Kconfig  |   2 +
 include/tlv_eeprom.h | 272 
 lib/Kconfig  |   2 +
 lib/Makefile |   2 +
 lib/tlv/Kconfig  |  18 ++
 lib/tlv/Makefile |   4 +
 lib/tlv/tlv_eeprom.c | 599 +++
 7 files changed, 899 insertions(+)
 create mode 100644 lib/tlv/Kconfig
 create mode 100644 lib/tlv/Makefile
 create mode 100644 lib/tlv/tlv_eeprom.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 365371fb511..9c2149dc881 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -191,6 +191,7 @@ config CMD_REGINFO
 config CMD_TLV_EEPROM
bool "tlv_eeprom"
depends on I2C_EEPROM
+   depends on !EEPROM_TLV_LIB
select CRC32
help
  Display and program the system EEPROM data block in ONIE Tlvinfo
@@ -199,6 +200,7 @@ config CMD_TLV_EEPROM
 config SPL_CMD_TLV_EEPROM
bool "tlv_eeprom for SPL"
depends on SPL_I2C_EEPROM
+   depends on !SPL_EEPROM_TLV_LIB
select SPL_DRIVERS_MISC
select SPL_CRC32
help
diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
index fd45e5f6ebb..b08c98a5833 100644
--- a/include/tlv_eeprom.h
+++ b/include/tlv_eeprom.h
@@ -7,6 +7,8 @@
 #ifndef __TLV_EEPROM_H_
 #define __TLV_EEPROM_H_
 
+#if !defined(CONFIG_EEPROM_TLV_LIB) && !defined(CONFIG_SPL_EEPROM_TLV_LIB)
+
 /*
  *  The Definition of the TlvInfo EEPROM format can be found at onie.org or
  *  github.com/onie
@@ -150,4 +152,274 @@ static inline bool is_valid_tlvinfo_header(struct 
tlvinfo_header *hdr)
(be16_to_cpu(hdr->totallen) <= TLV_TOTAL_LEN_MAX));
 }
 
+#else
+
+/*
+ *  The Definition of the TlvInfo EEPROM format can be found at onie.org or
+ *  github.com/onie
+ */
+
+#include 
+#include 
+#include 
+
+/* tlv library internal state, per each tlv structure */
+struct tlvinfo_priv;
+
+/*
+ * TlvInfo header: Layout of the header for the TlvInfo format
+ *
+ * See the end of this file for details of this eeprom format
+ */
+struct __attribute__ ((__packed__)) tlvinfo_header {
+   charsignature[8]; /* 0x00 - 0x07 EEPROM Tag "TlvInfo" */
+   u8  version;  /* 0x08Structure version*/
+   u16 totallen; /* 0x09 - 0x0A Length of all data which follows */
+};
+
+// Header Field Constants
+#define TLV_INFO_HEADER_SIZEsizeof(struct tlvinfo_header)
+#define TLV_INFO_ID_STRING  "TlvInfo"
+#define TLV_INFO_VERSION0x01
+#define TLV_INFO_MAX_LEN2048
+#define TLV_TOTAL_LEN_MAX   (TLV_INFO_MAX_LEN - TLV_INFO_HEADER_SIZE)
+
+/*
+ * TlvInfo TLV: Layout of a TLV field
+ */
+struct __attribute__ ((__packed__)) tlvinfo_tlv {
+   u8  type;
+   u8  length;
+   u8  value[];
+};
+
+#define TLV_INFO_ENTRY_SIZE  sizeof(struct tlvinfo_tlv)
+/* Maximum length of a TLV value in bytes */
+#define TLV_VALUE_MAX_LEN255
+
+/**
+ *  The TLV Types.
+ *
+ *  Keep these in sync with tlv_code_list in cmd/tlv_eeprom.c
+ */
+#define TLV_CODE_PRODUCT_NAME   0x21
+#define TLV_CODE_PART_NUMBER0x22
+#define TLV_CODE_SERIAL_NUMBER  0x23
+#define TLV_CODE_MAC_BASE   0x24
+#define TLV_CODE_MANUF_DATE 0x25
+#define TLV_CODE_DEVICE_VERSION 0x26
+#define TLV_CODE_LABEL_REVISION 0x27
+#define TLV_CODE_PLATFORM_NAME  0x28
+#define TLV_CODE_ONIE_VERSION   0x29
+#define TLV_CODE_MAC_SIZE   0x2A
+#define TLV_CODE_MANUF_NAME 0x2B
+#define TLV_CODE_MANUF_COUNTRY  0x2C
+#define TLV_CODE_VENDOR_NAME0x2D
+#define TLV_CODE_DIAG_VERSION   0x2E
+#define TLV_CODE_SERVICE_TAG0x2F
+#define TLV_CODE_VENDOR_EXT 0xFD
+#define TLV_CODE_CRC_32 0xFE
+
+/* how many EEPROMs can be used */
+#define MAX_TLV_DEVICES2
+
+/*
+ * EEPROM<->TLV API
+ */
+
+/**
+ * Find EEPROM device by index.
+ *
+ * @index: index of eeprom in the system, 0 = first.
+ * @return: handle to eeprom device on success, error pointer otherwise.
+ */
+struct udevice *tlv_eeprom_get_by_index(unsigned int index);
+
+/**
+ * Read TLV formatted data from EEPROM.
+ *
+ * @dev: EEPROM device handle.
+ * @offset: Offset into EEPROM to read from.
+ * @buffer: Buffer for storing TLV structure.
+ * @buffer_size: Size of the buffer.
+ * @return: Buffer initialised with TLV structure, or error pointer.
+ */
+struct tlvinfo_priv *tlv_eeprom_read(struct udevice *dev, int offset, u8 
*buffer, size_t buffer_size);
+
+/**
+ * Write TLV formatted data to EEPROM.
+ *
+ * @dev: EEPROM device handle.
+ * @offset: Offset into EEPROM to write to.
+ * @tlv: Pointer to TLV structure.
+ * @re

[RFC v2 2/3] mvebu: clearfog: convert tlv parsing to use new library

2023-05-16 Thread Josua Mayer
Update the existing code reading tlv data from eeprom to use the new tlv
library functions rather than relying on tlv_eeprom command internals.

Signed-off-by: Josua Mayer 
---
 board/solidrun/common/tlv_data.c | 46 
 configs/clearfog_defconfig   |  4 ++-
 2 files changed, 20 insertions(+), 30 deletions(-)

diff --git a/board/solidrun/common/tlv_data.c b/board/solidrun/common/tlv_data.c
index 11d6e4a1380..31b4b473c75 100644
--- a/board/solidrun/common/tlv_data.c
+++ b/board/solidrun/common/tlv_data.c
@@ -5,6 +5,7 @@
 
 #include 
 #include 
+#include 
 #include 
 #include "tlv_data.h"
 
@@ -50,44 +51,31 @@ static void parse_tlv_vendor_ext(struct tlvinfo_tlv 
*tlv_entry,
td->ram_size = val[5];
 }
 
-static void parse_tlv_data(u8 *eeprom, struct tlvinfo_header *hdr,
-  struct tlvinfo_tlv *entry, struct tlv_data *td)
+static void parse_tlv_data(u8 *eeprom, struct tlvinfo_priv *tlv,
+  struct tlv_data *td)
 {
-   unsigned int tlv_offset, tlv_len;
-
-   tlv_offset = sizeof(struct tlvinfo_header);
-   tlv_len = sizeof(struct tlvinfo_header) + be16_to_cpu(hdr->totallen);
-   while (tlv_offset < tlv_len) {
-   entry = (struct tlvinfo_tlv *)[tlv_offset];
-
-   switch (entry->type) {
-   case TLV_CODE_PRODUCT_NAME:
-   store_product_name(entry, td);
-   break;
-   case TLV_CODE_VENDOR_EXT:
-   parse_tlv_vendor_ext(entry, td);
-   break;
-   default:
-   break;
-   }
-
-   tlv_offset += sizeof(struct tlvinfo_tlv) + entry->length;
-   }
+   struct tlvinfo_tlv *entry;
+
+   entry = tlv_entry_next_by_code(tlv, NULL, TLV_CODE_PRODUCT_NAME);
+   if (!IS_ERR(entry))
+   store_product_name(entry, td);
+
+   entry = tlv_entry_next_by_code(tlv, NULL, TLV_CODE_VENDOR_EXT);
+   if (!IS_ERR(entry))
+   parse_tlv_vendor_ext(entry, td);
 }
 
 void read_tlv_data(struct tlv_data *td)
 {
u8 eeprom_data[TLV_TOTAL_LEN_MAX];
-   struct tlvinfo_header *tlv_hdr;
-   struct tlvinfo_tlv *tlv_entry;
-   int ret, i;
+   struct tlvinfo_priv *priv;
+   int i;
 
for (i = 0; i < 2; i++) {
-   ret = read_tlvinfo_tlv_eeprom(eeprom_data, _hdr,
- _entry, i);
-   if (ret < 0)
+   priv = tlv_eeprom_read(tlv_eeprom_get_by_index(i), 0, 
eeprom_data, ARRAY_SIZE(eeprom_data));
+   if (IS_ERR(priv))
continue;
-   parse_tlv_data(eeprom_data, tlv_hdr, tlv_entry, td);
+   parse_tlv_data(eeprom_data, priv, td);
}
 }
 
diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
index b3ed1ec7bbe..fa86b23ef40 100644
--- a/configs/clearfog_defconfig
+++ b/configs/clearfog_defconfig
@@ -35,7 +35,7 @@ CONFIG_SPL_SYS_MALLOC_SIMPLE=y
 # CONFIG_SPL_SHARES_INIT_SP_ADDR is not set
 CONFIG_SPL_I2C=y
 CONFIG_SYS_MAXARGS=32
-CONFIG_CMD_TLV_EEPROM=y
+# CONFIG_CMD_TLV_EEPROM is not set
 CONFIG_SPL_CMD_TLV_EEPROM=y
 # CONFIG_CMD_FLASH is not set
 CONFIG_CMD_GPIO=y
@@ -81,3 +81,5 @@ CONFIG_USB=y
 CONFIG_USB_XHCI_HCD=y
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_SECTOR=0x1
 CONFIG_SYS_MMCSD_RAW_MODE_U_BOOT_DATA_PART_OFFSET=0x0
+CONFIG_EEPROM_TLV_LIB=y
+CONFIG_SPL_EEPROM_TLV_LIB=y
-- 
2.35.3



[RFC v2 0/3] lib: tlv_eeprom: refactor API

2023-05-16 Thread Josua Mayer
The existing tlv_eeprom functionality has been designed as a single
application living as a uboot command.
The split into individual library and command, as well as attempts using
this functionality extensively for board-identification have revealed
several short-comings, such as:
- Inconsistent naming convention
- Inconsistent order of arguments
- Stateful functions
- Complex inter-dependencies between functions
- Control of low-level information such as header checksums by command
- Difficult to extend
- No clear separation between eeprom access and tlv format
- No support for multiple entries with same code (e.g. vendor extension)

Introduce a new API with clear names and support for duplicate tlv
codes. Further rewrite the tlv_eeprom command, and the solidrun clearfog board
logic to utilize these new functions.

This is the second draft, I am looking for:
- comments on the library functions names + descriptions
- comments on the use of udevice pointers, especially wrt.
  "tlv_eeprom_get_by_index" function that I am not sure about keeping

Please note that there are still bugs and untested functions in this version.
I am planning to provide a complete result in the coming weeks.

Josua Mayer (3):
  lib: add tlv_eeprom library
  mvebu: clearfog: convert tlv parsing to use new library
  cmd: tlv_eeprom: port to new shared tlv library

 board/solidrun/common/tlv_data.c |  46 +--
 cmd/Kconfig  |   7 +-
 cmd/tlv_eeprom.c | 507 +-
 configs/clearfog_defconfig   |   2 +
 include/tlv_eeprom.h | 242 ++---
 lib/Kconfig  |   2 +
 lib/Makefile |   2 +
 lib/tlv/Kconfig  |  18 +
 lib/tlv/Makefile |   4 +
 lib/tlv/tlv_eeprom.c | 599 +++
 10 files changed, 918 insertions(+), 511 deletions(-)
 create mode 100644 lib/tlv/Kconfig
 create mode 100644 lib/tlv/Makefile
 create mode 100644 lib/tlv/tlv_eeprom.c

Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 
-- 
2.35.3



[PATCH] cmd: tlv_eeprom: fix signature for populate_serial_number function

2023-05-16 Thread Josua Mayer
populate_serial_number is not used internally for the tlv_eeprom
command, but rather provided as a library function for external use..
Remove the devnum that had recently been added by mistake, returning the
function to its original signature.

Instead place a 0-initialised member variable inside the function to
same purpose, along with a node that it only supports reading from the
first EEPROM in the system.

Fixes: dfda0c0 ("cmd: tlv_eeprom: remove use of global variable current_dev")
Signed-off-by: Josua Mayer 
Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 
---
 cmd/tlv_eeprom.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 79796394c5c..0ca4d714645 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -1100,11 +1100,12 @@ int mac_read_from_eeprom(void)
  *
  *  This function must be called after relocation.
  */
-int populate_serial_number(int devnum)
+int populate_serial_number(void)
 {
char serialstr[257];
int eeprom_index;
struct tlvinfo_tlv *eeprom_tlv;
+   int devnum = 0; // TODO: support multiple EEPROMs
 
if (env_get("serial#"))
return 0;
-- 
2.35.3



Re: [[PATCH v2] 4/4] cmd: tlv_eeprom: enable 'dev' subcommand before 'read'

2023-05-13 Thread Josua Mayer

Hi Simon,

I have a private rewrite of the tlv functionality as a seperate library 
outside of the command implementation.
Originally I had hoped to finish upstreaming that about a year ago but 
got sidetracked.


Hopefully within the coming weeks I can submit another version as RFC 
that can address your concerns more easily than the current eeprom_tlv 
implementation.


Sincerely
Josua Mayer

Am 08.05.23 um 17:42 schrieb Simon Glass:

Hi,

On Fri, 5 May 2023 at 02:22, Josua Mayer  wrote:

Move the handler for "tlv_eeprom dev X" command to the beginning of
do_tlv_eeprom, to allow using it before issuing a "read" command for
currently selected eeprom.

Also remove the check if eeprom exists, since that can only work after
the first execution of read_eeprom triggered device lookup.
Instead accept values up to the defined array size (MAX_TLV_DEVICES).

Signed-off-by: Josua Mayer 
Reviewed-by: Stefan Roese 
Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 
---
  cmd/tlv_eeprom.c | 26 --
  1 file changed, 16 insertions(+), 10 deletions(-)

Can someone take a look at fixing up the driver model code?

For example:
- it maintains its own array of eproms, when it should use driver
model to find them.
- the has_been_read flag seems to track only one EEPROM, but the code
indicates there might be two (should be in the device's private data)
- current_dev should be a pointer to a device
- ideally all state should be either in the device or the uclass, so
it can be used before relocation, etc.

Regards,
Simon


[[PATCH v2] 4/4] cmd: tlv_eeprom: enable 'dev' subcommand before 'read'

2023-05-05 Thread Josua Mayer
Move the handler for "tlv_eeprom dev X" command to the beginning of
do_tlv_eeprom, to allow using it before issuing a "read" command for
currently selected eeprom.

Also remove the check if eeprom exists, since that can only work after
the first execution of read_eeprom triggered device lookup.
Instead accept values up to the defined array size (MAX_TLV_DEVICES).

Signed-off-by: Josua Mayer 
Reviewed-by: Stefan Roese 
Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 
---
 cmd/tlv_eeprom.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 636c1fe32ef..79796394c5c 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -450,6 +450,22 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
// "reset" will both be treated as "read".
cmd = argv[1][0];
 
+   // select device
+   if (cmd == 'd') {
+/* 'dev' command */
+   unsigned int devnum;
+
+   devnum = simple_strtoul(argv[2], NULL, 0);
+   if (devnum >= MAX_TLV_DEVICES) {
+   printf("Invalid device number\n");
+   return 0;
+   }
+   current_dev = devnum;
+   has_been_read = 0;
+
+   return 0;
+   }
+
// Read the EEPROM contents
if (cmd == 'r') {
has_been_read = 0;
@@ -508,16 +524,6 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
tlvinfo_delete_tlv(eeprom, tcode);
if (argc == 4)
tlvinfo_add_tlv(eeprom, tcode, argv[3]);
-   } else if (cmd == 'd') { /* 'dev' command */
-   unsigned int devnum;
-
-   devnum = simple_strtoul(argv[2], NULL, 0);
-   if (devnum > MAX_TLV_DEVICES || !tlv_devices[devnum]) {
-   printf("Invalid device number\n");
-   return 0;
-   }
-   current_dev = devnum;
-   has_been_read = 0;
} else {
return CMD_RET_USAGE;
}
-- 
2.35.3



[[PATCH v2] 2/4] cmd: tlv_eeprom: remove use of global variable has_been_read

2023-05-05 Thread Josua Mayer
has_been_read is only used as an optimization for do_tlv_eeprom.
Explicitly use and set inside this function, thus making read_eeprom
stateless.

Signed-off-by: Josua Mayer 
Reviewed-by: Stefan Roese 
Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 
---
 cmd/tlv_eeprom.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 8049bf9843c..d36401e9138 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -42,8 +42,6 @@ static int set_date(char *buf, const char *string);
 static int set_bytes(char *buf, const char *string, int *converted_accum);
 static void show_tlv_devices(int current_dev);
 
-/* Set to 1 if we've read EEPROM into memory */
-static int has_been_read;
 /* The EERPOM contents after being read into memory */
 static u8 eeprom[TLV_INFO_MAX_LEN];
 
@@ -130,9 +128,6 @@ static int read_eeprom(int devnum, u8 *eeprom)
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
struct tlvinfo_tlv *eeprom_tlv = to_entry([HDR_SIZE]);
 
-   if (has_been_read)
-   return 0;
-
/* Read the header */
ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
/* If the header was successfully read, read the TLVs */
@@ -149,10 +144,8 @@ static int read_eeprom(int devnum, u8 *eeprom)
update_crc(eeprom);
}
 
-   has_been_read = 1;
-
 #ifdef DEBUG
-   show_eeprom(eeprom);
+   show_eeprom(devnum, eeprom);
 #endif
 
return ret;
@@ -432,10 +425,16 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
char cmd;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
static unsigned int current_dev;
+   /* Set to 1 if we've read EEPROM into memory */
+   static int has_been_read;
+   int ret;
 
// If no arguments, read the EERPOM and display its contents
if (argc == 1) {
-   read_eeprom(current_dev, eeprom);
+   if (!has_been_read) {
+   if (read_eeprom(current_dev, eeprom) == 0)
+   has_been_read = 1;
+   }
show_eeprom(current_dev, eeprom);
return 0;
}
@@ -447,8 +446,10 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
// Read the EEPROM contents
if (cmd == 'r') {
has_been_read = 0;
-   if (!read_eeprom(current_dev, eeprom))
+   if (read_eeprom(current_dev, eeprom) == 0) {
printf("EEPROM data loaded from device to memory.\n");
+   has_been_read = 1;
+   }
return 0;
}
 
-- 
2.35.3



[[PATCH v2] 3/4] cmd: tlv_eeprom: handle -ENODEV error from read_eeprom function

2023-05-05 Thread Josua Mayer
When tlv eeprom does not exist, return error code instead of quietly
making up tlv structure in memory.

Signed-off-by: Josua Mayer 
Reviewed-by: Stefan Roese 
Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 
---
 cmd/tlv_eeprom.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index d36401e9138..636c1fe32ef 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -134,6 +134,8 @@ static int read_eeprom(int devnum, u8 *eeprom)
if (ret == 0 && is_valid_tlvinfo_header(eeprom_hdr))
ret = read_tlv_eeprom((void *)eeprom_tlv, HDR_SIZE,
  be16_to_cpu(eeprom_hdr->totallen), 
devnum);
+   else if (ret == -ENODEV)
+   return ret;
 
// If the contents are invalid, start over with default contents
if (!is_valid_tlvinfo_header(eeprom_hdr) ||
@@ -432,8 +434,13 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
// If no arguments, read the EERPOM and display its contents
if (argc == 1) {
if (!has_been_read) {
-   if (read_eeprom(current_dev, eeprom) == 0)
-   has_been_read = 1;
+   ret = read_eeprom(current_dev, eeprom);
+   if (ret) {
+   printf("Failed to read EEPROM data from 
device.\n");
+   return 0;
+   }
+
+   has_been_read = 1;
}
show_eeprom(current_dev, eeprom);
return 0;
@@ -446,11 +453,14 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
// Read the EEPROM contents
if (cmd == 'r') {
has_been_read = 0;
-   if (read_eeprom(current_dev, eeprom) == 0) {
-   printf("EEPROM data loaded from device to memory.\n");
-   has_been_read = 1;
+   ret = read_eeprom(current_dev, eeprom);
+   if (ret) {
+   printf("Failed to read EEPROM data from device.\n");
+   return 0;
}
-   return 0;
+
+   printf("EEPROM data loaded from device to memory.\n");
+   has_been_read = 1;
}
 
// Subsequent commands require that the EEPROM has already been read.
-- 
2.35.3



[[PATCH v2] 1/4] cmd: tlv_eeprom: remove use of global variable current_dev

2023-05-05 Thread Josua Mayer
Make tlv_eeprom command device selection an explicit parameter of all
function calls.

Signed-off-by: Josua Mayer 
Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 
---
 cmd/tlv_eeprom.c | 50 ++--
 include/tlv_eeprom.h |  3 ++-
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 4591ff336bb..8049bf9843c 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -29,18 +29,18 @@ DECLARE_GLOBAL_DATA_PTR;
 
 /* File scope function prototypes */
 static bool is_checksum_valid(u8 *eeprom);
-static int read_eeprom(u8 *eeprom);
-static void show_eeprom(u8 *eeprom);
+static int read_eeprom(int devnum, u8 *eeprom);
+static void show_eeprom(int devnum, u8 *eeprom);
 static void decode_tlv(struct tlvinfo_tlv *tlv);
 static void update_crc(u8 *eeprom);
-static int prog_eeprom(u8 *eeprom);
+static int prog_eeprom(int devnum, u8 *eeprom);
 static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
 static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
 static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
 static int set_mac(char *buf, const char *string);
 static int set_date(char *buf, const char *string);
 static int set_bytes(char *buf, const char *string, int *converted_accum);
-static void show_tlv_devices(void);
+static void show_tlv_devices(int current_dev);
 
 /* Set to 1 if we've read EEPROM into memory */
 static int has_been_read;
@@ -48,7 +48,6 @@ static int has_been_read;
 static u8 eeprom[TLV_INFO_MAX_LEN];
 
 static struct udevice *tlv_devices[MAX_TLV_DEVICES];
-static unsigned int current_dev;
 
 #define to_header(p) ((struct tlvinfo_header *)p)
 #define to_entry(p) ((struct tlvinfo_tlv *)p)
@@ -125,7 +124,7 @@ static bool is_checksum_valid(u8 *eeprom)
  *
  *  Read the EEPROM into memory, if it hasn't already been read.
  */
-static int read_eeprom(u8 *eeprom)
+static int read_eeprom(int devnum, u8 *eeprom)
 {
int ret;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
@@ -135,12 +134,11 @@ static int read_eeprom(u8 *eeprom)
return 0;
 
/* Read the header */
-   ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, current_dev);
+   ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
/* If the header was successfully read, read the TLVs */
if (ret == 0 && is_valid_tlvinfo_header(eeprom_hdr))
ret = read_tlv_eeprom((void *)eeprom_tlv, HDR_SIZE,
- be16_to_cpu(eeprom_hdr->totallen),
- current_dev);
+ be16_to_cpu(eeprom_hdr->totallen), 
devnum);
 
// If the contents are invalid, start over with default contents
if (!is_valid_tlvinfo_header(eeprom_hdr) ||
@@ -165,7 +163,7 @@ static int read_eeprom(u8 *eeprom)
  *
  *  Display the contents of the EEPROM
  */
-static void show_eeprom(u8 *eeprom)
+static void show_eeprom(int devnum, u8 *eeprom)
 {
int tlv_end;
int curr_tlv;
@@ -180,7 +178,7 @@ static void show_eeprom(u8 *eeprom)
return;
}
 
-   printf("TLV: %u\n", current_dev);
+   printf("TLV: %u\n", devnum);
printf("TlvInfo Header:\n");
printf("   Id String:%s\n", eeprom_hdr->signature);
printf("   Version:  %d\n", eeprom_hdr->version);
@@ -389,7 +387,7 @@ static void update_crc(u8 *eeprom)
  *
  *  Write the EEPROM data from CPU memory to the hardware.
  */
-static int prog_eeprom(u8 *eeprom)
+static int prog_eeprom(int devnum, u8 *eeprom)
 {
int ret = 0;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
@@ -398,7 +396,7 @@ static int prog_eeprom(u8 *eeprom)
update_crc(eeprom);
 
eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
-   ret = write_tlv_eeprom(eeprom, eeprom_len);
+   ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
if (ret) {
printf("Programming failed.\n");
return -1;
@@ -433,11 +431,12 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
 {
char cmd;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+   static unsigned int current_dev;
 
// If no arguments, read the EERPOM and display its contents
if (argc == 1) {
-   read_eeprom(eeprom);
-   show_eeprom(eeprom);
+   read_eeprom(current_dev, eeprom);
+   show_eeprom(current_dev, eeprom);
return 0;
}
 
@@ -448,7 +447,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
// Read the EEPROM contents
if (cmd == 'r') {
has_been_read = 0;
-   if (!read_eeprom(eeprom))
+   if (!read_eeprom(current_dev, e

[[PATCH v2] 0/4] cmd: tlv_eeprom: global variables and error cleanup

2023-05-05 Thread Josua Mayer
This patch-set removes some uses of global variables, and improves error
reporting for the "read" command.
It is intended to help switching to a split tlv library eventually,
but general enough to apply independently.

Changes since v1:
- rebased on master 0160d58
- added first patch!!! I had forgotten to send in v1
- added reviewed-tags by Stefan for patches 2-4

Josua Mayer (4):
  cmd: tlv_eeprom: remove use of global variable current_dev
  cmd: tlv_eeprom: remove use of global variable has_been_read
  cmd: tlv_eeprom: handle -ENODEV error from read_eeprom function
  cmd: tlv_eeprom: enable 'dev' subcommand before 'read'

 cmd/tlv_eeprom.c | 107 +--
 include/tlv_eeprom.h |   3 +-
 2 files changed, 64 insertions(+), 46 deletions(-)

-- 
2.35.3



Re: [PATCH 1/3] cmd: tlv_eeprom: remove use of global variable has_been_read

2023-05-05 Thread Josua Mayer

Hi Stefan,

Thanks for reviewing. I just rebased on 
https://source.denx.de/u-boot/u-boot.git / master,

and will send new version soon.

- Josua Mayer

Am 03.05.23 um 09:55 schrieb Stefan Roese:

Hi Josua,

On 4/29/23 11:15, Josua Mayer wrote:

has_been_read is only used as an optimization for do_tlv_eeprom.
Explicitly use and set inside this function, thus making read_eeprom
stateless.

Signed-off-by: Josua Mayer 
Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 


This patchset does not apply to master. Could you please check, rebase
and send again?

Thanks,
Stefan


---
  cmd/tlv_eeprom.c | 21 +++--
  1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 8049bf9843c..d36401e9138 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -42,8 +42,6 @@ static int set_date(char *buf, const char *string);
  static int set_bytes(char *buf, const char *string, int 
*converted_accum);

  static void show_tlv_devices(int current_dev);
  -/* Set to 1 if we've read EEPROM into memory */
-static int has_been_read;
  /* The EERPOM contents after being read into memory */
  static u8 eeprom[TLV_INFO_MAX_LEN];
  @@ -130,9 +128,6 @@ static int read_eeprom(int devnum, u8 *eeprom)
  struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
  struct tlvinfo_tlv *eeprom_tlv = to_entry([HDR_SIZE]);
  -    if (has_been_read)
-    return 0;
-
  /* Read the header */
  ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
  /* If the header was successfully read, read the TLVs */
@@ -149,10 +144,8 @@ static int read_eeprom(int devnum, u8 *eeprom)
  update_crc(eeprom);
  }
  -    has_been_read = 1;
-
  #ifdef DEBUG
-    show_eeprom(eeprom);
+    show_eeprom(devnum, eeprom);
  #endif
    return ret;
@@ -432,10 +425,16 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int 
flag, int argc, char *const argv[])

  char cmd;
  struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
  static unsigned int current_dev;
+    /* Set to 1 if we've read EEPROM into memory */
+    static int has_been_read;
+    int ret;
    // If no arguments, read the EERPOM and display its contents
  if (argc == 1) {
-    read_eeprom(current_dev, eeprom);
+    if (!has_been_read) {
+    if (read_eeprom(current_dev, eeprom) == 0)
+    has_been_read = 1;
+    }
  show_eeprom(current_dev, eeprom);
  return 0;
  }
@@ -447,8 +446,10 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int 
flag, int argc, char *const argv[])

  // Read the EEPROM contents
  if (cmd == 'r') {
  has_been_read = 0;
-    if (!read_eeprom(current_dev, eeprom))
+    if (read_eeprom(current_dev, eeprom) == 0) {
  printf("EEPROM data loaded from device to memory.\n");
+    has_been_read = 1;
+    }
  return 0;
  }


Viele Grüße,
Stefan Roese



[PATCH 2/3] cmd: tlv_eeprom: handle -ENODEV error from read_eeprom function

2023-04-29 Thread Josua Mayer
When tlv eeprom does not exist, return error code instead of quietly
making up tlv structure in memory.

Signed-off-by: Josua Mayer 
Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 
---
 cmd/tlv_eeprom.c | 22 --
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index d36401e9138..636c1fe32ef 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -134,6 +134,8 @@ static int read_eeprom(int devnum, u8 *eeprom)
if (ret == 0 && is_valid_tlvinfo_header(eeprom_hdr))
ret = read_tlv_eeprom((void *)eeprom_tlv, HDR_SIZE,
  be16_to_cpu(eeprom_hdr->totallen), 
devnum);
+   else if (ret == -ENODEV)
+   return ret;
 
// If the contents are invalid, start over with default contents
if (!is_valid_tlvinfo_header(eeprom_hdr) ||
@@ -432,8 +434,13 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
// If no arguments, read the EERPOM and display its contents
if (argc == 1) {
if (!has_been_read) {
-   if (read_eeprom(current_dev, eeprom) == 0)
-   has_been_read = 1;
+   ret = read_eeprom(current_dev, eeprom);
+   if (ret) {
+   printf("Failed to read EEPROM data from 
device.\n");
+   return 0;
+   }
+
+   has_been_read = 1;
}
show_eeprom(current_dev, eeprom);
return 0;
@@ -446,11 +453,14 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
// Read the EEPROM contents
if (cmd == 'r') {
has_been_read = 0;
-   if (read_eeprom(current_dev, eeprom) == 0) {
-   printf("EEPROM data loaded from device to memory.\n");
-   has_been_read = 1;
+   ret = read_eeprom(current_dev, eeprom);
+   if (ret) {
+   printf("Failed to read EEPROM data from device.\n");
+   return 0;
}
-   return 0;
+
+   printf("EEPROM data loaded from device to memory.\n");
+   has_been_read = 1;
}
 
// Subsequent commands require that the EEPROM has already been read.
-- 
2.35.3



[PATCH 3/3] cmd: tlv_eeprom: enable 'dev' subcommand before 'read'

2023-04-29 Thread Josua Mayer
Move the handler for "tlv_eeprom dev X" command to the beginning of
do_tlv_eeprom, to allow using it before issuing a "read" command for
currently selected eeprom.

Also remove the check if eeprom exists, since that can only work after
the first execution of read_eeprom triggered device lookup.
Instead accept values up to the defined array size (MAX_TLV_DEVICES).

Signed-off-by: Josua Mayer 
Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 
---
 cmd/tlv_eeprom.c | 26 --
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 636c1fe32ef..79796394c5c 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -450,6 +450,22 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
// "reset" will both be treated as "read".
cmd = argv[1][0];
 
+   // select device
+   if (cmd == 'd') {
+/* 'dev' command */
+   unsigned int devnum;
+
+   devnum = simple_strtoul(argv[2], NULL, 0);
+   if (devnum >= MAX_TLV_DEVICES) {
+   printf("Invalid device number\n");
+   return 0;
+   }
+   current_dev = devnum;
+   has_been_read = 0;
+
+   return 0;
+   }
+
// Read the EEPROM contents
if (cmd == 'r') {
has_been_read = 0;
@@ -508,16 +524,6 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
tlvinfo_delete_tlv(eeprom, tcode);
if (argc == 4)
tlvinfo_add_tlv(eeprom, tcode, argv[3]);
-   } else if (cmd == 'd') { /* 'dev' command */
-   unsigned int devnum;
-
-   devnum = simple_strtoul(argv[2], NULL, 0);
-   if (devnum > MAX_TLV_DEVICES || !tlv_devices[devnum]) {
-   printf("Invalid device number\n");
-   return 0;
-   }
-   current_dev = devnum;
-   has_been_read = 0;
} else {
return CMD_RET_USAGE;
}
-- 
2.35.3



[PATCH 1/3] cmd: tlv_eeprom: remove use of global variable has_been_read

2023-04-29 Thread Josua Mayer
has_been_read is only used as an optimization for do_tlv_eeprom.
Explicitly use and set inside this function, thus making read_eeprom
stateless.

Signed-off-by: Josua Mayer 
Cc: Stefan Roese 
Cc: Baruch Siach 
Cc: Heinrich Schuchardt 
---
 cmd/tlv_eeprom.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 8049bf9843c..d36401e9138 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -42,8 +42,6 @@ static int set_date(char *buf, const char *string);
 static int set_bytes(char *buf, const char *string, int *converted_accum);
 static void show_tlv_devices(int current_dev);
 
-/* Set to 1 if we've read EEPROM into memory */
-static int has_been_read;
 /* The EERPOM contents after being read into memory */
 static u8 eeprom[TLV_INFO_MAX_LEN];
 
@@ -130,9 +128,6 @@ static int read_eeprom(int devnum, u8 *eeprom)
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
struct tlvinfo_tlv *eeprom_tlv = to_entry([HDR_SIZE]);
 
-   if (has_been_read)
-   return 0;
-
/* Read the header */
ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
/* If the header was successfully read, read the TLVs */
@@ -149,10 +144,8 @@ static int read_eeprom(int devnum, u8 *eeprom)
update_crc(eeprom);
}
 
-   has_been_read = 1;
-
 #ifdef DEBUG
-   show_eeprom(eeprom);
+   show_eeprom(devnum, eeprom);
 #endif
 
return ret;
@@ -432,10 +425,16 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
char cmd;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
static unsigned int current_dev;
+   /* Set to 1 if we've read EEPROM into memory */
+   static int has_been_read;
+   int ret;
 
// If no arguments, read the EERPOM and display its contents
if (argc == 1) {
-   read_eeprom(current_dev, eeprom);
+   if (!has_been_read) {
+   if (read_eeprom(current_dev, eeprom) == 0)
+   has_been_read = 1;
+   }
show_eeprom(current_dev, eeprom);
return 0;
}
@@ -447,8 +446,10 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
// Read the EEPROM contents
if (cmd == 'r') {
has_been_read = 0;
-   if (!read_eeprom(current_dev, eeprom))
+   if (read_eeprom(current_dev, eeprom) == 0) {
printf("EEPROM data loaded from device to memory.\n");
+   has_been_read = 1;
+   }
return 0;
}
 
-- 
2.35.3



[PATCH 0/3] cmd: tlv_eeprom: global variables and error cleanup

2023-04-29 Thread Josua Mayer
This patch-set removes some uses of global variables, and improves error
reporting for the "read" command.
It is intended to help switching to a split tlv library eventually,
but general enough to apply independently.

Josua Mayer (3):
  cmd: tlv_eeprom: remove use of global variable has_been_read
  cmd: tlv_eeprom: handle -ENODEV error from read_eeprom function
  cmd: tlv_eeprom: enable 'dev' subcommand before 'read'

 cmd/tlv_eeprom.c | 61 +++-
 1 file changed, 39 insertions(+), 22 deletions(-)

-- 
2.35.3



Re: [PATCH 2/2] arm: mvebu: clearfog: Add defconfig for SPI booting

2023-02-25 Thread Josua Mayer

Hi Pali, Martin,

Am 25.02.23 um 10:48 schrieb Martin Rowe:

On Sat, 25 Feb 2023 at 07:41, Pali Rohár  wrote:

On Saturday 25 February 2023 11:42:20 Martin Rowe wrote:

This new clearfog_spi_defconfig file is copy of existing
clearfog_defconfig file and changed to instruct build system to
generate final kwbimage for SPI booting and to store the
environment in SPI as well.

Signed-off-by: Martin Rowe
---
  .../{clearfog_sata_defconfig => clearfog_spi_defconfig}| 7 ++-
  1 file changed, 6 insertions(+), 1 deletion(-)
  copy configs/{clearfog_sata_defconfig => clearfog_spi_defconfig} (93%)

diff --git a/configs/clearfog_sata_defconfig b/configs/clearfog_spi_defconfig
similarity index 93%
copy from configs/clearfog_sata_defconfig
copy to configs/clearfog_spi_defconfig
index 84f900bf50..31b1e9fce8 100644
--- a/configs/clearfog_sata_defconfig
+++ b/configs/clearfog_spi_defconfig
@@ -3,20 +3,24 @@ CONFIG_ARCH_CPU_INIT=y
  CONFIG_SYS_THUMB_BUILD=y
  CONFIG_ARCH_MVEBU=y
  CONFIG_TEXT_BASE=0x0080
+CONFIG_SPL_GPIO=y
  CONFIG_SPL_LIBCOMMON_SUPPORT=y
  CONFIG_SPL_LIBGENERIC_SUPPORT=y
  CONFIG_NR_DRAM_BANKS=2
  CONFIG_HAS_CUSTOM_SYS_INIT_SP_ADDR=y
  CONFIG_CUSTOM_SYS_INIT_SP_ADDR=0xff
  CONFIG_TARGET_CLEARFOG=y
-CONFIG_MVEBU_SPL_BOOT_DEVICE_SATA=y
+CONFIG_ENV_SECT_SIZE=0x1
  CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
  CONFIG_SPL_TEXT_BASE=0x4030
+CONFIG_SPL_MMC=y
  CONFIG_SPL_SERIAL=y
  CONFIG_SPL_STACK=0x4002c000
  CONFIG_SPL=y
  CONFIG_DEBUG_UART_BASE=0xf1012000
  CONFIG_DEBUG_UART_CLOCK=25000
+CONFIG_SPL_LIBDISK_SUPPORT=y
+# CONFIG_SPL_SPI is not set

Any reason to disable it?

It's the same as MMC and SATA defconfigs this way, it just needs to be
explicit here because of something SPI related that we've enabled.
SPL_DM_SPI is enabled in all of the configs, so I'm guessing that it
is providing driver support because everything seems to work without
SPL_SPI.

I re-tested with and without the config and it doesn't seem to change
anything. With it enabled the file is 588K instead of 580K without it,
but that's not really an issue with 4M of flash storage available.


While I haven't tested this patchset - I have tested SPI booting with 
v2022.01 after applying lots of patches.
I found that the magic value reported by bootrom when booting from SPI 
will be 0x34 on clearfog.
The current sources only handle 0x32, hence having or not having SPL_SPI 
will always lead to returning to the bootrom.


Please consider pulling in this patch:
https://github.com/SolidRun/u-boot/commit/f4f8a69740a8415c05359e01e51650f445cda03d
I can send it separately if you like.

which I have tested by adding these config options:

|CONFIG_MVEBU_SPL_BOOT_DEVICE_SPI=y # CONFIG_MVEBU_SPL_BOOT_DEVICE_MMC 
is not set # CONFIG_SPL_MMC is not set CONFIG_SPL_SPI_FLASH_SUPPORT=y 
CONFIG_SPL_SPI=y CONFIG_SPL_SPI_FLASH_TINY=y CONFIG_SPL_SPI_LOAD=y 
CONFIG_SYS_SPI_U_BOOT_OFFS=0x0 # CONFIG_ENV_IS_IN_MMC is not set 
CONFIG_ENV_IS_IN_SPI_FLASH=y # CONFIG_ENV_SECT_SIZE_AUTO is not set # 
CONFIG_USE_ENV_SPI_BUS is not set # CONFIG_USE_ENV_SPI_CS is not set # 
CONFIG_USE_ENV_SPI_MAX_HZ is not set # CONFIG_USE_ENV_SPI_MODE is not 
set # CONFIG_ENV_SPI_EARLY is not set CONFIG_ENV_ADDR=0x0|



  CONFIG_SYS_LOAD_ADDR=0x80
  CONFIG_DEBUG_UART=y
  CONFIG_AHCI=y
@@ -59,6 +63,7 @@ CONFIG_SYS_I2C_MVTWSI=y
  CONFIG_I2C_EEPROM=y
  CONFIG_SPL_I2C_EEPROM=y
  CONFIG_MMC_BROKEN_CD=y
+CONFIG_SPL_DM_MMC=y
  CONFIG_SUPPORT_EMMC_BOOT=y
  CONFIG_MMC_SDHCI=y
  CONFIG_MMC_SDHCI_SDMA=y
--
2.39.2



Re: [PATCH] mx6cuboxi: fix ethernet after synchronise device-tree

2022-07-28 Thread Josua Mayer
Please hold off merging this patch until someone tested it, I can not do so
this week.
@Tom Can you confirm if this fixes the networking on your Cubox?
Also note that the phy-handle property may or may not be required, I am not
sure.

sincerely
Josua Mayer

On Thu, Jul 28, 2022 at 7:05 AM Josua Mayer  wrote:

> The i.MX6 Cubox-i and HummingBoards can have different PHYs at varying
> addresses. U-Boot needs to auto-detect which phy is actually present,
> and at which address it is responding.
>
> Auto-detection from multiple phy nodes specified in device-tree does not
> currently work correct. As a work-around merge all three possible phys
> into one node with the special address 0x which indicates to the
> generic phy driver to probe all addresses.
> Also fixup this fake address before booting Linux, *if* booting with
> U-Boot's internal dtb.
>
> Signed-off-by: Josua Mayer 
> Fixes: d0399a46e7cd
> ---
>  arch/arm/dts/imx6qdl-sr-som.dtsi | 30 +---
>  board/solidrun/mx6cuboxi/mx6cuboxi.c |  6 +-
>  2 files changed, 14 insertions(+), 22 deletions(-)
>
> diff --git a/arch/arm/dts/imx6qdl-sr-som.dtsi
> b/arch/arm/dts/imx6qdl-sr-som.dtsi
> index ce543e325c..2d7cbc26b3 100644
> --- a/arch/arm/dts/imx6qdl-sr-som.dtsi
> +++ b/arch/arm/dts/imx6qdl-sr-som.dtsi
> @@ -53,6 +53,7 @@
>   {
> pinctrl-names = "default";
> pinctrl-0 = <_microsom_enet_ar8035>;
> +   phy-handle = <>;
> phy-mode = "rgmii-id";
>
> /*
> @@ -68,30 +69,17 @@
> #address-cells = <1>;
> #size-cells = <0>;
>
> -   /*
> -* The PHY can appear at either address 0 or 4 due to the
> -* configuration (LED) pin not being pulled sufficiently.
> -*/
> -   ethernet-phy@0 {
> -   reg = <0>;
> +   phy: ethernet-phy@0 {
> +   /*
> +* The PHY can appear either:
> +* - AR8035: at address 0 or 4
> +* - ADIN1300: at address 1
> +* Actual address being detected at runtime.
> +*/
> +   reg = <0x>;
> qca,clk-out-frequency = <12500>;
> qca,smarteee-tw-us-1g = <24>;
> -   };
> -
> -   ethernet-phy@4 {
> -   reg = <4>;
> -   qca,clk-out-frequency = <12500>;
> -   qca,smarteee-tw-us-1g = <24>;
> -   };
> -
> -   /*
> -* ADIN1300 (som rev 1.9 or later) is always at address 1.
> It
> -* will be enabled automatically by U-Boot if detected.
> -*/
> -   ethernet-phy@1 {
> -   reg = <1>;
> adi,phy-output-clock = "125mhz-free-running";
> -   status = "disabled";
> };
> };
>  };
> diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c
> b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> index debf4f6a3b..52172a03b1 100644
> --- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
> +++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
> @@ -446,7 +446,7 @@ static int find_ethernet_phy(void)
>   */
>  int ft_board_setup(void *fdt, struct bd_info *bd)
>  {
> -   int node_phy0, node_phy1, node_phy4;
> +   int node_phy, node_phy0, node_phy1, node_phy4;
> int ret, phy;
> bool enable_phy0 = false, enable_phy1 = false, enable_phy4 = false;
> enum board_type board;
> @@ -478,6 +478,10 @@ int ft_board_setup(void *fdt, struct bd_info *bd)
> return 0;
> }
>
> +   // update U-Boot's own unified phy node phy address, if present
> +   node_phy = fdt_path_offset(fdt, "/soc/bus@210/ethernet@2188000
> /mdio/phy");
> +   ret = fdt_setprop_u32(fdt, node_phy, "reg", phy);
> +
> // update all phy nodes status
> node_phy0 = fdt_path_offset(fdt, "/soc/bus@210
> /ethernet@2188000/mdio/ethernet-phy@0");
> ret = fdt_setprop_string(fdt, node_phy0, "status", enable_phy0 ?
> "okay" : "disabled");
> --
> 2.37.1
>
>


Re: Pull request: u-boot-imx u-boot-imx-20220726

2022-07-28 Thread Josua Mayer
Hi Fabio, Marcel, Tom,

The intuitive way to make ethernet work in case we do not know which phy is
assembled,and at what address it will be available, is having 3 phy nodes
in the dts, and setting all of their status fields to "okay" - so that they
would be probed in order until one succeeds.
This approach does work in Linux, but the maintainers there have rejected
it for producing unnecessary errors in the boot log. For U-Boot imo this
approach would be acceptable.

HOWEVER U-Boot does not currently work that way.
Back when I developed my patch I noticed a strange problem where U-Boot
would detect one of the PHYs, but then pass the *wrong* device-tree node to
the driver.
Therefore I intentionally merged them all into one phy node, as a
workaround.

I see that the phy-handle property has also been dropped. This may or may
not be triggering the situation Tom is reporting on - I did not look deep
enough.

So I would suggest to partially undo Marcels change by restoring the single
phy node as I had described it, for now.
It may also be beneficial to tweak the device-tree patching inside the
board-file to make this merged phy node compatible with Linux.

I can probably draft a patch to test this theory, later today.

sincerely
Josua Mayer

On Wed, Jul 27, 2022 at 12:14 PM Fabio Estevam  wrote:

> [Adding Josua]
>
> On Tue, Jul 26, 2022 at 3:12 PM Tom Rini  wrote:
>
> > So, funny issue here now.  With:
> > commit d0399a46e7cda63c07e3eb8558bef84cfb068028
> > Author: Marcel Ziswiler 
> > Date:   Thu Jul 21 15:27:26 2022 +0200
> >
> > imx6dl/imx6qdl: synchronise device trees with linux
> >
> > Synchronise device trees with linux-next next-20220708.
> >
> > Signed-off-by: Marcel Ziswiler 
> >
> > This breaks ethernet on my mx6cuboxi board, which in reality is an MX6
> > Hummingboard.  I think this means that we need some further tweak in the
> > board.c file, given the past history I recall of "oops, ethernet broke
> > again" here.  Since I don't want to block everything else and after
> > re-reading that commit, I don't think this is some systemic breakage but
> > more likely board specific, I have applied the PR.  I am hopeful that we
> > can resolve this issue fairly quickly.
>
> The sync with imx6 upstream  DT undoes some of the changes from:
>
> commit 17baba4682001cc11446ff8406c63850b46edf72
> Author: Josua Mayer 
> Date:   Thu May 19 12:31:58 2022 +0300
>
> ARM: dts: imx6qdl-sr-som: add support for alternate phy addresses
>
> The Cubox has an unstable phy address - which can appear at either
> address 0 (intended) or 4 (unintended).
>
> SoM revision 1.9 has replaced the ar8035 phy with an adin1300, which
> will always appear at address 1.
>
> Change the reg property of the phy node to the magic value 0x,
> which indicates to the generic phy driver that all addresses should be
> probed. That allows the same node (which is pinned by phy-handle) to
> match
> either the AR8035 PHY at both possible addresses, as well as the new
> one
> at address 1.
> Also add the new adi,phy-output-clock property for enabling the 125MHz
> clock used by the fec ethernet controller, as submitted to Linux [1].
>
> Linux solves this problem differently:
> For the ar8035 phy it will probe both phy nodes in device-tree in
> order,
> and use the one that succeeds. For the new adin1300 it expects U-Boot
> to
> patch the status field in the DTB before booting
>
> While at it also sync the reset-delay with the upstream Linux dtb.
>
> [1]
> https://patchwork.kernel.org/project/netdevbpf/patch/20220428082848.12191-4-jo...@solid-run.com/
>
> Signed-off-by: Josua Mayer 
>
> Josua, could you please take a look?
>


Re: [PATCH v2 4/5] mx6cuboxi: fixup dtb ethernet phy nodes before booting an OS

2022-06-16 Thread Josua Mayer

Hi Stefano,

Thank you for applying this patchset.
This patch had a functional flaw that I was intending to address in v3.

Instead I have now followed up with a separate patch:
[PATCH] mx6cuboxi: fix board detection while patching device-tree phy nodes
Please make sure this one is added too.

sincerely
Josua Mayer

Am 15.06.22 um 14:11 schrieb sba...@denx.de:

SoM revision 1.9 has replaced the ar8035 phy address 0 with an adin1300
at address 1. Because early SoMs had a hardware flaw, the ar8035 can
also appear at address 4 - making it a total of 3 phy nodes in the DTB.
To avoid confusing Linux with probe errors, fixup the dtb to only enable
the phy node that is detected at runtime.
Signed-off-by: Josua Mayer 

Applied to u-boot-imx, master, thanks !

Best regards,
Stefano Babic



[PATCH] mx6cuboxi: fix board detection while patching device-tree phy nodes

2022-06-16 Thread Josua Mayer
ft_board_setup relies on the board_type() function to optimize which phy
nodes need to be enabled for Linux.
Add calls to setup and release the board-detect GPIOs.

Also fix the switch-case statement to only enable phy address 4 for
Cubox and unknown devices.

Fixes: 741ce308 ("mx6cuboxi: fixup dtb ethernet phy nodes before booting an OS")
Signed-off-by: Josua Mayer 
---
 board/solidrun/mx6cuboxi/mx6cuboxi.c | 13 -
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c 
b/board/solidrun/mx6cuboxi/mx6cuboxi.c
index 42aa5cb63c..debf4f6a3b 100644
--- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
+++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
@@ -449,15 +449,26 @@ int ft_board_setup(void *fdt, struct bd_info *bd)
int node_phy0, node_phy1, node_phy4;
int ret, phy;
bool enable_phy0 = false, enable_phy1 = false, enable_phy4 = false;
+   enum board_type board;
+
+   // detect device
+   request_detect_gpios();
+   board = board_type();
+   free_detect_gpios();
 
// detect phy
phy = find_ethernet_phy();
if (phy == 0 || phy == 4) {
enable_phy0 = true;
-   switch (board_type()) {
+   switch (board) {
+   case HUMMINGBOARD:
+   case HUMMINGBOARD2:
+   /* atheros phy may appear only at address 0 */
+   break;
case CUBOXI:
case UNKNOWN:
default:
+   /* atheros phy may appear at either address 0 or 4 */
enable_phy4 = true;
}
} else if (phy == 1) {
-- 
2.35.3



Re: [PATCH v2 3/5] ARM: dts: imx6qdl-sr-som: add support for alternate phy addresses

2022-05-19 Thread Josua Mayer

Hi Mark,

Am 19.05.22 um 13:21 schrieb Mark Kettenis:

From: Josua Mayer 
Date: Thu, 19 May 2022 12:31:58 +0300

The Cubox has an unstable phy address - which can appear at either
address 0 (intended) or 4 (unintended).

SoM revision 1.9 has replaced the ar8035 phy with an adin1300, which
will always appear at address 1.

Change the reg property of the phy node to the magic value 0x,
which indicates to the generic phy driver that all addresses should be
probed. That allows the same node (which is pinned by phy-handle) to match
either the AR8035 PHY at both possible addresses, as well as the new one
at address 1.
Also add the new adi,phy-output-clock property for enabling the 125MHz
clock used by the fec ethernet controller, as submitted to Linux [1].

Linux solves this problem differently:
For the ar8035 phy it will probe both phy nodes in device-tree in order,
and use the one that succeeds. For the new adin1300 it expects U-Boot to
patch the status field in the DTB before booting


And U-Boot should do the same thing for its own device tree such that
the built-in DTB can simply be passed to the OS.  There should be no
difference between the U-Boot device tree and the Linux device tree.

Should, but that does not currently work.
I actually tried doing it before submitting v1, and I found that the phy 
driver would not find the device-tree properties I was adding to the 
phy@1 node.
So it did not enable the required clock output, hence the ethernet port 
was not functional.
We also have a non-Linux phy-handle property, so I suspect there were 
reasons to not use the Linux dts for i.MX6 ethernet.


This is why instead I added to the one single phy node.



While at it also sync the reset-delay with the upstream Linux dtb.

[1] 
https://patchwork.kernel.org/project/netdevbpf/patch/20220428082848.12191-4-jo...@solid-run.com/

Signed-off-by: Josua Mayer 
---
  arch/arm/dts/imx6qdl-sr-som.dtsi | 17 +++--
  1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/arm/dts/imx6qdl-sr-som.dtsi b/arch/arm/dts/imx6qdl-sr-som.dtsi
index b06577808f..c20bed2721 100644
--- a/arch/arm/dts/imx6qdl-sr-som.dtsi
+++ b/arch/arm/dts/imx6qdl-sr-som.dtsi
@@ -55,7 +55,13 @@
pinctrl-0 = <_microsom_enet_ar8035>;
phy-handle = <>;
phy-mode = "rgmii-id";
-   phy-reset-duration = <2>;
+
+   /*
+* The PHY seems to require a long-enough reset duration to avoid
+* some rare issues where the PHY gets stuck in an inconsistent and
+* non-functional state at boot-up. 10ms proved to be fine .
+*/
+   phy-reset-duration = <10>;
phy-reset-gpios = < 15 GPIO_ACTIVE_LOW>;
status = "okay";
  
@@ -64,8 +70,15 @@

#size-cells = <0>;
  
  		phy: ethernet-phy@0 {

-   reg = <0>;
+   /*
+* The PHY can appear either:
+* - AR8035: at address 0 or 4
+* - ADIN1300: at address 1
+* Actual address being detected at runtime.
+*/
+   reg = <0x>;
qca,clk-out-frequency = <12500>;
+   adi,phy-output-clock = "125mhz-free-running";
};
};
  };
--
2.35.3




Re: [PATCH v2 4/5] mx6cuboxi: fixup dtb ethernet phy nodes before booting an OS

2022-05-19 Thread Josua Mayer

Hold off on this one, I found a mistake in the switch case below.
While fully functional, it does enable both phy nodes for addresses 0 
and 4 unconditionally.
Cubox requires both, as do potentially any derivate devices - but the 
HummingBoards do not.


Am 19.05.22 um 12:31 schrieb Josua Mayer:

SoM revision 1.9 has replaced the ar8035 phy address 0 with an adin1300
at address 1. Because early SoMs had a hardware flaw, the ar8035 can
also appear at address 4 - making it a total of 3 phy nodes in the DTB.

To avoid confusing Linux with probe errors, fixup the dtb to only enable
the phy node that is detected at runtime.

Signed-off-by: Josua Mayer 
---
  board/solidrun/mx6cuboxi/mx6cuboxi.c | 78 
  configs/mx6cuboxi_defconfig  |  1 +
  2 files changed, 79 insertions(+)

diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c 
b/board/solidrun/mx6cuboxi/mx6cuboxi.c
index 6207bf8253..42aa5cb63c 100644
--- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
+++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
@@ -1,5 +1,7 @@
  // SPDX-License-Identifier: GPL-2.0+
  /*
+ * Copyright (C) 2022 Josua Mayer 
+ *
   * Copyright (C) 2015 Freescale Semiconductor, Inc.
   *
   * Author: Fabio Estevam 
@@ -39,6 +41,8 @@
  #include 
  #include 
  #include 
+#include 
+#include 
  
  DECLARE_GLOBAL_DATA_PTR;
  
@@ -407,6 +411,80 @@ out:

return 0;
  }
  
+static int find_ethernet_phy(void)

+{
+   struct mii_dev *bus = NULL;
+   struct phy_device *phydev = NULL;
+   int phy_addr = -ENOENT;
+
+#ifdef CONFIG_FEC_MXC
+   bus = fec_get_miibus(ENET_BASE_ADDR, -1);
+   if (!bus)
+   return -ENOENT;
+
+   // scan address 0, 1, 4
+   phydev = phy_find_by_mask(bus, 0b00010011);
+   if (!phydev) {
+   free(bus);
+   return -ENOENT;
+   }
+   pr_debug("%s: detected ethernet phy at address %d\n", __func__, 
phydev->addr);
+   phy_addr = phydev->addr;
+
+   free(phydev);
+#endif
+
+   return phy_addr;
+}
+
+#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP)
+/*
+ * Configure the correct ethernet PHYs nodes in device-tree:
+ * - AR8035 at addresses 0 or 4: Cubox
+ * - AR8035 at address 0: HummingBoard, HummingBoard 2
+ * - ADIN1300 at address 1: since SoM rev 1.9
+ */
+int ft_board_setup(void *fdt, struct bd_info *bd)
+{
+   int node_phy0, node_phy1, node_phy4;
+   int ret, phy;
+   bool enable_phy0 = false, enable_phy1 = false, enable_phy4 = false;
+
+   // detect phy
+   phy = find_ethernet_phy();
+   if (phy == 0 || phy == 4) {
+   enable_phy0 = true;
+   switch (board_type()) {
+   case CUBOXI:
+   case UNKNOWN:
+   default:
+   enable_phy4 = true;
+   }
+   } else if (phy == 1) {
+   enable_phy1 = true;
+   } else {
+   pr_err("%s: couldn't detect ethernet phy, not patching dtb!\n", 
__func__);
+   return 0;
+   }
+
+   // update all phy nodes status
+   node_phy0 = fdt_path_offset(fdt, 
"/soc/bus@210/ethernet@2188000/mdio/ethernet-phy@0");
+   ret = fdt_setprop_string(fdt, node_phy0, "status", enable_phy0 ? "okay" : 
"disabled");
+   if (ret < 0 && enable_phy0)
+   pr_err("%s: failed to enable ethernet phy at address 0 in 
dtb!\n", __func__);
+   node_phy1 = fdt_path_offset(fdt, 
"/soc/bus@210/ethernet@2188000/mdio/ethernet-phy@1");
+   ret = fdt_setprop_string(fdt, node_phy1, "status", enable_phy1 ? "okay" : 
"disabled");
+   if (ret < 0 && enable_phy1)
+   pr_err("%s: failed to enable ethernet phy at address 1 in 
dtb!\n", __func__);
+   node_phy4 = fdt_path_offset(fdt, 
"/soc/bus@210/ethernet@2188000/mdio/ethernet-phy@4");
+   ret = fdt_setprop_string(fdt, node_phy4, "status", enable_phy4 ? "okay" : 
"disabled");
+   if (ret < 0 && enable_phy4)
+   pr_err("%s: failed to enable ethernet phy at address 4 in 
dtb!\n", __func__);
+
+   return 0;
+}
+#endif
+
  /* Override the default implementation, DT model is not accurate */
  int show_board_info(void)
  {
diff --git a/configs/mx6cuboxi_defconfig b/configs/mx6cuboxi_defconfig
index 1e2e332af9..d3ac8eeeba 100644
--- a/configs/mx6cuboxi_defconfig
+++ b/configs/mx6cuboxi_defconfig
@@ -22,6 +22,7 @@ CONFIG_CMD_HDMIDETECT=y
  CONFIG_AHCI=y
  CONFIG_DISTRO_DEFAULTS=y
  CONFIG_FIT=y
+CONFIG_OF_BOARD_SETUP=y
  CONFIG_BOOTCOMMAND="run findfdt; run finduuid; run distro_bootcmd"
  CONFIG_USE_PREBOOT=y
  CONFIG_PREBOOT="if hdmidet; then usb start; setenv stdin  serial,usbkbd; setenv 
stdout serial,vidconsole; setenv stderr serial,vidconsole; else setenv stdin  serial; 
setenv stdout serial; setenv stderr serial; fi;"


[PATCH v2 4/5] mx6cuboxi: fixup dtb ethernet phy nodes before booting an OS

2022-05-19 Thread Josua Mayer
SoM revision 1.9 has replaced the ar8035 phy address 0 with an adin1300
at address 1. Because early SoMs had a hardware flaw, the ar8035 can
also appear at address 4 - making it a total of 3 phy nodes in the DTB.

To avoid confusing Linux with probe errors, fixup the dtb to only enable
the phy node that is detected at runtime.

Signed-off-by: Josua Mayer 
---
 board/solidrun/mx6cuboxi/mx6cuboxi.c | 78 
 configs/mx6cuboxi_defconfig  |  1 +
 2 files changed, 79 insertions(+)

diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c 
b/board/solidrun/mx6cuboxi/mx6cuboxi.c
index 6207bf8253..42aa5cb63c 100644
--- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
+++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
+ * Copyright (C) 2022 Josua Mayer 
+ *
  * Copyright (C) 2015 Freescale Semiconductor, Inc.
  *
  * Author: Fabio Estevam 
@@ -39,6 +41,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -407,6 +411,80 @@ out:
return 0;
 }
 
+static int find_ethernet_phy(void)
+{
+   struct mii_dev *bus = NULL;
+   struct phy_device *phydev = NULL;
+   int phy_addr = -ENOENT;
+
+#ifdef CONFIG_FEC_MXC
+   bus = fec_get_miibus(ENET_BASE_ADDR, -1);
+   if (!bus)
+   return -ENOENT;
+
+   // scan address 0, 1, 4
+   phydev = phy_find_by_mask(bus, 0b00010011);
+   if (!phydev) {
+   free(bus);
+   return -ENOENT;
+   }
+   pr_debug("%s: detected ethernet phy at address %d\n", __func__, 
phydev->addr);
+   phy_addr = phydev->addr;
+
+   free(phydev);
+#endif
+
+   return phy_addr;
+}
+
+#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP)
+/*
+ * Configure the correct ethernet PHYs nodes in device-tree:
+ * - AR8035 at addresses 0 or 4: Cubox
+ * - AR8035 at address 0: HummingBoard, HummingBoard 2
+ * - ADIN1300 at address 1: since SoM rev 1.9
+ */
+int ft_board_setup(void *fdt, struct bd_info *bd)
+{
+   int node_phy0, node_phy1, node_phy4;
+   int ret, phy;
+   bool enable_phy0 = false, enable_phy1 = false, enable_phy4 = false;
+
+   // detect phy
+   phy = find_ethernet_phy();
+   if (phy == 0 || phy == 4) {
+   enable_phy0 = true;
+   switch (board_type()) {
+   case CUBOXI:
+   case UNKNOWN:
+   default:
+   enable_phy4 = true;
+   }
+   } else if (phy == 1) {
+   enable_phy1 = true;
+   } else {
+   pr_err("%s: couldn't detect ethernet phy, not patching dtb!\n", 
__func__);
+   return 0;
+   }
+
+   // update all phy nodes status
+   node_phy0 = fdt_path_offset(fdt, 
"/soc/bus@210/ethernet@2188000/mdio/ethernet-phy@0");
+   ret = fdt_setprop_string(fdt, node_phy0, "status", enable_phy0 ? "okay" 
: "disabled");
+   if (ret < 0 && enable_phy0)
+   pr_err("%s: failed to enable ethernet phy at address 0 in 
dtb!\n", __func__);
+   node_phy1 = fdt_path_offset(fdt, 
"/soc/bus@210/ethernet@2188000/mdio/ethernet-phy@1");
+   ret = fdt_setprop_string(fdt, node_phy1, "status", enable_phy1 ? "okay" 
: "disabled");
+   if (ret < 0 && enable_phy1)
+   pr_err("%s: failed to enable ethernet phy at address 1 in 
dtb!\n", __func__);
+   node_phy4 = fdt_path_offset(fdt, 
"/soc/bus@210/ethernet@2188000/mdio/ethernet-phy@4");
+   ret = fdt_setprop_string(fdt, node_phy4, "status", enable_phy4 ? "okay" 
: "disabled");
+   if (ret < 0 && enable_phy4)
+   pr_err("%s: failed to enable ethernet phy at address 4 in 
dtb!\n", __func__);
+
+   return 0;
+}
+#endif
+
 /* Override the default implementation, DT model is not accurate */
 int show_board_info(void)
 {
diff --git a/configs/mx6cuboxi_defconfig b/configs/mx6cuboxi_defconfig
index 1e2e332af9..d3ac8eeeba 100644
--- a/configs/mx6cuboxi_defconfig
+++ b/configs/mx6cuboxi_defconfig
@@ -22,6 +22,7 @@ CONFIG_CMD_HDMIDETECT=y
 CONFIG_AHCI=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_FIT=y
+CONFIG_OF_BOARD_SETUP=y
 CONFIG_BOOTCOMMAND="run findfdt; run finduuid; run distro_bootcmd"
 CONFIG_USE_PREBOOT=y
 CONFIG_PREBOOT="if hdmidet; then usb start; setenv stdin  serial,usbkbd; 
setenv stdout serial,vidconsole; setenv stderr serial,vidconsole; else setenv 
stdin  serial; setenv stdout serial; setenv stderr serial; fi;"
-- 
2.35.3



[PATCH v2 5/5] mx6cuboxi: enable driver for adin1300 phy

2022-05-19 Thread Josua Mayer
Since SoMs revision 1.9 the ar8035 phy has been replaced by adin1300.
Enable the driver so that the new SoMs have functional networking.

Signed-off-by: Josua Mayer 
---
 configs/mx6cuboxi_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/mx6cuboxi_defconfig b/configs/mx6cuboxi_defconfig
index d3ac8eeeba..46634a1727 100644
--- a/configs/mx6cuboxi_defconfig
+++ b/configs/mx6cuboxi_defconfig
@@ -55,6 +55,7 @@ CONFIG_DWC_AHSATA=y
 CONFIG_SPL_SYS_I2C_LEGACY=y
 CONFIG_FSL_USDHC=y
 CONFIG_PHYLIB=y
+CONFIG_PHY_ADIN=y
 CONFIG_PHY_ATHEROS=y
 CONFIG_DM_ETH=y
 CONFIG_FEC_MXC=y
-- 
2.35.3



[PATCH v2 1/5] phy: adin: fix broken support for adi, phy-mode-override

2022-05-19 Thread Josua Mayer
From: Nate Drude 

Currently, the adin driver fails to compile.

The original patch introducing the adin driver used the function
phy_get_interface_by_name to support the adi,phy-mode-override
property. Unfortunately, a few days before the adin patch
was accepted, another patch removed support for phy_get_interface_by_name:

https://github.com/u-boot/u-boot/commit/123ca114e07ecf28aa2538748d733e2b22d8b8b5

This patch refactors adin_get_phy_mode_override, implementing the logic in
the new function, ofnode_read_phy_mode, from the patch above.

Signed-off-by: Nate Drude 
Tested-by: Josua Mayer 
Signed-off-by: Josua Mayer 
---
 drivers/net/phy/adin.c | 22 +++---
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index cff841ab3d..e60f288b9b 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -100,27 +100,27 @@ static u32 adin_get_reg_value(struct phy_device *phydev,
  * The function gets phy-mode string from property 'adi,phy-mode-override'
  * and return its index in phy_interface_strings table, or -1 in error case.
  */
-int adin_get_phy_mode_override(struct phy_device *phydev)
+phy_interface_t adin_get_phy_mode_override(struct phy_device *phydev)
 {
ofnode node = phy_get_ofnode(phydev);
const char *phy_mode_override;
const char *prop_phy_mode_override = "adi,phy-mode-override";
-   int override_interface;
+   int i;
 
phy_mode_override = ofnode_read_string(node, prop_phy_mode_override);
if (!phy_mode_override)
-   return -ENODEV;
+   return PHY_INTERFACE_MODE_NA;
 
debug("%s: %s = '%s'\n",
  __func__, prop_phy_mode_override, phy_mode_override);
 
-   override_interface = phy_get_interface_by_name(phy_mode_override);
+   for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++)
+   if (!strcmp(phy_mode_override, phy_interface_strings[i]))
+   return (phy_interface_t) i;
 
-   if (override_interface < 0)
-   printf("%s: %s = '%s' is not valid\n",
-  __func__, prop_phy_mode_override, phy_mode_override);
+   printf("%s: Invalid PHY interface '%s'\n", __func__, phy_mode_override);
 
-   return override_interface;
+   return PHY_INTERFACE_MODE_NA;
 }
 
 static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum)
@@ -148,10 +148,10 @@ static int adin_config_rgmii_mode(struct phy_device 
*phydev)
 {
u16 reg_val;
u32 val;
-   int phy_mode_override = adin_get_phy_mode_override(phydev);
+   phy_interface_t phy_mode_override = adin_get_phy_mode_override(phydev);
 
-   if (phy_mode_override >= 0) {
-   phydev->interface = (phy_interface_t) phy_mode_override;
+   if (phy_mode_override != PHY_INTERFACE_MODE_NA) {
+   phydev->interface = phy_mode_override;
}
 
reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
-- 
2.35.3



[PATCH v2 3/5] ARM: dts: imx6qdl-sr-som: add support for alternate phy addresses

2022-05-19 Thread Josua Mayer
The Cubox has an unstable phy address - which can appear at either
address 0 (intended) or 4 (unintended).

SoM revision 1.9 has replaced the ar8035 phy with an adin1300, which
will always appear at address 1.

Change the reg property of the phy node to the magic value 0x,
which indicates to the generic phy driver that all addresses should be
probed. That allows the same node (which is pinned by phy-handle) to match
either the AR8035 PHY at both possible addresses, as well as the new one
at address 1.
Also add the new adi,phy-output-clock property for enabling the 125MHz
clock used by the fec ethernet controller, as submitted to Linux [1].

Linux solves this problem differently:
For the ar8035 phy it will probe both phy nodes in device-tree in order,
and use the one that succeeds. For the new adin1300 it expects U-Boot to
patch the status field in the DTB before booting

While at it also sync the reset-delay with the upstream Linux dtb.

[1] 
https://patchwork.kernel.org/project/netdevbpf/patch/20220428082848.12191-4-jo...@solid-run.com/

Signed-off-by: Josua Mayer 
---
 arch/arm/dts/imx6qdl-sr-som.dtsi | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/arm/dts/imx6qdl-sr-som.dtsi b/arch/arm/dts/imx6qdl-sr-som.dtsi
index b06577808f..c20bed2721 100644
--- a/arch/arm/dts/imx6qdl-sr-som.dtsi
+++ b/arch/arm/dts/imx6qdl-sr-som.dtsi
@@ -55,7 +55,13 @@
pinctrl-0 = <_microsom_enet_ar8035>;
phy-handle = <>;
phy-mode = "rgmii-id";
-   phy-reset-duration = <2>;
+
+   /*
+* The PHY seems to require a long-enough reset duration to avoid
+* some rare issues where the PHY gets stuck in an inconsistent and
+* non-functional state at boot-up. 10ms proved to be fine .
+*/
+   phy-reset-duration = <10>;
phy-reset-gpios = < 15 GPIO_ACTIVE_LOW>;
status = "okay";
 
@@ -64,8 +70,15 @@
#size-cells = <0>;
 
phy: ethernet-phy@0 {
-   reg = <0>;
+   /*
+* The PHY can appear either:
+* - AR8035: at address 0 or 4
+* - ADIN1300: at address 1
+* Actual address being detected at runtime.
+*/
+   reg = <0x>;
qca,clk-out-frequency = <12500>;
+   adi,phy-output-clock = "125mhz-free-running";
};
};
 };
-- 
2.35.3



[PATCH v2 2/5] phy: adin: add support for clock output

2022-05-19 Thread Josua Mayer
The ADIN1300 supports generating certain clocks on its GP_CLK pin, as
well as providing the reference clock on CLK25_REF.

Add support for selecting the clock via device-tree properties.

This patch is based on the Linux implementation for this feature,
which has been added to netdev/net-next.git [1].

[2] 
https://patchwork.kernel.org/project/netdevbpf/cover/20220517085143.3749-1-jo...@solid-run.com/

Signed-off-by: Josua Mayer 
---
V1 -> V2: removed recovered clock options

 drivers/net/phy/adin.c | 42 ++
 1 file changed, 42 insertions(+)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index e60f288b9b..a5bfd960d9 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -4,6 +4,7 @@
  *
  * Copyright 2019 Analog Devices Inc.
  * Copyright 2022 Variscite Ltd.
+ * Copyright 2022 Josua Mayer 
  */
 #include 
 #include 
@@ -13,6 +14,16 @@
 #define PHY_ID_ADIN13000x0283bc30
 #define ADIN1300_EXT_REG_PTR   0x10
 #define ADIN1300_EXT_REG_DATA  0x11
+
+#define ADIN1300_GE_CLK_CFG_REG0xff1f
+#define   ADIN1300_GE_CLK_CFG_MASK GENMASK(5, 0)
+#define   ADIN1300_GE_CLK_CFG_RCVR_125 BIT(5)
+#define   ADIN1300_GE_CLK_CFG_FREE_125 BIT(4)
+#define   ADIN1300_GE_CLK_CFG_REF_EN   BIT(3)
+#define   ADIN1300_GE_CLK_CFG_HRT_RCVR BIT(2)
+#define   ADIN1300_GE_CLK_CFG_HRT_FREE BIT(1)
+#define   ADIN1300_GE_CLK_CFG_25   BIT(0)
+
 #define ADIN1300_GE_RGMII_CFG  0xff23
 #define ADIN1300_GE_RGMII_RX_MSK   GENMASK(8, 6)
 #define ADIN1300_GE_RGMII_RX_SEL(x)\
@@ -144,6 +155,33 @@ static int adin_ext_write(struct phy_device *phydev, const 
u32 regnum, const u16
return phy_write(phydev, MDIO_DEVAD_NONE, ADIN1300_EXT_REG_DATA, val);
 }
 
+static int adin_config_clk_out(struct phy_device *phydev)
+{
+   ofnode node = phy_get_ofnode(phydev);
+   const char *val = NULL;
+   u8 sel = 0;
+
+   val = ofnode_read_string(node, "adi,phy-output-clock");
+   if (!val) {
+   /* property not present, do not enable GP_CLK pin */
+   } else if (strcmp(val, "25mhz-reference") == 0) {
+   sel |= ADIN1300_GE_CLK_CFG_25;
+   } else if (strcmp(val, "125mhz-free-running") == 0) {
+   sel |= ADIN1300_GE_CLK_CFG_FREE_125;
+   } else if (strcmp(val, "adaptive-free-running") == 0) {
+   sel |= ADIN1300_GE_CLK_CFG_HRT_FREE;
+   } else {
+   pr_err("%s: invalid adi,phy-output-clock\n", __func__);
+   return -EINVAL;
+   }
+
+   if (ofnode_read_bool(node, "adi,phy-output-reference-clock"))
+   sel |= ADIN1300_GE_CLK_CFG_REF_EN;
+
+   return adin_ext_write(phydev, ADIN1300_GE_CLK_CFG_REG,
+ ADIN1300_GE_CLK_CFG_MASK & sel);
+}
+
 static int adin_config_rgmii_mode(struct phy_device *phydev)
 {
u16 reg_val;
@@ -202,6 +240,10 @@ static int adin1300_config(struct phy_device *phydev)
 
printf("ADIN1300 PHY detected at addr %d\n", phydev->addr);
 
+   ret = adin_config_clk_out(phydev);
+   if (ret < 0)
+   return ret;
+
ret = adin_config_rgmii_mode(phydev);
 
if (ret < 0)
-- 
2.35.3



[PATCH v2 0/5] mx6cuboxi: add eth support for SoMs revision 1.9+

2022-05-19 Thread Josua Mayer
As of Revision 1.9 the SolidRun i.MX6 SoMs ship with a new PHY - an
ADIN1300 at address 1. This patchset carries many small parts to
facilitate proper operation of the ethernet port in U-Boot as well as
Linux:

1. Fix a compile error in the recently added phy driver
2. Add support for configuring the clock output pins to the phy driver
3. update device-tree with support for the new phy
4. support runtime patching of device-tree for Linux:
   enable only the phy node detected during boot, to avoid
   warning messages when Linux probes the phys.
5. finally enable the phy driver in the cuboxi defconfig

I have included Nate's patch here only so that it won't get lost - it is fully 
independent and I have not made any changes.

Changes since v1:
- applied proper compile fix by Nate Drude
- removed -recovered clock options as requested by Linux maintainers

Josua Mayer (4):
  phy: adin: add support for clock output
  ARM: dts: imx6qdl-sr-som: add support for alternate phy addresses
  mx6cuboxi: fixup dtb ethernet phy nodes before booting an OS
  mx6cuboxi: enable driver for adin1300 phy

Nate Drude (1):
  phy: adin: fix broken support for adi,phy-mode-override

 arch/arm/dts/imx6qdl-sr-som.dtsi | 17 +-
 board/solidrun/mx6cuboxi/mx6cuboxi.c | 78 
 configs/mx6cuboxi_defconfig  |  2 +
 drivers/net/phy/adin.c   | 64 +++
 4 files changed, 148 insertions(+), 13 deletions(-)

-- 
2.35.3



Re: [PATCH 0/5] mx6cuboxi: add eth support for SoMs revision 1.9+

2022-05-19 Thread Josua Mayer

Greetings everybody,

The changes to the adin phy device-tree bindings and driver have been 
applied to netdev/net-next:

https://patchwork.kernel.org/project/netdevbpf/cover/20220517085143.3749-1-jo...@solid-run.com/

As there have been no further comments, I will send v2 soon including 
some changes the Linux maintainers asked for - and drop the first patch 
now that Nate Drude has submitted a fix:

phy: adin: fix broken support for adi,phy-mode-override

sincerely
Josua Mayer

Am 01.05.22 um 15:41 schrieb Josua Mayer:

As of Revision 1.9 the SolidRun i.MX6 SoMs ship with a new PHY - an
ADIN1300 at address 1. This patchset carries many small parts to
facilitate proper operation of the ethernet port in U-Boot as well as
Linux:

1. Fix a compile error in the recently phy driver
2. Add support for configuring the clock output pins to the phy driver
3. update device-tree with support for the neew phy
4. support runtime patching of device-tree for Linux:
enables only the phy node detected during boot, to avoid
warning messages during boot.
5. finally enable the phy driver in the cuboxi defconfig

Josua Mayer (5):
   phy: adin: remove broken support for adi,phy-mode-override
   phy: adin: add support for clock output
   ARM: dts: imx6qdl-sr-som: add support for alternate phy addresses
   mx6cuboxi: fixup dtb ethernet phy nodes before booting an OS
   mx6cuboxi: enable driver for adin1300 phy

  arch/arm/dts/imx6qdl-sr-som.dtsi | 17 +-
  board/solidrun/mx6cuboxi/mx6cuboxi.c | 78 +++
  configs/mx6cuboxi_defconfig  |  2 +
  drivers/net/phy/adin.c   | 80 
  4 files changed, 141 insertions(+), 36 deletions(-)



Re: [PATCH 1/5] phy: adin: remove broken support for adi,phy-mode-override

2022-05-08 Thread Josua Mayer

Hi Nate, Maintainers,

This patch is now superseded by Nate's
[PATCH] phy: adin: fix broken support for adi,phy-mode-override
submitted yesterday.

The remainder of this patchset cleanly applies on top of it though,
so I won't roll v2.

Let me know if there is any further feedback on this series, or if it 
can be applied.


Am 05.05.22 um 18:30 schrieb Josua Mayer:

\o/

Am 04.05.22 um 15:46 schrieb Nate Drude:

Hi Josua,

On Wed, 2022-05-04 at 11:51 +0300, Josua Mayer wrote:

Hi Nate,

Am 02.05.22 um 16:25 schrieb Nate Drude:

Hi Josua,

On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote:

The adin_get_phy_mode_override function does not compile, because
it
is
missing both declaration and implementation of
phy_get_interface_by_name.

Remove the whole function for now, since the missing
implementation
is
not included in mainline Linux - and thus can not be copied.

Signed-off-by: Josua Mayer 
---
   drivers/net/phy/adin.c | 34 --
   1 file changed, 34 deletions(-)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index cff841ab3d..2433e76fea 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct
phy_device
*phydev,
  return rc;
   }
   -/**
- * adin_get_phy_mode_override - Get phy-mode override for adin
PHY
- *
- * The function gets phy-mode string from property 'adi,phy-
mode-
override'
- * and return its index in phy_interface_strings table, or -1 in
error case.
- */
-int adin_get_phy_mode_override(struct phy_device *phydev)
-{
-   ofnode node = phy_get_ofnode(phydev);
-   const char *phy_mode_override;
-   const char *prop_phy_mode_override = "adi,phy-mode-
override";
-   int override_interface;
-
-   phy_mode_override = ofnode_read_string(node,
prop_phy_mode_override);
-   if (!phy_mode_override)
-   return -ENODEV;
-
-   debug("%s: %s = '%s'\n",
- __func__, prop_phy_mode_override,
phy_mode_override);
-
-   override_interface =
phy_get_interface_by_name(phy_mode_override);
-
-   if (override_interface < 0)
-   printf("%s: %s = '%s' is not valid\n",
-  __func__, prop_phy_mode_override,
phy_mode_override);
-
-   return override_interface;
-}
-
   static u16 adin_ext_read(struct phy_device *phydev, const u32
regnum)
   {
  u16 val;
@@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct
phy_device *phydev)
   {
  u16 reg_val;
  u32 val;
-   int phy_mode_override =
adin_get_phy_mode_override(phydev);
-
-   if (phy_mode_override >= 0) {
-   phydev->interface = (phy_interface_t)
phy_mode_override;
-   }
     reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);

The patch that introduced adin_get_phy_mode_override was tested
against
the U-Boot master branch. Unfortunately, phy_get_interface_by_name
was
removed just a few days before by:
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fu-boot%2Fu-boot%2Fcommit%2F123ca114e07ecf28aa2538748d733e2b22d8b8b5data=05%7C01%7CNate.D%40variscite.com%7C467af650626f4902ae2b08da2dab6225%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637872511359995609%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=Ok3YOO8rheEZuRa%2Fn04NZf37TsPQ7hzArUojFZ4QDUg%3Dreserved=0 


:(

Can we fix adin_get_phy_mode_override instead of removing it?

I will take a look in the coming days and see if I understand how to
fix
this.
I don't really mind if we fix it, or remove it - removing was the
faster

If you prefer, I can submit a separate patch to fix it. I'll wait to
hear from you to avoid duplicated work.
I won't be able to look at this before Sunday, so feel free to work on 
it.

way getting my patch-set out of course ...

We can
implement part of ofnode_read_phy_mode. This needs to be tested,
but
for example:

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index cff841ab3d..6a05b9fd05 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct
phy_device
*phydev,
    * The function gets phy-mode string from property 'adi,phy-mode-
override'
    * and return its index in phy_interface_strings table, or -1 in
error
case.
    */
-int adin_get_phy_mode_override(struct phy_device *phydev)
+phy_interface_t adin_get_phy_mode_override(struct phy_device
*phydev)
   {
  ofnode node = phy_get_ofnode(phydev);
  const char *phy_mode_override;
  const char *prop_phy_mode_override = "adi,phy-mode-
override";
-   int override_interface;
+   int i;
     phy_mode_override = ofnode_read_string(node,
prop_phy_mode_override);
+
  if (!phy_mode_override)
-   return -ENODEV;
+   return PHY_INTERFACE_MODE_NA;
     debug("%s: %s = '%s

Re: [PATCH] phy: adin: fix broken support for adi,phy-mode-override

2022-05-08 Thread Josua Mayer

Hi Nate,

That was quick! I have rebased my changes on top of this patch,
and can confirm that the adin phy driver compiles, and still works.

I did not test setting adi,[rt]x-internal-delay-ps in dts though.

So, I guess you can add my superficial
Tested-By: Josua Mayer 

[PATCH 0/5] mx6cuboxi: add eth support for SoMs revision 1.9+
patches 2-5 apply cleanly, so I won't immediately roll a v2.

Am 07.05.22 um 00:52 schrieb Nate Drude:

Currently, the adin driver fails to compile.

The original patch introducing the adin driver used the function
phy_get_interface_by_name to support the adi,phy-mode-override
property. Unfortunately, a few days before the adin patch
was accepted, another patch removed support for phy_get_interface_by_name:

https://github.com/u-boot/u-boot/commit/123ca114e07ecf28aa2538748d733e2b22d8b8b5

This patch refactors adin_get_phy_mode_override, implementing the logic in
the new function, ofnode_read_phy_mode, from the patch above.

Signed-off-by: Nate Drude 
---
  drivers/net/phy/adin.c | 22 +++---
  1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index cff841ab3d..e60f288b9b 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -100,27 +100,27 @@ static u32 adin_get_reg_value(struct phy_device *phydev,
   * The function gets phy-mode string from property 'adi,phy-mode-override'
   * and return its index in phy_interface_strings table, or -1 in error case.
   */
-int adin_get_phy_mode_override(struct phy_device *phydev)
+phy_interface_t adin_get_phy_mode_override(struct phy_device *phydev)
  {
ofnode node = phy_get_ofnode(phydev);
const char *phy_mode_override;
const char *prop_phy_mode_override = "adi,phy-mode-override";
-   int override_interface;
+   int i;
  
  	phy_mode_override = ofnode_read_string(node, prop_phy_mode_override);

if (!phy_mode_override)
-   return -ENODEV;
+   return PHY_INTERFACE_MODE_NA;
  
  	debug("%s: %s = '%s'\n",

  __func__, prop_phy_mode_override, phy_mode_override);
  
-	override_interface = phy_get_interface_by_name(phy_mode_override);

+   for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++)
+   if (!strcmp(phy_mode_override, phy_interface_strings[i]))
+   return (phy_interface_t) i;
  
-	if (override_interface < 0)

-   printf("%s: %s = '%s' is not valid\n",
-  __func__, prop_phy_mode_override, phy_mode_override);
+   printf("%s: Invalid PHY interface '%s'\n", __func__, phy_mode_override);
  
-	return override_interface;

+   return PHY_INTERFACE_MODE_NA;
  }
  
  static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum)

@@ -148,10 +148,10 @@ static int adin_config_rgmii_mode(struct phy_device 
*phydev)
  {
u16 reg_val;
u32 val;
-   int phy_mode_override = adin_get_phy_mode_override(phydev);
+   phy_interface_t phy_mode_override = adin_get_phy_mode_override(phydev);
  
-	if (phy_mode_override >= 0) {

-   phydev->interface = (phy_interface_t) phy_mode_override;
+   if (phy_mode_override != PHY_INTERFACE_MODE_NA) {
+   phydev->interface = phy_mode_override;
}
  
  	reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);


- Josua Mayer



Re: [PATCH 1/5] phy: adin: remove broken support for adi,phy-mode-override

2022-05-05 Thread Josua Mayer

\o/

Am 04.05.22 um 15:46 schrieb Nate Drude:

Hi Josua,

On Wed, 2022-05-04 at 11:51 +0300, Josua Mayer wrote:

Hi Nate,

Am 02.05.22 um 16:25 schrieb Nate Drude:

Hi Josua,

On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote:

The adin_get_phy_mode_override function does not compile, because
it
is
missing both declaration and implementation of
phy_get_interface_by_name.

Remove the whole function for now, since the missing
implementation
is
not included in mainline Linux - and thus can not be copied.

Signed-off-by: Josua Mayer 
---
   drivers/net/phy/adin.c | 34 --
   1 file changed, 34 deletions(-)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index cff841ab3d..2433e76fea 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct
phy_device
*phydev,
  return rc;
   }
   
-/**

- * adin_get_phy_mode_override - Get phy-mode override for adin
PHY
- *
- * The function gets phy-mode string from property 'adi,phy-
mode-
override'
- * and return its index in phy_interface_strings table, or -1 in
error case.
- */
-int adin_get_phy_mode_override(struct phy_device *phydev)
-{
-   ofnode node = phy_get_ofnode(phydev);
-   const char *phy_mode_override;
-   const char *prop_phy_mode_override = "adi,phy-mode-
override";
-   int override_interface;
-
-   phy_mode_override = ofnode_read_string(node,
prop_phy_mode_override);
-   if (!phy_mode_override)
-   return -ENODEV;
-
-   debug("%s: %s = '%s'\n",
- __func__, prop_phy_mode_override,
phy_mode_override);
-
-   override_interface =
phy_get_interface_by_name(phy_mode_override);
-
-   if (override_interface < 0)
-   printf("%s: %s = '%s' is not valid\n",
-  __func__, prop_phy_mode_override,
phy_mode_override);
-
-   return override_interface;
-}
-
   static u16 adin_ext_read(struct phy_device *phydev, const u32
regnum)
   {
  u16 val;
@@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct
phy_device *phydev)
   {
  u16 reg_val;
  u32 val;
-   int phy_mode_override =
adin_get_phy_mode_override(phydev);
-
-   if (phy_mode_override >= 0) {
-   phydev->interface = (phy_interface_t)
phy_mode_override;
-   }
   
  reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
   

The patch that introduced adin_get_phy_mode_override was tested
against
the U-Boot master branch. Unfortunately, phy_get_interface_by_name
was
removed just a few days before by:
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Fu-boot%2Fu-boot%2Fcommit%2F123ca114e07ecf28aa2538748d733e2b22d8b8b5data=05%7C01%7CNate.D%40variscite.com%7C467af650626f4902ae2b08da2dab6225%7C399ae6ac38f44ef094a8440b0ad581de%7C1%7C0%7C637872511359995609%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7Csdata=Ok3YOO8rheEZuRa%2Fn04NZf37TsPQ7hzArUojFZ4QDUg%3Dreserved=0

:(

Can we fix adin_get_phy_mode_override instead of removing it?

I will take a look in the coming days and see if I understand how to
fix
this.
I don't really mind if we fix it, or remove it - removing was the
faster

If you prefer, I can submit a separate patch to fix it. I'll wait to
hear from you to avoid duplicated work.

I won't be able to look at this before Sunday, so feel free to work on it.

way getting my patch-set out of course ...

We can
implement part of ofnode_read_phy_mode. This needs to be tested,
but
for example:

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index cff841ab3d..6a05b9fd05 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct
phy_device
*phydev,
    * The function gets phy-mode string from property 'adi,phy-mode-
override'
    * and return its index in phy_interface_strings table, or -1 in
error
case.
    */
-int adin_get_phy_mode_override(struct phy_device *phydev)
+phy_interface_t adin_get_phy_mode_override(struct phy_device
*phydev)
   {
  ofnode node = phy_get_ofnode(phydev);
  const char *phy_mode_override;
  const char *prop_phy_mode_override = "adi,phy-mode-
override";
-   int override_interface;
+   int i;
   
  phy_mode_override = ofnode_read_string(node,

prop_phy_mode_override);
+
  if (!phy_mode_override)
-   return -ENODEV;
+   return PHY_INTERFACE_MODE_NA;
   
  debug("%s: %s = '%s'\n",

    __func__, prop_phy_mode_override,
phy_mode_override);
   
-   override_interface =

phy_get_interface_by_name(phy_mode_override);
+   for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++)
+   if (!strcmp(phy_mode_override,
phy_interface_strings[i]))
+   return i;
   
-   if (override_interf

Re: [PATCH 1/5] phy: adin: remove broken support for adi,phy-mode-override

2022-05-04 Thread Josua Mayer

Hi Nate,

Am 02.05.22 um 16:25 schrieb Nate Drude:

Hi Josua,

On Sun, 2022-05-01 at 15:41 +0300, Josua Mayer wrote:

The adin_get_phy_mode_override function does not compile, because it
is
missing both declaration and implementation of
phy_get_interface_by_name.

Remove the whole function for now, since the missing implementation
is
not included in mainline Linux - and thus can not be copied.

Signed-off-by: Josua Mayer 
---
  drivers/net/phy/adin.c | 34 --
  1 file changed, 34 deletions(-)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index cff841ab3d..2433e76fea 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct phy_device
*phydev,
 return rc;
  }
  
-/**

- * adin_get_phy_mode_override - Get phy-mode override for adin PHY
- *
- * The function gets phy-mode string from property 'adi,phy-mode-
override'
- * and return its index in phy_interface_strings table, or -1 in
error case.
- */
-int adin_get_phy_mode_override(struct phy_device *phydev)
-{
-   ofnode node = phy_get_ofnode(phydev);
-   const char *phy_mode_override;
-   const char *prop_phy_mode_override = "adi,phy-mode-override";
-   int override_interface;
-
-   phy_mode_override = ofnode_read_string(node,
prop_phy_mode_override);
-   if (!phy_mode_override)
-   return -ENODEV;
-
-   debug("%s: %s = '%s'\n",
- __func__, prop_phy_mode_override, phy_mode_override);
-
-   override_interface =
phy_get_interface_by_name(phy_mode_override);
-
-   if (override_interface < 0)
-   printf("%s: %s = '%s' is not valid\n",
-  __func__, prop_phy_mode_override,
phy_mode_override);
-
-   return override_interface;
-}
-
  static u16 adin_ext_read(struct phy_device *phydev, const u32
regnum)
  {
 u16 val;
@@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct
phy_device *phydev)
  {
 u16 reg_val;
 u32 val;
-   int phy_mode_override = adin_get_phy_mode_override(phydev);
-
-   if (phy_mode_override >= 0) {
-   phydev->interface = (phy_interface_t)
phy_mode_override;
-   }
  
 reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
  

The patch that introduced adin_get_phy_mode_override was tested against
the U-Boot master branch. Unfortunately, phy_get_interface_by_name was
removed just a few days before by:
https://github.com/u-boot/u-boot/commit/123ca114e07ecf28aa2538748d733e2b22d8b8b5

:(

Can we fix adin_get_phy_mode_override instead of removing it?
I will take a look in the coming days and see if I understand how to fix 
this.
I don't really mind if we fix it, or remove it - removing was the faster 
way getting my patch-set out of course ...

We can
implement part of ofnode_read_phy_mode. This needs to be tested, but
for example:

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index cff841ab3d..6a05b9fd05 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -100,27 +100,28 @@ static u32 adin_get_reg_value(struct phy_device
*phydev,
   * The function gets phy-mode string from property 'adi,phy-mode-
override'
   * and return its index in phy_interface_strings table, or -1 in error
case.
   */
-int adin_get_phy_mode_override(struct phy_device *phydev)
+phy_interface_t adin_get_phy_mode_override(struct phy_device *phydev)
  {
 ofnode node = phy_get_ofnode(phydev);
 const char *phy_mode_override;
 const char *prop_phy_mode_override = "adi,phy-mode-override";
-   int override_interface;
+   int i;
  
 phy_mode_override = ofnode_read_string(node,

prop_phy_mode_override);
+
 if (!phy_mode_override)
-   return -ENODEV;
+   return PHY_INTERFACE_MODE_NA;
  
 debug("%s: %s = '%s'\n",

   __func__, prop_phy_mode_override, phy_mode_override);
  
-   override_interface =

phy_get_interface_by_name(phy_mode_override);
+   for (i = 0; i < PHY_INTERFACE_MODE_MAX; i++)
+   if (!strcmp(phy_mode_override,
phy_interface_strings[i]))
+   return i;
  
-   if (override_interface < 0)

-   printf("%s: %s = '%s' is not valid\n",
-  __func__, prop_phy_mode_override,
phy_mode_override);
+   debug("%s: Invalid PHY interface '%s'\n", __func__,
phy_mode_override);
  
-   return override_interface;

+   return PHY_INTERFACE_MODE_NA;
  }
  
  static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum)

@@ -148,10 +149,10 @@ static int adin_config_rgmii_mode(struct
phy_device *phydev)
  {
 u16 reg_val;
 u32 val;
-   int phy_mode_override = adin_get_phy_mode_override(phydev);
+   phy_interface_t phy_mode_override =
adin_get_phy_mode_override(phydev);
  
-   if (phy_mode_over

Re: [PATCH 04/12] cmd: tlv_eeprom: convert functions used by command to api functions

2022-05-03 Thread Josua Mayer

\o/

Am 03.05.22 um 13:54 schrieb Stefan Roese:

Hi Josua,

On 03.05.22 09:17, Josua Mayer wrote:

Am 03.05.22 um 09:16 schrieb Stefan Roese:

On 02.05.22 16:18, Josua Mayer wrote:

- prog_eeprom: write_tlvinfo_tlv_eeprom
- update_crc: tlvinfo_update_crc
- is_valid_tlv: is_valid_tlvinfo_entry
- is_checksum_valid: tlvinfo_check_crc


So while creating a new API it makes sense to prepend the function
names identical IMHO to not "pollute" the namespace. Something like

- tlv_is_valid_entry
- tlv_check_crc
...

Just examples, you get the idea.
Yes. The hard part in this particular implementation is that the 
naming is not consistent.


The most sense I could make is that prefix tlvinfo indicates all tlv 
data, i.e. working with the whole structure, while tlvinfo_tlv 
indicates working with one data entry. Further write, read and is_ 
are currently prefixed in the header, but for previously static 
functions in the C file it was put in the middle ...


I found it quite difficult to prepare for splitting off a library in 
a way that preserves history, i.e. diffs should still be readable for 
spotting mistakes.


Yes, a decent history would be welcome. But still, when going global
here with a new API this should be consistant.
My view more like - patches 1-10 are not really new API, in that I am 
only changing what is necessary to allow splitting the code.
I was considering to at the very end do a mass-rename and come up 
with better naming, something like

tlv_{set,get}_{blob,string,mac}
tlv_find_entry
tlv_{read,write}_eeprom

But this is pending a refactoring and extension of the tlv parsing 
code in board/solidrun/common/tlv_data.*, to figure out what is 
required or useful.


So your plan is to this:
a) Get this patchset included
b) Use it in board specific code, e.g. solidrun
c) Do the mass-rename

Is this correct?

This is close. I would say
1) get the split merged
2) rebase board-specific code
2a) figure out what kinds of set/get/add/remove functions are useful
3) redesign api
   - there are inconsistencies with the order of function arguments
   - read/write functions imo should use the tlv header to determine 
size, not a function argument
   - at least one of the tlv data types can appear multiple times, 
existing code does not support this

4) submit any renames and extensions to the tlv library
5) submit board-specific use of tlv eeprom data

If yes, why is it better to do the renaming at the end?
It is very difficult to work on a patch-set that touches the same code 
before and after moving it to a different file, which I found myself 
doing a lot while prototyping this.
So if now I went ahead and figured out proper names and purposes for all 
functions that I think should be the tlv api, then I have to divide that 
into those parts that are renames or refactoring of existing 
functionality, and those parts that are strictly new - putting the 
former before splitting off the library, and the latter to after.


I am not sure I can manage this level of complexity.

- Josua Mayer



Thanks,
Stefan



Thanks,
Stefan


Signed-off-by: Josua Mayer 
---
  cmd/tlv_eeprom.c | 56 
+++
  include/tlv_eeprom.h | 57 


  2 files changed, 76 insertions(+), 37 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 00c5b5f840..1b4f2537f6 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -28,13 +28,9 @@ DECLARE_GLOBAL_DATA_PTR;
  #define MAX_TLV_DEVICES    2
    /* File scope function prototypes */
-static bool is_checksum_valid(u8 *eeprom);
  static int read_eeprom(int devnum, u8 *eeprom);
  static void show_eeprom(int devnum, u8 *eeprom);
  static void decode_tlv(struct tlvinfo_tlv *tlv);
-static void update_crc(u8 *eeprom);
-static int prog_eeprom(int devnum, u8 *eeprom);
-static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int 
*eeprom_index);

  static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
  static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
  static int set_mac(char *buf, const char *string);
@@ -58,18 +54,6 @@ static inline bool is_digit(char c)
  return (c >= '0' && c <= '9');
  }
  -/**
- *  is_valid_tlv
- *
- *  Perform basic sanity checks on a TLV field. The TLV is pointed to
- *  by the parameter provided.
- *  1. The type code is not reserved (0x00 or 0xFF)
- */
-static inline bool is_valid_tlv(struct tlvinfo_tlv *tlv)
-{
-    return((tlv->type != 0x00) && (tlv->type != 0xFF));
-}
-
  /**
   *  is_hex
   *
@@ -83,14 +67,12 @@ static inline u8 is_hex(char p)
  }
    /**
- *  is_checksum_valid
- *
   *  Validate the checksum in the provided TlvInfo EEPROM data. 
First,

   *  verify that the TlvInfo header is valid, then make sure the last
   *  TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
   *  and compare it to the value stored in the EEPROM CRC-32 TLV.
   */
-static bool is_checksum_valid(u8 *eepr

Re: [PATCH 04/12] cmd: tlv_eeprom: convert functions used by command to api functions

2022-05-03 Thread Josua Mayer

Am 03.05.22 um 09:16 schrieb Stefan Roese:

On 02.05.22 16:18, Josua Mayer wrote:

- prog_eeprom: write_tlvinfo_tlv_eeprom
- update_crc: tlvinfo_update_crc
- is_valid_tlv: is_valid_tlvinfo_entry
- is_checksum_valid: tlvinfo_check_crc


So while creating a new API it makes sense to prepend the function
names identical IMHO to not "pollute" the namespace. Something like

- tlv_is_valid_entry
- tlv_check_crc
...

Just examples, you get the idea.
Yes. The hard part in this particular implementation is that the naming 
is not consistent.


The most sense I could make is that prefix tlvinfo indicates all tlv 
data, i.e. working with the whole structure, while tlvinfo_tlv indicates 
working with one data entry. Further write, read and is_ are currently 
prefixed in the header, but for previously static functions in the C 
file it was put in the middle ...


I found it quite difficult to prepare for splitting off a library in a 
way that preserves history, i.e. diffs should still be readable for 
spotting mistakes.


I was considering to at the very end do a mass-rename and come up with 
better naming, something like

tlv_{set,get}_{blob,string,mac}
tlv_find_entry
tlv_{read,write}_eeprom

But this is pending a refactoring and extension of the tlv parsing code 
in board/solidrun/common/tlv_data.*, to figure out what is required or 
useful.




Thanks,
Stefan


Signed-off-by: Josua Mayer 
---
  cmd/tlv_eeprom.c | 56 +++
  include/tlv_eeprom.h | 57 
  2 files changed, 76 insertions(+), 37 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 00c5b5f840..1b4f2537f6 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -28,13 +28,9 @@ DECLARE_GLOBAL_DATA_PTR;
  #define MAX_TLV_DEVICES    2
    /* File scope function prototypes */
-static bool is_checksum_valid(u8 *eeprom);
  static int read_eeprom(int devnum, u8 *eeprom);
  static void show_eeprom(int devnum, u8 *eeprom);
  static void decode_tlv(struct tlvinfo_tlv *tlv);
-static void update_crc(u8 *eeprom);
-static int prog_eeprom(int devnum, u8 *eeprom);
-static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
  static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
  static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
  static int set_mac(char *buf, const char *string);
@@ -58,18 +54,6 @@ static inline bool is_digit(char c)
  return (c >= '0' && c <= '9');
  }
  -/**
- *  is_valid_tlv
- *
- *  Perform basic sanity checks on a TLV field. The TLV is pointed to
- *  by the parameter provided.
- *  1. The type code is not reserved (0x00 or 0xFF)
- */
-static inline bool is_valid_tlv(struct tlvinfo_tlv *tlv)
-{
-    return((tlv->type != 0x00) && (tlv->type != 0xFF));
-}
-
  /**
   *  is_hex
   *
@@ -83,14 +67,12 @@ static inline u8 is_hex(char p)
  }
    /**
- *  is_checksum_valid
- *
   *  Validate the checksum in the provided TlvInfo EEPROM data. First,
   *  verify that the TlvInfo header is valid, then make sure the last
   *  TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
   *  and compare it to the value stored in the EEPROM CRC-32 TLV.
   */
-static bool is_checksum_valid(u8 *eeprom)
+bool tlvinfo_check_crc(u8 *eeprom)
  {
  struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
  struct tlvinfo_tlv    *eeprom_crc;
@@ -137,11 +119,11 @@ static int read_eeprom(int devnum, u8 *eeprom)
    // If the contents are invalid, start over with default contents
  if (!is_valid_tlvinfo_header(eeprom_hdr) ||
-    !is_checksum_valid(eeprom)) {
+    !tlvinfo_check_crc(eeprom)) {
  strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
  eeprom_hdr->version = TLV_INFO_VERSION;
  eeprom_hdr->totallen = cpu_to_be16(0);
-    update_crc(eeprom);
+    tlvinfo_update_crc(eeprom);
  }
    #ifdef DEBUG
@@ -183,7 +165,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
  tlv_end  = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
  while (curr_tlv < tlv_end) {
  eeprom_tlv = to_entry([curr_tlv]);
-    if (!is_valid_tlv(eeprom_tlv)) {
+    if (!is_valid_tlvinfo_entry(eeprom_tlv)) {
  printf("Invalid TLV field starting at EEPROM offset %d\n",
 curr_tlv);
  return;
@@ -193,7 +175,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
  }
    printf("Checksum is %s.\n",
-   is_checksum_valid(eeprom) ? "valid" : "invalid");
+   tlvinfo_check_crc(eeprom) ? "valid" : "invalid");
    #ifdef DEBUG
  printf("EEPROM dump: (0x%x bytes)", TLV_INFO_MAX_LEN);
@@ -340,13 +322,13 @@ static void decode_tlv(struct tlvinfo_tlv *tlv)
  }
    /**
- *  update_crc
+ *  tlvinfo_update_crc
   *
   *  This function updates the CRC-32 TLV. If there is no CRC-32 
TLV, then
   *  one is adde

Re: [PATCH 01/12] cmd: tlv_eeprom: remove use of global variable current_dev

2022-05-03 Thread Josua Mayer

Hi Stefan,

Thank you for starting to review this patchset!

Am 03.05.22 um 09:09 schrieb Stefan Roese:

Hi Josua,

a few general comments about this series:

- A cover letter for this patchset would be very helpful, so that
  reviewers have a summary of the changes.


I have sent a cover-letter to the list, but you are not in the To field 
- perhaps you can find it - as it does give an overview what I am trying 
to do (subject [PATCH 00/12] split tlv_eeprom command into a separate 
library)




- Please Cc the original author of this code Baruch Siach in the
  next version as well (if there is next version that is).

Made a mental note, will do.


On 02.05.22 16:18, Josua Mayer wrote:

Make tlv_eeprom command device selection an explicit parameter of all
function calls.

Signed-off-by: Josua Mayer 


Reviewed-by: Stefan Roese 

Thanks,
Stefan


---
  cmd/tlv_eeprom.c | 50 ++--
  include/tlv_eeprom.h |  3 ++-
  2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index bf8d453dc5..f91c11b304 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -29,18 +29,18 @@ DECLARE_GLOBAL_DATA_PTR;
    /* File scope function prototypes */
  static bool is_checksum_valid(u8 *eeprom);
-static int read_eeprom(u8 *eeprom);
-static void show_eeprom(u8 *eeprom);
+static int read_eeprom(int devnum, u8 *eeprom);
+static void show_eeprom(int devnum, u8 *eeprom);
  static void decode_tlv(struct tlvinfo_tlv *tlv);
  static void update_crc(u8 *eeprom);
-static int prog_eeprom(u8 *eeprom);
+static int prog_eeprom(int devnum, u8 *eeprom);
  static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
  static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
  static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
  static int set_mac(char *buf, const char *string);
  static int set_date(char *buf, const char *string);
  static int set_bytes(char *buf, const char *string, int 
*converted_accum);

-static void show_tlv_devices(void);
+static void show_tlv_devices(int current_dev);
    /* Set to 1 if we've read EEPROM into memory */
  static int has_been_read;
@@ -48,7 +48,6 @@ static int has_been_read;
  static u8 eeprom[TLV_INFO_MAX_LEN];
    static struct udevice *tlv_devices[MAX_TLV_DEVICES];
-static unsigned int current_dev;
    #define to_header(p) ((struct tlvinfo_header *)p)
  #define to_entry(p) ((struct tlvinfo_tlv *)p)
@@ -125,7 +124,7 @@ static bool is_checksum_valid(u8 *eeprom)
   *
   *  Read the EEPROM into memory, if it hasn't already been read.
   */
-static int read_eeprom(u8 *eeprom)
+static int read_eeprom(int devnum, u8 *eeprom)
  {
  int ret;
  struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
@@ -135,12 +134,11 @@ static int read_eeprom(u8 *eeprom)
  return 0;
    /* Read the header */
-    ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, 
current_dev);

+    ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
  /* If the header was successfully read, read the TLVs */
  if (ret == 0 && is_valid_tlvinfo_header(eeprom_hdr))
  ret = read_tlv_eeprom((void *)eeprom_tlv, HDR_SIZE,
-  be16_to_cpu(eeprom_hdr->totallen),
-  current_dev);
+  be16_to_cpu(eeprom_hdr->totallen), devnum);
    // If the contents are invalid, start over with default contents
  if (!is_valid_tlvinfo_header(eeprom_hdr) ||
@@ -165,7 +163,7 @@ static int read_eeprom(u8 *eeprom)
   *
   *  Display the contents of the EEPROM
   */
-static void show_eeprom(u8 *eeprom)
+static void show_eeprom(int devnum, u8 *eeprom)
  {
  int tlv_end;
  int curr_tlv;
@@ -180,7 +178,7 @@ static void show_eeprom(u8 *eeprom)
  return;
  }
  -    printf("TLV: %u\n", current_dev);
+    printf("TLV: %u\n", devnum);
  printf("TlvInfo Header:\n");
  printf("   Id String:    %s\n", eeprom_hdr->signature);
  printf("   Version:  %d\n", eeprom_hdr->version);
@@ -389,7 +387,7 @@ static void update_crc(u8 *eeprom)
   *
   *  Write the EEPROM data from CPU memory to the hardware.
   */
-static int prog_eeprom(u8 *eeprom)
+static int prog_eeprom(int devnum, u8 *eeprom)
  {
  int ret = 0;
  struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
@@ -398,7 +396,7 @@ static int prog_eeprom(u8 *eeprom)
  update_crc(eeprom);
    eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
-    ret = write_tlv_eeprom(eeprom, eeprom_len);
+    ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
  if (ret) {
  printf("Programming failed.\n");
  return -1;
@@ -433,11 +431,12 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int 
flag, int argc, char *const argv[])

  {
  char cmd;
  struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+    static unsigned int current_dev;
    // If no arguments, r

[PATCH 10/12] cmd: tlv_eeprom: split off tlv library from command

2022-05-02 Thread Josua Mayer
The eeprom command includes functions for reading and writing
tlv-formatted data from an eeprom, as well as an implementation of the
cli command tlv_eeprom.

Split off the parsing, read and write into a standalone tlv library.

Signed-off-by: Josua Mayer 
---
 cmd/Kconfig  |   2 +
 cmd/tlv_eeprom.c | 742 +-
 lib/Kconfig  |   2 +
 lib/Makefile |   2 +
 lib/tlv/Kconfig  |  15 +
 lib/tlv/Makefile |   5 +
 lib/tlv/tlv_eeprom.c | 750 +++
 7 files changed, 786 insertions(+), 732 deletions(-)
 create mode 100644 lib/tlv/Kconfig
 create mode 100644 lib/tlv/Makefile
 create mode 100644 lib/tlv/tlv_eeprom.c

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 2b575a2b42..821b5e9d6b 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -166,6 +166,7 @@ config CMD_REGINFO
 
 config CMD_TLV_EEPROM
bool "tlv_eeprom"
+   select EEPROM_TLV_LIB
depends on I2C_EEPROM
help
  Display and program the system EEPROM data block in ONIE Tlvinfo
@@ -173,6 +174,7 @@ config CMD_TLV_EEPROM
 
 config SPL_CMD_TLV_EEPROM
bool "tlv_eeprom for SPL"
+   select SPL_EEPROM_TLV_LIB
depends on SPL_I2C_EEPROM
select SPL_DRIVERS_MISC
help
diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index c66116b2c4..99b79cad8b 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -10,131 +10,26 @@
  * Copyright (C) 2022 Josua Mayer 
  */
 
-#include 
 #include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-#include 
-
-#include "tlv_eeprom.h"
-
-DECLARE_GLOBAL_DATA_PTR;
+#include 
+#include 
+#include 
+#include 
 
 /* File scope function prototypes */
-static int read_eeprom(int devnum, u8 *eeprom);
 static void show_eeprom(int devnum, u8 *eeprom);
-static void decode_tlv(struct tlvinfo_tlv *tlv);
-static int set_mac(char *buf, const char *string);
-static int set_date(char *buf, const char *string);
-static int set_bytes(char *buf, const char *string, int *converted_accum);
 static void show_tlv_devices(int current_dev);
+static inline const char *tlv_type2name(u8 type);
+static void decode_tlv(struct tlvinfo_tlv *tlv);
+static int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int argc, char 
*const argv[]);
+static void show_tlv_code_list(void);
 
 /* The EERPOM contents after being read into memory */
 static u8 eeprom[TLV_INFO_MAX_LEN];
 
-static struct udevice *tlv_devices[TLV_MAX_DEVICES];
-
 #define to_header(p) ((struct tlvinfo_header *)p)
 #define to_entry(p) ((struct tlvinfo_tlv *)p)
 
-/**
- * Check whether eeprom device exists.
- */
-bool exists_tlv_eeprom(int dev)
-{
-   return dev < TLV_MAX_DEVICES && tlv_devices[dev] != 0;
-}
-
-static inline bool is_digit(char c)
-{
-   return (c >= '0' && c <= '9');
-}
-
-/**
- *  is_hex
- *
- *  Tests if character is an ASCII hex digit
- */
-static inline u8 is_hex(char p)
-{
-   return (((p >= '0') && (p <= '9')) ||
-   ((p >= 'A') && (p <= 'F')) ||
-   ((p >= 'a') && (p <= 'f')));
-}
-
-/**
- *  Validate the checksum in the provided TlvInfo EEPROM data. First,
- *  verify that the TlvInfo header is valid, then make sure the last
- *  TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
- *  and compare it to the value stored in the EEPROM CRC-32 TLV.
- */
-bool tlvinfo_check_crc(u8 *eeprom)
-{
-   struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
-   struct tlvinfo_tlv*eeprom_crc;
-   unsigned int   calc_crc;
-   unsigned int   stored_crc;
-
-   // Is the eeprom header valid?
-   if (!is_valid_tlvinfo_header(eeprom_hdr))
-   return false;
-
-   // Is the last TLV a CRC?
-   eeprom_crc = to_entry([TLV_INFO_HEADER_SIZE +
-   be16_to_cpu(eeprom_hdr->totallen) - (TLV_INFO_ENTRY_SIZE + 4)]);
-   if (eeprom_crc->type != TLV_CODE_CRC_32 || eeprom_crc->length != 4)
-   return false;
-
-   // Calculate the checksum
-   calc_crc = crc32(0, (void *)eeprom,
-TLV_INFO_HEADER_SIZE + 
be16_to_cpu(eeprom_hdr->totallen) - 4);
-   stored_crc = (eeprom_crc->value[0] << 24) |
-   (eeprom_crc->value[1] << 16) |
-   (eeprom_crc->value[2] <<  8) |
-   eeprom_crc->value[3];
-   return calc_crc == stored_crc;
-}
-
-/**
- *  read_eeprom
- *
- *  Read the EEPROM into memory, if it hasn't already been read.
- */
-static int read_eeprom(int devnum, u8 *eeprom)
-{
-   int ret;
-   struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
-   struct tlvinfo_tlv *eeprom_tlv = 
to_entry([TLV_INFO_HEADER_SIZE]);
-
-   /* Read the header */
-   ret = read_tlv_eeprom((void *)eeprom_hdr, 0, TLV_INFO_HEADER_SIZE, 
devnum);
-   /* If the header was s

[PATCH 12/12] lib: tlv_eeprom: add function for reading one entry into a C string

2022-05-02 Thread Josua Mayer
This solves the potentially common problem of getting a specific tlv
entry from an eeprom in board-files, without having to introduce several
variables, error handling, memcpy and 0-terminating the string.

Signed-off-by: Josua Mayer 
---
 include/tlv_eeprom.h | 12 
 lib/tlv/tlv_eeprom.c | 25 +
 2 files changed, 37 insertions(+)

diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
index c81c58837d..5989c611f5 100644
--- a/include/tlv_eeprom.h
+++ b/include/tlv_eeprom.h
@@ -166,6 +166,18 @@ bool tlvinfo_add_tlv(u8 *eeprom, int code, char *strval);
  */
 bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
 
+/**
+ * Read the TLV entry with specified code to a buffer as terminated C string.
+ * @eeprom: Pointer to buffer holding the TLV EEPROM binary data.
+ * @code:   The TLV Code of the entry to read.
+ * @buffer: Pointer to buffer where the value will be stored. Must have 
capacity
+ *  for the string representation of the data including null 
terminator.
+ * @length: size of the buffer where the value will be stored.
+ *
+ * Return length of string on success, -1 on error.
+ */
+ssize_t tlvinfo_read_tlv(u8 *eeprom, u8 code, u8 *buffer, size_t length);
+
 /**
  *  tlvinfo_update_crc
  *
diff --git a/lib/tlv/tlv_eeprom.c b/lib/tlv/tlv_eeprom.c
index 464f0aa1fa..205960e8f2 100644
--- a/lib/tlv/tlv_eeprom.c
+++ b/lib/tlv/tlv_eeprom.c
@@ -350,6 +350,31 @@ bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval)
return true;
 }
 
+/**
+ * Read the TLV entry with specified code to a buffer as terminated C string.
+ */
+ssize_t tlvinfo_read_tlv(u8 *eeprom, u8 code, u8 *buffer, size_t length)
+{
+   int index;
+   struct tlvinfo_tlv *tlv;
+
+   // read sku from part-number field
+   if (tlvinfo_find_tlv(eeprom, code, )) {
+   tlv = (struct tlvinfo_tlv *)[index];
+   if (tlv->length > length) {
+   pr_err("%s: tlv value (%d) larger than buffer (%zu)!\n",
+  __func__, tlv->length + 1, length);
+   return -1;
+   }
+   memcpy(buffer, tlv->value, tlv->length);
+   buffer[tlv->length] = 0;
+
+   return tlv->length;
+   }
+
+   return -1;
+}
+
 /**
  *  set_mac
  *
-- 
2.34.1



[PATCH 09/12] cmd: tlv_eeprom: add my copyright

2022-05-02 Thread Josua Mayer
While each of the previous individual changes is small, accumulated they
amount to a substantial effort. Explicitly add a Copyright declaration.

Signed-off-by: Josua Mayer 
---
 cmd/tlv_eeprom.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index c110927cb5..c66116b2c4 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -7,6 +7,7 @@
  * Copyright (C) 2014 Srideep 
  * Copyright (C) 2013 Miles Tseng 
  * Copyright (C) 2014,2016 david_yang 
+ * Copyright (C) 2022 Josua Mayer 
  */
 
 #include 
-- 
2.34.1



[PATCH 11/12] arm: mvebu: clearfog: enable tlv library for spl in favour of eeprom cmd

2022-05-02 Thread Josua Mayer
The board file used CONFIG_SPL_CMD_TLV_EEPROM as a library to facilitate
reading tlv data with the memory size from eeprom.
Since the tlv library has been split off, only CONFIG_SPL_EEPROM_TLV_LIB
is required now.

Signed-off-by: Josua Mayer 
---
 configs/clearfog_defconfig | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configs/clearfog_defconfig b/configs/clearfog_defconfig
index 880f16a6e0..87ba6f29a7 100644
--- a/configs/clearfog_defconfig
+++ b/configs/clearfog_defconfig
@@ -12,6 +12,7 @@ CONFIG_DM_GPIO=y
 CONFIG_DEFAULT_DEVICE_TREE="armada-388-clearfog"
 CONFIG_SPL_TEXT_BASE=0x4030
 CONFIG_SPL_SERIAL=y
+CONFIG_SPL_DRIVERS_MISC=y
 CONFIG_SPL=y
 CONFIG_DEBUG_UART_BASE=0xd0012000
 CONFIG_DEBUG_UART_CLOCK=25000
@@ -26,7 +27,6 @@ CONFIG_SYS_CONSOLE_INFO_QUIET=y
 CONFIG_DISPLAY_BOARDINFO_LATE=y
 CONFIG_SPL_I2C=y
 CONFIG_CMD_TLV_EEPROM=y
-CONFIG_SPL_CMD_TLV_EEPROM=y
 # CONFIG_CMD_FLASH is not set
 CONFIG_CMD_GPIO=y
 CONFIG_CMD_I2C=y
@@ -71,3 +71,4 @@ CONFIG_SYS_NS16550=y
 CONFIG_KIRKWOOD_SPI=y
 CONFIG_USB=y
 CONFIG_USB_XHCI_HCD=y
+CONFIG_SPL_EEPROM_TLV_LIB=y
-- 
2.34.1



[PATCH 07/12] cmd: tlv_eeprom: hide access to static tlv_devices array behind accessor

2022-05-02 Thread Josua Mayer
The tlv_eeprom command logic checks the static tlv_devices array to
validate the eeprom number. This array will be move to a separate tlv
library.
Hide this access behind a new function exists_tlv_eeprom.

Signed-off-by: Josua Mayer 
---
 cmd/tlv_eeprom.c | 12 ++--
 include/tlv_eeprom.h |  7 +++
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 57468edb1c..f51a9666bf 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -44,6 +44,14 @@ static struct udevice *tlv_devices[MAX_TLV_DEVICES];
 #define to_header(p) ((struct tlvinfo_header *)p)
 #define to_entry(p) ((struct tlvinfo_tlv *)p)
 
+/**
+ * Check whether eeprom device exists.
+ */
+bool exists_tlv_eeprom(int dev)
+{
+   return dev < TLV_MAX_DEVICES && tlv_devices[dev] != 0;
+}
+
 static inline bool is_digit(char c)
 {
return (c >= '0' && c <= '9');
@@ -481,7 +489,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
unsigned int devnum;
 
devnum = simple_strtoul(argv[2], NULL, 0);
-   if (devnum > MAX_TLV_DEVICES || !tlv_devices[devnum]) {
+   if (!exists_tlv_eeprom(devnum)) {
printf("Invalid device number\n");
return 0;
}
@@ -866,7 +874,7 @@ static void show_tlv_devices(int current_dev)
unsigned int dev;
 
for (dev = 0; dev < MAX_TLV_DEVICES; dev++)
-   if (tlv_devices[dev])
+   if (exists_tlv_eeprom(dev))
printf("TLV: %u%s\n", dev,
   (dev == current_dev) ? " (*)" : "");
 }
diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
index dc7952da6b..c81c58837d 100644
--- a/include/tlv_eeprom.h
+++ b/include/tlv_eeprom.h
@@ -69,6 +69,13 @@ struct __attribute__ ((__packed__)) tlvinfo_tlv {
 /* how many EEPROMs can be used */
 #define TLV_MAX_DEVICES2
 
+/**
+ * Check whether eeprom device exists.
+ *
+ * @dev: EEPROM device to check.
+ */
+bool exists_tlv_eeprom(int dev);
+
 /**
  * read_tlv_eeprom - Read the EEPROM binary data from the hardware
  * @eeprom: Pointer to buffer to hold the binary data
-- 
2.34.1



[PATCH 08/12] cmd: tlv_eeprom: clean up two defines for one thing

2022-05-02 Thread Josua Mayer
MAX_TLV_DEVICES defined in C, and TLV_MAX_DEVICES defined in the header
serve the same purpose. Replace all occurences of the former by the
latter.

Signed-off-by: Josua Mayer 
---
 cmd/tlv_eeprom.c | 12 +---
 1 file changed, 5 insertions(+), 7 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index f51a9666bf..c110927cb5 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -25,8 +25,6 @@
 
 DECLARE_GLOBAL_DATA_PTR;
 
-#define MAX_TLV_DEVICES2
-
 /* File scope function prototypes */
 static int read_eeprom(int devnum, u8 *eeprom);
 static void show_eeprom(int devnum, u8 *eeprom);
@@ -39,7 +37,7 @@ static void show_tlv_devices(int current_dev);
 /* The EERPOM contents after being read into memory */
 static u8 eeprom[TLV_INFO_MAX_LEN];
 
-static struct udevice *tlv_devices[MAX_TLV_DEVICES];
+static struct udevice *tlv_devices[TLV_MAX_DEVICES];
 
 #define to_header(p) ((struct tlvinfo_header *)p)
 #define to_entry(p) ((struct tlvinfo_tlv *)p)
@@ -873,7 +871,7 @@ static void show_tlv_devices(int current_dev)
 {
unsigned int dev;
 
-   for (dev = 0; dev < MAX_TLV_DEVICES; dev++)
+   for (dev = 0; dev < TLV_MAX_DEVICES; dev++)
if (exists_tlv_eeprom(dev))
printf("TLV: %u%s\n", dev,
   (dev == current_dev) ? " (*)" : "");
@@ -890,7 +888,7 @@ static int find_tlv_devices(struct udevice **tlv_devices_p)
ret = uclass_next_device_check()) {
if (ret == 0)
tlv_devices_p[count_dev++] = dev;
-   if (count_dev >= MAX_TLV_DEVICES)
+   if (count_dev >= TLV_MAX_DEVICES)
break;
}
 
@@ -899,7 +897,7 @@ static int find_tlv_devices(struct udevice **tlv_devices_p)
 
 static struct udevice *find_tlv_device_by_index(int dev_num)
 {
-   struct udevice *local_tlv_devices[MAX_TLV_DEVICES] = {};
+   struct udevice *local_tlv_devices[TLV_MAX_DEVICES] = {};
struct udevice **tlv_devices_p;
int ret;
 
@@ -926,7 +924,7 @@ int read_tlv_eeprom(void *eeprom, int offset, int len, int 
dev_num)
 {
struct udevice *dev;
 
-   if (dev_num >= MAX_TLV_DEVICES)
+   if (dev_num >= TLV_MAX_DEVICES)
return -EINVAL;
 
dev = find_tlv_device_by_index(dev_num);
-- 
2.34.1



[PATCH 06/12] cmd: tlv_eeprom: move missing declarations and defines to header

2022-05-02 Thread Josua Mayer
In preparation of splitting the tlv_eeprom command into a separate
library, add function declarations and defines used by the command logic
to the tlv_eeprom header file.

Signed-off-by: Josua Mayer 
---
 cmd/tlv_eeprom.c | 59 
 include/tlv_eeprom.h | 29 --
 2 files changed, 54 insertions(+), 34 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 1b4f2537f6..57468edb1c 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -31,8 +31,6 @@ DECLARE_GLOBAL_DATA_PTR;
 static int read_eeprom(int devnum, u8 *eeprom);
 static void show_eeprom(int devnum, u8 *eeprom);
 static void decode_tlv(struct tlvinfo_tlv *tlv);
-static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
-static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
 static int set_mac(char *buf, const char *string);
 static int set_date(char *buf, const char *string);
 static int set_bytes(char *buf, const char *string, int *converted_accum);
@@ -46,9 +44,6 @@ static struct udevice *tlv_devices[MAX_TLV_DEVICES];
 #define to_header(p) ((struct tlvinfo_header *)p)
 #define to_entry(p) ((struct tlvinfo_tlv *)p)
 
-#define HDR_SIZE sizeof(struct tlvinfo_header)
-#define ENT_SIZE sizeof(struct tlvinfo_tlv)
-
 static inline bool is_digit(char c)
 {
return (c >= '0' && c <= '9');
@@ -84,14 +79,14 @@ bool tlvinfo_check_crc(u8 *eeprom)
return false;
 
// Is the last TLV a CRC?
-   eeprom_crc = to_entry([HDR_SIZE +
-   be16_to_cpu(eeprom_hdr->totallen) - (ENT_SIZE + 4)]);
+   eeprom_crc = to_entry([TLV_INFO_HEADER_SIZE +
+   be16_to_cpu(eeprom_hdr->totallen) - (TLV_INFO_ENTRY_SIZE + 4)]);
if (eeprom_crc->type != TLV_CODE_CRC_32 || eeprom_crc->length != 4)
return false;
 
// Calculate the checksum
calc_crc = crc32(0, (void *)eeprom,
-HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen) - 4);
+TLV_INFO_HEADER_SIZE + 
be16_to_cpu(eeprom_hdr->totallen) - 4);
stored_crc = (eeprom_crc->value[0] << 24) |
(eeprom_crc->value[1] << 16) |
(eeprom_crc->value[2] <<  8) |
@@ -108,13 +103,13 @@ static int read_eeprom(int devnum, u8 *eeprom)
 {
int ret;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
-   struct tlvinfo_tlv *eeprom_tlv = to_entry([HDR_SIZE]);
+   struct tlvinfo_tlv *eeprom_tlv = 
to_entry([TLV_INFO_HEADER_SIZE]);
 
/* Read the header */
-   ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
+   ret = read_tlv_eeprom((void *)eeprom_hdr, 0, TLV_INFO_HEADER_SIZE, 
devnum);
/* If the header was successfully read, read the TLVs */
if (ret == 0 && is_valid_tlvinfo_header(eeprom_hdr))
-   ret = read_tlv_eeprom((void *)eeprom_tlv, HDR_SIZE,
+   ret = read_tlv_eeprom((void *)eeprom_tlv, TLV_INFO_HEADER_SIZE,
  be16_to_cpu(eeprom_hdr->totallen), 
devnum);
 
// If the contents are invalid, start over with default contents
@@ -161,8 +156,8 @@ static void show_eeprom(int devnum, u8 *eeprom)
 
printf("TLV Name Code Len Value\n");
printf("  --- -\n");
-   curr_tlv = HDR_SIZE;
-   tlv_end  = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
+   curr_tlv = TLV_INFO_HEADER_SIZE;
+   tlv_end  = TLV_INFO_HEADER_SIZE + be16_to_cpu(eeprom_hdr->totallen);
while (curr_tlv < tlv_end) {
eeprom_tlv = to_entry([curr_tlv]);
if (!is_valid_tlvinfo_entry(eeprom_tlv)) {
@@ -171,7 +166,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
return;
}
decode_tlv(eeprom_tlv);
-   curr_tlv += ENT_SIZE + eeprom_tlv->length;
+   curr_tlv += TLV_INFO_ENTRY_SIZE + eeprom_tlv->length;
}
 
printf("Checksum is %s.\n",
@@ -339,10 +334,10 @@ void tlvinfo_update_crc(u8 *eeprom)
if (!tlvinfo_find_tlv(eeprom, TLV_CODE_CRC_32, _index)) {
unsigned int totallen = be16_to_cpu(eeprom_hdr->totallen);
 
-   if ((totallen + ENT_SIZE + 4) > TLV_TOTAL_LEN_MAX)
+   if ((totallen + TLV_INFO_ENTRY_SIZE + 4) > TLV_TOTAL_LEN_MAX)
return;
-   eeprom_index = HDR_SIZE + totallen;
-   eeprom_hdr->totallen = cpu_to_be16(totallen + ENT_SIZE + 4);
+   eeprom_index = TLV_INFO_HEADER_SIZE + totallen;
+   eeprom_hdr->totallen = cpu_to_be16(totallen + 
TLV_INFO_ENTRY_SIZE + 4);
}
eeprom_crc = to_entry([eeprom_index]);
eeprom_crc->type = TLV_CODE_CRC_32;
@@ -350,7 +345,7 @@ void tlvinfo_update_crc(u8 *eeprom)
 
// Calcula

[PATCH 04/12] cmd: tlv_eeprom: convert functions used by command to api functions

2022-05-02 Thread Josua Mayer
- prog_eeprom: write_tlvinfo_tlv_eeprom
- update_crc: tlvinfo_update_crc
- is_valid_tlv: is_valid_tlvinfo_entry
- is_checksum_valid: tlvinfo_check_crc

Signed-off-by: Josua Mayer 
---
 cmd/tlv_eeprom.c | 56 +++
 include/tlv_eeprom.h | 57 
 2 files changed, 76 insertions(+), 37 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index 00c5b5f840..1b4f2537f6 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -28,13 +28,9 @@ DECLARE_GLOBAL_DATA_PTR;
 #define MAX_TLV_DEVICES2
 
 /* File scope function prototypes */
-static bool is_checksum_valid(u8 *eeprom);
 static int read_eeprom(int devnum, u8 *eeprom);
 static void show_eeprom(int devnum, u8 *eeprom);
 static void decode_tlv(struct tlvinfo_tlv *tlv);
-static void update_crc(u8 *eeprom);
-static int prog_eeprom(int devnum, u8 *eeprom);
-static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
 static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
 static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
 static int set_mac(char *buf, const char *string);
@@ -58,18 +54,6 @@ static inline bool is_digit(char c)
return (c >= '0' && c <= '9');
 }
 
-/**
- *  is_valid_tlv
- *
- *  Perform basic sanity checks on a TLV field. The TLV is pointed to
- *  by the parameter provided.
- *  1. The type code is not reserved (0x00 or 0xFF)
- */
-static inline bool is_valid_tlv(struct tlvinfo_tlv *tlv)
-{
-   return((tlv->type != 0x00) && (tlv->type != 0xFF));
-}
-
 /**
  *  is_hex
  *
@@ -83,14 +67,12 @@ static inline u8 is_hex(char p)
 }
 
 /**
- *  is_checksum_valid
- *
  *  Validate the checksum in the provided TlvInfo EEPROM data. First,
  *  verify that the TlvInfo header is valid, then make sure the last
  *  TLV is a CRC-32 TLV. Then calculate the CRC over the EEPROM data
  *  and compare it to the value stored in the EEPROM CRC-32 TLV.
  */
-static bool is_checksum_valid(u8 *eeprom)
+bool tlvinfo_check_crc(u8 *eeprom)
 {
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
struct tlvinfo_tlv*eeprom_crc;
@@ -137,11 +119,11 @@ static int read_eeprom(int devnum, u8 *eeprom)
 
// If the contents are invalid, start over with default contents
if (!is_valid_tlvinfo_header(eeprom_hdr) ||
-   !is_checksum_valid(eeprom)) {
+   !tlvinfo_check_crc(eeprom)) {
strcpy(eeprom_hdr->signature, TLV_INFO_ID_STRING);
eeprom_hdr->version = TLV_INFO_VERSION;
eeprom_hdr->totallen = cpu_to_be16(0);
-   update_crc(eeprom);
+   tlvinfo_update_crc(eeprom);
}
 
 #ifdef DEBUG
@@ -183,7 +165,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
tlv_end  = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
while (curr_tlv < tlv_end) {
eeprom_tlv = to_entry([curr_tlv]);
-   if (!is_valid_tlv(eeprom_tlv)) {
+   if (!is_valid_tlvinfo_entry(eeprom_tlv)) {
printf("Invalid TLV field starting at EEPROM offset 
%d\n",
   curr_tlv);
return;
@@ -193,7 +175,7 @@ static void show_eeprom(int devnum, u8 *eeprom)
}
 
printf("Checksum is %s.\n",
-  is_checksum_valid(eeprom) ? "valid" : "invalid");
+  tlvinfo_check_crc(eeprom) ? "valid" : "invalid");
 
 #ifdef DEBUG
printf("EEPROM dump: (0x%x bytes)", TLV_INFO_MAX_LEN);
@@ -340,13 +322,13 @@ static void decode_tlv(struct tlvinfo_tlv *tlv)
 }
 
 /**
- *  update_crc
+ *  tlvinfo_update_crc
  *
  *  This function updates the CRC-32 TLV. If there is no CRC-32 TLV, then
  *  one is added. This function should be called after each update to the
  *  EEPROM structure, to make sure the CRC is always correct.
  */
-static void update_crc(u8 *eeprom)
+void tlvinfo_update_crc(u8 *eeprom)
 {
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
struct tlvinfo_tlv*eeprom_crc;
@@ -376,20 +358,20 @@ static void update_crc(u8 *eeprom)
 }
 
 /**
- *  prog_eeprom
+ *  write_tlvinfo_tlv_eeprom
  *
- *  Write the EEPROM data from CPU memory to the hardware.
+ *  Write the TLV data from CPU memory to the hardware.
  */
-static int prog_eeprom(int devnum, u8 *eeprom)
+int write_tlvinfo_tlv_eeprom(void *eeprom, int dev)
 {
int ret = 0;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
int eeprom_len;
 
-   update_crc(eeprom);
+   tlvinfo_update_crc(eeprom);
 
eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
-   ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
+   ret = write_tlv_eeprom(eeprom, eeprom_len, dev);
if (ret) {
printf("Programming failed.\n");
return -1;

[PATCH 05/12] cmd: tlv_eeprom: remove empty function implementations from header

2022-05-02 Thread Josua Mayer
tlv_eeprom exposed functions are independent from platforms, hence no
stubs are required.

Signed-off-by: Josua Mayer 
---
 include/tlv_eeprom.h | 24 ++--
 1 file changed, 2 insertions(+), 22 deletions(-)

diff --git a/include/tlv_eeprom.h b/include/tlv_eeprom.h
index 30626a1067..55fd72d6d2 100644
--- a/include/tlv_eeprom.h
+++ b/include/tlv_eeprom.h
@@ -65,7 +65,8 @@ struct __attribute__ ((__packed__)) tlvinfo_tlv {
 #define TLV_CODE_VENDOR_EXT 0xFD
 #define TLV_CODE_CRC_32 0xFE
 
-#if CONFIG_IS_ENABLED(CMD_TLV_EEPROM)
+/* how many EEPROMs can be used */
+#define TLV_MAX_DEVICES2
 
 /**
  * read_tlv_eeprom - Read the EEPROM binary data from the hardware
@@ -156,27 +157,6 @@ void tlvinfo_update_crc(u8 *eeprom);
  */
 bool tlvinfo_check_crc(u8 *eeprom);
 
-#else /* !CONFIG_IS_ENABLED(CMD_TLV_EEPROM) */
-
-static inline int read_tlv_eeprom(void *eeprom, int offset, int len, int dev)
-{
-   return -ENOSYS;
-}
-
-static inline int write_tlv_eeprom(void *eeprom, int len)
-{
-   return -ENOSYS;
-}
-
-static inline int
-read_tlvinfo_tlv_eeprom(void *eeprom, struct tlvinfo_header **hdr,
-   struct tlvinfo_tlv **first_entry, int dev)
-{
-   return -ENOSYS;
-}
-
-#endif /* CONFIG_IS_ENABLED(CMD_TLV_EEPROM) */
-
 /**
  *  is_valid_tlvinfo_header
  *
-- 
2.34.1



[PATCH 02/12] cmd: tlv_eeprom: remove use of global variable has_been_read

2022-05-02 Thread Josua Mayer
has_been_read is only used as an optimization for do_tlv_eeprom.
Explicitly use and set inside this function, thus making read_eeprom
stateless.

Signed-off-by: Josua Mayer 
---
 cmd/tlv_eeprom.c | 25 -
 1 file changed, 12 insertions(+), 13 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index f91c11b304..bfd4882e0d 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -42,8 +42,6 @@ static int set_date(char *buf, const char *string);
 static int set_bytes(char *buf, const char *string, int *converted_accum);
 static void show_tlv_devices(int current_dev);
 
-/* Set to 1 if we've read EEPROM into memory */
-static int has_been_read;
 /* The EERPOM contents after being read into memory */
 static u8 eeprom[TLV_INFO_MAX_LEN];
 
@@ -130,9 +128,6 @@ static int read_eeprom(int devnum, u8 *eeprom)
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
struct tlvinfo_tlv *eeprom_tlv = to_entry([HDR_SIZE]);
 
-   if (has_been_read)
-   return 0;
-
/* Read the header */
ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
/* If the header was successfully read, read the TLVs */
@@ -149,10 +144,8 @@ static int read_eeprom(int devnum, u8 *eeprom)
update_crc(eeprom);
}
 
-   has_been_read = 1;
-
 #ifdef DEBUG
-   show_eeprom(eeprom);
+   show_eeprom(devnum, eeprom);
 #endif
 
return ret;
@@ -432,10 +425,15 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
char cmd;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
static unsigned int current_dev;
+   /* Set to devnum if we've read EEPROM into memory */
+   static int has_been_read = -1;
 
// If no arguments, read the EERPOM and display its contents
if (argc == 1) {
-   read_eeprom(current_dev, eeprom);
+   if (has_been_read != current_dev) {
+   if (read_eeprom(current_dev, eeprom) == 0)
+   has_been_read = current_dev;
+   }
show_eeprom(current_dev, eeprom);
return 0;
}
@@ -446,14 +444,16 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
 
// Read the EEPROM contents
if (cmd == 'r') {
-   has_been_read = 0;
-   if (!read_eeprom(current_dev, eeprom))
+   has_been_read = -1;
+   if (read_eeprom(current_dev, eeprom) == 0) {
printf("EEPROM data loaded from device to memory.\n");
+   has_been_read = current_dev;
+   }
return 0;
}
 
// Subsequent commands require that the EEPROM has already been read.
-   if (!has_been_read) {
+   if (has_been_read != current_dev) {
printf("Please read the EEPROM data first, using the 
'tlv_eeprom read' command.\n");
return 0;
}
@@ -509,7 +509,6 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
return 0;
}
current_dev = devnum;
-   has_been_read = 0;
} else {
cmd_usage(cmdtp);
}
-- 
2.34.1



[PATCH 03/12] cmd: tlv_eeprom: do_tlv_eeprom: stop using non-api read_eeprom function

2022-05-02 Thread Josua Mayer
IN the scope of do_tlv_eeprom, the error-checking provided by the
read_eeprom function is not required.
Instead use the API function read_tlv_eeprom.

Signed-off-by: Josua Mayer 
---
 cmd/tlv_eeprom.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index bfd4882e0d..00c5b5f840 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -431,7 +431,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
// If no arguments, read the EERPOM and display its contents
if (argc == 1) {
if (has_been_read != current_dev) {
-   if (read_eeprom(current_dev, eeprom) == 0)
+   if (read_tlv_eeprom(eeprom, 0, TLV_INFO_MAX_LEN, 
current_dev) == 0)
has_been_read = current_dev;
}
show_eeprom(current_dev, eeprom);
@@ -445,7 +445,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
// Read the EEPROM contents
if (cmd == 'r') {
has_been_read = -1;
-   if (read_eeprom(current_dev, eeprom) == 0) {
+   if (read_tlv_eeprom(eeprom, 0, TLV_INFO_MAX_LEN, current_dev) 
== 0) {
printf("EEPROM data loaded from device to memory.\n");
has_been_read = current_dev;
}
-- 
2.34.1



[PATCH 01/12] cmd: tlv_eeprom: remove use of global variable current_dev

2022-05-02 Thread Josua Mayer
Make tlv_eeprom command device selection an explicit parameter of all
function calls.

Signed-off-by: Josua Mayer 
---
 cmd/tlv_eeprom.c | 50 ++--
 include/tlv_eeprom.h |  3 ++-
 2 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/cmd/tlv_eeprom.c b/cmd/tlv_eeprom.c
index bf8d453dc5..f91c11b304 100644
--- a/cmd/tlv_eeprom.c
+++ b/cmd/tlv_eeprom.c
@@ -29,18 +29,18 @@ DECLARE_GLOBAL_DATA_PTR;
 
 /* File scope function prototypes */
 static bool is_checksum_valid(u8 *eeprom);
-static int read_eeprom(u8 *eeprom);
-static void show_eeprom(u8 *eeprom);
+static int read_eeprom(int devnum, u8 *eeprom);
+static void show_eeprom(int devnum, u8 *eeprom);
 static void decode_tlv(struct tlvinfo_tlv *tlv);
 static void update_crc(u8 *eeprom);
-static int prog_eeprom(u8 *eeprom);
+static int prog_eeprom(int devnum, u8 *eeprom);
 static bool tlvinfo_find_tlv(u8 *eeprom, u8 tcode, int *eeprom_index);
 static bool tlvinfo_delete_tlv(u8 *eeprom, u8 code);
 static bool tlvinfo_add_tlv(u8 *eeprom, int tcode, char *strval);
 static int set_mac(char *buf, const char *string);
 static int set_date(char *buf, const char *string);
 static int set_bytes(char *buf, const char *string, int *converted_accum);
-static void show_tlv_devices(void);
+static void show_tlv_devices(int current_dev);
 
 /* Set to 1 if we've read EEPROM into memory */
 static int has_been_read;
@@ -48,7 +48,6 @@ static int has_been_read;
 static u8 eeprom[TLV_INFO_MAX_LEN];
 
 static struct udevice *tlv_devices[MAX_TLV_DEVICES];
-static unsigned int current_dev;
 
 #define to_header(p) ((struct tlvinfo_header *)p)
 #define to_entry(p) ((struct tlvinfo_tlv *)p)
@@ -125,7 +124,7 @@ static bool is_checksum_valid(u8 *eeprom)
  *
  *  Read the EEPROM into memory, if it hasn't already been read.
  */
-static int read_eeprom(u8 *eeprom)
+static int read_eeprom(int devnum, u8 *eeprom)
 {
int ret;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
@@ -135,12 +134,11 @@ static int read_eeprom(u8 *eeprom)
return 0;
 
/* Read the header */
-   ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, current_dev);
+   ret = read_tlv_eeprom((void *)eeprom_hdr, 0, HDR_SIZE, devnum);
/* If the header was successfully read, read the TLVs */
if (ret == 0 && is_valid_tlvinfo_header(eeprom_hdr))
ret = read_tlv_eeprom((void *)eeprom_tlv, HDR_SIZE,
- be16_to_cpu(eeprom_hdr->totallen),
- current_dev);
+ be16_to_cpu(eeprom_hdr->totallen), 
devnum);
 
// If the contents are invalid, start over with default contents
if (!is_valid_tlvinfo_header(eeprom_hdr) ||
@@ -165,7 +163,7 @@ static int read_eeprom(u8 *eeprom)
  *
  *  Display the contents of the EEPROM
  */
-static void show_eeprom(u8 *eeprom)
+static void show_eeprom(int devnum, u8 *eeprom)
 {
int tlv_end;
int curr_tlv;
@@ -180,7 +178,7 @@ static void show_eeprom(u8 *eeprom)
return;
}
 
-   printf("TLV: %u\n", current_dev);
+   printf("TLV: %u\n", devnum);
printf("TlvInfo Header:\n");
printf("   Id String:%s\n", eeprom_hdr->signature);
printf("   Version:  %d\n", eeprom_hdr->version);
@@ -389,7 +387,7 @@ static void update_crc(u8 *eeprom)
  *
  *  Write the EEPROM data from CPU memory to the hardware.
  */
-static int prog_eeprom(u8 *eeprom)
+static int prog_eeprom(int devnum, u8 *eeprom)
 {
int ret = 0;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
@@ -398,7 +396,7 @@ static int prog_eeprom(u8 *eeprom)
update_crc(eeprom);
 
eeprom_len = HDR_SIZE + be16_to_cpu(eeprom_hdr->totallen);
-   ret = write_tlv_eeprom(eeprom, eeprom_len);
+   ret = write_tlv_eeprom(eeprom, eeprom_len, devnum);
if (ret) {
printf("Programming failed.\n");
return -1;
@@ -433,11 +431,12 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
 {
char cmd;
struct tlvinfo_header *eeprom_hdr = to_header(eeprom);
+   static unsigned int current_dev;
 
// If no arguments, read the EERPOM and display its contents
if (argc == 1) {
-   read_eeprom(eeprom);
-   show_eeprom(eeprom);
+   read_eeprom(current_dev, eeprom);
+   show_eeprom(current_dev, eeprom);
return 0;
}
 
@@ -448,7 +447,7 @@ int do_tlv_eeprom(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
// Read the EEPROM contents
if (cmd == 'r') {
has_been_read = 0;
-   if (!read_eeprom(eeprom))
+   if (!read_eeprom(current_dev, eeprom))
printf("EEPROM data loaded from device t

[PATCH 00/12] split tlv_eeprom command into a separate library

2022-05-02 Thread Josua Mayer
The tlv_eeprom command provides much more than just a cli command:
- de- and encoding tlv format
- for reading and writing eeprom
- setting the eth?addr environment variable
- setting the serial# environment variable

One device (Clearfog) is already using the decoding functionality to
choose the correct memory size based on eeprom data.

This patchset massages the implementation in many places removing
stateful behaviour and global variables as much as possible and changing
some functions to be usable as a library.
Then finally everything but the handling of the cli command is split off
into a separate tlv library that can be used independently from the command.

SolidRun is starting to flash more devices with tlv data at the factory,
and we plan to use this information for device identification purposes
in future board files. This refactoring will make those implementations
easier and reduce the amount of duplicate code to carry around.

Josua Mayer (12):
  cmd: tlv_eeprom: remove use of global variable current_dev
  cmd: tlv_eeprom: remove use of global variable has_been_read
  cmd: tlv_eeprom: do_tlv_eeprom: stop using non-api read_eeprom
function
  cmd: tlv_eeprom: convert functions used by command to api functions
  cmd: tlv_eeprom: remove empty function implementations from header
  cmd: tlv_eeprom: move missing declarations and defines to header
  cmd: tlv_eeprom: hide access to static tlv_devices array behind
accessor
  cmd: tlv_eeprom: clean up two defines for one thing
  cmd: tlv_eeprom: add my copyright
  cmd: tlv_eeprom: split off tlv library from command
  arm: mvebu: clearfog: enable tlv library for spl in favour of eeprom
cmd
  lib: tlv_eeprom: add function for reading one entry into a C string

 cmd/Kconfig|   2 +
 cmd/tlv_eeprom.c   | 817 ++---
 configs/clearfog_defconfig |   3 +-
 include/tlv_eeprom.h   | 122 +-
 lib/Kconfig|   2 +
 lib/Makefile   |   2 +
 lib/tlv/Kconfig|  15 +
 lib/tlv/Makefile   |   5 +
 lib/tlv/tlv_eeprom.c   | 775 +++
 9 files changed, 944 insertions(+), 799 deletions(-)
 create mode 100644 lib/tlv/Kconfig
 create mode 100644 lib/tlv/Makefile
 create mode 100644 lib/tlv/tlv_eeprom.c

Cc: Jon Nettleton 
-- 
2.34.1



[PATCH 4/5] mx6cuboxi: fixup dtb ethernet phy nodes before booting an OS

2022-05-01 Thread Josua Mayer
SoM revision 1.9 has replaced the ar8035 phy address 0 with an adin1300
at address 1. Because early SoMs had a hardware flaw, the ar8035 can
also appear at address 4 - making it a total of 3 phy nodes in the DTB.

To avoid confusing Linux with probe errors, fixup the dtb to only enable
the phy node that is detected at runtime.

Signed-off-by: Josua Mayer 
---
 board/solidrun/mx6cuboxi/mx6cuboxi.c | 78 
 configs/mx6cuboxi_defconfig  |  1 +
 2 files changed, 79 insertions(+)

diff --git a/board/solidrun/mx6cuboxi/mx6cuboxi.c 
b/board/solidrun/mx6cuboxi/mx6cuboxi.c
index 6207bf8253..42aa5cb63c 100644
--- a/board/solidrun/mx6cuboxi/mx6cuboxi.c
+++ b/board/solidrun/mx6cuboxi/mx6cuboxi.c
@@ -1,5 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0+
 /*
+ * Copyright (C) 2022 Josua Mayer 
+ *
  * Copyright (C) 2015 Freescale Semiconductor, Inc.
  *
  * Author: Fabio Estevam 
@@ -39,6 +41,8 @@
 #include 
 #include 
 #include 
+#include 
+#include 
 
 DECLARE_GLOBAL_DATA_PTR;
 
@@ -407,6 +411,80 @@ out:
return 0;
 }
 
+static int find_ethernet_phy(void)
+{
+   struct mii_dev *bus = NULL;
+   struct phy_device *phydev = NULL;
+   int phy_addr = -ENOENT;
+
+#ifdef CONFIG_FEC_MXC
+   bus = fec_get_miibus(ENET_BASE_ADDR, -1);
+   if (!bus)
+   return -ENOENT;
+
+   // scan address 0, 1, 4
+   phydev = phy_find_by_mask(bus, 0b00010011);
+   if (!phydev) {
+   free(bus);
+   return -ENOENT;
+   }
+   pr_debug("%s: detected ethernet phy at address %d\n", __func__, 
phydev->addr);
+   phy_addr = phydev->addr;
+
+   free(phydev);
+#endif
+
+   return phy_addr;
+}
+
+#if defined(CONFIG_OF_LIBFDT) && defined(CONFIG_OF_BOARD_SETUP)
+/*
+ * Configure the correct ethernet PHYs nodes in device-tree:
+ * - AR8035 at addresses 0 or 4: Cubox
+ * - AR8035 at address 0: HummingBoard, HummingBoard 2
+ * - ADIN1300 at address 1: since SoM rev 1.9
+ */
+int ft_board_setup(void *fdt, struct bd_info *bd)
+{
+   int node_phy0, node_phy1, node_phy4;
+   int ret, phy;
+   bool enable_phy0 = false, enable_phy1 = false, enable_phy4 = false;
+
+   // detect phy
+   phy = find_ethernet_phy();
+   if (phy == 0 || phy == 4) {
+   enable_phy0 = true;
+   switch (board_type()) {
+   case CUBOXI:
+   case UNKNOWN:
+   default:
+   enable_phy4 = true;
+   }
+   } else if (phy == 1) {
+   enable_phy1 = true;
+   } else {
+   pr_err("%s: couldn't detect ethernet phy, not patching dtb!\n", 
__func__);
+   return 0;
+   }
+
+   // update all phy nodes status
+   node_phy0 = fdt_path_offset(fdt, 
"/soc/bus@210/ethernet@2188000/mdio/ethernet-phy@0");
+   ret = fdt_setprop_string(fdt, node_phy0, "status", enable_phy0 ? "okay" 
: "disabled");
+   if (ret < 0 && enable_phy0)
+   pr_err("%s: failed to enable ethernet phy at address 0 in 
dtb!\n", __func__);
+   node_phy1 = fdt_path_offset(fdt, 
"/soc/bus@210/ethernet@2188000/mdio/ethernet-phy@1");
+   ret = fdt_setprop_string(fdt, node_phy1, "status", enable_phy1 ? "okay" 
: "disabled");
+   if (ret < 0 && enable_phy1)
+   pr_err("%s: failed to enable ethernet phy at address 1 in 
dtb!\n", __func__);
+   node_phy4 = fdt_path_offset(fdt, 
"/soc/bus@210/ethernet@2188000/mdio/ethernet-phy@4");
+   ret = fdt_setprop_string(fdt, node_phy4, "status", enable_phy4 ? "okay" 
: "disabled");
+   if (ret < 0 && enable_phy4)
+   pr_err("%s: failed to enable ethernet phy at address 4 in 
dtb!\n", __func__);
+
+   return 0;
+}
+#endif
+
 /* Override the default implementation, DT model is not accurate */
 int show_board_info(void)
 {
diff --git a/configs/mx6cuboxi_defconfig b/configs/mx6cuboxi_defconfig
index 1e2e332af9..d3ac8eeeba 100644
--- a/configs/mx6cuboxi_defconfig
+++ b/configs/mx6cuboxi_defconfig
@@ -22,6 +22,7 @@ CONFIG_CMD_HDMIDETECT=y
 CONFIG_AHCI=y
 CONFIG_DISTRO_DEFAULTS=y
 CONFIG_FIT=y
+CONFIG_OF_BOARD_SETUP=y
 CONFIG_BOOTCOMMAND="run findfdt; run finduuid; run distro_bootcmd"
 CONFIG_USE_PREBOOT=y
 CONFIG_PREBOOT="if hdmidet; then usb start; setenv stdin  serial,usbkbd; 
setenv stdout serial,vidconsole; setenv stderr serial,vidconsole; else setenv 
stdin  serial; setenv stdout serial; setenv stderr serial; fi;"
-- 
2.34.1



[PATCH 5/5] mx6cuboxi: enable driver for adin1300 phy

2022-05-01 Thread Josua Mayer
Since SoMs revision 1.9 the ar8038 phy has been replaced by adin1300.
Enable the driver so that the new SoMs have functional networking.

Signed-off-by: Josua Mayer 
---
 configs/mx6cuboxi_defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/configs/mx6cuboxi_defconfig b/configs/mx6cuboxi_defconfig
index d3ac8eeeba..46634a1727 100644
--- a/configs/mx6cuboxi_defconfig
+++ b/configs/mx6cuboxi_defconfig
@@ -55,6 +55,7 @@ CONFIG_DWC_AHSATA=y
 CONFIG_SPL_SYS_I2C_LEGACY=y
 CONFIG_FSL_USDHC=y
 CONFIG_PHYLIB=y
+CONFIG_PHY_ADIN=y
 CONFIG_PHY_ATHEROS=y
 CONFIG_DM_ETH=y
 CONFIG_FEC_MXC=y
-- 
2.34.1



[PATCH 3/5] ARM: dts: imx6qdl-sr-som: add support for alternate phy addresses

2022-05-01 Thread Josua Mayer
The Cubox has an unstable phy address - which can appear at either
address 0 (intended) or 4 (unintended).

SoM revision 1.9 has replaced the ar8035 phy with an adin1300, which
will always appear at address 1.

Change the reg property of the phy node to the magic value 0x,
which indicates to the generic phy driver that all addresses should be
probed. That allows the same node (which is pinned by phy-handle) to match
either the AR8035 PHY at both possible addresses, as well as the new one
at address 1.
Also add the new adi,phy-output-clock property for enabling the 125MHz
clock used by the fec ethernet controller, as submitted to Linux [1].

Linux solves this problem differently:
For the ar8035 phy it will probe both phy nodes in device-tree in order,
and use the one that succeeds. For the new adin1300 it expects U-Boot to
patch the status field in the DTB before booting

While at it also sync the reset-delay with the upstream Linux dtb.

[1] 
https://patchwork.kernel.org/project/netdevbpf/patch/20220428082848.12191-4-jo...@solid-run.com/

Signed-off-by: Josua Mayer 
---
 arch/arm/dts/imx6qdl-sr-som.dtsi | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/arch/arm/dts/imx6qdl-sr-som.dtsi b/arch/arm/dts/imx6qdl-sr-som.dtsi
index b06577808f..c20bed2721 100644
--- a/arch/arm/dts/imx6qdl-sr-som.dtsi
+++ b/arch/arm/dts/imx6qdl-sr-som.dtsi
@@ -55,7 +55,13 @@
pinctrl-0 = <_microsom_enet_ar8035>;
phy-handle = <>;
phy-mode = "rgmii-id";
-   phy-reset-duration = <2>;
+
+   /*
+* The PHY seems to require a long-enough reset duration to avoid
+* some rare issues where the PHY gets stuck in an inconsistent and
+* non-functional state at boot-up. 10ms proved to be fine .
+*/
+   phy-reset-duration = <10>;
phy-reset-gpios = < 15 GPIO_ACTIVE_LOW>;
status = "okay";
 
@@ -64,8 +70,15 @@
#size-cells = <0>;
 
phy: ethernet-phy@0 {
-   reg = <0>;
+   /*
+* The PHY can appear either:
+* - AR8035: at address 0 or 4
+* - ADIN1300: at address 1
+* Actual address being detected at runtime.
+*/
+   reg = <0x>;
qca,clk-out-frequency = <12500>;
+   adi,phy-output-clock = "125mhz-free-running";
};
};
 };
-- 
2.34.1



[PATCH 2/5] phy: adin: add support for clock output

2022-05-01 Thread Josua Mayer
The ADIN1300 supports generating certain clocks on its GP_CLK pin, as
well as providing the reference clock on CLK25_REF.

Add support for selecting the clock via device-tree properties.

This patch is based on just submitted patches to Linux [1] [2].

[1] 
https://patchwork.kernel.org/project/netdevbpf/patch/20220428082848.12191-2-jo...@solid-run.com/
[2] 
https://patchwork.kernel.org/project/netdevbpf/patch/20220428082848.12191-4-jo...@solid-run.com/

Signed-off-by: Josua Mayer 
---
 drivers/net/phy/adin.c | 46 ++
 1 file changed, 46 insertions(+)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index 2433e76fea..485430b128 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -4,6 +4,7 @@
  *
  * Copyright 2019 Analog Devices Inc.
  * Copyright 2022 Variscite Ltd.
+ * Copyright 2022 Josua Mayer 
  */
 #include 
 #include 
@@ -13,6 +14,16 @@
 #define PHY_ID_ADIN13000x0283bc30
 #define ADIN1300_EXT_REG_PTR   0x10
 #define ADIN1300_EXT_REG_DATA  0x11
+
+#define ADIN1300_GE_CLK_CFG_REG0xff1f
+#define   ADIN1300_GE_CLK_CFG_MASK GENMASK(5, 0)
+#define   ADIN1300_GE_CLK_CFG_RCVR_125 BIT(5)
+#define   ADIN1300_GE_CLK_CFG_FREE_125 BIT(4)
+#define   ADIN1300_GE_CLK_CFG_REF_EN   BIT(3)
+#define   ADIN1300_GE_CLK_CFG_HRT_RCVR BIT(2)
+#define   ADIN1300_GE_CLK_CFG_HRT_FREE BIT(1)
+#define   ADIN1300_GE_CLK_CFG_25   BIT(0)
+
 #define ADIN1300_GE_RGMII_CFG  0xff23
 #define ADIN1300_GE_RGMII_RX_MSK   GENMASK(8, 6)
 #define ADIN1300_GE_RGMII_RX_SEL(x)\
@@ -115,6 +126,37 @@ static int adin_ext_write(struct phy_device *phydev, const 
u32 regnum, const u16
return phy_write(phydev, MDIO_DEVAD_NONE, ADIN1300_EXT_REG_DATA, val);
 }
 
+static int adin_config_clk_out(struct phy_device *phydev)
+{
+   ofnode node = phy_get_ofnode(phydev);
+   const char *val = NULL;
+   u8 sel = 0;
+
+   val = ofnode_read_string(node, "adi,phy-output-clock");
+   if (!val) {
+   /* property not present, do not enable GP_CLK pin */
+   } else if (strcmp(val, "25mhz-reference") == 0) {
+   sel |= ADIN1300_GE_CLK_CFG_25;
+   } else if (strcmp(val, "125mhz-free-running") == 0) {
+   sel |= ADIN1300_GE_CLK_CFG_FREE_125;
+   } else if (strcmp(val, "125mhz-recovered") == 0) {
+   sel |= ADIN1300_GE_CLK_CFG_RCVR_125;
+   } else if (strcmp(val, "adaptive-free-running") == 0) {
+   sel |= ADIN1300_GE_CLK_CFG_HRT_FREE;
+   } else if (strcmp(val, "adaptive-recovered") == 0) {
+   sel |= ADIN1300_GE_CLK_CFG_HRT_RCVR;
+   } else {
+   pr_err("%s: invalid adi,phy-output-clock\n", __func__);
+   return -EINVAL;
+   }
+
+   if (ofnode_read_bool(node, "adi,phy-output-reference-clock"))
+   sel |= ADIN1300_GE_CLK_CFG_REF_EN;
+
+   return adin_ext_write(phydev, ADIN1300_GE_CLK_CFG_REG,
+ ADIN1300_GE_CLK_CFG_MASK & sel);
+}
+
 static int adin_config_rgmii_mode(struct phy_device *phydev)
 {
u16 reg_val;
@@ -168,6 +210,10 @@ static int adin1300_config(struct phy_device *phydev)
 
printf("ADIN1300 PHY detected at addr %d\n", phydev->addr);
 
+   ret = adin_config_clk_out(phydev);
+   if (ret < 0)
+   return ret;
+
ret = adin_config_rgmii_mode(phydev);
 
if (ret < 0)
-- 
2.34.1



[PATCH 1/5] phy: adin: remove broken support for adi, phy-mode-override

2022-05-01 Thread Josua Mayer
The adin_get_phy_mode_override function does not compile, because it is
missing both declaration and implementation of
phy_get_interface_by_name.

Remove the whole function for now, since the missing implementation is
not included in mainline Linux - and thus can not be copied.

Signed-off-by: Josua Mayer 
---
 drivers/net/phy/adin.c | 34 --
 1 file changed, 34 deletions(-)

diff --git a/drivers/net/phy/adin.c b/drivers/net/phy/adin.c
index cff841ab3d..2433e76fea 100644
--- a/drivers/net/phy/adin.c
+++ b/drivers/net/phy/adin.c
@@ -94,35 +94,6 @@ static u32 adin_get_reg_value(struct phy_device *phydev,
return rc;
 }
 
-/**
- * adin_get_phy_mode_override - Get phy-mode override for adin PHY
- *
- * The function gets phy-mode string from property 'adi,phy-mode-override'
- * and return its index in phy_interface_strings table, or -1 in error case.
- */
-int adin_get_phy_mode_override(struct phy_device *phydev)
-{
-   ofnode node = phy_get_ofnode(phydev);
-   const char *phy_mode_override;
-   const char *prop_phy_mode_override = "adi,phy-mode-override";
-   int override_interface;
-
-   phy_mode_override = ofnode_read_string(node, prop_phy_mode_override);
-   if (!phy_mode_override)
-   return -ENODEV;
-
-   debug("%s: %s = '%s'\n",
- __func__, prop_phy_mode_override, phy_mode_override);
-
-   override_interface = phy_get_interface_by_name(phy_mode_override);
-
-   if (override_interface < 0)
-   printf("%s: %s = '%s' is not valid\n",
-  __func__, prop_phy_mode_override, phy_mode_override);
-
-   return override_interface;
-}
-
 static u16 adin_ext_read(struct phy_device *phydev, const u32 regnum)
 {
u16 val;
@@ -148,11 +119,6 @@ static int adin_config_rgmii_mode(struct phy_device 
*phydev)
 {
u16 reg_val;
u32 val;
-   int phy_mode_override = adin_get_phy_mode_override(phydev);
-
-   if (phy_mode_override >= 0) {
-   phydev->interface = (phy_interface_t) phy_mode_override;
-   }
 
reg_val = adin_ext_read(phydev, ADIN1300_GE_RGMII_CFG);
 
-- 
2.34.1



[PATCH 0/5] mx6cuboxi: add eth support for SoMs revision 1.9+

2022-05-01 Thread Josua Mayer
As of Revision 1.9 the SolidRun i.MX6 SoMs ship with a new PHY - an
ADIN1300 at address 1. This patchset carries many small parts to
facilitate proper operation of the ethernet port in U-Boot as well as
Linux:

1. Fix a compile error in the recently phy driver
2. Add support for configuring the clock output pins to the phy driver
3. update device-tree with support for the neew phy
4. support runtime patching of device-tree for Linux:
   enables only the phy node detected during boot, to avoid
   warning messages during boot.
5. finally enable the phy driver in the cuboxi defconfig

Josua Mayer (5):
  phy: adin: remove broken support for adi,phy-mode-override
  phy: adin: add support for clock output
  ARM: dts: imx6qdl-sr-som: add support for alternate phy addresses
  mx6cuboxi: fixup dtb ethernet phy nodes before booting an OS
  mx6cuboxi: enable driver for adin1300 phy

 arch/arm/dts/imx6qdl-sr-som.dtsi | 17 +-
 board/solidrun/mx6cuboxi/mx6cuboxi.c | 78 +++
 configs/mx6cuboxi_defconfig  |  2 +
 drivers/net/phy/adin.c   | 80 
 4 files changed, 141 insertions(+), 36 deletions(-)

-- 
2.34.1



[PATCH] arm: mvebu: clearfog: add scsi target to distro-boot

2020-02-17 Thread Josua Mayer
Support for sata devices via the scsi command is available and already
enabled by default for the Clearfog Base and Pro. This change adds scsi
to the list of boot targets used by distro-boot.

Signed-off-by: Josua Mayer 
Cc: Stefan Roese 
---
 include/configs/clearfog.h | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/include/configs/clearfog.h b/include/configs/clearfog.h
index 633187d86f..33c71b3d51 100644
--- a/include/configs/clearfog.h
+++ b/include/configs/clearfog.h
@@ -104,6 +104,12 @@
 #define BOOT_TARGET_DEVICES_MMC(func)
 #endif
 
+#ifdef CONFIG_SCSI
+#define BOOT_TARGET_DEVICES_SCSI(func) func(SCSI, scsi, 0)
+#else
+#define BOOT_TARGET_DEVICES_SCSI(func)
+#endif
+
 #ifdef CONFIG_USB_STORAGE
 #define BOOT_TARGET_DEVICES_USB(func) func(USB, usb, 0)
 #else
@@ -112,6 +118,7 @@
 
 #define BOOT_TARGET_DEVICES(func) \
BOOT_TARGET_DEVICES_MMC(func) \
+   BOOT_TARGET_DEVICES_SCSI(func) \
BOOT_TARGET_DEVICES_USB(func) \
func(PXE, pxe, na) \
func(DHCP, dhcp, na)
-- 
2.25.0



[U-Boot] [PATCH 1/1] add Kconfig for fsuuid command

2017-04-24 Thread Josua Mayer
CONFIG_CMD_FS_UUID was neither whitelisted, nor was it declared in
Kconfig.
Now it can be enabled in .config and defconfig as expected.

Signed-off-by: Josua Mayer <josua.maye...@gmail.com>
---
 cmd/Kconfig | 5 +
 1 file changed, 5 insertions(+)

diff --git a/cmd/Kconfig b/cmd/Kconfig
index 13dc46a174..50888236db 100644
--- a/cmd/Kconfig
+++ b/cmd/Kconfig
@@ -831,6 +831,11 @@ config CMD_FS_GENERIC
  Enables filesystem commands (e.g. load, ls) that work for multiple
  fs types.
 +config CMD_FS_UUID
+   bool "fsuuid command"
+   help
+ Enables fsuuid command for filesystem UUID.
+
 config CMD_MTDPARTS
depends on ARCH_SUNXI
bool "MTD partition support"
-- 
2.12.2

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot


[U-Boot] [PATCH 0/1] Allow fsuuid command to be built

2017-04-24 Thread Josua Mayer
Hi everybody,
With the fsuuid command not being listed in Kconfig, and not being
whitelisted, it was impossible to be build it.
This patch rectifies that situation by adding a Kconfig piece for it.

br
Josua Mayer

Josua Mayer (1):
  clearfog: enable distro boot code

 Kconfig|  1 +
 configs/clearfog_defconfig |  1 +
 include/configs/clearfog.h | 46
+-
 3 files changed, 43 insertions(+), 5 deletions(-)

-- 
2.12.2

___
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot