Re: [PATCH] i2c: designware: Remove i2c_dw_remove_lock_support()

[Nathan Chancellor <nathan@kernel.org>]

On Tue, Oct 14, 2025 at 04:58:56PM -0500, Mario Limonciello (AMD) (kernel.org) wrote:
> On 10/14/2025 3:33 PM, Bjorn Helgaas wrote:
> > I'm totally fine with the patch itself, but I think the commit log
> > could be trimmed to something like the following with no loss:
> > 
> >    Remove struct i2c_dw_semaphore_callbacks.remove() and
> >    i2c_dw_remove_lock_support().
> > 
> >    440da737cf8d ("i2c: designware: Use PCI PSP driver for
> >    communication") removed the last place that set
> >    i2c_dw_semaphore_callbacks.remove(), which made
> >    i2c_dw_remove_lock_support() a no-op.
> > 
> >    This has the side effect of avoiding this kCFI warning (see Link):
> > 
> >      dw_i2c_plat_remove+0x3c: no-cfi indirect call!
> > 
> >    Link: https://lore.kernel.org/r/20251013-dw_i2c_plat_remove-avoid-objtool-no-cfi-warning-v1-1-8cc4842967bf@kernel.org
> > 
> > FWIW,
> > Reviewed-by: Bjorn Helgaas <bhelgaas@google.com>
> 
> I echo Bjorn's comments on the lengthy commit message.
> Code change looks fine.
> 
> Reviewed-by: Mario Limonciello (AMD) <superm1@kernel.org>

I have no objections to trimming the commit message if so desired but I
think the solution (removing unused code) is more tangential to the
problem (potentially accessing an array out of bounds). I am sometimes
looking at changes from ten years ago where something was done to avoid
a problem but the problem was never mentioned in the message but may
have been elsewhere. Maybe nobody ever needs .remove() again but what if
new IP comes out that necessitates it and they go to revert this change
without avoiding this problem? I could try to make the analysis shorter
if that would help.

Cheers,
Nathan