Re: [PATCH] soc: add polarfire soc system controller

[<Conor.Dooley@microchip.com>]

On 21/10/2021 19:34, Arnd Bergmann wrote:
>>>> +int mpfs_blocking_transaction(struct mpfs_sys_controller *mpfs_client, void *msg)
>>>> +{
>>>> +    int ret;
>>>> +
>>>> +    mutex_lock_interruptible(&transaction_lock);
> 
> When you do a mutex_lock_interruptible(), you have to check its return code and
> handle the interruption, usually by passing down -EINTR to the caller.
> 
>>>> +struct mpfs_sys_controller *
>>>> +mpfs_sys_controller_get(struct device_node *mss_node)
>>>> +{
>>>> +    struct platform_device *pdev = of_find_device_by_node(mss_node);
>>>> +
>>>> +    if (!pdev)
>>>> +            return NULL;
>>>> +
>>>> +    return platform_get_drvdata(pdev);
>>>> +}
>>>> +EXPORT_SYMBOL(mpfs_sys_controller_get);
> 
> There should probably be a check in here to ensure that this is actually
> a system controller and it's bound do this driver, rather than returning a
> random device's driver data >
> It might also help to make this take a phandle instead of a device node
> for lookup, to spare the client the extra phandle to node conversion.
Finally got around to this again, is it sufficient to just check that 
platform_get_drvdata returns non null, or should I also check if the 
pdev matches the drivers compatible strings? Only found one example of 
the latter and a mix of the former & what I had done when I went looking 
around at other drivers.
Conor.

_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv