Re: [PATCH v6 08/40] soc: Add SoC driver for Cirrus ep93xx

[Arnd Bergmann]

On Sat, Dec 23, 2023, at 10:06, Nikita Shubin wrote:
> On Wed, 2023-12-13 at 20:37 +0200, Andy Shevchenko wrote:
>> On Tue, Dec 12, 2023 at 11:20:25AM +0300, Nikita Shubin wrote:

>> 
>> Maybe
>> 
>>         strict device *dev = &pdev->dev;
>>         enum ep93xx_soc_model model;
>>         ...
>>         model = (enum
>> ep93xx_soc_model)(uintptr_t)device_get_match_data(dev);
>> 
>> ?

You can just skip the second cast:

    model = (uintptr_t)device_get_match_data(dev);


or even better, since the only thing you look up by model is the
string put the string directly into the data:

+static const struct of_device_id ep9301_syscon_of_device_ids[] = {
+	{ .compatible	= "cirrus,ep9301-syscon", .data = "pinctrl-ep9301" },
+	{ .compatible	= "cirrus,ep9302-syscon", .data = "pinctrl-ep9301" },
+	{ .compatible	= "cirrus,ep9307-syscon", .data = "pinctrl-ep9307" },
+	{ .compatible	= "cirrus,ep9312-syscon", .data = "pinctrl-ep9312" },
+	{ .compatible	= "cirrus,ep9315-syscon", .data = "pinctrl-ep9312" },
+	{ /* sentinel */ }
+};

Since the strings are constants, gcc should even be able to deduplicate them.

>> 
>> > +       dev_info(dev, "EP93xx SoC revision %s\n", attrs->revision);
>> 
>> Hmm... Is this message anyhow useful?
>> 
>
> Can we keep it please ? It makes us happy when we see it in logs for
> historical reasons - it's been there since 2.4.

In general we don't do these at all, but since you are likely the only
person paying attention to this particular one, you may as well keep it.

     Arnd