Johannes,
Thanks a lot for your comments.
Cheers,
Avri
> -----Original Message-----
> From: linux-scsi-owner@xxxxxxxxxxxxxxx <linux-scsi-owner@xxxxxxxxxxxxxxx>
> On Behalf Of Johannes Thumshirn
> Sent: Wednesday, August 01, 2018 2:17 PM
> To: Avri Altman <Avri.Altman@xxxxxxx>
> Cc: Christoph Hellwig <hch@xxxxxx>; Hannes Reinecke <hare@xxxxxxxx>;
> Bart Van Assche <Bart.VanAssche@xxxxxxx>; James E.J. Bottomley
> <jejb@xxxxxxxxxxxxxxxxxx>; Martin K. Petersen
> <martin.petersen@xxxxxxxxxx>; linux-scsi@xxxxxxxxxxxxxxx; Stanislav
> Nijnikov <Stanislav.Nijnikov@xxxxxxx>; Avi Shchislowski
> <Avi.Shchislowski@xxxxxxx>; Alex Lemberg <Alex.Lemberg@xxxxxxx>;
> Subhash Jadavani <subhashj@xxxxxxxxxxxxxx>; Vinayak Holikatti
> <Vinayak.Holikatti@xxxxxxx>
> 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
> jthumshirn@xxxxxxx +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
