Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <CAMdYzYquceSBrOsvO8rW9wmJA_RO=HSwv_waVoS=0hsP414T-A@mail.gmail.com>
 <YesxfEDbK/qFizFG@robh.at.kernel.org>
In-Reply-To: <YesxfEDbK/qFizFG@robh.at.kernel.org>
From:   Peter Geis <pgwipeout@gmail.com>
Date:   Fri, 21 Jan 2022 20:27:17 -0500
Message-ID: <CAMdYzYrnahMuZTBjwNRrHnnNhn4wB7Ze=UA_HhzurqHC9U6oJg@mail.gmail.com>
Subject: Re: [BUG] device_property_read_u16 reads out only zero
To:     Rob Herring <robh@kernel.org>
Cc:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "Rafael J. Wysocki" <rafael@kernel.org>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Fri, Jan 21, 2022 at 5:19 PM Rob Herring <robh@kernel.org> wrote:
>
> On Fri, Jan 14, 2022 at 05:48:52PM -0500, Peter Geis wrote:
> > Good Evening,
> >
> > I seem to have come across a strange bug with drivers/base/property.c
> > while expanding the new cyttsp5 driver to handle device-tree
> > overrides.
> >
> > With the property:
> > touchscreen-size-x = <1863>;
> > The following code:
> > u32 test_u32 = 32; /* canary to catch writing a zero */
> > u16 test_u16 = 16; /* canary to catch writing a zero */
> > int ret;
> >
> > ret = device_property_read_u32(ts->dev, "touchscreen-size-x", &test_u32);
> > if(ret)
> > dev_err(ts->dev, "read_u32 failed ret: %d\n", ret);
> >
> > ret = device_property_read_u16(ts->dev, "touchscreen-size-x", &test_u16);
> > if(ret)
> > dev_err(ts->dev, "read_u16 failed ret: %d\n", ret);
> >
> > dev_err(ts->dev, "read_u32: %d, read_u16: %d\n", test_u32, test_u16);
> >
> > returns the following:
> > [    1.010876] cyttsp5 5-0024: read_u32: 1863, read_u16: 0
> >
> > This was as of 5.16-rc8, using the
> > gcc-arm-10.2-2020.11-x86_64-aarch64-none-linux-gnu compiler.
> > I honestly am at a loss here, any insight you can provide here would
> > be appreciated.
>
> The property "touchscreen-size-x" is a u32. Calling
> device_property_read_u16 is an error though one is not returned here.
> You get 0 because that is what the first 2 bytes of the property
> contain. DT data is big-endian.

I figured this was the case, but I was hopeful the operators would be
smart enough to handle endian translations.
Wouldn't all DT numeric properties be u32, meaning
device_property_read_u16 and device_property_read_u8 are meaningless
on little endian devices?
Or is there a way to force smaller values in the DT?

>
> I suspect making this a hard error would break some users, but we could
> try a WARN.

I don't suspect it would be trivial to implement endian translation
for these functions?

>
> Rob

Always,
Peter