diff mbox

soc: ti: wkup_m3_ipc: switch to using remoteproc OF infrastructure

Message ID 20160812003529.519-1-s-anna@ti.com (mailing list archive)
State New, archived
Headers show

Commit Message

Suman Anna Aug. 12, 2016, 12:35 a.m. UTC
The remoteproc framework has been enhanced recently to provide new
OF API to retrieve a remoteproc handle by client drivers through a
standard 'rprocs' property in client nodes. The wkup_m3_ipc driver
has been using a custom 'ti,rproc' property until now, switch this
to use this new OF infrastructure. The wkup_m3_ipc driver has been
fixed up to provide backward compatibility for older DTBs by
replacing the older property with the standard newer property.

The wkup_m3_ipc binding has also been updated accordingly.

Signed-off-by: Suman Anna <s-anna@ti.com>
---
This patch is based on the discussion [1] for introducing new standard
OF API in remoteproc core series [2] and the exporting of couple of
functions in OF base code [3].

With this in place, the remoteproc core need not be looking for the
TI specific property anymore. I will submit the DTS changes once this
makes it to mainline.

regards
Suman

[1] https://patchwork.kernel.org/patch/9237767/
[2] http://marc.info/?l=linux-kernel&m=146894341701010&w=2
[3] https://patchwork.kernel.org/patch/9276181/

 .../devicetree/bindings/soc/ti/wkup_m3_ipc.txt     |  9 ++++++--
 drivers/soc/ti/wkup_m3_ipc.c                       | 26 +++++++++++++++++++++-
 2 files changed, 32 insertions(+), 3 deletions(-)

Comments

kernel test robot Aug. 12, 2016, 6:17 a.m. UTC | #1
Hi Suman,

[auto build test ERROR on robh/for-next]
[also build test ERROR on v4.8-rc1 next-20160811]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Suman-Anna/soc-ti-wkup_m3_ipc-switch-to-using-remoteproc-OF-infrastructure/20160812-085217
base:   https://git.kernel.org/pub/scm/linux/kernel/git/robh/linux.git for-next
config: arm-allmodconfig (attached as .config)
compiler: arm-linux-gnueabi-gcc (Debian 5.4.0-6) 5.4.0 20160609
reproduce:
        wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=arm 

All errors (new ones prefixed by >>):

>> ERROR: "of_update_property" [drivers/soc/ti/wkup_m3_ipc.ko] undefined!
>> ERROR: "of_remove_property" [drivers/soc/ti/wkup_m3_ipc.ko] undefined!

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
Lee Jones Aug. 12, 2016, 11:02 a.m. UTC | #2
On Thu, 11 Aug 2016, Suman Anna wrote:

> The remoteproc framework has been enhanced recently to provide new
> OF API to retrieve a remoteproc handle by client drivers through a
> standard 'rprocs' property in client nodes. The wkup_m3_ipc driver
> has been using a custom 'ti,rproc' property until now, switch this
> to use this new OF infrastructure. The wkup_m3_ipc driver has been
> fixed up to provide backward compatibility for older DTBs by
> replacing the older property with the standard newer property.
> 
> The wkup_m3_ipc binding has also been updated accordingly.
> 
> Signed-off-by: Suman Anna <s-anna@ti.com>
> ---
> This patch is based on the discussion [1] for introducing new standard
> OF API in remoteproc core series [2] and the exporting of couple of
> functions in OF base code [3].
> 
> With this in place, the remoteproc core need not be looking for the
> TI specific property anymore. I will submit the DTS changes once this
> makes it to mainline.
> 
> regards
> Suman
> 
> [1] https://patchwork.kernel.org/patch/9237767/
> [2] http://marc.info/?l=linux-kernel&m=146894341701010&w=2
> [3] https://patchwork.kernel.org/patch/9276181/
> 
>  .../devicetree/bindings/soc/ti/wkup_m3_ipc.txt     |  9 ++++++--
>  drivers/soc/ti/wkup_m3_ipc.c                       | 26 +++++++++++++++++++++-
>  2 files changed, 32 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt b/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
> index 401550487ed6..2ea7dd91acff 100644
> --- a/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
> +++ b/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
> @@ -23,12 +23,17 @@ Required properties:
>  			with the Wakeup M3 processor
>  - interrupts:		Contains the interrupt information for the wkup_m3
>  			interrupt that signals the MPU.
> -- ti,rproc:		phandle to the wkup_m3 rproc node so the IPC driver
> +- rprocs:		phandle to the wkup_m3 rproc node so the IPC driver
>  			can boot it.
>  - mboxes:		phandles used by IPC framework to get correct mbox
>  			channel for communication. Must point to appropriate
>  			mbox_wkupm3 child node.
>  
> +Deprecated properties:
> +----------------------
> +- ti,rproc:		This property has been replaced with the "rprocs"
> +			property.
> +
>  Example:
>  --------
>  /* AM33xx */
> @@ -48,7 +53,7 @@ Example:
>  				compatible = "ti,am3352-wkup-m3-ipc";
>  				reg = <0x1324 0x24>;
>  				interrupts = <78>;
> -				ti,rproc = <&wkup_m3>;
> +				rprocs = <&wkup_m3>;
>  				mboxes = <&mailbox &mbox_wkupm3>;
>  			};
>  
> diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c
> index 8823cc81ae45..86085f9bf6a8 100644
> --- a/drivers/soc/ti/wkup_m3_ipc.c
> +++ b/drivers/soc/ti/wkup_m3_ipc.c
> @@ -1,7 +1,7 @@
>  /*
>   * AMx3 Wkup M3 IPC driver
>   *
> - * Copyright (C) 2015 Texas Instruments, Inc.
> + * Copyright (C) 2015-2016 Texas Instruments, Inc.
>   *
>   * Dave Gerlach <d-gerlach@ti.com>
>   *
> @@ -390,6 +390,8 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
>  	struct resource *res;
>  	struct task_struct *task;
>  	struct wkup_m3_ipc *m3_ipc;
> +	struct property *nprop, *oprop;
> +	const char nprop_name[] = "rprocs";
>  
>  	m3_ipc = devm_kzalloc(dev, sizeof(*m3_ipc), GFP_KERNEL);
>  	if (!m3_ipc)
> @@ -415,6 +417,28 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
>  		return ret;
>  	}
>  
> +	/* provide compatibility for older DTBs using ti,rproc property */
> +	nprop = of_find_property(dev->of_node, "rprocs", NULL);
> +	if (!nprop) {
> +		oprop = of_find_property(dev->of_node, "ti,rproc", NULL);
> +		if (!oprop) {
> +			dev_err(&pdev->dev, "node is missing ti,rproc property\n");
> +			return -ENODEV;
> +		}
> +
> +		nprop = kzalloc(sizeof(*nprop) + sizeof(nprop_name),
> +				GFP_KERNEL);
> +		if (!nprop)
> +			return -ENOMEM;
> +
> +		nprop->name = (char *)nprop + sizeof(*nprop);
> +		snprintf(nprop->name, sizeof(nprop_name), nprop_name);
> +		nprop->value = oprop->value;
> +		nprop->length = oprop->length;
> +		of_update_property(dev->of_node, nprop);
> +		of_remove_property(dev->of_node, oprop);
> +	}
> +

+1 for getting the functionality out of core code.

-100 for having to jump though all these hoops.

If you are going to keep all of this, I would at least tuck it away in
a header file or something.

>  	m3_ipc->mbox_client.dev = dev;
>  	m3_ipc->mbox_client.tx_done = NULL;
>  	m3_ipc->mbox_client.tx_prepare = NULL;
Suman Anna Aug. 12, 2016, 4 p.m. UTC | #3
On 08/12/2016 06:02 AM, Lee Jones wrote:
> On Thu, 11 Aug 2016, Suman Anna wrote:
> 
>> The remoteproc framework has been enhanced recently to provide new
>> OF API to retrieve a remoteproc handle by client drivers through a
>> standard 'rprocs' property in client nodes. The wkup_m3_ipc driver
>> has been using a custom 'ti,rproc' property until now, switch this
>> to use this new OF infrastructure. The wkup_m3_ipc driver has been
>> fixed up to provide backward compatibility for older DTBs by
>> replacing the older property with the standard newer property.
>>
>> The wkup_m3_ipc binding has also been updated accordingly.
>>
>> Signed-off-by: Suman Anna <s-anna@ti.com>
>> ---
>> This patch is based on the discussion [1] for introducing new standard
>> OF API in remoteproc core series [2] and the exporting of couple of
>> functions in OF base code [3].
>>
>> With this in place, the remoteproc core need not be looking for the
>> TI specific property anymore. I will submit the DTS changes once this
>> makes it to mainline.
>>
>> regards
>> Suman
>>
>> [1] https://patchwork.kernel.org/patch/9237767/
>> [2] http://marc.info/?l=linux-kernel&m=146894341701010&w=2
>> [3] https://patchwork.kernel.org/patch/9276181/
>>
>>  .../devicetree/bindings/soc/ti/wkup_m3_ipc.txt     |  9 ++++++--
>>  drivers/soc/ti/wkup_m3_ipc.c                       | 26 +++++++++++++++++++++-
>>  2 files changed, 32 insertions(+), 3 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt b/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
>> index 401550487ed6..2ea7dd91acff 100644
>> --- a/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
>> +++ b/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
>> @@ -23,12 +23,17 @@ Required properties:
>>  			with the Wakeup M3 processor
>>  - interrupts:		Contains the interrupt information for the wkup_m3
>>  			interrupt that signals the MPU.
>> -- ti,rproc:		phandle to the wkup_m3 rproc node so the IPC driver
>> +- rprocs:		phandle to the wkup_m3 rproc node so the IPC driver
>>  			can boot it.
>>  - mboxes:		phandles used by IPC framework to get correct mbox
>>  			channel for communication. Must point to appropriate
>>  			mbox_wkupm3 child node.
>>  
>> +Deprecated properties:
>> +----------------------
>> +- ti,rproc:		This property has been replaced with the "rprocs"
>> +			property.
>> +
>>  Example:
>>  --------
>>  /* AM33xx */
>> @@ -48,7 +53,7 @@ Example:
>>  				compatible = "ti,am3352-wkup-m3-ipc";
>>  				reg = <0x1324 0x24>;
>>  				interrupts = <78>;
>> -				ti,rproc = <&wkup_m3>;
>> +				rprocs = <&wkup_m3>;
>>  				mboxes = <&mailbox &mbox_wkupm3>;
>>  			};
>>  
>> diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c
>> index 8823cc81ae45..86085f9bf6a8 100644
>> --- a/drivers/soc/ti/wkup_m3_ipc.c
>> +++ b/drivers/soc/ti/wkup_m3_ipc.c
>> @@ -1,7 +1,7 @@
>>  /*
>>   * AMx3 Wkup M3 IPC driver
>>   *
>> - * Copyright (C) 2015 Texas Instruments, Inc.
>> + * Copyright (C) 2015-2016 Texas Instruments, Inc.
>>   *
>>   * Dave Gerlach <d-gerlach@ti.com>
>>   *
>> @@ -390,6 +390,8 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
>>  	struct resource *res;
>>  	struct task_struct *task;
>>  	struct wkup_m3_ipc *m3_ipc;
>> +	struct property *nprop, *oprop;
>> +	const char nprop_name[] = "rprocs";
>>  
>>  	m3_ipc = devm_kzalloc(dev, sizeof(*m3_ipc), GFP_KERNEL);
>>  	if (!m3_ipc)
>> @@ -415,6 +417,28 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
>>  		return ret;
>>  	}
>>  
>> +	/* provide compatibility for older DTBs using ti,rproc property */
>> +	nprop = of_find_property(dev->of_node, "rprocs", NULL);
>> +	if (!nprop) {
>> +		oprop = of_find_property(dev->of_node, "ti,rproc", NULL);
>> +		if (!oprop) {
>> +			dev_err(&pdev->dev, "node is missing ti,rproc property\n");
>> +			return -ENODEV;
>> +		}
>> +
>> +		nprop = kzalloc(sizeof(*nprop) + sizeof(nprop_name),
>> +				GFP_KERNEL);
>> +		if (!nprop)
>> +			return -ENOMEM;
>> +
>> +		nprop->name = (char *)nprop + sizeof(*nprop);
>> +		snprintf(nprop->name, sizeof(nprop_name), nprop_name);
>> +		nprop->value = oprop->value;
>> +		nprop->length = oprop->length;
>> +		of_update_property(dev->of_node, nprop);
>> +		of_remove_property(dev->of_node, oprop);
>> +	}
>> +
> 
> +1 for getting the functionality out of core code.
> 
> -100 for having to jump though all these hoops.
> 
> If you are going to keep all of this, I would at least tuck it away in
> a header file or something.

Hiding it somewhere else won't make much of a difference though. If this
is such an eyesore, we still can do this cleanly without jumping through
these hoops. I would say this is a direct result of the removal of the
rproc_get_by_phandle API in Patch 2 of your OF remoteproc series [2].
That API is still an agnostic API. We can just add the of_rproc_get()
convenience helper in your Patch 1 and drop patch 2 completely with no
references to ti,rproc in the core code in the first place. The other
option, if we do want to remove that API is to add an additional
property name argument (NULL to mean "rprocs") to
of_rproc_get_by_index(), and it can be hidden away under of_rproc_get()
API - which is what I am expecting most of the new client users would
use anyway.

Once "rprocs" hits mainline, I will definitely switch over the
wkup_m3_ipc nodes to use that standard property and can fix this driver
independently.

Anyway, I have given all sorts of inputs to arrive at a clean solution
in both remoteproc core and this driver. I will leave it to Bjorn and
Tony/Santosh to decide what they want in each driver. As it is, this
patch will be moot if the core OF exports patch falls through.

regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bjorn Andersson Aug. 12, 2016, 4:43 p.m. UTC | #4
On Fri 12 Aug 09:00 PDT 2016, Suman Anna wrote:

> On 08/12/2016 06:02 AM, Lee Jones wrote:
> > On Thu, 11 Aug 2016, Suman Anna wrote:
> > 
> >> The remoteproc framework has been enhanced recently to provide new
> >> OF API to retrieve a remoteproc handle by client drivers through a
> >> standard 'rprocs' property in client nodes. The wkup_m3_ipc driver
> >> has been using a custom 'ti,rproc' property until now, switch this
> >> to use this new OF infrastructure. The wkup_m3_ipc driver has been
> >> fixed up to provide backward compatibility for older DTBs by
> >> replacing the older property with the standard newer property.
> >>
> >> The wkup_m3_ipc binding has also been updated accordingly.
> >>
> >> Signed-off-by: Suman Anna <s-anna@ti.com>
> >> ---
> >> This patch is based on the discussion [1] for introducing new standard
> >> OF API in remoteproc core series [2] and the exporting of couple of
> >> functions in OF base code [3].
> >>
> >> With this in place, the remoteproc core need not be looking for the
> >> TI specific property anymore. I will submit the DTS changes once this
> >> makes it to mainline.
> >>
> >> regards
> >> Suman
> >>
> >> [1] https://patchwork.kernel.org/patch/9237767/
> >> [2] http://marc.info/?l=linux-kernel&m=146894341701010&w=2
> >> [3] https://patchwork.kernel.org/patch/9276181/
> >>
> >>  .../devicetree/bindings/soc/ti/wkup_m3_ipc.txt     |  9 ++++++--
> >>  drivers/soc/ti/wkup_m3_ipc.c                       | 26 +++++++++++++++++++++-
> >>  2 files changed, 32 insertions(+), 3 deletions(-)
> >>
> >> diff --git a/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt b/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
> >> index 401550487ed6..2ea7dd91acff 100644
> >> --- a/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
> >> +++ b/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
> >> @@ -23,12 +23,17 @@ Required properties:
> >>  			with the Wakeup M3 processor
> >>  - interrupts:		Contains the interrupt information for the wkup_m3
> >>  			interrupt that signals the MPU.
> >> -- ti,rproc:		phandle to the wkup_m3 rproc node so the IPC driver
> >> +- rprocs:		phandle to the wkup_m3 rproc node so the IPC driver
> >>  			can boot it.
> >>  - mboxes:		phandles used by IPC framework to get correct mbox
> >>  			channel for communication. Must point to appropriate
> >>  			mbox_wkupm3 child node.
> >>  
> >> +Deprecated properties:
> >> +----------------------
> >> +- ti,rproc:		This property has been replaced with the "rprocs"
> >> +			property.
> >> +
> >>  Example:
> >>  --------
> >>  /* AM33xx */
> >> @@ -48,7 +53,7 @@ Example:
> >>  				compatible = "ti,am3352-wkup-m3-ipc";
> >>  				reg = <0x1324 0x24>;
> >>  				interrupts = <78>;
> >> -				ti,rproc = <&wkup_m3>;
> >> +				rprocs = <&wkup_m3>;
> >>  				mboxes = <&mailbox &mbox_wkupm3>;
> >>  			};
> >>  
> >> diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c
> >> index 8823cc81ae45..86085f9bf6a8 100644
> >> --- a/drivers/soc/ti/wkup_m3_ipc.c
> >> +++ b/drivers/soc/ti/wkup_m3_ipc.c
> >> @@ -1,7 +1,7 @@
> >>  /*
> >>   * AMx3 Wkup M3 IPC driver
> >>   *
> >> - * Copyright (C) 2015 Texas Instruments, Inc.
> >> + * Copyright (C) 2015-2016 Texas Instruments, Inc.
> >>   *
> >>   * Dave Gerlach <d-gerlach@ti.com>
> >>   *
> >> @@ -390,6 +390,8 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
> >>  	struct resource *res;
> >>  	struct task_struct *task;
> >>  	struct wkup_m3_ipc *m3_ipc;
> >> +	struct property *nprop, *oprop;
> >> +	const char nprop_name[] = "rprocs";
> >>  
> >>  	m3_ipc = devm_kzalloc(dev, sizeof(*m3_ipc), GFP_KERNEL);
> >>  	if (!m3_ipc)
> >> @@ -415,6 +417,28 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
> >>  		return ret;
> >>  	}
> >>  
> >> +	/* provide compatibility for older DTBs using ti,rproc property */
> >> +	nprop = of_find_property(dev->of_node, "rprocs", NULL);
> >> +	if (!nprop) {
> >> +		oprop = of_find_property(dev->of_node, "ti,rproc", NULL);
> >> +		if (!oprop) {
> >> +			dev_err(&pdev->dev, "node is missing ti,rproc property\n");
> >> +			return -ENODEV;
> >> +		}
> >> +
> >> +		nprop = kzalloc(sizeof(*nprop) + sizeof(nprop_name),
> >> +				GFP_KERNEL);
> >> +		if (!nprop)
> >> +			return -ENOMEM;
> >> +
> >> +		nprop->name = (char *)nprop + sizeof(*nprop);
> >> +		snprintf(nprop->name, sizeof(nprop_name), nprop_name);
> >> +		nprop->value = oprop->value;
> >> +		nprop->length = oprop->length;
> >> +		of_update_property(dev->of_node, nprop);
> >> +		of_remove_property(dev->of_node, oprop);
> >> +	}
> >> +
> > 
> > +1 for getting the functionality out of core code.
> > 
> > -100 for having to jump though all these hoops.
> > 
> > If you are going to keep all of this, I would at least tuck it away in
> > a header file or something.
> 
> Hiding it somewhere else won't make much of a difference though. If this
> is such an eyesore, we still can do this cleanly without jumping through
> these hoops. I would say this is a direct result of the removal of the
> rproc_get_by_phandle API in Patch 2 of your OF remoteproc series [2].
> That API is still an agnostic API. We can just add the of_rproc_get()
> convenience helper in your Patch 1 and drop patch 2 completely with no
> references to ti,rproc in the core code in the first place. The other
> option, if we do want to remove that API is to add an additional
> property name argument (NULL to mean "rprocs") to
> of_rproc_get_by_index(), and it can be hidden away under of_rproc_get()
> API - which is what I am expecting most of the new client users would
> use anyway.
> 
> Once "rprocs" hits mainline, I will definitely switch over the
> wkup_m3_ipc nodes to use that standard property and can fix this driver
> independently.
> 

We're stuck with this problem all over the place, as the world continues
to evolve we will have issues with DT being static. This has been
discussed many times before and the suggested solution is always what
you implemented here - make the code deal with both versions, preferably
by patching.

The fact that you had to export the of_ operations indicates that no-one
has tried this before and I'm happy you did. I'm however not happy about
the size of the chunk of code it takes to do this dance.

I think for this to be practical we need to provide higher level
operations for DT modification - in this case a of_rename_property().

@Rob, any comments on this?

> Anyway, I have given all sorts of inputs to arrive at a clean solution
> in both remoteproc core and this driver. I will leave it to Bjorn and
> Tony/Santosh to decide what they want in each driver. As it is, this
> patch will be moot if the core OF exports patch falls through.
> 

I appreciate your input and we need to settle above discussion
regardless of this case - and by then have it implemented as you have
suggested.

Regards,
Bjorn
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Aug. 16, 2016, 2:54 p.m. UTC | #5
On Fri, Aug 12, 2016 at 11:43 AM, Bjorn Andersson
<bjorn.andersson@linaro.org> wrote:
> On Fri 12 Aug 09:00 PDT 2016, Suman Anna wrote:
>
>> On 08/12/2016 06:02 AM, Lee Jones wrote:
>> > On Thu, 11 Aug 2016, Suman Anna wrote:
>> >
>> >> The remoteproc framework has been enhanced recently to provide new
>> >> OF API to retrieve a remoteproc handle by client drivers through a
>> >> standard 'rprocs' property in client nodes. The wkup_m3_ipc driver
>> >> has been using a custom 'ti,rproc' property until now, switch this
>> >> to use this new OF infrastructure. The wkup_m3_ipc driver has been
>> >> fixed up to provide backward compatibility for older DTBs by
>> >> replacing the older property with the standard newer property.
>> >>
>> >> The wkup_m3_ipc binding has also been updated accordingly.
>> >>
>> >> Signed-off-by: Suman Anna <s-anna@ti.com>
>> >> ---
>> >> This patch is based on the discussion [1] for introducing new standard
>> >> OF API in remoteproc core series [2] and the exporting of couple of
>> >> functions in OF base code [3].
>> >>
>> >> With this in place, the remoteproc core need not be looking for the
>> >> TI specific property anymore. I will submit the DTS changes once this
>> >> makes it to mainline.
>> >>
>> >> regards
>> >> Suman
>> >>
>> >> [1] https://patchwork.kernel.org/patch/9237767/
>> >> [2] http://marc.info/?l=linux-kernel&m=146894341701010&w=2
>> >> [3] https://patchwork.kernel.org/patch/9276181/
>> >>
>> >>  .../devicetree/bindings/soc/ti/wkup_m3_ipc.txt     |  9 ++++++--
>> >>  drivers/soc/ti/wkup_m3_ipc.c                       | 26 +++++++++++++++++++++-
>> >>  2 files changed, 32 insertions(+), 3 deletions(-)
>> >>
>> >> diff --git a/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt b/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
>> >> index 401550487ed6..2ea7dd91acff 100644
>> >> --- a/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
>> >> +++ b/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
>> >> @@ -23,12 +23,17 @@ Required properties:
>> >>                    with the Wakeup M3 processor
>> >>  - interrupts:             Contains the interrupt information for the wkup_m3
>> >>                    interrupt that signals the MPU.
>> >> -- ti,rproc:               phandle to the wkup_m3 rproc node so the IPC driver
>> >> +- rprocs:         phandle to the wkup_m3 rproc node so the IPC driver
>> >>                    can boot it.
>> >>  - mboxes:         phandles used by IPC framework to get correct mbox
>> >>                    channel for communication. Must point to appropriate
>> >>                    mbox_wkupm3 child node.
>> >>
>> >> +Deprecated properties:
>> >> +----------------------
>> >> +- ti,rproc:               This property has been replaced with the "rprocs"
>> >> +                  property.
>> >> +
>> >>  Example:
>> >>  --------
>> >>  /* AM33xx */
>> >> @@ -48,7 +53,7 @@ Example:
>> >>                            compatible = "ti,am3352-wkup-m3-ipc";
>> >>                            reg = <0x1324 0x24>;
>> >>                            interrupts = <78>;
>> >> -                          ti,rproc = <&wkup_m3>;
>> >> +                          rprocs = <&wkup_m3>;
>> >>                            mboxes = <&mailbox &mbox_wkupm3>;
>> >>                    };
>> >>
>> >> diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c
>> >> index 8823cc81ae45..86085f9bf6a8 100644
>> >> --- a/drivers/soc/ti/wkup_m3_ipc.c
>> >> +++ b/drivers/soc/ti/wkup_m3_ipc.c
>> >> @@ -1,7 +1,7 @@
>> >>  /*
>> >>   * AMx3 Wkup M3 IPC driver
>> >>   *
>> >> - * Copyright (C) 2015 Texas Instruments, Inc.
>> >> + * Copyright (C) 2015-2016 Texas Instruments, Inc.
>> >>   *
>> >>   * Dave Gerlach <d-gerlach@ti.com>
>> >>   *
>> >> @@ -390,6 +390,8 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
>> >>    struct resource *res;
>> >>    struct task_struct *task;
>> >>    struct wkup_m3_ipc *m3_ipc;
>> >> +  struct property *nprop, *oprop;
>> >> +  const char nprop_name[] = "rprocs";
>> >>
>> >>    m3_ipc = devm_kzalloc(dev, sizeof(*m3_ipc), GFP_KERNEL);
>> >>    if (!m3_ipc)
>> >> @@ -415,6 +417,28 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
>> >>            return ret;
>> >>    }
>> >>
>> >> +  /* provide compatibility for older DTBs using ti,rproc property */
>> >> +  nprop = of_find_property(dev->of_node, "rprocs", NULL);
>> >> +  if (!nprop) {
>> >> +          oprop = of_find_property(dev->of_node, "ti,rproc", NULL);
>> >> +          if (!oprop) {
>> >> +                  dev_err(&pdev->dev, "node is missing ti,rproc property\n");
>> >> +                  return -ENODEV;
>> >> +          }
>> >> +
>> >> +          nprop = kzalloc(sizeof(*nprop) + sizeof(nprop_name),
>> >> +                          GFP_KERNEL);
>> >> +          if (!nprop)
>> >> +                  return -ENOMEM;
>> >> +
>> >> +          nprop->name = (char *)nprop + sizeof(*nprop);
>> >> +          snprintf(nprop->name, sizeof(nprop_name), nprop_name);
>> >> +          nprop->value = oprop->value;
>> >> +          nprop->length = oprop->length;
>> >> +          of_update_property(dev->of_node, nprop);
>> >> +          of_remove_property(dev->of_node, oprop);
>> >> +  }
>> >> +
>> >
>> > +1 for getting the functionality out of core code.
>> >
>> > -100 for having to jump though all these hoops.
>> >
>> > If you are going to keep all of this, I would at least tuck it away in
>> > a header file or something.
>>
>> Hiding it somewhere else won't make much of a difference though. If this
>> is such an eyesore, we still can do this cleanly without jumping through
>> these hoops. I would say this is a direct result of the removal of the
>> rproc_get_by_phandle API in Patch 2 of your OF remoteproc series [2].
>> That API is still an agnostic API. We can just add the of_rproc_get()
>> convenience helper in your Patch 1 and drop patch 2 completely with no
>> references to ti,rproc in the core code in the first place. The other
>> option, if we do want to remove that API is to add an additional
>> property name argument (NULL to mean "rprocs") to
>> of_rproc_get_by_index(), and it can be hidden away under of_rproc_get()
>> API - which is what I am expecting most of the new client users would
>> use anyway.
>>
>> Once "rprocs" hits mainline, I will definitely switch over the
>> wkup_m3_ipc nodes to use that standard property and can fix this driver
>> independently.
>>
>
> We're stuck with this problem all over the place, as the world continues
> to evolve we will have issues with DT being static. This has been
> discussed many times before and the suggested solution is always what
> you implemented here - make the code deal with both versions, preferably
> by patching.
>
> The fact that you had to export the of_ operations indicates that no-one
> has tried this before and I'm happy you did. I'm however not happy about
> the size of the chunk of code it takes to do this dance.
>
> I think for this to be practical we need to provide higher level
> operations for DT modification - in this case a of_rename_property().
>
> @Rob, any comments on this?

I agree. Pantelis submitted some helpers in this area a while back
(for the changeset API IIRC). I believe they were mostly fine, but
needed some users.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suman Anna Aug. 18, 2016, 9:30 p.m. UTC | #6
Hi Rob,

On 08/16/2016 09:54 AM, Rob Herring wrote:
> On Fri, Aug 12, 2016 at 11:43 AM, Bjorn Andersson
> <bjorn.andersson@linaro.org> wrote:
>> On Fri 12 Aug 09:00 PDT 2016, Suman Anna wrote:
>>
>>> On 08/12/2016 06:02 AM, Lee Jones wrote:
>>>> On Thu, 11 Aug 2016, Suman Anna wrote:
>>>>
>>>>> The remoteproc framework has been enhanced recently to provide new
>>>>> OF API to retrieve a remoteproc handle by client drivers through a
>>>>> standard 'rprocs' property in client nodes. The wkup_m3_ipc driver
>>>>> has been using a custom 'ti,rproc' property until now, switch this
>>>>> to use this new OF infrastructure. The wkup_m3_ipc driver has been
>>>>> fixed up to provide backward compatibility for older DTBs by
>>>>> replacing the older property with the standard newer property.
>>>>>
>>>>> The wkup_m3_ipc binding has also been updated accordingly.
>>>>>
>>>>> Signed-off-by: Suman Anna <s-anna@ti.com>
>>>>> ---
>>>>> This patch is based on the discussion [1] for introducing new standard
>>>>> OF API in remoteproc core series [2] and the exporting of couple of
>>>>> functions in OF base code [3].
>>>>>
>>>>> With this in place, the remoteproc core need not be looking for the
>>>>> TI specific property anymore. I will submit the DTS changes once this
>>>>> makes it to mainline.
>>>>>
>>>>> regards
>>>>> Suman
>>>>>
>>>>> [1] https://patchwork.kernel.org/patch/9237767/
>>>>> [2] http://marc.info/?l=linux-kernel&m=146894341701010&w=2
>>>>> [3] https://patchwork.kernel.org/patch/9276181/
>>>>>
>>>>>  .../devicetree/bindings/soc/ti/wkup_m3_ipc.txt     |  9 ++++++--
>>>>>  drivers/soc/ti/wkup_m3_ipc.c                       | 26 +++++++++++++++++++++-
>>>>>  2 files changed, 32 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt b/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
>>>>> index 401550487ed6..2ea7dd91acff 100644
>>>>> --- a/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
>>>>> +++ b/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
>>>>> @@ -23,12 +23,17 @@ Required properties:
>>>>>                    with the Wakeup M3 processor
>>>>>  - interrupts:             Contains the interrupt information for the wkup_m3
>>>>>                    interrupt that signals the MPU.
>>>>> -- ti,rproc:               phandle to the wkup_m3 rproc node so the IPC driver
>>>>> +- rprocs:         phandle to the wkup_m3 rproc node so the IPC driver
>>>>>                    can boot it.
>>>>>  - mboxes:         phandles used by IPC framework to get correct mbox
>>>>>                    channel for communication. Must point to appropriate
>>>>>                    mbox_wkupm3 child node.
>>>>>
>>>>> +Deprecated properties:
>>>>> +----------------------
>>>>> +- ti,rproc:               This property has been replaced with the "rprocs"
>>>>> +                  property.
>>>>> +
>>>>>  Example:
>>>>>  --------
>>>>>  /* AM33xx */
>>>>> @@ -48,7 +53,7 @@ Example:
>>>>>                            compatible = "ti,am3352-wkup-m3-ipc";
>>>>>                            reg = <0x1324 0x24>;
>>>>>                            interrupts = <78>;
>>>>> -                          ti,rproc = <&wkup_m3>;
>>>>> +                          rprocs = <&wkup_m3>;
>>>>>                            mboxes = <&mailbox &mbox_wkupm3>;
>>>>>                    };
>>>>>
>>>>> diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c
>>>>> index 8823cc81ae45..86085f9bf6a8 100644
>>>>> --- a/drivers/soc/ti/wkup_m3_ipc.c
>>>>> +++ b/drivers/soc/ti/wkup_m3_ipc.c
>>>>> @@ -1,7 +1,7 @@
>>>>>  /*
>>>>>   * AMx3 Wkup M3 IPC driver
>>>>>   *
>>>>> - * Copyright (C) 2015 Texas Instruments, Inc.
>>>>> + * Copyright (C) 2015-2016 Texas Instruments, Inc.
>>>>>   *
>>>>>   * Dave Gerlach <d-gerlach@ti.com>
>>>>>   *
>>>>> @@ -390,6 +390,8 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
>>>>>    struct resource *res;
>>>>>    struct task_struct *task;
>>>>>    struct wkup_m3_ipc *m3_ipc;
>>>>> +  struct property *nprop, *oprop;
>>>>> +  const char nprop_name[] = "rprocs";
>>>>>
>>>>>    m3_ipc = devm_kzalloc(dev, sizeof(*m3_ipc), GFP_KERNEL);
>>>>>    if (!m3_ipc)
>>>>> @@ -415,6 +417,28 @@ static int wkup_m3_ipc_probe(struct platform_device *pdev)
>>>>>            return ret;
>>>>>    }
>>>>>
>>>>> +  /* provide compatibility for older DTBs using ti,rproc property */
>>>>> +  nprop = of_find_property(dev->of_node, "rprocs", NULL);
>>>>> +  if (!nprop) {
>>>>> +          oprop = of_find_property(dev->of_node, "ti,rproc", NULL);
>>>>> +          if (!oprop) {
>>>>> +                  dev_err(&pdev->dev, "node is missing ti,rproc property\n");
>>>>> +                  return -ENODEV;
>>>>> +          }
>>>>> +
>>>>> +          nprop = kzalloc(sizeof(*nprop) + sizeof(nprop_name),
>>>>> +                          GFP_KERNEL);
>>>>> +          if (!nprop)
>>>>> +                  return -ENOMEM;
>>>>> +
>>>>> +          nprop->name = (char *)nprop + sizeof(*nprop);
>>>>> +          snprintf(nprop->name, sizeof(nprop_name), nprop_name);
>>>>> +          nprop->value = oprop->value;
>>>>> +          nprop->length = oprop->length;
>>>>> +          of_update_property(dev->of_node, nprop);
>>>>> +          of_remove_property(dev->of_node, oprop);
>>>>> +  }
>>>>> +
>>>>
>>>> +1 for getting the functionality out of core code.
>>>>
>>>> -100 for having to jump though all these hoops.
>>>>
>>>> If you are going to keep all of this, I would at least tuck it away in
>>>> a header file or something.
>>>
>>> Hiding it somewhere else won't make much of a difference though. If this
>>> is such an eyesore, we still can do this cleanly without jumping through
>>> these hoops. I would say this is a direct result of the removal of the
>>> rproc_get_by_phandle API in Patch 2 of your OF remoteproc series [2].
>>> That API is still an agnostic API. We can just add the of_rproc_get()
>>> convenience helper in your Patch 1 and drop patch 2 completely with no
>>> references to ti,rproc in the core code in the first place. The other
>>> option, if we do want to remove that API is to add an additional
>>> property name argument (NULL to mean "rprocs") to
>>> of_rproc_get_by_index(), and it can be hidden away under of_rproc_get()
>>> API - which is what I am expecting most of the new client users would
>>> use anyway.
>>>
>>> Once "rprocs" hits mainline, I will definitely switch over the
>>> wkup_m3_ipc nodes to use that standard property and can fix this driver
>>> independently.
>>>
>>
>> We're stuck with this problem all over the place, as the world continues
>> to evolve we will have issues with DT being static. This has been
>> discussed many times before and the suggested solution is always what
>> you implemented here - make the code deal with both versions, preferably
>> by patching.
>>
>> The fact that you had to export the of_ operations indicates that no-one
>> has tried this before and I'm happy you did. I'm however not happy about
>> the size of the chunk of code it takes to do this dance.
>>
>> I think for this to be practical we need to provide higher level
>> operations for DT modification - in this case a of_rename_property().
>>
>> @Rob, any comments on this?
> 
> I agree. Pantelis submitted some helpers in this area a while back
> (for the changeset API IIRC). I believe they were mostly fine, but
> needed some users.
> 

Is this the series you are talking about?
http://marc.info/?l=devicetree&m=146341689512653&w=2

It looks like that series is effective only when OF_DYNAMIC is enabled.
Probably a dumb question, but is this limited to DT Overlays? This
particular usage is a one-time change (first-time module is insmod'd)
mainly to provide compatibility for older DTBs, thereafter we wouldn't
be required to make any changes. It is definitely not a bulk update and
we don't want to unroll the changes even if we removed the module.

regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Rob Herring Aug. 19, 2016, 9:21 p.m. UTC | #7
On Thu, Aug 18, 2016 at 4:30 PM, Suman Anna <s-anna@ti.com> wrote:
> Hi Rob,
>
> On 08/16/2016 09:54 AM, Rob Herring wrote:
>> On Fri, Aug 12, 2016 at 11:43 AM, Bjorn Andersson
>> <bjorn.andersson@linaro.org> wrote:
>>> On Fri 12 Aug 09:00 PDT 2016, Suman Anna wrote:
>>>
>>>> On 08/12/2016 06:02 AM, Lee Jones wrote:
>>>>> On Thu, 11 Aug 2016, Suman Anna wrote:
>>>>>

[...]

>>>> Once "rprocs" hits mainline, I will definitely switch over the
>>>> wkup_m3_ipc nodes to use that standard property and can fix this driver
>>>> independently.
>>>>
>>>
>>> We're stuck with this problem all over the place, as the world continues
>>> to evolve we will have issues with DT being static. This has been
>>> discussed many times before and the suggested solution is always what
>>> you implemented here - make the code deal with both versions, preferably
>>> by patching.
>>>
>>> The fact that you had to export the of_ operations indicates that no-one
>>> has tried this before and I'm happy you did. I'm however not happy about
>>> the size of the chunk of code it takes to do this dance.
>>>
>>> I think for this to be practical we need to provide higher level
>>> operations for DT modification - in this case a of_rename_property().
>>>
>>> @Rob, any comments on this?
>>
>> I agree. Pantelis submitted some helpers in this area a while back
>> (for the changeset API IIRC). I believe they were mostly fine, but
>> needed some users.
>>
>
> Is this the series you are talking about?
> http://marc.info/?l=devicetree&m=146341689512653&w=2
>
> It looks like that series is effective only when OF_DYNAMIC is enabled.

Yes, as changeset API allows adding/removing nodes. It's probably just
a matter of time until OF_DYNAMIC is always enabled.

> Probably a dumb question, but is this limited to DT Overlays?

No, changesets are the mechanism overlays use to apply changes.

> This
> particular usage is a one-time change (first-time module is insmod'd)
> mainly to provide compatibility for older DTBs, thereafter we wouldn't
> be required to make any changes. It is definitely not a bulk update and
> we don't want to unroll the changes even if we removed the module.

If you only need property changes, then we could do similar helpers to
make it easier for callers.

Rob
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Suman Anna Aug. 19, 2016, 9:35 p.m. UTC | #8
On 08/19/2016 04:21 PM, Rob Herring wrote:
> On Thu, Aug 18, 2016 at 4:30 PM, Suman Anna <s-anna@ti.com> wrote:
>> Hi Rob,
>>
>> On 08/16/2016 09:54 AM, Rob Herring wrote:
>>> On Fri, Aug 12, 2016 at 11:43 AM, Bjorn Andersson
>>> <bjorn.andersson@linaro.org> wrote:
>>>> On Fri 12 Aug 09:00 PDT 2016, Suman Anna wrote:
>>>>
>>>>> On 08/12/2016 06:02 AM, Lee Jones wrote:
>>>>>> On Thu, 11 Aug 2016, Suman Anna wrote:
>>>>>>
> 
> [...]
> 
>>>>> Once "rprocs" hits mainline, I will definitely switch over the
>>>>> wkup_m3_ipc nodes to use that standard property and can fix this driver
>>>>> independently.
>>>>>
>>>>
>>>> We're stuck with this problem all over the place, as the world continues
>>>> to evolve we will have issues with DT being static. This has been
>>>> discussed many times before and the suggested solution is always what
>>>> you implemented here - make the code deal with both versions, preferably
>>>> by patching.
>>>>
>>>> The fact that you had to export the of_ operations indicates that no-one
>>>> has tried this before and I'm happy you did. I'm however not happy about
>>>> the size of the chunk of code it takes to do this dance.
>>>>
>>>> I think for this to be practical we need to provide higher level
>>>> operations for DT modification - in this case a of_rename_property().
>>>>
>>>> @Rob, any comments on this?
>>>
>>> I agree. Pantelis submitted some helpers in this area a while back
>>> (for the changeset API IIRC). I believe they were mostly fine, but
>>> needed some users.
>>>
>>
>> Is this the series you are talking about?
>> http://marc.info/?l=devicetree&m=146341689512653&w=2
>>
>> It looks like that series is effective only when OF_DYNAMIC is enabled.
> 
> Yes, as changeset API allows adding/removing nodes. It's probably just
> a matter of time until OF_DYNAMIC is always enabled.
> 
>> Probably a dumb question, but is this limited to DT Overlays?
> 
> No, changesets are the mechanism overlays use to apply changes.
> 
>> This
>> particular usage is a one-time change (first-time module is insmod'd)
>> mainly to provide compatibility for older DTBs, thereafter we wouldn't
>> be required to make any changes. It is definitely not a bulk update and
>> we don't want to unroll the changes even if we removed the module.
> 
> If you only need property changes, then we could do similar helpers to
> make it easier for callers.

So if you are ok with an of_rename_property() API, I can submit a patch
for the same.

regards
Suman
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
diff mbox

Patch

diff --git a/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt b/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
index 401550487ed6..2ea7dd91acff 100644
--- a/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
+++ b/Documentation/devicetree/bindings/soc/ti/wkup_m3_ipc.txt
@@ -23,12 +23,17 @@  Required properties:
 			with the Wakeup M3 processor
 - interrupts:		Contains the interrupt information for the wkup_m3
 			interrupt that signals the MPU.
-- ti,rproc:		phandle to the wkup_m3 rproc node so the IPC driver
+- rprocs:		phandle to the wkup_m3 rproc node so the IPC driver
 			can boot it.
 - mboxes:		phandles used by IPC framework to get correct mbox
 			channel for communication. Must point to appropriate
 			mbox_wkupm3 child node.
 
+Deprecated properties:
+----------------------
+- ti,rproc:		This property has been replaced with the "rprocs"
+			property.
+
 Example:
 --------
 /* AM33xx */
@@ -48,7 +53,7 @@  Example:
 				compatible = "ti,am3352-wkup-m3-ipc";
 				reg = <0x1324 0x24>;
 				interrupts = <78>;
-				ti,rproc = <&wkup_m3>;
+				rprocs = <&wkup_m3>;
 				mboxes = <&mailbox &mbox_wkupm3>;
 			};
 
diff --git a/drivers/soc/ti/wkup_m3_ipc.c b/drivers/soc/ti/wkup_m3_ipc.c
index 8823cc81ae45..86085f9bf6a8 100644
--- a/drivers/soc/ti/wkup_m3_ipc.c
+++ b/drivers/soc/ti/wkup_m3_ipc.c
@@ -1,7 +1,7 @@ 
 /*
  * AMx3 Wkup M3 IPC driver
  *
- * Copyright (C) 2015 Texas Instruments, Inc.
+ * Copyright (C) 2015-2016 Texas Instruments, Inc.
  *
  * Dave Gerlach <d-gerlach@ti.com>
  *
@@ -390,6 +390,8 @@  static int wkup_m3_ipc_probe(struct platform_device *pdev)
 	struct resource *res;
 	struct task_struct *task;
 	struct wkup_m3_ipc *m3_ipc;
+	struct property *nprop, *oprop;
+	const char nprop_name[] = "rprocs";
 
 	m3_ipc = devm_kzalloc(dev, sizeof(*m3_ipc), GFP_KERNEL);
 	if (!m3_ipc)
@@ -415,6 +417,28 @@  static int wkup_m3_ipc_probe(struct platform_device *pdev)
 		return ret;
 	}
 
+	/* provide compatibility for older DTBs using ti,rproc property */
+	nprop = of_find_property(dev->of_node, "rprocs", NULL);
+	if (!nprop) {
+		oprop = of_find_property(dev->of_node, "ti,rproc", NULL);
+		if (!oprop) {
+			dev_err(&pdev->dev, "node is missing ti,rproc property\n");
+			return -ENODEV;
+		}
+
+		nprop = kzalloc(sizeof(*nprop) + sizeof(nprop_name),
+				GFP_KERNEL);
+		if (!nprop)
+			return -ENOMEM;
+
+		nprop->name = (char *)nprop + sizeof(*nprop);
+		snprintf(nprop->name, sizeof(nprop_name), nprop_name);
+		nprop->value = oprop->value;
+		nprop->length = oprop->length;
+		of_update_property(dev->of_node, nprop);
+		of_remove_property(dev->of_node, oprop);
+	}
+
 	m3_ipc->mbox_client.dev = dev;
 	m3_ipc->mbox_client.tx_done = NULL;
 	m3_ipc->mbox_client.tx_prepare = NULL;