On 6/5/2013 2:38 PM, Grant Likely wrote:
On Sun, 02 Jun 2013 19:31:33 -0700, Rohit Vaswani <rvaswani@xxxxxxxxxxxxxx> wrote:
On 6/1/2013 1:09 AM, Grant Likely wrote:
On Sat, Jun 1, 2013 at 1:22 AM, Rohit Vaswani <rvaswani@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx>
@@ -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-doc" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
