Re: [PATCH v2 2/6] mtd: spi-nor: Avoid forward declaration of internal functions

[<Tudor.Ambarus@xxxxxxxxxxxxx>]

Hi,

On 12/05/2018 10:09 AM, Boris Brezillon wrote:
[cut]

>>> -static int spi_nor_hwcaps_read2cmd(u32 hwcaps);
>>> +static int spi_nor_hwcaps2cmd(u32 hwcaps, const int table[][2], size_t size)
>>> +{
>>> +	size_t i;
>>> +
>>> +	for (i = 0; i < size; i++)
>>> +		if (table[i][0] == (int)hwcaps)
>>> +			return table[i][1];
>>> +
>>> +	return -EINVAL;
>>> +}
>>> +
>>> +static int spi_nor_hwcaps_read2cmd(u32 hwcaps)
>>> +{
>>> +	static const int hwcaps_read2cmd[][2] = {
>>> +		{ SNOR_HWCAPS_READ,		SNOR_CMD_READ },
>>> +		{ SNOR_HWCAPS_READ_FAST,	SNOR_CMD_READ_FAST },
>>> +		{ SNOR_HWCAPS_READ_1_1_1_DTR,	SNOR_CMD_READ_1_1_1_DTR },
>>> +		{ SNOR_HWCAPS_READ_1_1_2,	SNOR_CMD_READ_1_1_2 },
>>> +		{ SNOR_HWCAPS_READ_1_2_2,	SNOR_CMD_READ_1_2_2 },
>>> +		{ SNOR_HWCAPS_READ_2_2_2,	SNOR_CMD_READ_2_2_2 },
>>> +		{ SNOR_HWCAPS_READ_1_2_2_DTR,	SNOR_CMD_READ_1_2_2_DTR },
>>> +		{ SNOR_HWCAPS_READ_1_1_4,	SNOR_CMD_READ_1_1_4 },
>>> +		{ SNOR_HWCAPS_READ_1_4_4,	SNOR_CMD_READ_1_4_4 },
>>> +		{ SNOR_HWCAPS_READ_4_4_4,	SNOR_CMD_READ_4_4_4 },
>>> +		{ SNOR_HWCAPS_READ_1_4_4_DTR,	SNOR_CMD_READ_1_4_4_DTR },
>>> +		{ SNOR_HWCAPS_READ_1_1_8,	SNOR_CMD_READ_1_1_8 },
>>> +		{ SNOR_HWCAPS_READ_1_8_8,	SNOR_CMD_READ_1_8_8 },
>>> +		{ SNOR_HWCAPS_READ_8_8_8,	SNOR_CMD_READ_8_8_8 },
>>> +		{ SNOR_HWCAPS_READ_1_8_8_DTR,	SNOR_CMD_READ_1_8_8_DTR },
>>> +	};
>>> +
>>> +	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_read2cmd,
>>> +				  ARRAY_SIZE(hwcaps_read2cmd));
>>> +}
>>> +
>>> +static int spi_nor_hwcaps_pp2cmd(u32 hwcaps)
>>> +{
>>> +	static const int hwcaps_pp2cmd[][2] = {
>>> +		{ SNOR_HWCAPS_PP,		SNOR_CMD_PP },
>>> +		{ SNOR_HWCAPS_PP_1_1_4,		SNOR_CMD_PP_1_1_4 },
>>> +		{ SNOR_HWCAPS_PP_1_4_4,		SNOR_CMD_PP_1_4_4 },
>>> +		{ SNOR_HWCAPS_PP_4_4_4,		SNOR_CMD_PP_4_4_4 },
>>> +		{ SNOR_HWCAPS_PP_1_1_8,		SNOR_CMD_PP_1_1_8 },
>>> +		{ SNOR_HWCAPS_PP_1_8_8,		SNOR_CMD_PP_1_8_8 },
>>> +		{ SNOR_HWCAPS_PP_8_8_8,		SNOR_CMD_PP_8_8_8 },
>>> +	};
>>> +
>>> +	return spi_nor_hwcaps2cmd(hwcaps, hwcaps_pp2cmd,
>>> +				  ARRAY_SIZE(hwcaps_pp2cmd));
>>> +}  
>>
>> Would it be a better place to put these immediately after
>> spi_nor_set_pp_settings()? It would be consistent with how they were declared
>> back in cfc5604c488ccd17936b69008af0c9ae050f4a08.
> 
> I thought it would be preferable to have the xx2cmd[] conversion tables
> grouped together, but I can move this one next to
> spi_nor_set_pp_settings() if you prefer.
> 

I thought of moving the entire chunk (_hwcaps2cmd(), _read2cmd(), _pp2cmd())
just after spi_nor_set_pp_settings(). You have in that section of code the
spi_nor_read/pp_command_index enums followed by spi_nor_set_read/pp_settings().
We can have all the read/pp stuff grouped together.

Cheers,
ta
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/