From mboxrd@z Thu Jan 1 00:00:00 1970 From: Ken Ma Date: Wed, 5 Apr 2017 08:38:40 +0000 Subject: [U-Boot] [EXT] Re: [PATCH 1/7] scsi: move base, max_lun and max_id to uclass plat data In-Reply-To: References: <1490261347-11896-1-git-send-email-make@marvell.com> <1490261347-11896-2-git-send-email-make@marvell.com> Message-ID: List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 8bit To: u-boot@lists.denx.de Hi Simon Please see my inline reply, thanks a lot! -----Original Message----- From: sjg@google.com [mailto:sjg at google.com] On Behalf Of Simon Glass Sent: 2017年4月1日 12:22 To: Ken Ma Cc: U-Boot Mailing List; Stefan Roese; Michal Simek Subject: [EXT] Re: [PATCH 1/7] scsi: move base, max_lun and max_id to uclass plat data External Email ---------------------------------------------------------------------- Hi Ken, On 23 March 2017 at 03:29, wrote: > From: Ken Ma > > - The members in scsi_platdata(base, max_lun and max_id) are generic, > so now they are taken from fdt by the uclass_platdata instead of > platdata code upon call to post bind callback. > > Signed-off-by: Ken Ma > Cc: Simon Glass > Cc: Stefan Roese > Cc: Michal Simek > Reviewed-on: http://vgitil04.il.marvell.com:8080/35304 > Tested-by: iSoC Platform CI > Reviewed-by: Omri Itach > Reviewed-by: Kostya Porotchkin > --- > common/scsi.c | 2 +- > drivers/block/ahci.c | 2 +- > drivers/block/scsi-uclass.c | 29 +++++++++++++++++++++++++++++ > 3 files changed, 31 insertions(+), 2 deletions(-) > > diff --git a/common/scsi.c b/common/scsi.c index fb5b407..117c682 > 100644 > --- a/common/scsi.c > +++ b/common/scsi.c > @@ -574,7 +574,7 @@ int scsi_scan(int mode) > return ret; > > /* Get controller platdata */ > - plat = dev_get_platdata(dev); > + plat = dev_get_uclass_platdata(dev); > > for (i = 0; i < plat->max_id; i++) { > for (lun = 0; lun < plat->max_lun; lun++) { > diff --git a/drivers/block/ahci.c b/drivers/block/ahci.c index > 3fa14a7..368816e 100644 > --- a/drivers/block/ahci.c > +++ b/drivers/block/ahci.c > @@ -479,7 +479,7 @@ static int ahci_init_one(pci_dev_t dev) > pci_write_config_byte(dev, 0x41, 0xa1); #endif #else > - struct scsi_platdata *plat = dev_get_platdata(dev); > + struct scsi_platdata *plat = dev_get_uclass_platdata(dev); > probe_ent->mmio_base = (void *)plat->base; #endif > > diff --git a/drivers/block/scsi-uclass.c b/drivers/block/scsi-uclass.c > index 05da6cd..3bf026b 100644 > --- a/drivers/block/scsi-uclass.c > +++ b/drivers/block/scsi-uclass.c > @@ -11,8 +11,11 @@ > > #include > #include > +#include > #include > > +DECLARE_GLOBAL_DATA_PTR; > + > static int scsi_post_probe(struct udevice *dev) { > debug("%s: device %p\n", __func__, dev); @@ -20,8 +23,34 @@ > static int scsi_post_probe(struct udevice *dev) > return 0; > } > > +static void scsi_ofdata_to_uclass_platdata(struct udevice *dev) Please make this return an int since functions that decode the DT should return an error code. > +{ > + struct scsi_platdata *plat = dev_get_uclass_platdata(dev); > + const void *blob = gd->fdt_blob; > + int node = dev->of_offset; > + > + plat->base = (unsigned long)dev_get_addr_ptr(dev); > + plat->max_id = fdtdec_get_uint(blob, > + node, > + "max-id", > + CONFIG_SYS_SCSI_MAX_SCSI_ID); > + plat->max_lun = fdtdec_get_uint(blob, > + node, > + "max-lun", > + CONFIG_SYS_SCSI_MAX_LUN); > + return; return 0 [Ken] got it. > +} > + > +static int scsi_post_bind(struct udevice *dev) { > + /* Get uclass plat data from fdt */ > + scsi_ofdata_to_uclass_platdata(dev); Do we need to have this info in bind(), or could it wait until of_to_platdata()? [Ken] Stefan shows me a patch https://patchwork.ozlabs.org/patch/743160/ , it fills the two field members(max_lun and max_id) in ahci's scsi_low_level_init() As below, I think it's OK to get active link port number in ahci_host_init(), and in my opinion, it's better to be a default way to get max_lun and max_id in ahci driver if the two members are not set in fdt since actually max_lun * max_id = ffs(linkmap) and we can also set max_lun = 2 and max_id = fls(linkmap)/2; And another question is whther ffs() or fls() for max_id? > + if (plat) { > + plat->max_lun = 1; > + plat->max_id = ffs(linkmap); > + } Also, return the error code here. [Ken] got it. > +} > + > UCLASS_DRIVER(scsi) = { > .id = UCLASS_SCSI, > .name = "scsi", > + .post_bind = scsi_post_bind, > .post_probe = scsi_post_probe, > + .per_device_platdata_auto_alloc_size = sizeof(struct > + scsi_platdata), > }; > -- > 1.9.1 > Regards, Simon