From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750815AbdAXUHy (ORCPT ); Tue, 24 Jan 2017 15:07:54 -0500 Received: from mail-pf0-f196.google.com ([209.85.192.196]:33036 "EHLO mail-pf0-f196.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750713AbdAXUHw (ORCPT ); Tue, 24 Jan 2017 15:07:52 -0500 Subject: Re: [PATCH] platform/x86: Add IMS/ZII SCU driver To: Andy Shevchenko References: <20170111212644.9217-1-f.fainelli@gmail.com> Cc: Platform Driver , Chris Healy , Guenter Roeck , Darren Hart , open list From: Florian Fainelli Message-ID: Date: Tue, 24 Jan 2017 12:07:50 -0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.5.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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