Hi Rohit,
Thanks for the new version!
I have few more comments below.
On 05/23/2013 03:29 AM, Rohit Vaswani 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>
> ---
> .../devicetree/bindings/gpio/gpio-msm.txt | 26 +++
> arch/arm/boot/dts/msm8660-surf.dts | 11 ++
> arch/arm/boot/dts/msm8960-cdp.dts | 11 ++
> drivers/gpio/Kconfig | 2 +-
> drivers/gpio/gpio-msm-v2.c | 168 +++++++++++++-------
> 5 files changed, 157 insertions(+), 61 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/gpio/gpio-msm.txt
>
<cut>
> static int msm_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> {
> - return MSM_GPIO_TO_INT(chip->base + offset);
> + struct msm_gpio_dev *g_dev = to_msm_gpio_dev(chip);
> + struct irq_domain *domain = g_dev->domain;
Could you add blank line here.
> + return irq_create_mapping(domain, offset);
> }
>
> static inline int msm_irq_to_gpio(struct gpio_chip *chip, unsigned irq)
> {
> - return irq - MSM_GPIO_TO_INT(chip->base);
> + struct irq_data *irq_data = irq_get_irq_data(irq);
Blank line please.
> + return irq_data->hwirq;
> }
>
> static struct msm_gpio_dev msm_gpio = {
> .gpio_chip = {
> .base = 0,
> - .ngpio = NR_GPIO_IRQS,
> .direction_input = msm_gpio_direction_input,
> .direction_output = msm_gpio_direction_output,
> .get = msm_gpio_get,
> @@ -226,9 +234,9 @@ static void msm_gpio_update_dual_edge_pos(unsigned gpio)
> if (intstat || val == val2)
> return;
> } while (loop_limit-- > 0);
> - pr_err("dual-edge irq failed to stabilize, "
> + pr_err("%s: dual-edge irq failed to stabilize, "
> "interrupts dropped. %#08x != %#08x\n",
> - val, val2);
> + __func__, val, val2);
> }
>
> static void msm_gpio_irq_ack(struct irq_data *d)
> @@ -312,10 +320,11 @@ static void msm_summary_irq_handler(unsigned int irq, struct irq_desc *desc)
> {
> unsigned long i;
> struct irq_chip *chip = irq_desc_get_chip(desc);
> + int ngpio = msm_gpio.gpio_chip.ngpio;
>
> chained_irq_enter(chip, desc);
>
> - for_each_set_bit(i, msm_gpio.enabled_irqs, NR_GPIO_IRQS) {
> + for_each_set_bit(i, msm_gpio.enabled_irqs, ngpio) {
> if (readl(GPIO_INTR_STATUS(i)) & BIT(INTR_STATUS))
> generic_handle_irq(msm_gpio_to_irq(&msm_gpio.gpio_chip,
> i));
As you decided to call irq_create_mapping() from msm_gpio_to_irq, then
in irq handler you must call irq_find_mapping() as stated in
IRQ-domain.txt and probably check return value.
<cut>
> +static int msm_gpio_irqdomain_init(struct device_node *node, int ngpio)
> +{
> + msm_gpio.domain = irq_domain_add_linear(node, ngpio,
> + &msm_gpio_irq_domain_ops, &msm_gpio);
> + if (!msm_gpio.domain)
> + return -ENOMEM;
> +
> + return 0;
> +}
> +
This function seems meaningless, could you call irq_domain_add_linear
from .probe directly?
- Stan
--
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
