Hi,

Some nits below. Overall looks good.

On Tue, Oct 11, 2011 at 02:23:57PM -0600, Stephen Warren wrote:
> @@ -76,16 +75,20 @@ struct tegra_gpio_bank {
>  };
>  
>  
> -static struct tegra_gpio_bank tegra_gpio_banks[] = {
> -     {.bank = 0, .irq = INT_GPIO1},
> -     {.bank = 1, .irq = INT_GPIO2},
> -     {.bank = 2, .irq = INT_GPIO3},
> -     {.bank = 3, .irq = INT_GPIO4},
> -     {.bank = 4, .irq = INT_GPIO5},
> -     {.bank = 5, .irq = INT_GPIO6},
> -     {.bank = 6, .irq = INT_GPIO7},
> +void __iomem *regs;

Should be static.

> +static struct tegra_gpio_bank tegra_gpio_banks[7] = {
>  };

Can be on the same line as the rest, i.e.  ... = { };

> +static inline void tegra_gpio_writel(u32 val, u32 reg)
> +{
> +     __raw_writel(val, regs + reg);
> +}
> +
> +static inline u32 tegra_gpio_readl(u32 reg)
> +{
> +     return __raw_readl(regs + reg);
> +}
> +
>  static int tegra_gpio_compose(int bank, int port, int bit)
>  {
>       return (bank << 5) | ((port & 0x3) << 3) | (bit & 0x7);

[...]

> @@ -333,28 +336,55 @@ static struct irq_chip tegra_gpio_irq_chip = {
>   */
>  static struct lock_class_key gpio_lock_class;
>  
> -static int __init tegra_gpio_init(void)
> +static int __init tegra_gpio_probe(struct platform_device *pdev)
>  {
> +     struct resource *res;
>       struct tegra_gpio_bank *bank;
>       int gpio;
>       int i;
>       int j;
>  
> +     for (i = 0; i < ARRAY_SIZE(tegra_gpio_banks); i++) {
> +             res = platform_get_resource(pdev, IORESOURCE_IRQ, i);
> +             if (!res) {
> +                     dev_err(&pdev->dev, "Missing IRQ resource\n");
> +                     return -ENODEV;
> +             }
> +
> +             bank = &tegra_gpio_banks[i];
> +             bank->bank = i;
> +             bank->irq = res->start;
> +     }
> +
> +     res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> +     if (!res) {
> +             dev_err(&pdev->dev, "Missing MEM resource\n");
> +             return -ENODEV;
> +     }
> +
> +     if (!devm_request_mem_region(&pdev->dev, res->start,
> +                                  resource_size(res),
> +                                  dev_name(&pdev->dev))) {
> +             dev_err(&pdev->dev, "Couldn't request MEM resource\n");
> +             return -ENODEV;
> +     }
> +
> +     regs = devm_ioremap(&pdev->dev, res->start, resource_size(res));
> +     if (!regs) {
> +             dev_err(&pdev->dev, "Couldn't ioremap regs\n");
> +             return -ENODEV;

Should you release the mem region requested above here?


-Olof
--
To unsubscribe from this list: send the line "unsubscribe linux-tegra" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to