| Message ID | 1490261347-11896-2-git-send-email-make@marvell.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Simon Glass |
| Headers | show |
Hi Ken, On 23 March 2017 at 03:29, <make@marvell.com> wrote: > From: Ken Ma <make@marvell.com> > > - 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 <make@marvell.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Stefan Roese <sr@denx.de> > Cc: Michal Simek <michal.simek@xilinx.com> > Reviewed-on: http://vgitil04.il.marvell.com:8080/35304 > Tested-by: iSoC Platform CI <ykjenk@marvell.com> > Reviewed-by: Omri Itach <omrii@marvell.com> > Reviewed-by: Kostya Porotchkin <kostap@marvell.com> > --- > 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 <common.h> > #include <dm.h> > +#include <dm/device-internal.h> > #include <scsi.h> > > +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 > +} > + > +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()? Also, return the error code here. > +} > + > 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
Hi Simon Please see my inline reply, thanks a lot! -----Original Message----- From: sjg@google.com [mailto:sjg@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, <make@marvell.com> wrote: > From: Ken Ma <make@marvell.com> > > - 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 <make@marvell.com> > Cc: Simon Glass <sjg@chromium.org> > Cc: Stefan Roese <sr@denx.de> > Cc: Michal Simek <michal.simek@xilinx.com> > Reviewed-on: http://vgitil04.il.marvell.com:8080/35304 > Tested-by: iSoC Platform CI <ykjenk@marvell.com> > Reviewed-by: Omri Itach <omrii@marvell.com> > Reviewed-by: Kostya Porotchkin <kostap@marvell.com> > --- > 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 <common.h> > #include <dm.h> > +#include <dm/device-internal.h> > #include <scsi.h> > > +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
Hi Ken, On 5 April 2017 at 02:38, Ken Ma <make@marvell.com> wrote: > Hi Simon > > Please see my inline reply, thanks a lot! > > -----Original Message----- > From: sjg@google.com [mailto:sjg@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, <make@marvell.com> wrote: >> From: Ken Ma <make@marvell.com> >> >> - 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 <make@marvell.com> >> Cc: Simon Glass <sjg@chromium.org> >> Cc: Stefan Roese <sr@denx.de> >> Cc: Michal Simek <michal.simek@xilinx.com> >> Reviewed-on: http://vgitil04.il.marvell.com:8080/35304 >> Tested-by: iSoC Platform CI <ykjenk@marvell.com> >> Reviewed-by: Omri Itach <omrii@marvell.com> >> Reviewed-by: Kostya Porotchkin <kostap@marvell.com> >> --- >> 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 <common.h> >> #include <dm.h> >> +#include <dm/device-internal.h> >> #include <scsi.h> >> >> +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? OK well since it doesn't take any time, I am OK with it. I'm not sure about your last question. > >> + 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
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 <common.h> #include <dm.h> +#include <dm/device-internal.h> #include <scsi.h> +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) +{ + 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; +} + +static int scsi_post_bind(struct udevice *dev) +{ + /* Get uclass plat data from fdt */ + scsi_ofdata_to_uclass_platdata(dev); +} + 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), };