Re: [PATCH net-next 1/3] i2c: xiic: allow platforms to require 32-bit register accesses

Re: [PATCH net-next 1/3] i2c: xiic: allow platforms to require 32-bit register accesses

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found more severe
issue(s) in other patches of this series. This mail is only an FYI, in case
you want to address the nit picks below when you respin.

Low priority / nit picks for your consideration:

- [Low] Commit "i2c: xiic: allow platforms to require 32-bit register
  accesses" adds xiic_i2c_platform_data::force_32bit_access and the…

--

i2c: xiic: allow platforms to require 32-bit register accesses

This commit adds a force_32bit_access flag to xiic_i2c_platform_data.
When the flag is set, xiic_setreg8(), xiic_getreg8() and xiic_setreg16()
perform aligned 32-bit accesses in the detected endianness. The target is
the LitePCIe bridge on the WiWi Time Card, which cannot handle subword
MMIO accesses.

> diff --git a/drivers/i2c/busses/i2c-xiic.c b/drivers/i2c/busses/i2c-xiic.c
> index ba560462d4513..d13cfb0127a20 100644
> --- a/drivers/i2c/busses/i2c-xiic.c
> +++ b/drivers/i2c/busses/i2c-xiic.c

[ ... ]

> @@ -1480,6 +1498,8 @@ static int xiic_i2c_probe(struct platform_device *pdev)
>  		return irq;
>  
>  	pdata = dev_get_platdata(dev);
> +	if (pdata)
> +		i2c->force_32bit_access = pdata->force_32bit_access;

[Severity: Low]
This isn't a bug, but nothing in the tree sets force_32bit_access at this
commit. The ptp_ocp xiic tables and timberdale both leave it false, so the
new paths in xiic_setreg8(), xiic_getreg8() and xiic_setreg16() can't be
reached yet.

The later patch in this series, "ptp: ocp: add WiWi Time Card production
image support", adds the user in ptp_ocp_i2c_bus():

    if (bp->wiwi) {
        wiwi_data = *(struct xiic_i2c_platform_data *)info->data;
        wiwi_data.force_32bit_access = true;
        data = &wiwi_data;
    }

That copy is then passed to platform_device_register_resndata(), so the
flag has an in-tree user once the series is applied.

Could the commit message say that the user of this flag comes in that
follow-up patch?

>  
>  	/* hook up driver to tree */
>  	platform_set_drvdata(pdev, i2c);

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261111041904.48916-1-ahmadexp%40gmail.com