Re: [RFC v2 3/5] clk: bcm2835: use firmware interface to update pllb
From: Nicolas Saenz Julienne
Date: Tue May 21 2019 - 08:21:28 EST
On Tue, 2019-05-21 at 14:14 +0200, Petr Tesarik wrote:
> On Tue, 21 May 2019 13:39:31 +0200
> Nicolas Saenz Julienne <nsaenzjulienne@xxxxxxx> wrote:
>
> > Hi Oliver, thanks for the review.
> >
> > On Mon, 2019-05-20 at 14:43 +0200, Oliver Neukum wrote:
> > > On Mo, 2019-05-20 at 12:47 +0200, Nicolas Saenz Julienne wrote:
> > > > + * For more information on the firmware interface check:
> > > > + *
> > > > https://github.com/raspberrypi/firmware/wiki/Mailbox-property-interface
> > > > + */
> > > > +struct bcm2835_firmware_prop {
> > > > + u32 id;
> > > > + u32 val;
> > > > + u32 disable_turbo;
> > > > +} __packed;
> > >
> > > Hi,
> > >
> > > technically we are not in arch and those fields have a defined
> > > endianness.
> > >
> >
> > Well I set it as packed since it's 'sent' through a memory mapped firmware
> > interface. Hence the need for the structure format to be fixed. So I guessed
> > we're safer with it, as I'm not 100% sure what the different compilers are
> > going to do with it (although it's very likely it'll stay the same). BTW
> > this
> > will be built both for arm & arm64.
>
> I believe that's not the point Oliver was trying to make. You should
> use __le32 instead of u32.
>
> That's because u32 means "host byte order" and this code is not located
> under arch/, so host endianness is unknown, but the mailbox interface
> requires little-endian.
>
> It's nit-picking, and that's why Oliver writes 'technically'; there is
> probably no way this firmware interface could be used on a big-endian
> CPU...
Understood, thanks for the clarification.
Regards,
Nicolas
Attachment:
signature.asc
Description: This is a digitally signed message part