On 19/11/2024 10:57, Andrei Stefanescu wrote: > 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. > >>> - 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. > > 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. > > Would it be better to add another file which would contain the old probing > function(and match the old binding) so clients would be able to select the > old implementation? I don't understand that. Your driver is supposed to keep ABI. Not through some selection but just as is. Best regards, Krzysztof
