Messages in this thread | | | From | Baolin Wang <> | Date | Thu, 30 Nov 2017 10:04:13 +0800 | Subject | Re: [PATCH v3] mfd: syscon: Add hardware spinlock support |
| |
Hi Arnd,
On 29 November 2017 at 18:07, Arnd Bergmann <arnd@arndb.de> wrote: > On Mon, Nov 20, 2017 at 7:54 AM, Baolin Wang <baolin.wang@linaro.org> wrote: >> Some system control registers need hardware spinlock to synchronize >> between the multiple subsystems, so we should add hardware spinlock >> support for syscon. >> > >> @@ -87,6 +88,12 @@ static struct syscon *of_syscon_register(struct device_node *np) >> if (ret) >> reg_io_width = 4; >> >> + ret = of_hwspin_lock_get_id(np, 0); >> + if (ret > 0) { >> + syscon_config.hwlock_id = ret; >> + syscon_config.hwlock_mode = HWLOCK_IRQSTATE; >> + } > > Hi Baolin, > > The error handling here seems insufficient, I think we have to treat > the following cases separately: > > - ret>0 (this is fine) > - no spinlock in DT: of_hwspin_lock_get_id currently returns -EINVAL, > we probably want to use a different return code (maybe -ESRCH ?) > here so we can tell the difference beween no spinlock, and an > invalid spinlock > - deferred probing: if the spinlock is there but the driver is not > yet available, we may need to propagate the -EPROBE_DEFER > here > - other error: this would probably be a fatal condition here, and > we should print a warning and clean up.
You are correct and I will add some error handling. Thanks for your comments.
-- Baolin.wang Best Regards
| |