* [U-Boot] [PATCH] blk: Check if_type in blk_get_devnum_by_typename
@ 2019-11-24 18:09 Juha Sarlin
2019-11-24 18:37 ` Heinrich Schuchardt
0 siblings, 1 reply; 4+ messages in thread
From: Juha Sarlin @ 2019-11-24 18:09 UTC (permalink / raw)
To: u-boot
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 */
--
2.20.1.98.gecbdaf0899
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] blk: Check if_type in blk_get_devnum_by_typename
2019-11-24 18:09 [U-Boot] [PATCH] blk: Check if_type in blk_get_devnum_by_typename Juha Sarlin
@ 2019-11-24 18:37 ` Heinrich Schuchardt
2019-11-24 23:56 ` Juha Sarlin
0 siblings, 1 reply; 4+ messages in thread
From: Heinrich Schuchardt @ 2019-11-24 18:37 UTC (permalink / raw)
To: u-boot
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 */
>
^ permalink raw reply [flat|nested] 4+ messages in thread
* [U-Boot] [PATCH] blk: Check if_type in blk_get_devnum_by_typename
2019-11-24 18:37 ` Heinrich Schuchardt
@ 2019-11-24 23:56 ` Juha Sarlin
2019-12-27 16:41 ` Simon Glass
0 siblings, 1 reply; 4+ messages in thread
From: Juha Sarlin @ 2019-11-24 23:56 UTC (permalink / raw)
To: u-boot
> 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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH] blk: Check if_type in blk_get_devnum_by_typename
2019-11-24 23:56 ` Juha Sarlin
@ 2019-12-27 16:41 ` Simon Glass
0 siblings, 0 replies; 4+ messages in thread
From: Simon Glass @ 2019-12-27 16:41 UTC (permalink / raw)
To: u-boot
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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2019-12-27 16:41 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-11-24 18:09 [U-Boot] [PATCH] blk: Check if_type in blk_get_devnum_by_typename Juha Sarlin
2019-11-24 18:37 ` Heinrich Schuchardt
2019-11-24 23:56 ` Juha Sarlin
2019-12-27 16:41 ` Simon Glass
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.