linux-kernel - Re: [PATCHv4 3/3] gpio: msm: Add device tree and irqdomain support for gpio-msm-v2

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: <51B65747.9030006@codeaurora.org>
Date:	Mon, 10 Jun 2013 15:46:31 -0700
From:	Rohit Vaswani <rvaswani@...eaurora.org>
To:	Grant Likely <grant.likely@...aro.org>
CC:	Linus Walleij <linus.walleij@...aro.org>,
	Rob Herring <rob.herring@...xeda.com>,
	Rob Landley <rob@...dley.net>,
	Russell King <linux@....linux.org.uk>,
	David Brown <davidb@...eaurora.org>,
	Bryan Huntsman <bryanh@...eaurora.org>,
	"linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	"linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>
Subject: Re: [PATCHv4 3/3] gpio: msm: Add device tree and irqdomain support
 for gpio-msm-v2

On 6/5/2013 2:38 PM, Grant Likely wrote:
> On Sun, 02 Jun 2013 19:31:33 -0700, Rohit Vaswani <rvaswani@...eaurora.org> wrote:
>> On 6/1/2013 1:09 AM, Grant Likely wrote:
>>> On Sat, Jun 1, 2013 at 1:22 AM, Rohit Vaswani <rvaswani@...eaurora.org> wrote:
>>>> This cleans up the gpio-msm-v2 driver of all the global define usage.
>>>> The number of gpios are now defined in the device tree. This enables
>>>> adding irqdomain support as well.
>>>>
>>>> Signed-off-by: Rohit Vaswani <rvaswani@...eaurora.org>
>>>> @@ -101,11 +96,27 @@ enum {
>>>>     */
>>>>    struct msm_gpio_dev {
>>>>           struct gpio_chip gpio_chip;
>>>> -       DECLARE_BITMAP(enabled_irqs, NR_GPIO_IRQS);
>>>> -       DECLARE_BITMAP(wake_irqs, NR_GPIO_IRQS);
>>>> -       DECLARE_BITMAP(dual_edge_irqs, NR_GPIO_IRQS);
>>>> +       unsigned long *enabled_irqs;
>>>> +       unsigned long *wake_irqs;
>>>> +       unsigned long *dual_edge_irqs;
>>> Was there a reason you ignored the comment to leave these bitmaps as
>>> statically allocated?
>>>
>>> [...]
>>>
>>>> +       msm_gpio.enabled_irqs = devm_kzalloc(&pdev->dev,
>>>> +                                               sizeof(unsigned long) *
>>>> +                                               BITS_TO_LONGS(ngpio) * 3,
>>>> +                                               GFP_KERNEL);
>>>> +       msm_gpio.wake_irqs = &msm_gpio.enabled_irqs[BITS_TO_LONGS(ngpio)];
>>>> +       msm_gpio.dual_edge_irqs =
>>>> +                       &msm_gpio.enabled_irqs[BITS_TO_LONGS(ngpio * 2)];
>>> I should have just deleted my comment about doing it this way. I was
>>> making the point that one allocation is better that three; but then I
>>> also said that it was better to not allocate at all. Go back to the
>>> statically allocated bitmap array. It is far better than this.
>>>
>>> g.
>> I agree that DECLARE_BITMAP is the most efficient way, but
>> DECLARE_BITMAP takes a statically defined number of gpios as an
>> argument. Since we get the ngpio from device tree, these had to go as
>> well. Under this scheme, 1 allocation was better than 3 and went ahead
>> with your suggestion.
> Think about it for a moment; You *know* all the devices that are going
> to use the driver. The largest size for ngpios that I see is 173 which
> is a mere 18 long words for 3 bitmaps. Even if you were to quadruple
> that amount the total for all the bitmasks is 72 long words. You're
> optimizing at the wrong place. Changing it to dynamic allocation makes
> the code slower, more complicated, and probably doesn't do anything to
> save on real memory consumption.
>
> The solution is simple; choose a maximum value of ngpios that the driver
> will likely need to work support. If a device appears with more GPIOs,
> then the driver should throw a warning and work with the maximum it can
> support. Part of enablement for the new device will be increasing the
> driver to handle more gpios.
>
> g.

Got it.

Thanks,
Rohit Vaswani

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, hosted by The Linux Foundation

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/