Messages in this thread | | | Date | Mon, 10 Jun 2013 15:46:31 -0700 | From | Rohit Vaswani <> | 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@codeaurora.org> wrote: >> On 6/1/2013 1:09 AM, Grant Likely wrote: >>> On Sat, Jun 1, 2013 at 1:22 AM, Rohit Vaswani <rvaswani@codeaurora.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@codeaurora.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
| |