Re: [PATCH] mwifiex: parse hscfg_gpio info from device tree

Re: [PATCH] mwifiex: parse hscfg_gpio info from device tree

[Kalle Valo]

+ devicetree list

Amitkumar Karwar <akarwar-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org> writes:

> From: Xinming Hu <huxm-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
>
> This patch reads hscfg_gpio from device tree and update
> internal variable
>
> Signed-off-by: Xinming Hu <huxm-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Amitkumar Karwar <akarwar-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> ---
>  drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
>
> diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> index e486867..d28a53f 100644
> --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> @@ -1459,10 +1459,21 @@ int mwifiex_dnld_dt_cfgdata(struct mwifiex_private *priv,
>  #ifdef CONFIG_OF
>  	struct property *prop;
>  	size_t len = strlen(prefix);
> +	u32 data;
>  	int ret;
>  
>  	/* look for all matching property names */
>  	for_each_property_of_node(node, prop) {
> +		if (!strncmp(prop->name, "marvell,hscfg_gpio",
> +			     strlen("marvell,hscfg_gpio"))) {
> +			if (!of_property_read_u32(priv->adapter->dt_node,
> +						  prop->name, &data)) {
> +				dev_dbg(priv->adapter->dev,
> +					"hscfg gpio = 0x%x\n", data);
> +				priv->adapter->hs_cfg.gpio = data;
> +			}
> +		}

I don't see this documented in Documentation/devicetree/bindings. Please
create a binding document and review it with the device tree
maintainers.

Actually when looking mwifiex close I see that it uses more undocumented
device tree interfaces:

marvell_cfgdata
marvell,caldata
marvell,00_txpwrlimit

I think these all should be properly documented and reviewed. But I'll
let the device tree people chime in what's the best way.

-- 
Kalle Valo
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] mwifiex: parse hscfg_gpio info from device tree

[Amitkumar Karwar]

Hi Kalle,

> -----Original Message-----
> From: Kalle Valo [mailto:kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org]
> Sent: Friday, December 11, 2015 2:04 PM
> To: Amitkumar Karwar
> Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Nishant Sarmukadam; Xinming Hu;
> devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Subject: Re: [PATCH] mwifiex: parse hscfg_gpio info from device tree
> 
> + devicetree list
> 
> Amitkumar Karwar <akarwar-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org> writes:
> 
> > From: Xinming Hu <huxm-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> >
> > This patch reads hscfg_gpio from device tree and update internal
> > variable
> >
> > Signed-off-by: Xinming Hu <huxm-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> > Signed-off-by: Amitkumar Karwar <akarwar-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
> > ---
> >  drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> > b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> > index e486867..d28a53f 100644
> > --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> > +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> > @@ -1459,10 +1459,21 @@ int mwifiex_dnld_dt_cfgdata(struct
> > mwifiex_private *priv,  #ifdef CONFIG_OF
> >  	struct property *prop;
> >  	size_t len = strlen(prefix);
> > +	u32 data;
> >  	int ret;
> >
> >  	/* look for all matching property names */
> >  	for_each_property_of_node(node, prop) {
> > +		if (!strncmp(prop->name, "marvell,hscfg_gpio",
> > +			     strlen("marvell,hscfg_gpio"))) {
> > +			if (!of_property_read_u32(priv->adapter->dt_node,
> > +						  prop->name, &data)) {
> > +				dev_dbg(priv->adapter->dev,
> > +					"hscfg gpio = 0x%x\n", data);
> > +				priv->adapter->hs_cfg.gpio = data;
> > +			}
> > +		}
> 
> I don't see this documented in Documentation/devicetree/bindings. Please
> create a binding document and review it with the device tree
> maintainers.
> 
> Actually when looking mwifiex close I see that it uses more undocumented
> device tree interfaces:
> 
> marvell_cfgdata
> marvell,caldata
> marvell,00_txpwrlimit
> 
> I think these all should be properly documented and reviewed. But I'll
> let the device tree people chime in what's the best way.
> 

Thanks for the review.
Sure. We will document existing DT interfaces and create updated version of this patch which includes documentation in bindings.

Regards,
Amit
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH] mwifiex: parse hscfg_gpio info from device tree

[Julian Calaby]

Hi Amitkumar,

On Fri, Dec 11, 2015 at 8:23 PM, Amitkumar Karwar <akarwar-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org> wrote:
> Hi Kalle,
>
>> -----Original Message-----
>> From: Kalle Valo [mailto:kvalo-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org]
>> Sent: Friday, December 11, 2015 2:04 PM
>> To: Amitkumar Karwar
>> Cc: linux-wireless-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; Nishant Sarmukadam; Xinming Hu;
>> devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Subject: Re: [PATCH] mwifiex: parse hscfg_gpio info from device tree
>>
>> + devicetree list
>>
>> Amitkumar Karwar <akarwar-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org> writes:
>>
>> > From: Xinming Hu <huxm-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
>> >
>> > This patch reads hscfg_gpio from device tree and update internal
>> > variable
>> >
>> > Signed-off-by: Xinming Hu <huxm-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
>> > Signed-off-by: Amitkumar Karwar <akarwar-eYqpPyKDWXRBDgjK7y7TUQ@public.gmane.org>
>> > ---
>> >  drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 11 +++++++++++
>> >  1 file changed, 11 insertions(+)
>> >
>> > diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
>> > b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
>> > index e486867..d28a53f 100644
>> > --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
>> > +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
>> > @@ -1459,10 +1459,21 @@ int mwifiex_dnld_dt_cfgdata(struct
>> > mwifiex_private *priv,  #ifdef CONFIG_OF
>> >     struct property *prop;
>> >     size_t len = strlen(prefix);
>> > +   u32 data;
>> >     int ret;
>> >
>> >     /* look for all matching property names */
>> >     for_each_property_of_node(node, prop) {
>> > +           if (!strncmp(prop->name, "marvell,hscfg_gpio",
>> > +                        strlen("marvell,hscfg_gpio"))) {
>> > +                   if (!of_property_read_u32(priv->adapter->dt_node,
>> > +                                             prop->name, &data)) {
>> > +                           dev_dbg(priv->adapter->dev,
>> > +                                   "hscfg gpio = 0x%x\n", data);
>> > +                           priv->adapter->hs_cfg.gpio = data;
>> > +                   }
>> > +           }
>>
>> I don't see this documented in Documentation/devicetree/bindings. Please
>> create a binding document and review it with the device tree
>> maintainers.
>>
>> Actually when looking mwifiex close I see that it uses more undocumented
>> device tree interfaces:
>>
>> marvell_cfgdata
>> marvell,caldata
>> marvell,00_txpwrlimit
>>
>> I think these all should be properly documented and reviewed. But I'll
>> let the device tree people chime in what's the best way.
>>
>
> Thanks for the review.
> Sure. We will document existing DT interfaces and create updated version of this patch which includes documentation in bindings.

It's been a while since this was reviewed. Do you have an updated
version of this patch?

Thanks,

-- 
Julian Calaby

Email: julian.calaby-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org
Profile: http://www.google.com/profiles/julian.calaby/
--
To unsubscribe from this list: send the line "unsubscribe linux-wireless" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

RE: [PATCH] mwifiex: parse hscfg_gpio info from device tree

[Amitkumar Karwar]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3592 bytes --]

Hi Julian,

> From: Julian Calaby [mailto:julian.calaby@gmail.com]
> Sent: Thursday, March 17, 2016 4:56 AM
> To: Amitkumar Karwar
> Cc: Kalle Valo; linux-wireless@vger.kernel.org; Nishant Sarmukadam;
> Xinming Hu; devicetree@vger.kernel.org
> Subject: Re: [PATCH] mwifiex: parse hscfg_gpio info from device tree
> 
> Hi Amitkumar,
> 
> On Fri, Dec 11, 2015 at 8:23 PM, Amitkumar Karwar <akarwar@marvell.com>
> wrote:
> > Hi Kalle,
> >
> >> -----Original Message-----
> >> From: Kalle Valo [mailto:kvalo@codeaurora.org]
> >> Sent: Friday, December 11, 2015 2:04 PM
> >> To: Amitkumar Karwar
> >> Cc: linux-wireless@vger.kernel.org; Nishant Sarmukadam; Xinming Hu;
> >> devicetree@vger.kernel.org
> >> Subject: Re: [PATCH] mwifiex: parse hscfg_gpio info from device tree
> >>
> >> + devicetree list
> >>
> >> Amitkumar Karwar <akarwar@marvell.com> writes:
> >>
> >> > From: Xinming Hu <huxm@marvell.com>
> >> >
> >> > This patch reads hscfg_gpio from device tree and update internal
> >> > variable
> >> >
> >> > Signed-off-by: Xinming Hu <huxm@marvell.com>
> >> > Signed-off-by: Amitkumar Karwar <akarwar@marvell.com>
> >> > ---
> >> >  drivers/net/wireless/marvell/mwifiex/sta_cmd.c | 11 +++++++++++
> >> >  1 file changed, 11 insertions(+)
> >> >
> >> > diff --git a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> >> > b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> >> > index e486867..d28a53f 100644
> >> > --- a/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> >> > +++ b/drivers/net/wireless/marvell/mwifiex/sta_cmd.c
> >> > @@ -1459,10 +1459,21 @@ int mwifiex_dnld_dt_cfgdata(struct
> >> > mwifiex_private *priv,  #ifdef CONFIG_OF
> >> >     struct property *prop;
> >> >     size_t len = strlen(prefix);
> >> > +   u32 data;
> >> >     int ret;
> >> >
> >> >     /* look for all matching property names */
> >> >     for_each_property_of_node(node, prop) {
> >> > +           if (!strncmp(prop->name, "marvell,hscfg_gpio",
> >> > +                        strlen("marvell,hscfg_gpio"))) {
> >> > +                   if (!of_property_read_u32(priv->adapter-
> >dt_node,
> >> > +                                             prop->name, &data)) {
> >> > +                           dev_dbg(priv->adapter->dev,
> >> > +                                   "hscfg gpio = 0x%x\n", data);
> >> > +                           priv->adapter->hs_cfg.gpio = data;
> >> > +                   }
> >> > +           }
> >>
> >> I don't see this documented in Documentation/devicetree/bindings.
> >> Please create a binding document and review it with the device tree
> >> maintainers.
> >>
> >> Actually when looking mwifiex close I see that it uses more
> >> undocumented device tree interfaces:
> >>
> >> marvell_cfgdata
> >> marvell,caldata
> >> marvell,00_txpwrlimit
> >>
> >> I think these all should be properly documented and reviewed. But
> >> I'll let the device tree people chime in what's the best way.
> >>
> >
> > Thanks for the review.
> > Sure. We will document existing DT interfaces and create updated
> version of this patch which includes documentation in bindings.
> 
> It's been a while since this was reviewed. Do you have an updated
> version of this patch?
> 

We will be submitting updated version of below patch series in a day or two. Hence updated version of this patch is not needed.

https://lkml.org/lkml/2016/2/29/394

Regards,
Amitkumar
N‹§²æìr¸›yúèšØb²X¬¶Ç§vØ^–)Þº{.nÇ+‰·¥Š{±Â*Þ•ë,Š{ayº\x1dʇڙë,j\a­¢f£¢·hš‹»öì\x17/oSc¾™Ú³9˜uÀ¦æå‰È&jw¨®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿïêäz¹Þ–Šàþf£¢·hšˆ§~ˆmš