Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Subject: Re: [PATCH] net: dsa: b53: mmap: Add device tree support
To:     Florian Fainelli <f.fainelli@gmail.com>, jonas.gorski@gmail.com,
        Andrew Lunn <andrew@lunn.ch>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Vladimir Oltean <olteanv@gmail.com>,
        "David S. Miller" <davem@davemloft.net>,
        Jakub Kicinski <kuba@kernel.org>, netdev@vger.kernel.org,
        linux-kernel@vger.kernel.org
References: <20210308180715.18571-1-noltari@gmail.com>
 <06dab800-d554-e807-8a72-427c6e99e4de@gmail.com>
From:   =?UTF-8?Q?=c3=81lvaro_Fern=c3=a1ndez_Rojas?= <noltari@gmail.com>
Message-ID: <15aa76d9-9399-b41b-151a-856597046af4@gmail.com>
Date:   Mon, 8 Mar 2021 19:34:10 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101
 Thunderbird/78.8.0
MIME-Version: 1.0
In-Reply-To: <06dab800-d554-e807-8a72-427c6e99e4de@gmail.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Language: es-ES
Content-Transfer-Encoding: 8bit
Precedence: bulk

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.