Re: [PATCH bluetooth-next] at86rf230: add support for setting external xtal

[Marc Kleine-Budde <mkl@pengutronix.de>]

[-- Attachment #1: Type: text/plain, Size: 1898 bytes --]

On 02/23/2015 04:41 PM, Alexander Aring wrote:
> Marc,
> 
> On Mon, Feb 23, 2015 at 04:13:29PM +0100, Alexander Aring wrote:
>> On Mon, Feb 23, 2015 at 04:10:58PM +0100, Marc Kleine-Budde wrote:
>>> On 02/23/2015 04:05 PM, Alexander Aring wrote:
>>>> This patch adds support for setting external xtal. This is recommended
> ...
>>>>  	rc = at86rf230_read_subreg(lp, SR_DVDD_OK, &dvdd);
>>>>  	if (rc)
>>>>  		return rc;
>>>> @@ -1392,6 +1403,16 @@ at86rf230_get_pdata(struct spi_device *spi)
>>>>  	pdata->rstn = of_get_named_gpio(spi->dev.of_node, "reset-gpio", 0);
>>>>  	pdata->slp_tr = of_get_named_gpio(spi->dev.of_node, "sleep-gpio", 0);
>>>>  
>>>> +	pdata->xtal = of_property_read_bool(spi->dev.of_node, "external-xtal");
>>>
>>> The platform data should be considered read only by the driver. Better
>>> put the information into your per-driver private data struct.
>>>
>>
>> ah, yes you are completely right here. Sorry :)
>>
> 
> I am not sure if this is okay here in that case. For non devicetree this
> code will never touched and I grab this code from some i2c driver like
> [0].
> 
> They also use the platform-data for setting the devicetree properties.

That doesn't make it better.

> The only thing which I can see now is the check if (xtal > 0xF) which is
> not available on non devicetree settings... but I can also remove this
> check, somebody should make something complete wrong if this value is
> above 0xF and regmap should mask this value.

IMHO this driver has been poorly converted to DT. The usual way would be
to add data to at86rf230_local.

Marc

-- 
Pengutronix e.K.                  | Marc Kleine-Budde           |
Industrial Linux Solutions        | Phone: +49-231-2826-924     |
Vertretung West/Dortmund          | Fax:   +49-5121-206917-5555 |
Amtsgericht Hildesheim, HRA 2686  | http://www.pengutronix.de   |


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]