Hi Ken,
On 5 April 2017 at 02:38, Ken Ma <m...@marvell.com> wrote:
> Hi Simon
>
> Please see my inline reply, thanks a lot!
>
> -----Original Message-----
> From: s...@google.com [mailto:s...@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, <m...@marvell.com> wrote:
>> From: Ken Ma <m...@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 <m...@marvell.com>
>> Cc: Simon Glass <s...@chromium.org>
>> Cc: Stefan Roese <s...@denx.de>
>> Cc: Michal Simek <michal.si...@xilinx.com>
>> Reviewed-on: http://vgitil04.il.marvell.com:8080/35304
>> Tested-by: iSoC Platform CI <ykj...@marvell.com>
>> Reviewed-by: Omri Itach <om...@marvell.com>
>> Reviewed-by: Kostya Porotchkin <kos...@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
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
https://lists.denx.de/listinfo/u-boot
