[v2,11/21] nvmem: core: handle the absence of expected layouts

Message ID 20230307165359.225361-12-miquel.raynal@bootlin.com
State New
Headers
Series nvmem: Layouts support |

Commit Message

Miquel Raynal March 7, 2023, 4:53 p.m. UTC
  Make nvmem_layout_get() return -EPROBE_DEFER while the expected layout
is not available. This condition cannot be triggered today as nvmem
layout drivers are initialed as part of an early init call, but soon
these drivers will be converted into modules and be initialized with a
standard priority, so the unavailability of the drivers might become a
reality that must be taken care of.

Let's anticipate this by telling the caller the layout might not yet be
available. A probe deferral is requested in this case.

Please note this does not affect any nvmem device not using layouts,
because an early check against the "nvmem-layout" container presence
will return NULL in this case.

Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
Tested-by: Michael Walle <michael@walle.cc>
---
 drivers/nvmem/core.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)
  

Comments

Srinivas Kandagatla March 10, 2023, 10:30 a.m. UTC | #1
On 07/03/2023 16:53, Miquel Raynal wrote:
> Make nvmem_layout_get() return -EPROBE_DEFER while the expected layout
> is not available. This condition cannot be triggered today as nvmem
> layout drivers are initialed as part of an early init call, but soon
> these drivers will be converted into modules and be initialized with a
> standard priority, so the unavailability of the drivers might become a
> reality that must be taken care of.
> 
> Let's anticipate this by telling the caller the layout might not yet be
> available. A probe deferral is requested in this case.
> 
> Please note this does not affect any nvmem device not using layouts,
> because an early check against the "nvmem-layout" container presence
> will return NULL in this case.
> 
> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> Tested-by: Michael Walle <michael@walle.cc>
> ---
>   drivers/nvmem/core.c | 10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> index b9be1faeb7be..51fd792b8d70 100644
> --- a/drivers/nvmem/core.c
> +++ b/drivers/nvmem/core.c
> @@ -755,7 +755,7 @@ EXPORT_SYMBOL_GPL(nvmem_layout_unregister);
>   static struct nvmem_layout *nvmem_layout_get(struct nvmem_device *nvmem)
>   {

Any reason why this is not part of 10/21?

kernel doc for nvmem_layout_get needs updating with this behavior.


--srini

>   	struct device_node *layout_np, *np = nvmem->dev.of_node;
> -	struct nvmem_layout *l, *layout = NULL;
> +	struct nvmem_layout *l, *layout = ERR_PTR(-EPROBE_DEFER);
>   
>   	layout_np = of_get_child_by_name(np, "nvmem-layout");
>   	if (!layout_np)
> @@ -938,6 +938,13 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   	 * pointer will be NULL and nvmem_layout_put() will be a noop.
>   	 */
>   	nvmem->layout = config->layout ?: nvmem_layout_get(nvmem);
> +	if (IS_ERR(nvmem->layout)) {
> +		rval = PTR_ERR(nvmem->layout);
> +		nvmem->layout = NULL;
> +
> +		if (rval == -EPROBE_DEFER)
> +			goto err_teardown_compat;
> +	}
>   
>   	if (config->cells) {
>   		rval = nvmem_add_cells(nvmem, config->cells, config->ncells);
> @@ -970,6 +977,7 @@ struct nvmem_device *nvmem_register(const struct nvmem_config *config)
>   err_remove_cells:
>   	nvmem_device_remove_all_cells(nvmem);
>   	nvmem_layout_put(nvmem->layout);
> +err_teardown_compat:
>   	if (config->compat)
>   		nvmem_sysfs_remove_compat(nvmem, config);
>   err_put_device:
  
Miquel Raynal March 10, 2023, 10:45 a.m. UTC | #2
Hi Srinivas,

srinivas.kandagatla@linaro.org wrote on Fri, 10 Mar 2023 10:30:14 +0000:

> On 07/03/2023 16:53, Miquel Raynal wrote:
> > Make nvmem_layout_get() return -EPROBE_DEFER while the expected layout
> > is not available. This condition cannot be triggered today as nvmem
> > layout drivers are initialed as part of an early init call, but soon
> > these drivers will be converted into modules and be initialized with a
> > standard priority, so the unavailability of the drivers might become a
> > reality that must be taken care of.
> > 
> > Let's anticipate this by telling the caller the layout might not yet be
> > available. A probe deferral is requested in this case.
> > 
> > Please note this does not affect any nvmem device not using layouts,
> > because an early check against the "nvmem-layout" container presence
> > will return NULL in this case.
> > 
> > Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
> > Tested-by: Michael Walle <michael@walle.cc>
> > ---
> >   drivers/nvmem/core.c | 10 +++++++++-
> >   1 file changed, 9 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
> > index b9be1faeb7be..51fd792b8d70 100644
> > --- a/drivers/nvmem/core.c
> > +++ b/drivers/nvmem/core.c
> > @@ -755,7 +755,7 @@ EXPORT_SYMBOL_GPL(nvmem_layout_unregister);
> >   static struct nvmem_layout *nvmem_layout_get(struct nvmem_device *nvmem)
> >   {  
> 
> Any reason why this is not part of 10/21?

Yes, I would like to credit everybody for his work, so Michael for the
base implementation and myself for the module sitaution handling,
arguing this is two different features. May we keep these separated?

> kernel doc for nvmem_layout_get needs updating with this behavior.

There is no kdoc for nvmem_layout_get, do you want one ? I thought the
comment where this function is called would be more descriptive (and
read by interested people).

Thanks,
Miquèl
  
Srinivas Kandagatla March 10, 2023, 10:49 a.m. UTC | #3
On 10/03/2023 10:45, Miquel Raynal wrote:
> Hi Srinivas,
> 
> srinivas.kandagatla@linaro.org wrote on Fri, 10 Mar 2023 10:30:14 +0000:
> 
>> On 07/03/2023 16:53, Miquel Raynal wrote:
>>> Make nvmem_layout_get() return -EPROBE_DEFER while the expected layout
>>> is not available. This condition cannot be triggered today as nvmem
>>> layout drivers are initialed as part of an early init call, but soon
>>> these drivers will be converted into modules and be initialized with a
>>> standard priority, so the unavailability of the drivers might become a
>>> reality that must be taken care of.
>>>
>>> Let's anticipate this by telling the caller the layout might not yet be
>>> available. A probe deferral is requested in this case.
>>>
>>> Please note this does not affect any nvmem device not using layouts,
>>> because an early check against the "nvmem-layout" container presence
>>> will return NULL in this case.
>>>
>>> Signed-off-by: Miquel Raynal <miquel.raynal@bootlin.com>
>>> Tested-by: Michael Walle <michael@walle.cc>
>>> ---
>>>    drivers/nvmem/core.c | 10 +++++++++-
>>>    1 file changed, 9 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>>> index b9be1faeb7be..51fd792b8d70 100644
>>> --- a/drivers/nvmem/core.c
>>> +++ b/drivers/nvmem/core.c
>>> @@ -755,7 +755,7 @@ EXPORT_SYMBOL_GPL(nvmem_layout_unregister);
>>>    static struct nvmem_layout *nvmem_layout_get(struct nvmem_device *nvmem)
>>>    {
>>
>> Any reason why this is not part of 10/21?
> 
> Yes, I would like to credit everybody for his work, so Michael for the
> base implementation and myself for the module sitaution handling,
> arguing this is two different features. May we keep these separated?

Thanks for clarifying this, that should be fine.

--srini
  

Patch

diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
index b9be1faeb7be..51fd792b8d70 100644
--- a/drivers/nvmem/core.c
+++ b/drivers/nvmem/core.c
@@ -755,7 +755,7 @@  EXPORT_SYMBOL_GPL(nvmem_layout_unregister);
 static struct nvmem_layout *nvmem_layout_get(struct nvmem_device *nvmem)
 {
 	struct device_node *layout_np, *np = nvmem->dev.of_node;
-	struct nvmem_layout *l, *layout = NULL;
+	struct nvmem_layout *l, *layout = ERR_PTR(-EPROBE_DEFER);
 
 	layout_np = of_get_child_by_name(np, "nvmem-layout");
 	if (!layout_np)
@@ -938,6 +938,13 @@  struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 	 * pointer will be NULL and nvmem_layout_put() will be a noop.
 	 */
 	nvmem->layout = config->layout ?: nvmem_layout_get(nvmem);
+	if (IS_ERR(nvmem->layout)) {
+		rval = PTR_ERR(nvmem->layout);
+		nvmem->layout = NULL;
+
+		if (rval == -EPROBE_DEFER)
+			goto err_teardown_compat;
+	}
 
 	if (config->cells) {
 		rval = nvmem_add_cells(nvmem, config->cells, config->ncells);
@@ -970,6 +977,7 @@  struct nvmem_device *nvmem_register(const struct nvmem_config *config)
 err_remove_cells:
 	nvmem_device_remove_all_cells(nvmem);
 	nvmem_layout_put(nvmem->layout);
+err_teardown_compat:
 	if (config->compat)
 		nvmem_sysfs_remove_compat(nvmem, config);
 err_put_device: