Re: [PATCH net-next v2 3/4] mlxbf_gige: add BlueField-3 Serdes configuration

[Jakub Kicinski <kuba@kernel.org>]

On Wed, 16 Nov 2022 18:01:41 -0800 Saeed Mahameed wrote:
> >	fw = request_firmware("whatever_name.ftb");
> >	
> >	abc = fw_table_get(fw, "table_abc");
> >	/* use abc */
> >  
> 
> abc is just a byte buffer ? right ?

pointer to const struct fw_table32, which is just;

struct fw_table32 {
	u32 addr;
	u32 value;
};

Actually we need the length as well, so perhaps this is better:

struct fw_table32 {
	u32 addr;
	u32 val;
};

struct fw_table_result {
	uint cnt;
	union {
		const struct fw_table32 *tb32;
	};
};

User:

	struct fw_table_result tab;

	fw = request_firmware("whatever_name.ftb");
	if (!fw)
		...

	err = fw_table_get(fw, "table_abc", &tab);
	if (err)
		...
	for (i = 0; i < tab.cnt; i++) /* use abc */
		write_or_whatever(hw, tab.tb32[i].addr, tab.tb32[i].val);

	def = fw_table_get(fw, "table_def");
	if (err)
		...
	for (i = 0; i < tab.cnt; i++) /* use def */
		write_or_whatever(hw, tab.tb32[i].addr, tab.tb32[i].val);

	release_firmware(fw)

> >	def = fw_table_get(fw, "table_def");
> >	/* use def */
> >  
> 
> And what goes here? any constraints on how the driver must interpret
> and handle abc/def blobs ? 

In the example I assumed a typical buffer with addr / value pairs.
But we can define more table types as needed.

> >	release_firmware(fw)  
> 
> What if the same abc blob structure/table format is used to setup dynamic link
> properties, say via ethtool -s ? Then the whole request firmware will be
> redundant since "struct abc {};" must be defined in the driver src code.

No complex structures, we'd be only targeting register init 
and small FW blobs (IOW { u32 addr; u32 val; } and { u8 val; }).
Stuff which Windows? drivers tend to put into the code as static array.

> I like the idea, i am just trying to figure how we are going to define it
> and how developers will differentiate between when to use this or when to
> use standard APIs to setup their devices.