On Wed, 2015-08-19 at 23:37 +0200, Laurent Dufour wrote:
> On 03/08/2015 13:00, David Laight wrote:
From: Laurent Dufour
> > > Sent: 31 July 2015 10:30
> > > This patch fixes several endianness issues detected when running
> > > the HVSI
> > > driver in little endian mode.
> > >
> > > These issues are raised in little endian mode because the data
> > > exchanged in
> > > memory between the kernel and the hypervisor has to be in big
> > > endian
> > > format.
> > ...
> > > diff --git a/drivers/tty/hvc/hvsi.c b/drivers/tty/hvc/hvsi.c
> > > index 41901997c0d6..a75146f600cb 100644
> > > --- a/drivers/tty/hvc/hvsi.c
> > > +++ b/drivers/tty/hvc/hvsi.c
> > > @@ -240,9 +240,9 @@ static void hvsi_recv_control(struct
> > > hvsi_struct *hp, uint8_t *packet,
> > > {
> > > struct hvsi_control *header = (struct hvsi_control
> > > *)packet;
> > >
> > > - switch (header->verb) {
> > > + switch (be16_to_cpu(header->verb)) {
> > > case VSV_MODEM_CTL_UPDATE:
> > > - if ((header->word & HVSI_TSCD) == 0) {
> > > + if ((be32_to_cpu(header->word) &
> > > HVSI_TSCD) == 0) {
> >
> > It is generally best to byteswap constants.
> >
> > David
>
> Thanks David for your review.
> Regarding the byte swapping of the constants, I'm wondering if this
> the
> best way here.
> For instance, Benjamin wrote a similar patch to fix another
> endianness
> issue (99fc1d91b8fc) and he doesn't convert the constant neither.
> It think that byte swapping the constant value will impact more code,
> and may not ease code reading.
Right, I disagree with byteswapping the constants at their definition
point, however maybe he meant using cpu_to_be16(CONSTANT) ?
In any case, it's pretty moot as we have the lhbrx instruction which
will do the load and byteswap and for HVSI, even if it was a tad slower
than a normal load, it would not make a noticeable difference.
Ben.
_______________________________________________
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
