[U-Boot,U-Boot,v2,3/7] drivers: block: disk-uclass: implement scsi_init()

On Wednesday 24 February 2016 09:50 PM, Tom Rini wrote:
> On Wed, Feb 03, 2016 at 05:29:36PM +0530, Mugunthan V N wrote:
> 
>> Implement scsi_init() api to probe driver model based sata
>> devices.
>>
>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>> Reviewed-by: Simon Glass <sjg@chromium.org>
>> ---
>>  drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++
>>  1 file changed, 39 insertions(+)
>>
>> diff --git a/drivers/block/disk-uclass.c b/drivers/block/disk-uclass.c
>> index d665b35..4bd7b56 100644
>> --- a/drivers/block/disk-uclass.c
>> +++ b/drivers/block/disk-uclass.c
>> @@ -7,6 +7,45 @@
>>  
>>  #include <common.h>
>>  #include <dm.h>
>> +#include <dm/uclass-internal.h>
>> +#include <dm/device-internal.h>
>> +#include <scsi.h>
>> +
>> +int scsi_get_device(int index, struct udevice **devp)
>> +{
>> +	struct udevice *dev;
>> +	int ret;
>> +
>> +	ret = uclass_find_device(UCLASS_DISK, index, &dev);
>> +	if (ret || !dev) {
>> +		printf("%d device not found\n", index);
>> +		return ret;
>> +	}
>> +
>> +	ret = device_probe(dev);
>> +	if (ret) {
>> +		error("device probe error\n");
>> +		return ret;
>> +	}
>> +
>> +	*devp = dev;
>> +
>> +	return ret;
>> +}
>> +
>> +void scsi_init(void)
>> +{
>> +	struct udevice *dev;
>> +	int ret;
>> +
>> +	ret = scsi_get_device(0, &dev);
>> +	if (ret || !dev) {
>> +		error("scsi device not found\n");
>> +		return;
>> +	}
>> +
>> +	scsi_scan(1);
>> +}
>>  
>>  UCLASS_DRIVER(disk) = {
>>  	.id		= UCLASS_DISK,
> 
> OK, this patch is a problem.  Many platforms already define scsi_init()
> and aren't moved over so now fail to build.  Mele_M5 is one of many
> examples here, thanks!
> 

Oops, sorry I didn't run buildman before submitting the patches, will
make sure running buildman before submitting patches in future.

Issue is when a platform is converted to DM, by default CONFIG_DISK is
selected through Kconfig whether the platform has block device or not,
disk_uclass driver is compile which results in build break when the
platform has scsi_init already defined and not not converted to DM.

The following diff solves the issue, and CONFIG_DISK has to be selected
for platforms which supports disk (sata, ide etc)

Simon, Are you Okay with the patch, so that I can send it as a separate
fixup patch.


Regards
Mugunthan V N

Hi Mugunthan,

On 25 February 2016 at 02:34, Mugunthan V N <mugunthanvnm@ti.com> wrote:
> On Wednesday 24 February 2016 09:50 PM, Tom Rini wrote:
>> On Wed, Feb 03, 2016 at 05:29:36PM +0530, Mugunthan V N wrote:
>>
>>> Implement scsi_init() api to probe driver model based sata
>>> devices.
>>>
>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>> ---
>>>  drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 39 insertions(+)
>>>
>>> diff --git a/drivers/block/disk-uclass.c b/drivers/block/disk-uclass.c
>>> index d665b35..4bd7b56 100644
>>> --- a/drivers/block/disk-uclass.c
>>> +++ b/drivers/block/disk-uclass.c
>>> @@ -7,6 +7,45 @@
>>>
>>>  #include <common.h>
>>>  #include <dm.h>
>>> +#include <dm/uclass-internal.h>
>>> +#include <dm/device-internal.h>
>>> +#include <scsi.h>
>>> +
>>> +int scsi_get_device(int index, struct udevice **devp)
>>> +{
>>> +    struct udevice *dev;
>>> +    int ret;
>>> +
>>> +    ret = uclass_find_device(UCLASS_DISK, index, &dev);
>>> +    if (ret || !dev) {
>>> +            printf("%d device not found\n", index);
>>> +            return ret;
>>> +    }
>>> +
>>> +    ret = device_probe(dev);
>>> +    if (ret) {
>>> +            error("device probe error\n");
>>> +            return ret;
>>> +    }
>>> +
>>> +    *devp = dev;
>>> +
>>> +    return ret;
>>> +}
>>> +
>>> +void scsi_init(void)
>>> +{
>>> +    struct udevice *dev;
>>> +    int ret;
>>> +
>>> +    ret = scsi_get_device(0, &dev);
>>> +    if (ret || !dev) {
>>> +            error("scsi device not found\n");
>>> +            return;
>>> +    }
>>> +
>>> +    scsi_scan(1);
>>> +}
>>>
>>>  UCLASS_DRIVER(disk) = {
>>>      .id             = UCLASS_DISK,
>>
>> OK, this patch is a problem.  Many platforms already define scsi_init()
>> and aren't moved over so now fail to build.  Mele_M5 is one of many
>> examples here, thanks!
>>
>
> Oops, sorry I didn't run buildman before submitting the patches, will
> make sure running buildman before submitting patches in future.
>
> Issue is when a platform is converted to DM, by default CONFIG_DISK is
> selected through Kconfig whether the platform has block device or not,
> disk_uclass driver is compile which results in build break when the
> platform has scsi_init already defined and not not converted to DM.
>
> The following diff solves the issue, and CONFIG_DISK has to be selected
> for platforms which supports disk (sata, ide etc)
>
> Simon, Are you Okay with the patch, so that I can send it as a separate
> fixup patch.

It is OK, but please enable CONFIG_DISK in chromebook_link_defconfig.
It is the only user at present.

With driver model we actually don't want scsi_init() to be implemented
in the end. It's fine for now. The driver-model block-device series
will make some changes here down the track. We'll have UCLASS_SATA (or
similar) rather than something as generic as UCLASS_DISK.

>
> diff --git a/drivers/block/Kconfig b/drivers/block/Kconfig
> index 915c1eb..e62bf75 100644
> --- a/drivers/block/Kconfig
> +++ b/drivers/block/Kconfig
> @@ -1,7 +1,7 @@
>  config DISK
>         bool "Support disk controllers with driver model"
>         depends on DM
> -       default y if DM
> +       default n if DM
>         help
>           This enables a uclass for disk controllers in U-Boot. Various driver
>           types can use this, such as AHCI/SATA. It does not provide any standard
>
> Regards
> Mugunthan V N
> _______________________________________________
> U-Boot mailing list
> U-Boot@lists.denx.de
> http://lists.denx.de/mailman/listinfo/u-boot

Regards,
Simon

Hi Tom

On Thursday 25 February 2016 09:00 PM, Simon Glass wrote:
> Hi Mugunthan,
> 
> On 25 February 2016 at 02:34, Mugunthan V N <mugunthanvnm@ti.com> wrote:
>> On Wednesday 24 February 2016 09:50 PM, Tom Rini wrote:
>>> On Wed, Feb 03, 2016 at 05:29:36PM +0530, Mugunthan V N wrote:
>>>
>>>> Implement scsi_init() api to probe driver model based sata
>>>> devices.
>>>>
>>>> Signed-off-by: Mugunthan V N <mugunthanvnm@ti.com>
>>>> Reviewed-by: Simon Glass <sjg@chromium.org>
>>>> ---
>>>>  drivers/block/disk-uclass.c | 39 +++++++++++++++++++++++++++++++++++++++
>>>>  1 file changed, 39 insertions(+)
>>>>
>>>> diff --git a/drivers/block/disk-uclass.c b/drivers/block/disk-uclass.c
>>>> index d665b35..4bd7b56 100644
>>>> --- a/drivers/block/disk-uclass.c
>>>> +++ b/drivers/block/disk-uclass.c
>>>> @@ -7,6 +7,45 @@
>>>>
>>>>  #include <common.h>
>>>>  #include <dm.h>
>>>> +#include <dm/uclass-internal.h>
>>>> +#include <dm/device-internal.h>
>>>> +#include <scsi.h>
>>>> +
>>>> +int scsi_get_device(int index, struct udevice **devp)
>>>> +{
>>>> +    struct udevice *dev;
>>>> +    int ret;
>>>> +
>>>> +    ret = uclass_find_device(UCLASS_DISK, index, &dev);
>>>> +    if (ret || !dev) {
>>>> +            printf("%d device not found\n", index);
>>>> +            return ret;
>>>> +    }
>>>> +
>>>> +    ret = device_probe(dev);
>>>> +    if (ret) {
>>>> +            error("device probe error\n");
>>>> +            return ret;
>>>> +    }
>>>> +
>>>> +    *devp = dev;
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>> +void scsi_init(void)
>>>> +{
>>>> +    struct udevice *dev;
>>>> +    int ret;
>>>> +
>>>> +    ret = scsi_get_device(0, &dev);
>>>> +    if (ret || !dev) {
>>>> +            error("scsi device not found\n");
>>>> +            return;
>>>> +    }
>>>> +
>>>> +    scsi_scan(1);
>>>> +}
>>>>
>>>>  UCLASS_DRIVER(disk) = {
>>>>      .id             = UCLASS_DISK,
>>>
>>> OK, this patch is a problem.  Many platforms already define scsi_init()
>>> and aren't moved over so now fail to build.  Mele_M5 is one of many
>>> examples here, thanks!
>>>
>>
>> Oops, sorry I didn't run buildman before submitting the patches, will
>> make sure running buildman before submitting patches in future.
>>
>> Issue is when a platform is converted to DM, by default CONFIG_DISK is
>> selected through Kconfig whether the platform has block device or not,
>> disk_uclass driver is compile which results in build break when the
>> platform has scsi_init already defined and not not converted to DM.
>>
>> The following diff solves the issue, and CONFIG_DISK has to be selected
>> for platforms which supports disk (sata, ide etc)
>>
>> Simon, Are you Okay with the patch, so that I can send it as a separate
>> fixup patch.
> 
> It is OK, but please enable CONFIG_DISK in chromebook_link_defconfig.
> It is the only user at present.
> 
> With driver model we actually don't want scsi_init() to be implemented
> in the end. It's fine for now. The driver-model block-device series
> will make some changes here down the track. We'll have UCLASS_SATA (or
> similar) rather than something as generic as UCLASS_DISK.
> 

Will send a fixup patch by tomorrow.

Regards
Mugunthan V N