[PATCH linux dev-4.10 v3 18/31] drivers: fsi: occ: Use big-endian values

[Andrew Jeffery]

On Thu, 2017-10-05 at 19:41 -0500, Eddie James wrote:
> 
> On 10/05/2017 06:03 PM, Andrew Jeffery wrote:
> > On Thu, 2017-10-05 at 14:24 -0500, Eddie James wrote:
> > > > > > From: "Edward A. James" <eajames at us.ibm.com>
> > >   
> > > Switch to __be16 from u16 and __be32 from u32. Also, use kernel access
> > > of unaligned be16 instead of manually creating a big-endian value from
> > > the occ response buffer.
> > >   
> > > > > > Signed-off-by: Edward A. James <eajames at us.ibm.com>
> > > ---
> > >   drivers/fsi/occ.c | 14 ++++++++------
> > >   1 file changed, 8 insertions(+), 6 deletions(-)
> > >   
> > > diff --git a/drivers/fsi/occ.c b/drivers/fsi/occ.c
> > > index 82a60c5..55f293d 100644
> > > --- a/drivers/fsi/occ.c
> > > +++ b/drivers/fsi/occ.c
> > > @@ -50,9 +50,9 @@ struct occ_response {
> > > > > >   	u8 seq_no;
> > > > > >   	u8 cmd_type;
> > > > > >   	u8 return_status;
> > > > > > -	u16 data_length;
> > > > > > +	__be16 data_length;
> > > > > >   	u8 data[OCC_RESP_DATA_BYTES];
> > > > > > -	u16 checksum;
> > > > > > +	__be16 checksum;
> > >   } __packed;
> > >   
> > >   /*
> > > @@ -425,7 +425,7 @@ static int occ_getsram(struct device *sbefifo, u32 address, u8 *data,
> > >   {
> > > > > >   	int rc;
> > > > > >   	u8 *resp;
> > > > > > -	u32 buf[5];
> > > > > > +	__be32 buf[5];
> > > > > >   	u32 data_len = ((len + 7) / 8) * 8;
> > > > > >   	struct sbefifo_client *client;
> > >   
> > > @@ -476,7 +476,7 @@ static int occ_putsram(struct device *sbefifo, u32 address, u8 *data,
> > > > > >   		       ssize_t len)
> > >   {
> > > > > >   	int rc;
> > > > > > -	u32 *buf;
> > > > > > +	__be32 *buf;
> > > > > >   	u32 data_len = ((len + 7) / 8) * 8;
> > > > > >   	size_t cmd_len = data_len + 20;
> > > > > >   	struct sbefifo_client *client;
> > > @@ -522,7 +522,7 @@ static int occ_putsram(struct device *sbefifo, u32 address, u8 *data,
> > >   static int occ_trigger_attn(struct device *sbefifo)
> > >   {
> > > > > >   	int rc;
> > > > > > -	u32 buf[6];
> > > > > > +	__be32 buf[6];
> > > > > >   	struct sbefifo_client *client;
> > >   
> > > > > >   	buf[0] = cpu_to_be32(0x6);
> > > @@ -560,6 +560,7 @@ static void occ_worker(struct work_struct *work)
> > > > > >   	int rc = 0, empty, waiting, canceled;
> > > > > >   	u16 resp_data_length;
> > > > > >   	struct occ_xfr *xfr;
> > > > > > +	struct occ_response *resp;
> > > > > >   	struct occ_client *client;
> > > > > >   	struct occ *occ = container_of(work, struct occ, work);
> > > > > >   	struct device *sbefifo = occ->sbefifo;
> > > @@ -573,6 +574,7 @@ static void occ_worker(struct work_struct *work)
> > > > > >   		return;
> > > > > >   	}
> > >   
> > > > > > +	resp = (struct occ_response *)xfr->buf;
> > > > > >   	set_bit(XFR_IN_PROGRESS, &xfr->flags);
> > >   
> > > > > >   	spin_unlock_irq(&occ->list_lock);
> > > 

> > > @@ -591,7 +593,7 @@ static void occ_worker(struct work_struct *work)
> > > > > >   	if (rc)
> > > > > >   		goto done;
> > >   
> > > > > > -	resp_data_length = (xfr->buf[3] << 8) + xfr->buf[4];
> > > > > > +	resp_data_length = get_unaligned_be16(&resp->data_length);
> > >   	if (resp_data_length > OCC_RESP_DATA_BYTES) {
> > 
> > You need to do be16_to_cpu() before any comparisons.
> 
> Hmm, I'm not convinced. I see this in access_ok.h
> 
> static  __always_inline 
> > <http://elixir.free-electrons.com/linux/latest/ident/__always_inline>  u16  get_unaligned_be16 
> <http://elixir.free-electrons.com/linux/latest/ident/get_unaligned_be16>(const  void  *p)
> {
> 	return  be16_to_cpup 
> <http://elixir.free-electrons.com/linux/latest/ident/be16_to_cpup>((__be16 
> > <http://elixir.free-electrons.com/linux/latest/ident/__be16>  *)p);
> } I think it will do the translation as well. Thanks, Eddie

Ugh, yeah, brain fart. Sorry about that.

Reviewed-by: Andrew Jeffery <andrew at aj.id.au>


> 
> > 
> > > > > >   		rc = -EDOM;
> > >   		goto done;
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 801 bytes
Desc: This is a digitally signed message part
URL: <http://lists.ozlabs.org/pipermail/openbmc/attachments/20171006/9390394e/attachment.sig>