Johannes, Thanks a lot for your comments. Cheers, Avri
> -----Original Message----- > From: [email protected] <[email protected]> > On Behalf Of Johannes Thumshirn > Sent: Wednesday, August 01, 2018 2:17 PM > To: Avri Altman <[email protected]> > Cc: Christoph Hellwig <[email protected]>; Hannes Reinecke <[email protected]>; > Bart Van Assche <[email protected]>; James E.J. Bottomley > <[email protected]>; Martin K. Petersen > <[email protected]>; [email protected]; Stanislav > Nijnikov <[email protected]>; Avi Shchislowski > <[email protected]>; Alex Lemberg <[email protected]>; > Subhash Jadavani <[email protected]>; Vinayak Holikatti > <[email protected]> > Subject: Re: [PATCH 1/6] scsi: Add ufs transport class > > Hi Avri, > > On Wed, Aug 01, 2018 at 11:04:27AM +0300, Avri Altman wrote: > [... > > > +#include <linux/bsg.h> > > Why do you include bsg.h here and bsg-lib.h in the scsi_transport_ufs.h? > > [...] Right, will move both to the same place. > > > +#define to_ufs_internal(tmpl) container_of(tmpl, struct > ufs_internal, t) > > I'd personally prefer this to be a inline function instead of a define > for type safety reasons. Ok. > > > + > > +struct ufs_host_attrs { > > + atomic_t next_port_id; > > +}; > > +#define to_ufs_host_attrs(x) ((struct ufs_host_attrs *)(x)->shost_data) > > Ditto. Ok. > > [...] > > > + > > + port->id = atomic_inc_return(&ufs_host->next_port_id); > > Any reason you can't use an IDA for the port->id? Ok. Will change to use it. > > [...] > > + > > + error = device_add(dev); > > + > > + if (error) > > + return error; > > No blank line please. Done. > > [...] > > > +#define dev_to_ufs_port(d) \ > > + container_of((d), struct ufs_port, dev) > > Inline function as well, please. Done. > > -- > Johannes Thumshirn Storage > [email protected] +49 911 74053 689 > SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg > GF: Felix Imendörffer, Jane Smithard, Graham Norton > HRB 21284 (AG Nürnberg) > Key fingerprint = EC38 9CAB C2C4 F25D 8600 D0D0 0393 969D 2D76 0850
