Subject: Re: [PATCH v1 1/2] spi: deduplicate spi_match_id() in
 __spi_register_driver()
To:     Andy Shevchenko <andriy.shevchenko@linux.intel.com>,
        Mark Brown <broonie@kernel.org>, linux-spi@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20211119173718.52938-1-andriy.shevchenko@linux.intel.com>
Cc:     "linux-tegra@vger.kernel.org" <linux-tegra@vger.kernel.org>
From:   Jon Hunter <jonathanh@nvidia.com>
Message-ID: <1572a2ff-dcfb-422f-c4c3-5a454a36d31d@nvidia.com>
Date:   Tue, 23 Nov 2021 15:22:38 +0000
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.14.0
In-Reply-To: <20211119173718.52938-1-andriy.shevchenko@linux.intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: en-US
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?R2NyQ0RjZUFjM0NtVzF5cWRRRkEwV2ZpeUNVaDdiQTRqRStUUVRFQUpyL09K?=
 =?utf-8?B?aHJ1b21lZm9DRXY0V3JYQ3pDckFkbHdCbGNCWEhtRlpJUFhuemRYTGJyK2dP?=
 =?utf-8?B?M0txaGdyQlFwREZDemcwUm9BS1E1OXNSRG1xclJSUDFWTG9yM1lHU1oyTE9C?=
 =?utf-8?B?eUVSK09aN0pya1MrZHJ0dUJDSXowRjJNR1lEZDdOaFhXWTlHOXZQa1NnR1By?=
 =?utf-8?B?YlNxVVIxOStNMjIxV3lQWXRxTStQN0IvWEU0c0o5TWl0azZGRHRiSU9jdXVX?=
 =?utf-8?B?UmtDNXRwRmt5bVV2TllFT1ZNU0dtdlpQTSswQkg1ZXBlRGNmbm5iUmRJeTZo?=
 =?utf-8?B?dTdJVGo5YnRBYWwrNmFoaC9CUFJlUG04T25kR0VXNVZnTFNTYnR0L3J5Qmdu?=
 =?utf-8?B?aXhTQWVXUjRKVUhzaHh0QXN5STFuOUdWVmMrVzNnaldUQldRNldSR3VUSTRH?=
 =?utf-8?B?YUZFTGkzbTB3bUVld0lwK0gyS0UvcnRGZEM5MXIzMVRrb1RzV1FpM2M1RUxl?=
 =?utf-8?B?QUZ3VllNdXh4QUgwb1BobUlEcmZLZFFtMDcxOEhRWFpLOHZ3UU84WTh0Zy9l?=
 =?utf-8?B?UE5pcCsyQkFUcGk2RjhUR0FZak9ZSHQ4eWxEUFk3ZkJOSEVGQU00bnJVNEZY?=
 =?utf-8?B?VEJlMFRXRDBrSW0vbEdScDBOU09xc2ZCc2VLeXpYSzF5cmlZdEZBUzNCUnZN?=
 =?utf-8?B?TTBFMkM0MGpsRUpSU1FFV2Y3NkxqcEtNRFZIbWZ0NDlPd2tJbzRjVmI4Q01i?=
 =?utf-8?B?TmIvdHJwUmdTRjZSZEgzaTZ1WWpzdExFL3g4dU9Zek55Ulc5MXRGUW5zN04r?=
 =?utf-8?B?TXR5YVFrVkk2d053WXg3RXp1NzFUYzBvampuVEIxRmNsbnBwUVc2VEpDbWVw?=
 =?utf-8?B?MTViM0g1TWEyTXZSV3Y1R1YyOVRqZ2c4UElBZGdKRDlHSHlyNGdNNC9TOXBl?=
 =?utf-8?B?U0x1dEVGT1RoWm9qUlRuY0NsVVh3cnZxR1Jxc2h3V1p6ekdSbDJ6U2srR3FG?=
 =?utf-8?B?d2ZLM1JZekNTV2c4MFB5cUR4MHdiRjROOG16UDhuaFk3aGZJVTN2b0N2aWZO?=
 =?utf-8?B?L0NTcmhhZVhhSklxNlREazNRYXM1aE5kVVlSOVBneTB4cUQ2RVk3dVQ1UTl5?=
 =?utf-8?B?ZHRlcXV0QTltTC9hY3Rva2o4MFpPQm1ZL2pON3VBY0VMdVNwcUdDOWYxSGpH?=
 =?utf-8?B?NWIxdWh6UXBWdjc5NUl1NzZBRDVFRTR2VDFMWGRYcmlIMHdkQ2FYSmtBTm10?=
 =?utf-8?B?dCtoUTZFOVRpVGZqRGZEYkdqcXhrMXpIYWRJUTIzWGZ5R3Z0elZ1VnRlcXNv?=
 =?utf-8?B?ZUxQc1FDUUxvSUNpTC9PUUc0KzJBam90SUF2b2p6NkNOYktHWXhzOHZaaEZN?=
 =?utf-8?B?dGdhOXY0SUYwT05yYWx0Q2daeDNvUW9pc2hFRHJjZ05WNGxteXVIdUN5WFpJ?=
 =?utf-8?B?TWt5NkFsRWxTTWJjZ3dldTluMmUrZldFRnJiMEJnMEpjM3FFRHBWZnJBOEQ0?=
 =?utf-8?B?NEpDRFFNalVVaFVkbjQrU1BMWFdDSlQ1SlNDejNvaDQvTGFwOWcycmFOZnlO?=
 =?utf-8?B?Z2toS1k3QTEzWTl4NDJ0WCsyUFRaL1dqYXhvVlRDQkc4S0IyaUNjRTBuQ3R4?=
 =?utf-8?B?T0FvVUFETEx6Y2pjVEhiZzNTN1kwT2RQdkJGOFJraTV1UThjd21FRmUyNjdq?=
 =?utf-8?B?SW9USEN4RWxFZTA4dDkxWk9SblZySEdobEwwWDFRU1dWWXFZMVZibEFlVE9h?=
 =?utf-8?B?TWxobHVCNjVGdzdSUitrUWU4Slo2SWRVZFJmTmRLTkl1cEp1OTFHR1pVZzlE?=
 =?utf-8?B?NmIwS3B4UnFtUjhTWXFzODQrK3BXV3FrWjREWEFvVCtyRlF3S1U1angrUW1h?=
 =?utf-8?B?R0k1TWExYzU1YjBXN1FsTm9nallrSjl3cmNic3ZlOEJTL1VKajNXZ21yWHYx?=
 =?utf-8?B?dkw0U0V2dVJ2VGJiUFBYcHJMWHM2SFdYQTNaK1RDbjZlWlhxeDEvTlg2bmdS?=
 =?utf-8?B?c1ljcUJaRjV5RkRTZ3Q1MGllalJCTzFLYjVNNXVLY09rZWJwOHBvWW5VaTdI?=
 =?utf-8?B?NkUyMFVzcitzTGRoMEgvdTZjNGFWcmNMR2xxUkppb3RmNS9keHE4c29hRjB1?=
 =?utf-8?B?elJCMHFMOXZQZ1I2blNpUVlEeWhEbE9TMXFxK055MlJNZ3hJR0xDbnBseS9n?=
 =?utf-8?B?cUtlWnRpdlUwT1lsVkloTVFNMGZIY3B4OU1HMzN4SXZrR2Rnb01WVEFhSWND?=
 =?utf-8?Q?OIiVzHuHQTtbq+0mN3Wc3tuENXyaShu1Bn200OugIM=3D?=
X-OriginatorOrg: Nvidia.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1eea12b4-bee7-4686-2fea-08d9ae951a0f
X-MS-Exchange-CrossTenant-AuthSource: CO6PR12MB5444.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Nov 2021 15:22:46.4977
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 43083d15-7273-40c1-b7db-39efd9ccc17a
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 3c0l3Q9z2j5DfExcKIrvnRKQrznV7mzfgEEHyxTVPhlVzy1HmermDeIJ9OvL0ZXVaTCl+yiwE8evy1Vj0WoPOg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO6PR12MB5459
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Andy,

On 19/11/2021 17:37, Andy Shevchenko wrote:
> The same logic is used in spi_match_id() and in the __spi_register_driver().
> By switching the former from taking struct spi_device * to const char * as
> the second parameter we may deduplicate the code.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>   drivers/spi/spi.c | 17 ++++++-----------
>   1 file changed, 6 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/spi/spi.c b/drivers/spi/spi.c
> index b23e675953e1..dfa06103150e 100644
> --- a/drivers/spi/spi.c
> +++ b/drivers/spi/spi.c
> @@ -315,11 +315,10 @@ static void spi_statistics_add_transfer_stats(struct spi_statistics *stats,
>    * and the sysfs version makes coldplug work too.
>    */
>   
> -static const struct spi_device_id *spi_match_id(const struct spi_device_id *id,
> -						const struct spi_device *sdev)
> +static const struct spi_device_id *spi_match_id(const struct spi_device_id *id, const char *name)
>   {
>   	while (id->name[0]) {
> -		if (!strcmp(sdev->modalias, id->name))
> +		if (!strcmp(name, id->name))
>   			return id;
>   		id++;
>   	}
> @@ -330,7 +329,7 @@ const struct spi_device_id *spi_get_device_id(const struct spi_device *sdev)
>   {
>   	const struct spi_driver *sdrv = to_spi_driver(sdev->dev.driver);
>   
> -	return spi_match_id(sdrv->id_table, sdev);
> +	return spi_match_id(sdrv->id_table, sdev->modalias);
>   }
>   EXPORT_SYMBOL_GPL(spi_get_device_id);
>   
> @@ -352,7 +351,7 @@ static int spi_match_device(struct device *dev, struct device_driver *drv)
>   		return 1;
>   
>   	if (sdrv->id_table)
> -		return !!spi_match_id(sdrv->id_table, spi);
> +		return !!spi_match_id(sdrv->id_table, spi->modalias);
>   
>   	return strcmp(spi->modalias, drv->name) == 0;
>   }
> @@ -474,12 +473,8 @@ int __spi_register_driver(struct module *owner, struct spi_driver *sdrv)
>   			if (sdrv->id_table) {
>   				const struct spi_device_id *spi_id;
>   
> -				for (spi_id = sdrv->id_table; spi_id->name[0];
> -				     spi_id++)
> -					if (strcmp(spi_id->name, of_name) == 0)
> -						break;
> -
> -				if (spi_id->name[0])
> +				spi_id = spi_match_id(sdrv->id_table, of_name);
> +				if (!spi_id)
>   					continue;
>   			} else {
>   				if (strcmp(sdrv->driver.name, of_name) == 0)
> 


Following this change I am seeing the following warnings again although 
most of these have now been fixed ...

  WARNING KERN SPI driver mtd_dataflash has no spi_device_id for atmel,at45
  WARNING KERN SPI driver mtd_dataflash has no spi_device_id for 
atmel,dataflash
  WARNING KERN SPI driver spi-nor has no spi_device_id for jedec,spi-nor
  WARNING KERN SPI driver mmc_spi has no spi_device_id for mmc-spi-slot
  WARNING KERN SPI driver cros-ec-spi has no spi_device_id for 
google,cros-ec-spi

I have not looked any further yet, but this appears to cause the SPI ID 
match to fail.

Cheers
Jon

-- 
nvpublic