diff mbox series

fsi: core: Fix legacy minor numbering

Message ID 20230413162440.3313036-1-eajames@linux.ibm.com
State New
Headers
Series fsi: core: Fix legacy minor numbering |

Commit Message

Eddie James April 13, 2023, 4:24 p.m. UTC 
  FSI reserves the first 64 minor numbers for the legacy numbering
based on the chip id. However the legacy number shifts the chip
id too much, resulting in overlap between legacy and non-legacy
numbers. Reduce the chip id bit shift since the type field only
takes 2 bits.

Signed-off-by: Eddie James <eajames@linux.ibm.com>
---
 drivers/fsi/fsi-core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Joel Stanley May 31, 2023, 1:54 a.m. UTC | #1 
On Thu, 13 Apr 2023 at 16:24, Eddie James <eajames@linux.ibm.com> wrote:
>
> FSI reserves the first 64 minor numbers for the legacy numbering
> based on the chip id. However the legacy number shifts the chip
> id too much, resulting in overlap between legacy and non-legacy
> numbers. Reduce the chip id bit shift since the type field only
> takes 2 bits.

I don't know much about the legacy numbering. Was that something we
used before the device tree descriptions were in place? Do we still
need it if we have updated device trees?

>
> Signed-off-by: Eddie James <eajames@linux.ibm.com>
> ---
>  drivers/fsi/fsi-core.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
> index 0b927c9f4267..b9f410170655 100644
> --- a/drivers/fsi/fsi-core.c
> +++ b/drivers/fsi/fsi-core.c
> @@ -950,7 +950,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type,
>         /* Check if we qualify for legacy numbering */
>         if (cid >= 0 && cid < 16 && type < 4) {
>                 /* Try reserving the legacy number */

Would it help to put some of your commit message info in this comment,
so we know what the magic shift is doing?

> -               id = (cid << 4) | type;
> +               id = (cid << 2) | type;
>                 id = ida_simple_get(&fsi_minor_ida, id, id + 1, GFP_KERNEL);
>                 if (id >= 0) {
>                         *out_index = fsi_adjust_index(cid);
> --
> 2.31.1
>
Eddie James June 8, 2023, 4:44 p.m. UTC | #2 
On 5/30/23 20:54, Joel Stanley wrote:
> On Thu, 13 Apr 2023 at 16:24, Eddie James <eajames@linux.ibm.com> wrote:
>> FSI reserves the first 64 minor numbers for the legacy numbering
>> based on the chip id. However the legacy number shifts the chip
>> id too much, resulting in overlap between legacy and non-legacy
>> numbers. Reduce the chip id bit shift since the type field only
>> takes 2 bits.
> I don't know much about the legacy numbering. Was that something we
> used before the device tree descriptions were in place? Do we still
> need it if we have updated device trees?


Its a way to number the devices based on the "chip id", which came from 
the device tree anyway. Now with the aliasing patch we could do away 
with it I believe, but we would need to update older system device trees 
(and scom devices on p10) to not break stuff.

>
>> Signed-off-by: Eddie James <eajames@linux.ibm.com>
>> ---
>>   drivers/fsi/fsi-core.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
>> index 0b927c9f4267..b9f410170655 100644
>> --- a/drivers/fsi/fsi-core.c
>> +++ b/drivers/fsi/fsi-core.c
>> @@ -950,7 +950,7 @@ static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type,
>>          /* Check if we qualify for legacy numbering */
>>          if (cid >= 0 && cid < 16 && type < 4) {
>>                  /* Try reserving the legacy number */
> Would it help to put some of your commit message info in this comment,
> so we know what the magic shift is doing?


Sure thing.

Thanks,

Eddie


>
>> -               id = (cid << 4) | type;
>> +               id = (cid << 2) | type;
>>                  id = ida_simple_get(&fsi_minor_ida, id, id + 1, GFP_KERNEL);
>>                  if (id >= 0) {
>>                          *out_index = fsi_adjust_index(cid);
>> --
>> 2.31.1
>>
diff mbox series

Patch

diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
index 0b927c9f4267..b9f410170655 100644
--- a/drivers/fsi/fsi-core.c
+++ b/drivers/fsi/fsi-core.c
@@ -950,7 +950,7 @@  static int __fsi_get_new_minor(struct fsi_slave *slave, enum fsi_dev_type type,
 	/* Check if we qualify for legacy numbering */
 	if (cid >= 0 && cid < 16 && type < 4) {
 		/* Try reserving the legacy number */
-		id = (cid << 4) | type;
+		id = (cid << 2) | type;
 		id = ida_simple_get(&fsi_minor_ida, id, id + 1, GFP_KERNEL);
 		if (id >= 0) {
 			*out_index = fsi_adjust_index(cid);