linux-kernel - Re: [PATCH] platform/x86: Add IMS/ZII SCU driver

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <b9f42c24-9a5f-5486-f097-2b65a3c809ab@gmail.com>
Date:   Tue, 24 Jan 2017 12:07:50 -0800
From:   Florian Fainelli <f.fainelli@...il.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Platform Driver <platform-driver-x86@...r.kernel.org>,
        Chris Healy <cphealy@...il.com>,
        Guenter Roeck <linux@...ck-us.net>,
        Darren Hart <dvhart@...radead.org>,
        open list <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] platform/x86: Add IMS/ZII SCU driver

On 01/24/2017 11:39 AM, Andy Shevchenko wrote:
>> Are you concerned with storage, or what motivates the preference for
>> bitfields vs. bools?
> 
> Just matter of style. Up to you.
> 
>>>> +       {                       /* bit 5 */
>>>> +        .name = "SD6:a:error",
>>>> +        .gpio = SCU_SD_ERROR_6_GPIO,
>>>> +        .active_low = 1,
>>>> +        .default_trigger = "none",
>>>> +        .default_state = LEDS_GPIO_DEFSTATE_OFF,
>>>> +       }
>>>> +};
>>>
>>> Hmm... Can you utilize device tree for that?
>>
>> Not really an option here
> 
> Why not?

The platform is several years old, it is currently deployed with a boot
loader that does not support passing a Device Tree blob pointer in one
of the special e820 regions, I looked into that before, but I am not
proficient enough with grub's source code to make that happen nor do I
think this is going to be worth the trouble for these specific platforms.

Last I worked on x86 and Device Tree on CE4100 (2013 or so) there was
quite a lot of ad-hoc and not so generic code early FDT/OF
initialization code that had to be provided which AFAIR would hinder the
multiplatform capability of the kernel on x86 if there was a second DT
enabled platform that would show up. I did started that route, and it
did not look pretty, also, I would have to make *all* drivers used by
this platform (kempld and friends) to become DT aware, and make sure
that they do propagate of_node/child of_node correctly, major pain.

Finally, writing a complete Device Tree for this platform is a major
pain because a lot of the peripherals are being a PCI host bridge and
some of them are several levels deep (SPI over I2C expander over CPLD
over...).

> 
>>> Or built-in device properties?
>>
>> Not clear what you mean by that, can you expand?
> 
> include/linux/properties.h (platform data in a form of DT resource provider)
> 
>>>> +static void pca_leds_register(struct device *parent,
>>>> +                             struct scu_data *data)
>>>> +{
>>>> +       data->leds_pdev[0] =
>>>> +               platform_device_register_data(parent, "leds-gpio", 1,
>>>> +                                             &pca_gpio_led_info1,
>>>> +                                             sizeof(pca_gpio_led_info1));
>>>> +       data->leds_pdev[1] =
>>>> +               platform_device_register_data(parent, "leds-gpio", 2,
>>>> +                                             &pca_gpio_led_info2,
>>>> +                                             sizeof(pca_gpio_led_info2));
>>>> +       data->leds_pdev[2] =
>>>> +               platform_device_register_data(parent, "leds-gpio", 3,
>>>> +                                             &pca_gpio_led_info3,
>>>> +                                             sizeof(pca_gpio_led_info3));
>>>> +}
>>>
>>> It really sounds like MFD to me.
>>
>> It's more of a board description of attached peripherals (all of them),
>> more than a multi-function device, the whole module is by nature, "multi
>> function" since it has a bunch of different I/Os and on-module peripherals.
> 
> So, there is neither ACPI, nor DT provider for such resources?

Nope.

> We used to have a hell of a time to handle board files for Intel MID
> platforms under arch/x86/platform/intel-mid (formerly known as
> mrst.c), so, I don't like the idea of board files.

I understand the pain, but we also have to work with the tools we have,
and neither DT nor ACPI are such tools at the moment for this platform.

Thanks
-- 
Florian