Re: [PATCH v2] net/cxgbe: read firmware configuration file from filesystem

[Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>]

On Monday, May 05/16/22, 2022 at 12:06:01 +0100, Ferruh Yigit wrote:
> On 5/16/2022 11:27 AM, Rahul Lakkireddy wrote:
> > Add support to read firmware configuration file from
> > /lib/firmware/cxgb4/ path in the filesystem. The firmware
> > config file is used to enable or disable NIC features before
> > firmware initialization to help retrieve better debug data to
> > analyze firmware init failures. The config file can also
> > be used to redistribute resources, like queues, TCAMs, etc.,
> > from disabled physical functions (PFs) to main PF, before
> > firmware init.
> 
> Hi Rahul,
> 
> Please find comments below.
> 
> Also can you please send a new version for both 4/5 and 5/5 (this patch)
> from original series?
> 

I had already posted v2 for 4/5 from original series at below location.

https://patches.dpdk.org/project/dpdk/patch/61ae717665c2f4e38712652bec2f9d0fe5ca7d32.1651842841.git.rahul.lakkireddy@chelsio.com/

Do you want me to send a new patch series with 4/5 and 5/5 in a
single patch series?

> > 
> > Signed-off-by: Rahul Lakkireddy <rahul.lakkireddy@chelsio.com>
> > ---
> > v2:
> > - Update cxgbe pmd doc about the firmware config file
> > - Update commit message to explain more about firmware config file
> > 
> >   doc/guides/nics/cxgbe.rst               |  28 ++
> >   drivers/net/cxgbe/base/t4fw_interface.h |   1 +
> >   drivers/net/cxgbe/cxgbe_main.c          | 329 ++++++++++++++++--------
> >   3 files changed, 245 insertions(+), 113 deletions(-)
> > 
> > diff --git a/doc/guides/nics/cxgbe.rst b/doc/guides/nics/cxgbe.rst
> > index a1d30c488b..fc8a5751f1 100644
> > --- a/doc/guides/nics/cxgbe.rst
> > +++ b/doc/guides/nics/cxgbe.rst
> > @@ -838,3 +838,31 @@ to configure the mtu of all the ports with a single command.
> >        testpmd> port stop all
> >        testpmd> port config all max-pkt-len 9000
> > +
> > +Hardware Configuration and Debugging
> > +------------------------------------
> > +
> > +Firmware Configuration File
> > +~~~~~~~~~~~~~~~~~~~~~~~~~~~
> > +
> > +To enable or disable Chelsio NIC features before firmware initialization,
> > +the Chelsio firmware configuration file can be placed in following
> > +directory. The CXGBE PMD will search and pick up the firmware
> > +configuration file during the Chelsio NIC probe, before initializing
> > +the firmware.
> > +
> 
> Does it worth to mention what happens if the FW config file doesn't exist?
> Or mention from FW config file load order, as you described in mail list, to
> understand the relation with 'cxgbtool'?
> 

Will do.

> > +.. code-block:: console
> > +
> > +   cp <path_to_config_file>/t6-config.txt /lib/firmware/cxgb4/t6-config.txt
> > +
> 
> There is also 't5-config.txt' in the code.
> 

Yes, t5-config.txt is for Chelsio T5 NIC series and t6-config.txt
is for Chelsio T6 NIC series. Will update t5-config.txt as well.

> > +The firmware configuration file is mainly intended to be used to debug
> > +firmware initialization failures. It can also be used to redistribute
> > +NIC resources from disabled physical functions (PFs) to main PF before
> > +initializing firmware.
> > +
> > +.. warning::
> > +
> > +   Note that the Chelsio firmware configuration file contains very low
> > +   level details that is specific to the Chelsio NIC. Hence, the
> > +   firmware configuration file must not be modified without expert
> > +   guidance from Chelsio support team.
> 
> Will it be too much detail to document what config can be changed via this
> FW config?
> 

Yes, this will be difficult to document. The config file just looks like
register=value pairs and are often dependent on each other based on
the selected configuration. A portion of sample config file is given
below. I'm not sure how to document this kind of info.

[global]
    reg[0x100c] = 0x22222222
    reg[0x10a0] = 0x01040810
    reg[0x1044] = 4096
    reg[0x1048] = 65536
    reg[0x104c] = 1536
    reg[0x1050] = 9024
    reg[0x1054] = 9216
    reg[0x1058] = 2048
    reg[0x105c] = 128
    reg[0x1060] = 8192
    reg[0x1064] = 16384
[...]

Thanks,
Rahul