[RFC PATCH 00/16] Introduce ICSSG Ethernet driver

MD Danish Anwar danishanwar at ti.com
Tue Jan 9 10:49:57 CET 2024



On 09/01/24 2:56 pm, Roger Quadros wrote:
> 
> 
> On 08/01/2024 12:25, MD Danish Anwar wrote:
>> On 08/01/24 3:00 pm, Roger Quadros wrote:
>>>
>>>
>>> On 05/01/2024 12:15, Anwar, Md Danish wrote:
>>>>
>>>>
>>>> On 1/5/2024 1:49 PM, Roger Quadros wrote:
>>>>>
>>>>>
>>>>> On 03/01/2024 12:27, MD Danish Anwar wrote:
>>>>>>
>>>>>>
>>>>>> On 02/01/24 7:26 pm, Andrew Davis wrote:
>>>>>>> On 12/27/23 12:56 AM, MD Danish Anwar wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On 22/12/23 6:13 pm, Roger Quadros wrote:
>>>>>>>>>
>>>>>>>>> On 22/12/2023 12:26, MD Danish Anwar wrote:
>>>>>>>>>> Hi Roger,
>>>>>>>>>>
>>>>>>>>>> On 20/12/23 3:29 pm, Roger Quadros wrote:
>>>>>>>>>>> Hi,
>>>>>>>>>>>
>>>>>>>>>>> On 19/12/2023 12:11, MD Danish Anwar wrote:
>>>>>>>>>>>> Introduce ICSSG PRUETH support in uboot. The ICSSG driver is used
>>>>>>>>>>>> in TI
>>>>>>>>>>>> AM654 SR2.0.
>>>>>>>>>>>>
>>>>>>>>>>>> The ICSSG PRU Sub-system runs on EMAC firmware. This series
>>>>>>>>>>>> Introduces
>>>>>>>>>>>> support for ICSSG driver in uboot. This series also adds the driver's
>>>>>>>>>>>> dependencies.
>>>>>>>>>>>>
>>>>>>>>>>>> The ICSSG2 node is added in device tree overlay so that it remains in
>>>>>>>>>>>> sync with linux kernel.
>>>>>>>>>>>>
>>>>>>>>>>>> The series introduces device tree and config changes and AM65x
>>>>>>>>>>>> to enable ICSSG driver. The series also enables
>>>>>>>>>>>> SPL_LOAD_FIT_APPLY_OVERLAY
>>>>>>>>>>>> for AM65x in order to load overlay over spl.
>>>>>>>>>>>>
>>>>>>>>>>>> This series has been tested on AM65x SR2.0, and the ICSSG
>>>>>>>>>>>> interface is
>>>>>>>>>>>> able to ping / dhcp and boot kernel using tftp in uboot.
>>>>>>>>>>>>
>>>>>>>>>>>> To use ICSSG2 ethernet, the ICSSG firmware needs to be loaded to
>>>>>>>>>>>> PRU RPROC
>>>>>>>>>>>> cores and RPROC cores need to be booted with the firmware. This
>>>>>>>>>>>> step is
>>>>>>>>>>>> done inside driver in kernel as kernel supports APIs like
>>>>>>>>>>>> rproc_set_firmware() and rproc_fw_boot(). But as u-boot doesn't
>>>>>>>>>>>> have these
>>>>>>>>>>>> APIs, the same needs to be done via u-boot cmds.
>>>>>>>>>>>>
>>>>>>>>>>>> To make sure icssg-eth works we need to do below steps.
>>>>>>>>>>>>
>>>>>>>>>>>> 1. Initialize rproc cores i.e. rproc_init()
>>>>>>>>>>>> 2. Load $firmware_file from partition '1:2' (root) on device (mmc
>>>>>>>>>>>> in this
>>>>>>>>>>>>     example)
>>>>>>>>>>>> 3. Load the firmware file to rproc cores passing. i.e. rproc_load()
>>>>>>>>>>>>     taking rproc_id, loadaddr and file size as arguments.
>>>>>>>>>>>> 4. Start rproc cores. i.e. rproc_start() taking rproc_id as arguments
>>>>>>>>>>>>
>>>>>>>>>>>> The above steps are done by running the below commands at u-boot
>>>>>>>>>>>> prompt.
>>>>>>>>>>>>
>>>>>>>>>>>> => setenv start_icssg2 'rproc start 14; rproc start 15; rproc
>>>>>>>>>>>> start 16; rproc start 17; rproc start 18; rproc start 19'
>>>>>>>>>>>> => setenv stop_icssg2 'rproc stop 14; rproc stop 15; rproc stop
>>>>>>>>>>>> 16; rproc stop 17; rproc stop 18; rproc stop 19'
>>>>>>>>>>>> => setenv firmware_dir '/lib/firmware/ti-pruss'
>>>>>>>>>>>> => setenv get_firmware_mmc 'load mmc ${bootpart} ${loadaddr}
>>>>>>>>>>>> ${firmware_dir}/${firmware_file}'
>>>>>>>>>>>>
>>>>>>>>>>>> => setenv init_icssg2 'setenv ethact icssg2-eth; setenv autoload
>>>>>>>>>>>> no; rproc init; setenv loadaddr 0x80000000; \
>>>>>>>>>>>>      setenv firmware_file am65x-sr2-pru0-prueth-fw.elf; run
>>>>>>>>>>>> get_firmware_mmc;  rproc load 14 0x80000000 ${filesize}; \
>>>>>>>>>>>>      setenv loadaddr 0x89000000; setenv firmware_file
>>>>>>>>>>>> am65x-sr2-rtu0-prueth-fw.elf; run get_firmware_mmc; rproc load 15
>>>>>>>>>>>> 0x89000000 ${filesize}; \
>>>>>>>>>>>>      setenv loadaddr 0x90000000; setenv firmware_file
>>>>>>>>>>>> am65x-sr2-txpru0-prueth-fw.elf; run get_firmware_mmc; rproc load
>>>>>>>>>>>> 16 0x90000000 ${filesize}; \
>>>>>>>>>>>>      setenv loadaddr 0x80000000; setenv firmware_file
>>>>>>>>>>>> am65x-sr2-pru1-prueth-fw.elf; run get_firmware_mmc; rproc load 17
>>>>>>>>>>>> 0x80000000 ${filesize}; \
>>>>>>>>>>>>      setenv loadaddr 0x89000000; setenv firmware_file
>>>>>>>>>>>> am65x-sr2-rtu1-prueth-fw.elf; run get_firmware_mmc; rproc load 18
>>>>>>>>>>>> 0x89000000 ${filesize}; \
>>>>>>>>>>>>      setenv loadaddr 0x90000000; setenv firmware_file
>>>>>>>>>>>> am65x-sr2-txpru1-prueth-fw.elf; run get_firmware_mmc; rproc load
>>>>>>>>>>>> 19 0x90000000 ${filesize}; \
>>>>>>>>>>>>      run start_icssg2;'
>>>>>>>>>>>
>>>>>>>>>>> A whole bunch of commands are required to get ethernet functional.
>>>>>>>>>>> This is not at all user friendly and will be a maintenance nightmare.
>>>>>>>>>>> What worries me is tracking the 6 different rproc cores and the 6
>>>>>>>>>>> different firmware files to start 1 ethernet device.
>>>>>>>>>>> This will get even more interesting when we have to deal with
>>>>>>>>>>> different ICSSG instances on different boards.
>>>>>>>>>>>
>>>>>>>>>>> What is preventing the driver from starting the rproc cores it
>>>>>>>>>>> needs so user doesn't need to care about it?
>>>>>>>>>>> All the necessary information is in the Device tree. At least this
>>>>>>>>>>> is how it is done on Linux.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> I tried removing the need for these commands and implementing them
>>>>>>>>>> inside the driver only. I am able to load the firmware from driver
>>>>>>>>>> using
>>>>>>>>>> the fs_loader API request_firmware_into_buf(). It requires changes to
>>>>>>>>>> dt. A DT node called fs-loader needs to be added also CONFIG_FS_LOADER
>>>>>>>>>> needs to enabled. In the DT node we need to specify the storage media
>>>>>>>>>> that we are using i.e. mmc, ospi, usb. It's upto user to modify the
>>>>>>>>>> storage media, the driver will take the media from DT and try to laod
>>>>>>>>>> firmware from their.
>>>>>>>>>>
>>>>>>>>>> For loading firmwares to rproc cores, rproc_load() API is needed. Now
>>>>>>>>>> this API takes rproc_id, loadaddr and firmware_size as arguments.
>>>>>>>>>> loadaddr is fixed for all three pru cores. firmware_size is obtained
>>>>>>>>>> from request_firmware_into_buf() but I couldn't find a way to get the
>>>>>>>>>> rproc_id. For now based on the ICSSG instance and slice number I am
>>>>>>>>>> figuring out the rproc_id and passing it to rproc_load() and
>>>>>>>>>> rproc_start() APIs. Please let me know if you have any other / better
>>>>>>>>>> way of finding rproc_id.
>>>>>>>>>>
>>>>>>>>>> Below is the entire diff to remove these commands and move their
>>>>>>>>>> functionality to driver. Please have a look and let me know if this
>>>>>>>>>> is ok.
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Good to see you got something working so quickly.
>>>>>>>>> It has some rough edges but nothing that is blocking.
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Save New Duplicate & Edit Just Text Twitter
>>>>>>>>>> diff --git a/arch/arm/dts/k3-am654-base-board.dts
>>>>>>>>>> b/arch/arm/dts/k3-am654-base-board.dts
>>>>>>>>>> index cfbcebfa37..c8da72e49c 100644
>>>>>>>>>> --- a/arch/arm/dts/k3-am654-base-board.dts
>>>>>>>>>> +++ b/arch/arm/dts/k3-am654-base-board.dts
>>>>>>>>>> @@ -16,6 +16,13 @@
>>>>>>>>>>       chosen {
>>>>>>>>>>           stdout-path = "serial2:115200n8";
>>>>>>>>>>           bootargs = "earlycon=ns16550a,mmio32,0x02800000";
>>>>>>>>>> +        firmware-loader = <&fs_loader0>;
>>>>>>>>>> +    };
>>>>>>>>>> +
>>>>>>>>>> +    fs_loader0: fs-loader {
>>>>>>>>>> +        bootph-all;
>>>>>>>>>> +        compatible = "u-boot,fs-loader";
>>>>>>>>>> +        phandlepart = <&sdhci1 2>;
>>>>>>>>>>       };
>>>>>>>>>
>>>>>>>>> This is has 2 issues
>>>>>>>>> 1) It will not be accepted in Kernel DT. Maybe it could be done in
>>>>>>>>> -u-boot.dtsi file.
>>>>>>>>
>>>>>>>> Sure. I'll move this to k3-am654-base-board-u-boot.dtsi
>>>>>>>>
>>>>>>>>> 2) You cannot assume boot medium is always sdhci1 2
>>>>>>>>>
>>>>>>>>
>>>>>>>> Yes. It's upto user to provide the boot medium and boot partition for
>>>>>>>> loading firmware. By default the firmware is loacated in root partion of
>>>>>>>> shdci1 on am65x so I am adding this as default. But user can easily
>>>>>>>> modify this to any other medium / partition needed.
>>>>>>>>
>>>>>>>
>>>>>>> Users should not have to modify DT to pick a boot medium/partition.
>>>>>>> What would you do for complex cases or non block devices like eth/uart
>>>>>>
>>>>> I agree with Andrew here.
>>>>>
>>>>>> In order to load firmware files from driver, we need to add the node for
>>>>>> fs-loader. The fs-loader has a phandlepart property which takes the boot
>>>>>> medium and the partition as input. Based on the medium and partition it
>>>>>> looks for the requested file and loads it to a given address. I am not
>>>>>> sure if there is any other way to load firmware from driver without
>>>>>> using the fs-loader.
>>>>>>
>>>>>>
>>>>>>> booting? This is one reason kernel moved firmware loading to
>>>>>>> userspace. The equivalant in U-Boot is the shell and scripts. Your
>>>>>>> original command based loading was the correct solution IMHO.
>>>>>>
>>>>>> I intended to go ahead with command base approach only but as Roger
>>>>>> pointed out the command based approach is not very user friendly.
>>>>>>
>>>>>> We need to align on what should be the correct approach for loading
>>>>>> firmware.
>>>>>>
>>>>>> Roger, can you please chime in here.
>>>>>
>>>>> My intention was to make it user friendly so he does not have to
>>>>> deal with 6 different Rproc IDs that can change between
>>>>> platforms. The solution also has to be seamless between different
>>>>> boot mediums. I think we can assume that the firmware will come from the
>>>>> same medium that the platform was booted.
>>>>>
>>>>
>>>> Yes and the with changes done by me using ICSSG port seems much easier
>>>> and user friendly. Just doing `dhcp` at u-boot prompt is enough to use
>>>> ICSSG port so the driver changes effectively makes the usage for user
>>>> friendly. But only way I have found to load the firmware files is to use
>>>> fs-loader that requires the boot medium and boot partition. I am not
>>>> sure how can I re-use the the boot medium used for booting u-boot images.
>>>>
>>>> Also in SD card, u-boot images are located in /boot partition where as
>>>> firmware is located inside /root partition. So we'll need to specify the
>>>> partition.
>>>>
>>>> Currently I don't have any way to load the firmware files from driver
>>>> without using below DT change
>>>>
>>>> fs_loader0: fs-loader {
>>>>         bootph-all;
>>>>         compatible = "u-boot,fs-loader";
>>>>         phandlepart = <&sdhci1 2>;
>>>> };
>>>>
>>>
>>> Fromdoc/develop/driver-model/fs_firmware_loader.rst
>>>
>>> "Firmware loader driver is also designed to support U-Boot environment
>>> variables, so all these data from FDT can be overwritten
>>> through the U-Boot environment variable during run time.
>>>
>>> For examples:
>>>
>>> storage_interface:
>>>   Storage interface, it can be "mmc", "usb", "sata" or "ubi".
>>> fw_dev_part:
>>>   Block device number and its partition, it can be "0:1".
>>> fw_ubi_mtdpart:
>>>   UBI device mtd partition, it can be "UBI".
>>> fw_ubi_volume:
>>>   UBI volume, it can be "ubi0".
>>>
>>> When above environment variables are set, environment values would be
>>> used instead of data from FDT.
>>> The benefit of this design allows user to change storage attribute data
>>> at run time through U-Boot console and saving the setting as default
>>> environment values in the storage for the next power cycle, so no
>>> compilation is required for both driver and FDT."
>>>
>>> So looks like we should provide this in environment variables instead of DT?
>>>
>>
>> Thanks Roger for digging this up. I tried setting the below environment
>> values and it works.
>>
>> 	storage_interface=mmc
>> 	fw_dev_part=1:2
>>
>> No need to add the fs-loader node in DT. We can directly set the env
>> values and fs_loader will use them to load file. I will drop the DT
>> change for fs-loader. I think this env approach is same as running the
>> load command at u-boot (the initial approach). I think any medium that
>> could be used using load command, can be used here by setting
>> appropriate env values.
>>
>> Roger, Should I add the below code to include/env/ti/ti_common.env or
>> board/ti/am65x/am65x.env so that we don't need to set these variables
>> manually everytime we try to use ICSSG Ethernet and by default these
>> variables will be set to mmc and 1:2. User can definately modify these
>> at u-boot prompt and set appropriate values before running `dhcp`.
>>
>> diff --git a/include/env/ti/ti_common.env b/include/env/ti/ti_common.env
>> index f0f89a2287..0861d3be34 100644
>> --- a/include/env/ti/ti_common.env
>> +++ b/include/env/ti/ti_common.env
>> @@ -35,3 +35,8 @@ bootcmd_ti_mmc=
>>         else;
>>                 run get_kern_${boot}; run get_fdt_${boot}; run
>> get_overlay_${boot}; run run_kern;
>>         fi;
>> +
>> +#if CONFIG_TI_ICSSG_PRUETH
>> +storage_interface=mmc
> 
> "storage_interface" is vague. It should be updated to fw_storage_interface in fs-loader driver
> and environment. It could fallback to storage_interface to not break existing implementations.
> 

Sure. But changing "storage_interface" to "fw_storage_interface" in  the
fs-loader driver will also require changing the variable name in all the
users drivers (e.g. arch/arm/mach-k3/common.c)

>> +fw_dev_part=1:2
>> +#endif
>>
> 
> I think it should go in the respective <board>.env file as not all TI platforms
> have the same boot/firmware partition.
> so I would put it in board/ti/am65x/am65x.env
> 

Sure, I'll keep it in board/ti/am65x/am65x.env.

Andrew, Can you please confirm if you are okay with this approach. I
will post v2 soon, once you confirm.

>>
>>>>>>
>>>>>>>
>>>>>>> If we want to try to hide some of that then we need a way to
>>>>>>> run a user provided script from the environement to handle
>>>>>>> the general cases for firmware loading.
>>>>>
>>>>> Assuming we go with the "script provided in environment: route,
>>>>> how do we deal with the Rproc IDs? I'm not sure if they are constant
>>>>> and can probably change based on which Rproc is registered first.
>>>>> So there needs to be some way to make sure user can reference
>>>>> the correct Rproc.
>>>>>
>>>>
>>>> The Rproc IDs aren't constant. I think the IDs depend on the sequence of
>>>> init. The only way to know the IDs of different Rprocs is to run `rproc
>>>> init` and then `rproc list`. Mostly on AM65x I have seen Rporc ID 0 and
>>>> 1 to be of r5 rprocs and from 2 to 19 of differnet PRUs (Total 18, 6 per
>>>> ICSSG instance). If r5 is not enabled, the sequence changes. Similarly
>>>> if some other rproc is enabled sequence will change.
>>>>
>>>> The script should be able to read the list and determine which rproc
>>>> needs to be loaded based on the port we want to use. I am not sure how
>>>> to do this during u-boot.
>>>>
>>>
>>> rproc list gives an output in the following format.
>>>
>>> => rproc list
>>> 0 - Name:'r5f at 41000000' type:'internal memory mapped' supports: load start stop reset
>>> 1 - Name:'r5f at 41400000' type:'internal memory mapped' supports: load start stop reset
>>> 2 - Name:'r5f at 5c00000' type:'internal memory mapped' supports: load start stop reset
>>> 3 - Name:'r5f at 5d00000' type:'internal memory mapped' supports: load start stop reset
>>> 4 - Name:'r5f at 5e00000' type:'internal memory mapped' supports: load start stop reset
>>> 5 - Name:'r5f at 5f00000' type:'internal memory mapped' supports: load start stop reset
>>> 6 - Name:'dsp at 4d80800000' type:'internal memory mapped' supports: load start stop reset
>>> 7 - Name:'dsp at 4d81800000' type:'internal memory mapped' supports: load start stop reset
>>> 8 - Name:'dsp at 64800000' type:'internal memory mapped' supports: load start stop reset
>>>
>>> How can the script know which rproc to start for a specific Ethernet interface?
>>>
>>
>> I am not sure. For ICSSGx port-y, we need to start pru_x_y, rtu_x_y,
>> tx_pru_x_y. The script could search for these core names and start the
>> core number corresponding to these names in the result of `rproc list`.
>>
>> I think it will not be needed now as we can modify the envs to select
>> different mediums, instead of hardcoding DT.
>>
> 
> OK.
> 

-- 
Thanks and Regards,
Danish


More information about the U-Boot mailing list