Johannes,
Thanks a lot for your comments.
Cheers,
Avri
> -----Original Message-----
> From: linux-scsi-ow...@vger.kernel.org <linux-scsi-ow...@vger.kernel.org>
> On Behalf Of Johannes Thumshirn
> Sent: Wednesday, August 01, 2018 2:17 PM
> To: Avri Altman <avri.alt...@wdc.com>
> Cc: Christoph Hellwig <h...@lst.de>; Hannes Reinecke <h...@suse.com>;
> Bart Van Assche <bart.vanass...@wdc.com>; James E.J. Bottomley
> <j...@linux.vnet.ibm.com>; Martin K. Petersen
> <martin.peter...@oracle.com>; linux-scsi@vger.kernel.org; Stanislav
> Nijnikov <stanislav.nijni...@wdc.com>; Avi Shchislowski
> <avi.shchislow...@wdc.com>; Alex Lemberg <alex.lemb...@wdc.com>;
> Subhash Jadavani <subha...@codeaurora.org>; Vinayak Holikatti
> <vinayak.holika...@wdc.com>
> 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
> jthumsh...@suse.de +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
