[PATCH linux dev-4.7] drivers: fsi: Fix incremeting dev nums

Joel Stanley joel at jms.id.au
Fri Feb 17 11:52:11 AEDT 2017


On Fri, Feb 17, 2017 at 11:14 AM, Jeremy Kerr <jk at ozlabs.org> wrote:
> Hi Eddie,
>
>> From: "Edward A. James" <eajames at us.ibm.com>
>>
>> both for FSI master and scom client
>>
>> Signed-off-by: Edward A. James <eajames at us.ibm.com>
>> ---
>>  drivers/fsi/fsi-core.c | 4 +++-
>>  drivers/fsi/fsi-scom.c | 2 ++
>>  2 files changed, 5 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/fsi/fsi-core.c b/drivers/fsi/fsi-core.c
>> index e8a3618..7816482 100644
>> --- a/drivers/fsi/fsi-core.c
>> +++ b/drivers/fsi/fsi-core.c
>> @@ -46,7 +46,7 @@ static const int engine_page_size = 0x400;
>>  static struct task_struct *master_ipoll;
>>  static unsigned int fsi_ipoll_period_ms = 100;
>>
>> -static atomic_t master_idx = ATOMIC_INIT(-1);
>> +static atomic_t master_idx = ATOMIC_INIT(0);
>
> This will mean that our first master is numbered 1 instead of zero. Not
> a huge problem, but it doesn't sound like that's what you wanted to do
> here.
>
>>  struct fsi_slave {
>>       struct list_head        list_link;      /* Master's list of slaves */
>> @@ -617,6 +617,8 @@ void fsi_master_unregister(struct fsi_master *master)
>>               kthread_stop(master_ipoll);
>>               master_ipoll = NULL;
>>       }
>> +
>> +     atomic_dec(&master_idx);
>
> This doesn't work. Consider (starting from -1):
>
>   - master_idx at -1
>   - master added, assigned index 0, master_idx now at 0
>   - master added, assigned index 1, master idx now at 1
>   - master 0 removed, master_idx now at 0
>   - master added, assigned index 1
>
> now we have two masters with index 1.
>
> Either we need to check for present masters, or use the ida functions
> (as greg kh has suggested).
>
> So, nak from me on this.

Thanks Jeremy. I missed this case when reviewing.

I have committed a revert of this patch.

Cheers,

Joel


More information about the openbmc mailing list