[U-Boot] [PATCH] blk: Check if_type in blk_get_devnum_by_typename
While searching for a BLK device, this function checks only for a matching devnum. It should check if_type, too.
Signed-off-by: Juha Sarlin jsub@sarlin.mobi ---
drivers/block/blk-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index ca8978f0e1..78f2bcab09 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -112,7 +112,7 @@ struct blk_desc *blk_get_devnum_by_typename(const char *if_typename, int devnum)
debug("%s: if_type=%d, devnum=%d: %s, %d, %d\n", __func__, if_type, devnum, dev->name, desc->if_type, desc->devnum); - if (desc->devnum != devnum) + if (desc->if_type != if_type || desc->devnum != devnum) continue;
/* Find out the parent device uclass */
On 11/24/19 7:09 PM, Juha Sarlin wrote:
While searching for a BLK device, this function checks only for a matching devnum. It should check if_type, too.
Could you, please, describe in which cases you have observed a problem and how it can be reproduced.
According to the function description the relevant interface type check is device_get_uclass_id(dev->parent) != uclass_id
Signed-off-by: Juha Sarlin jsub@sarlin.mobi
drivers/block/blk-uclass.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/block/blk-uclass.c b/drivers/block/blk-uclass.c index ca8978f0e1..78f2bcab09 100644 --- a/drivers/block/blk-uclass.c +++ b/drivers/block/blk-uclass.c @@ -112,7 +112,7 @@ struct blk_desc *blk_get_devnum_by_typename(const char *if_typename, int devnum)
Should the logic need changing, please, update the function description. Please, use Sphinx style for the function description.
Best regards
Heinrich
debug("%s: if_type=%d, devnum=%d: %s, %d, %d\n", __func__, if_type, devnum, dev->name, desc->if_type, desc->devnum);
if (desc->devnum != devnum)
if (desc->if_type != if_type || desc->devnum != devnum) continue;/* Find out the parent device uclass */
On 24 Nov 2019, at 19:37, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/24/19 7:09 PM, Juha Sarlin wrote:
While searching for a BLK device, this function checks only for a matching devnum. It should check if_type, too.
Could you, please, describe in which cases you have observed a problem and how it can be reproduced.
According to the function description the relevant interface type check is device_get_uclass_id(dev->parent) != uclass_id
I was wrong, it isn't really a bug. I was misled by all the other blk-finding functions that check if_type instead of parent class. I think that checking if_type would work here, too.
Or perhaps this case is the first step towards removing the if_type field in the future? There seems to be a 1-1 mapping from almost every if_type to uclass_id.
Hi Juha,
On Sun, 24 Nov 2019 at 16:57, Juha Sarlin jsub@sarlin.mobi wrote:
On 24 Nov 2019, at 19:37, Heinrich Schuchardt xypron.glpk@gmx.de wrote:
On 11/24/19 7:09 PM, Juha Sarlin wrote:
While searching for a BLK device, this function checks only for a matching devnum. It should check if_type, too.
Could you, please, describe in which cases you have observed a problem and how it can be reproduced.
According to the function description the relevant interface type check is device_get_uclass_id(dev->parent) != uclass_id
I was wrong, it isn't really a bug. I was misled by all the other blk-finding functions that check if_type instead of parent class. I think that checking if_type would work here, too.
Or perhaps this case is the first step towards removing the if_type field in the future? There seems to be a 1-1 mapping from almost every if_type to uclass_id.
Thanks for the patch. In this case it is the intended behaviour I think, because if_type_to_uclass_id() gives us the uclass to use, and we check that immediately below your patch. Since there is a 1:1 correspondence between if_type and uclass_id (apart from those we want to remove) we don't need to check both.
Yes it would be nice to remove if_type one day.
Regards, Simon
participants (3)
-
Heinrich Schuchardt -
Juha Sarlin -
Simon Glass