Re: [PATCH] net: dsa: b53: mmap: Add device tree support

["Álvaro Fernández Rojas" <noltari@gmail.com>]

Hi Florian,

El 08/03/2021 a las 19:29, Florian Fainelli escribió:
> On 3/8/21 10:07 AM, Álvaro Fernández Rojas wrote:
>> Add device tree support to b53_mmap.c while keeping platform devices support.
>>
>> Signed-off-by: Álvaro Fernández Rojas <noltari@gmail.com>
>> ---
>>   drivers/net/dsa/b53/b53_mmap.c | 36 ++++++++++++++++++++++++++++++++++
>>   1 file changed, 36 insertions(+)
>>
>> diff --git a/drivers/net/dsa/b53/b53_mmap.c b/drivers/net/dsa/b53/b53_mmap.c
>> index c628d0980c0b..b897b4263930 100644
>> --- a/drivers/net/dsa/b53/b53_mmap.c
>> +++ b/drivers/net/dsa/b53/b53_mmap.c
>> @@ -228,12 +228,48 @@ static const struct b53_io_ops b53_mmap_ops = {
>>   	.write64 = b53_mmap_write64,
>>   };
>>   
>> +static int b53_mmap_probe_of(struct platform_device *pdev,
>> +			     struct b53_platform_data **ppdata)
>> +{
>> +	struct device *dev = &pdev->dev;
>> +	struct device_node *np = dev->of_node;
>> +	struct b53_platform_data *pdata;
>> +	void __iomem *mem;
>> +
>> +	mem = devm_platform_ioremap_resource(pdev, 0);
>> +	if (IS_ERR(mem))
>> +		return PTR_ERR(mem);
>> +
>> +	pdata = devm_kzalloc(dev, sizeof(struct b53_platform_data),
>> +			     GFP_KERNEL);
>> +	if (!pdata)
>> +		return -ENOMEM;
>> +
>> +	pdata->regs = mem;
>> +	pdata->chip_id = BCM63XX_DEVICE_ID;
>> +	pdata->big_endian = of_property_read_bool(np, "big-endian");
>> +	of_property_read_u16(np, "brcm,ports", &pdata->enabled_ports);
>> +
>> +	*ppdata = pdata;
>> +
>> +	return 0;
>> +}
>> +
>>   static int b53_mmap_probe(struct platform_device *pdev)
>>   {
>> +	struct device_node *np = pdev->dev.of_node;
>>   	struct b53_platform_data *pdata = pdev->dev.platform_data;
>>   	struct b53_mmap_priv *priv;
>>   	struct b53_device *dev;
>>   
>> +	if (np) {
>> +		int ret = b53_mmap_probe_of(pdev, &pdata);
>> +		if (ret) {
>> +			dev_err(&pdev->dev, "OF probe error\n");
>> +			return ret;
>> +		}
>> +	}
> 
> I would be keen on making this less "OF-centric" and just have it happen
> whenever pdata is NULL such that we have an easier transition path if we
> wanted to migrate bcm63xx to passing down the switch base register
> address a platform_device source in the future (not that I expect it to
> happen though).

Honestly, it's probably easier to switch to bmips instead of migrating 
bcm63xx...

> 
> Other than that, the logic looks sound.
> 

Best regards,
Álvaro.