Apologies for belaboring this point, but doesn't your logic mean that
the code for selecting the "standard" (i.e. non-grlib) accessors is
wrong? Putting my broken HW aside, if a device tree doesn't specify
big_endian, assuming that the default is big_endian, then won't these
lines of code, which are in the mainline driver:
bool be = pdata ? pdata->big_endian :
of_device_is_big_endian(pdev->dev.of_node);
.
.
.
case 2:
i2c->setreg = be ? oc_setreg_16be : oc_setreg_16;
i2c->getreg = be ? oc_getreg_16be : oc_getreg_16;
break;
then use the little endian (i.e. oc_setreg_16) accessors? If so,
shouldn't the use of big_endian in this driver be replaced with
little_endian, and the corresponding code updated?
On Tue, Aug 18, 2020 at 10:14 PM Andrew Lunn <andrew@xxxxxxx> wrote:
>
> On Tue, Aug 18, 2020 at 06:14:31PM -0400, Mohammed Billoo wrote:
> > Andrew,
> >
> > Thanks for your comments. Does it make sense to replace the big_endian
> > bool with a small_endian bool? The code to choose the appropriate
> > non-grlib accessors assumes that big_endian will be specified, either
> > in a device tree blob or via platform_data:
> >
> > if (!i2c->setreg || !i2c->getreg) {
> > bool be = pdata ? pdata->big_endian :
> > of_device_is_big_endian(pdev->dev.of_node);
> > .
> > .
> > .
> > case 2:
> > i2c->setreg = be ? oc_setreg_16be : oc_setreg_16;
> >
> > And so if endianess isn't specified (assuming the default is big
> > endian), it will actually default to small endian.
>
> You have to assume there is no indication of endianness in device tree
> by default. For your broken hardware you will indicate little endian
> in device tree. If you want, you could add support for DT indicating
> big endian, but it is not required.
>
> Andrew
--
Mohammed A Billoo
Founder
MAB Labs, LLC
www.mab-labs.com
201-338-2022
