linux-kernel - Re: [PATCH v6 4/7] pinctrl: s32: convert the driver into an mfd cell

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <8bdf05e8-ee92-4551-80a8-1efe948d7a18@oss.nxp.com>
Date: Wed, 20 Nov 2024 11:29:30 +0200
From: Andrei Stefanescu <andrei.stefanescu@....nxp.com>
To: Krzysztof Kozlowski <krzk@...nel.org>,
 Linus Walleij <linus.walleij@...aro.org>, Bartosz Golaszewski
 <brgl@...ev.pl>, Rob Herring <robh@...nel.org>,
 Krzysztof Kozlowski <krzk+dt@...nel.org>, Conor Dooley
 <conor+dt@...nel.org>, Chester Lin <chester62515@...il.com>,
 Matthias Brugger <mbrugger@...e.com>,
 Ghennadi Procopciuc <Ghennadi.Procopciuc@....com>,
 Larisa Grigore <larisa.grigore@....com>,
 Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
 "Rafael J. Wysocki" <rafael@...nel.org>, Lee Jones <lee@...nel.org>,
 Shawn Guo <shawnguo@...nel.org>, Sascha Hauer <s.hauer@...gutronix.de>,
 Fabio Estevam <festevam@...il.com>, Dong Aisheng <aisheng.dong@....com>,
 Jacky Bai <ping.bai@....com>
Cc: linux-gpio@...r.kernel.org, devicetree@...r.kernel.org,
 linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
 NXP S32 Linux Team <s32@....com>, Christophe Lizzi <clizzi@...hat.com>,
 Alberto Ruiz <aruizrui@...hat.com>, Enric Balletbo <eballetb@...hat.com>,
 Pengutronix Kernel Team <kernel@...gutronix.de>, imx@...ts.linux.dev
Subject: Re: [PATCH v6 4/7] pinctrl: s32: convert the driver into an mfd cell

Hi Krzysztof,

>>>> +	if (npins < 0)
>>>> +		return dev_err_probe(dev, -EINVAL,
>>>> +				     "Failed to read 'pinmux' in node %s\n",
>>>> +				     grp->data.name);
>>>
>>> I do not see how this change is related. Looks you are mixing cleanups
>>> with refactoring into MFD cell. These are two different things.
>>
>> Yes, I also included some small refactoring changes. I didn't think they were
>> important enough to include them in a separate commit. Would you like me to separate
>> them in another commit?
> 
> You cannot include such changes along other, meaningful changes. This
> does not apply to this patch only but all contributions. There is a
> clear policy how cleanups, bugs and new things are being upstreamed:
> https://elixir.bootlin.com/linux/v6.12/source/Documentation/process/submitting-patches.rst#L168
> 
> Please read above document very carefully. This is v6 and we still
> circle around absolute basics.

I will split these changes in two commits in v7.

> 
>>
>>>> -	if (mem_regions == 0 || mem_regions >= 10000) {
>>>> -		dev_err(&pdev->dev, "mem_regions is invalid: %u\n", mem_regions);
>>>> -		return -EINVAL;
>>>> -	}
>>>> +	/* one MSCR and one IMCR region per SIUL2 module */
>>>
>>> How is this related to converion into MFD cell?
>>
>> We no longer parse the device tree to configure the regmaps, we instead
>> get them from the mfd driver. This is the main point of converting this
>> driver into an mfd cell. 
>>
>>>
>>> Still looks like an ABI break.
>>>
>>
>> Yes, the driver no longer adheres to the nxp,s32g2-siul2-pinctrl.yaml binding.
> 
> I did not find in commit msg explanation that this is ABI break and why
> it is allowed. I asked for it.

I will explicitly mention this in v7.

> 
>>
>> The intention is to deprecate that binding since it doesn't correctly describe
>> the hardware. I am not sure on how to do this. I thought that changing this
>> driver and removing the old compatible would be enough.
> 
> No, you cannot break the ABI. Either you deprecate this or just don't touch.

Yes, I would like to deprecate the binding. I am not sure on how to do this.
I saw that some bindings have a "deprecated: true" property at the top-level.
Should I also add a commit which does that in v7 or are there other steps
for deprecating a binding?

Best regards,
Andrei