Re: [PATCHv3 bluetooth-next 2/2] at86rf230: add support for external xtal trim

[Alexander Aring <alex.aring@gmail.com>]

On Tue, Feb 24, 2015 at 11:34:17AM +0100, Marc Kleine-Budde wrote:
> On 02/24/2015 11:28 AM, Alexander Aring wrote:
> > Hi Marc,
> > 
> > On Tue, Feb 24, 2015 at 11:21:43AM +0100, Marc Kleine-Budde wrote:
> >> On 02/24/2015 11:11 AM, Alexander Aring wrote:
> >>> This patch adds support for setting the xtal trim register. Some at86rf2xx
> >>> transceiver boards needs fine tuning the xtal capacitor.
> >>>
> >>> Signed-off-by: Alexander Aring <alex.aring@gmail.com>
> >>> ---
> >>>  .../devicetree/bindings/net/ieee802154/at86rf230.txt         |  3 +++
> >>>  drivers/net/ieee802154/at86rf230.c                           | 12 ++++++++++++
> >>>  include/linux/spi/at86rf230.h                                |  1 +
> >>>  3 files changed, 16 insertions(+)
> >>>
> > ...
> >>> @@ -1390,6 +1397,10 @@ static int at86rf230_get_pdata(struct spi_device *spi,
> >>>  
> >>>  	cfg->rstn = of_get_named_gpio(spi->dev.of_node, "reset-gpio", 0);
> >>>  	cfg->slp_tr = of_get_named_gpio(spi->dev.of_node, "sleep-gpio", 0);
> >>> +	ret = of_property_read_u8(spi->dev.of_node, "xtal-trim",
> >>> +				  &cfg->xtal_trim);
> >>> +	if (ret < 0 && ret != -EINVAL)
> >>> +		return ret;
> >>>  
> >>>  	return 0;
> >>>  }
> >>> @@ -1545,6 +1556,7 @@ static int at86rf230_probe(struct spi_device *spi)
> >>>  	lp = hw->priv;
> >>>  	lp->hw = hw;
> >>>  	lp->spi = spi;
> >>> +	lp->cfg = cfg;
> >>
> >> This doesn't look correct.
> > 
> > You mean the line:
> > 
> > "lp->cfg = cfg;" or everything?
> 
> Just that line, it's in the wrong patch.
> 

yes, but in the other patch it makes no sense. I only use the platform
data in probe function and can't access the platform data over lp->cfg
there, because lp isn't allocated. So I can add it in patch 1/2 but then
I will never access the lp->cfg then.

devm_request_gpio thing will remember which gpio was requested. I never
read the platform_data again.


Now in this patch (2/2) I will access the platform data in hw_init and the lp
is allocated already in this function and can access xtal_trim over
lp->cfg->xtal_trim.

We don't really need after probe the platform data again. Further when
we will add support for the slp_tr pin we need that. This pin is used
after probing. This means copying platform data in at86rf230_local is a
good thing to prepare for future.

Nevertheless I will move it to 1/2.

- Alex