Re: [PATCH 2/3] drivers: gpio: siul2-s32g2: add NXP S32G2/S32G3 SoCs support

[Andrei Stefanescu]

On 06/09/2024 12:53, Krzysztof Kozlowski wrote:
> On 06/09/2024 11:45, Andrei Stefanescu wrote:
>> On 06/09/2024 12:39, Krzysztof Kozlowski wrote:
>>> On 06/09/2024 10:43, Andrei Stefanescu wrote:
>>>> Hi Krzysztof,
>>>>
>>>>
>>>>>> +static struct regmap *common_regmap_init(struct platform_device *pdev,
>>>>>> +					 struct regmap_config *conf,
>>>>>> +					 const char *name)
>>>>>> +{
>>>>>> +	struct device *dev = &pdev->dev;
>>>>>> +	struct resource *res;
>>>>>> +	resource_size_t size;
>>>>>> +	void __iomem *base;
>>>>>> +
>>>>>> +	res = platform_get_resource_byname(pdev, IORESOURCE_MEM, name);
>>>>>> +	if (!res) {
>>>>>> +		dev_err(&pdev->dev, "Failed to get MEM resource: %s\n", name);
>>>>>> +		return ERR_PTR(-EINVAL);
>>>>>> +	}
>>>>>> +
>>>>>> +	base = devm_ioremap_resource(dev, res);
>>>>>
>>>>> There is a wrapper for both calls above, so use it.
>>>>
>>>> I am not sure I can change this because I also use the `resource_size`
>>>> call below in order to initialize the regmap_config. 
>>>> Unfortunately, `devm_platform_ioremap_resource_byname` doesn't also retrieve
>>>> the resource via a pointer.
>>>>
>>>> I saw the `devm_platform_get_and_ioremap_resource` function but that one
>>>> retrieves the resource based on the index. I would like to keep identifying
>>>> the resource by its name instead of its index.
>>>
>>> So add the wrapper. Or explain what's wrong with indices?
>>
>> There's nothing wrong but I prefer to not force an order. I will
>> add a wrapper then.
> 
> Order is forced. You cannot change it. I don't understand your rationale.
> 
> Best regards,
> Krzysztof
> 

By order I mean the order in which the memory region descriptions are laid out
in the reg property. Currently, there is no issue if we, let's say, swap the order
of opads0 and opads1(as long as we keep the same change for the reg-names).

Just to double check, this would imply adding a new wrapper named
`devm_platform_get_and_ioremap_resource_byname` which would basically be
very similar to `devm_platform_get_and_ioremap_resource`. Is that ok?

Best regards,
Andrei