[RFC PATCH 00/16] Introduce ICSSG Ethernet driver
Roger Quadros
rogerq at kernel.org
Tue Jan 9 13:34:29 CET 2024
On 09/01/2024 11:49, MD Danish Anwar wrote:
>
>
> 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)
>
That's why I said to fallback to "storage_interface" i.e. not drop it entirely.
FS-loader Documentation will also need to be updated.
>>> +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.
>>
>
--
cheers,
-roger
More information about the U-Boot
mailing list