Re: [PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
On Tue, Feb 10, 2015 at 7:24 AM, Haojian Zhuang wrote: > On Tue, Feb 3, 2015 at 9:21 PM, Linus Walleij > wrote: >> On Wed, Jan 28, 2015 at 3:30 AM, Chao Xie wrote: >> >>> From: Chao Xie >>> >>> For some old PXA series, they used PXA GPIO driver. >>> The IP of GPIO changes since PXA988 which is Marvell MMP >>> series. >>> It will use new way to control the GPIO level, direction >>> and edge status. >>> >>> Signed-off-by: Chao Xie >> >> (...) >> > > Just a silly question. What's the meaning of (...)? There're a lot of > comments as (...). It's my first time to see this. It just means I deleted and skipped over a few lines in the patch that was looking OK or irrelevant to comment on, so as to keep the mail to just the relevant parts. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
On Tue, Feb 10, 2015 at 7:24 AM, Haojian Zhuang haojian.zhu...@gmail.com wrote: On Tue, Feb 3, 2015 at 9:21 PM, Linus Walleij linus.wall...@linaro.org wrote: On Wed, Jan 28, 2015 at 3:30 AM, Chao Xie chao@marvell.com wrote: From: Chao Xie chao@marvell.com For some old PXA series, they used PXA GPIO driver. The IP of GPIO changes since PXA988 which is Marvell MMP series. It will use new way to control the GPIO level, direction and edge status. Signed-off-by: Chao Xie chao@marvell.com (...) Just a silly question. What's the meaning of (...)? There're a lot of comments as (...). It's my first time to see this. It just means I deleted and skipped over a few lines in the patch that was looking OK or irrelevant to comment on, so as to keep the mail to just the relevant parts. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
On Tue, Feb 3, 2015 at 9:21 PM, Linus Walleij wrote: > On Wed, Jan 28, 2015 at 3:30 AM, Chao Xie wrote: > >> From: Chao Xie >> >> For some old PXA series, they used PXA GPIO driver. >> The IP of GPIO changes since PXA988 which is Marvell MMP >> series. >> It will use new way to control the GPIO level, direction >> and edge status. >> >> Signed-off-by: Chao Xie > > (...) > Just a silly question. What's the meaning of (...)? There're a lot of comments as (...). It's my first time to see this. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
On Tue, Feb 3, 2015 at 9:21 PM, Linus Walleij linus.wall...@linaro.org wrote: On Wed, Jan 28, 2015 at 3:30 AM, Chao Xie chao@marvell.com wrote: From: Chao Xie chao@marvell.com For some old PXA series, they used PXA GPIO driver. The IP of GPIO changes since PXA988 which is Marvell MMP series. It will use new way to control the GPIO level, direction and edge status. Signed-off-by: Chao Xie chao@marvell.com (...) Just a silly question. What's the meaning of (...)? There're a lot of comments as (...). It's my first time to see this. -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
On Wed, Feb 4, 2015 at 3:10 AM, Chao Xie wrote: > At 2015-02-03 21:21:43, "Linus Walleij" wrote: >>Since this looks like a basic MMIO driver I think >>you should also use: >> >>select GPIO_GENERIC >> > > I think the gpio-mmp is not same as gpio-generic. > gpio-mmp need control the level/direction/rising edge detect enable/falling > edge detect enable. > For each of them, there are three registers: status register, setting > register and clear register. This is quite common. Notice that GPIO_GENERIC does not require you to use the library for *all* reading/writing of registers. Just those that you select. > For example, for direction, if you configure it as output. > You need SET the bit in setting register. The library function bgpio_dir_out first calls gc->set() (which may also be a generic implementation) and then sets the direction bit. This is the nominal behaviour. > If you want to configure it as input > You need SET the bit in clear register. OK this is different from the default implementation so override the dirin function to get the right behaviour. You may still use the library for everything else. > It is same for level/rising edege detect enable/falling edge detect enable. irqchip is completely orthogonal and implemented separately in the struct irq_chip callbacks. > If you want to read the status of the pin. For example, the current level of > the pin. > You CAN NOT read the setting/clear register. You need read > the level status register. That's cool. You pass a separate register for reading. Check how it actually works. >>> +#define mmp_gpio_to_bank_idx(gpio) ((gpio) >> BANK_GPIO_ORDER) >>> +#define mmp_gpio_to_bank_offset(gpio) ((gpio) & BANK_GPIO_MASK) >>> +#define mmp_bank_to_gpio(idx, offset) (((idx) << BANK_GPIO_ORDER) \ >>> + | ((offset) & >>> BANK_GPIO_MASK)) >> >>This looks convoluted. Why not just register each bank as a separate >>device instead of trying to figure out bank index like this? >> > > There are the following reasons > 1. There is only one IRQ for the whole GPIO, even there are 3 or more banks. > 2. The registers are not formatted into group. They are interleaved. > For example, there are three banks, So for the registers order are > LEVEL_STATUS_BANK0, LEVEL_STASTUS_BANK1, LEVEL_STATUS_BANK2 > DIRECTION_STATUS_BANK0, DIRECTION_STATUS_BANK1, DIRECTION_STATUS_BANK2 > 3. each bank has 32 bits. Formatting them into one driver will make other > driver easier. > For example, the MMC driver has GPIO detection for card. So it knows the > GPIO is GPIO56. > In the device tree of MMC driver, you can simple add as > cd-gpios = < 56 X> > if you format them into different devices, the mmc driver owner need to > know how much bits a bank is, and calculate out correct GPIOx and offset > cd-gpios = < 24 X> OK I buy this, atleast points 1 & 2. I think you can still use GPIOLIB_IRQCHIP though, since it's just one linear array of GPIO numbers. Yours, Linus Walleij -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: Re: [PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
On Wed, Feb 4, 2015 at 3:10 AM, Chao Xie xiechao_m...@163.com wrote: At 2015-02-03 21:21:43, Linus Walleij linus.wall...@linaro.org wrote: Since this looks like a basic MMIO driver I think you should also use: select GPIO_GENERIC I think the gpio-mmp is not same as gpio-generic. gpio-mmp need control the level/direction/rising edge detect enable/falling edge detect enable. For each of them, there are three registers: status register, setting register and clear register. This is quite common. Notice that GPIO_GENERIC does not require you to use the library for *all* reading/writing of registers. Just those that you select. For example, for direction, if you configure it as output. You need SET the bit in setting register. The library function bgpio_dir_out first calls gc-set() (which may also be a generic implementation) and then sets the direction bit. This is the nominal behaviour. If you want to configure it as input You need SET the bit in clear register. OK this is different from the default implementation so override the dirin function to get the right behaviour. You may still use the library for everything else. It is same for level/rising edege detect enable/falling edge detect enable. irqchip is completely orthogonal and implemented separately in the struct irq_chip callbacks. If you want to read the status of the pin. For example, the current level of the pin. You CAN NOT read the setting/clear register. You need read the level status register. That's cool. You pass a separate register for reading. Check how it actually works. +#define mmp_gpio_to_bank_idx(gpio) ((gpio) BANK_GPIO_ORDER) +#define mmp_gpio_to_bank_offset(gpio) ((gpio) BANK_GPIO_MASK) +#define mmp_bank_to_gpio(idx, offset) (((idx) BANK_GPIO_ORDER) \ + | ((offset) BANK_GPIO_MASK)) This looks convoluted. Why not just register each bank as a separate device instead of trying to figure out bank index like this? There are the following reasons 1. There is only one IRQ for the whole GPIO, even there are 3 or more banks. 2. The registers are not formatted into group. They are interleaved. For example, there are three banks, So for the registers order are LEVEL_STATUS_BANK0, LEVEL_STASTUS_BANK1, LEVEL_STATUS_BANK2 DIRECTION_STATUS_BANK0, DIRECTION_STATUS_BANK1, DIRECTION_STATUS_BANK2 3. each bank has 32 bits. Formatting them into one driver will make other driver easier. For example, the MMC driver has GPIO detection for card. So it knows the GPIO is GPIO56. In the device tree of MMC driver, you can simple add as cd-gpios = gpio 56 X if you format them into different devices, the mmc driver owner need to know how much bits a bank is, and calculate out correct GPIOx and offset cd-gpios = gpio1 24 X OK I buy this, atleast points 1 2. I think you can still use GPIOLIB_IRQCHIP though, since it's just one linear array of GPIO numbers. Yours, Linus Walleij -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re:Re: [PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
At 2015-02-03 21:21:43, "Linus Walleij" wrote:
>On Wed, Jan 28, 2015 at 3:30 AM, Chao Xie wrote:
>
>> From: Chao Xie
>>
>> For some old PXA series, they used PXA GPIO driver.
>> The IP of GPIO changes since PXA988 which is Marvell MMP
>> series.
>> It will use new way to control the GPIO level, direction
>> and edge status.
>>
>> Signed-off-by: Chao Xie
>
>(...)
>
>> +config GPIO_MMP
>> + bool "MMP GPIO support"
>> + depends on ARCH_MMP
>
>All new simple drivers with IRQ should
>
>select GPIOLIB_IRQCHIP
>
Thanks for review.
I will check GPIOLIB_IRQCHIP and make use of it.
>Since this looks like a basic MMIO driver I think
>you should also use:
>
>select GPIO_GENERIC
>
I think the gpio-mmp is not same as gpio-generic.
gpio-mmp need control the level/direction/rising edge detect enable/falling
edge detect enable.
For each of them, there are three registers: status register, setting register
and clear register.
For example, for direction, if you configure it as output.
You need SET the bit in setting register.
If you want to configure it as input
You need SET the bit in clear register.
The bits will be cleared by hardware if the operation is completed by hardware.
It is same for level/rising edege detect enable/falling edge detect enable.
If you want to read the status of the pin. For example, the current level of
the pin.
You CAN NOT read the setting/clear register. You need read the level status
register.
>And set up simple getter/setter functions with a
>
>
>> + help
>> + Say yes here to support the MMP GPIO device at
>> PXA1088/PXA1908/PXA1928.
>> + Comparing with PXA GPIO device, the IP of MMP GPIO changes a lot.
>> +
>
>(...)
>
>> +++ b/drivers/gpio/gpio-mmp.c
>
>> +#include
>> +#include
>> +#include
>
>You don't need this include with GPIOLIB_IRQCHIP
>
I will fix it.
>> +#include
>> +#include
>> +#include
>
>#include
>
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>
>get rid of these two includes in favor of using GPIOLIB_IRQCHIP
>
I will fix it.
>> +#include
>
>Add:
>#include
>
>And implement generic GPIO using bgpio_init()
>
>(...)
The reasons are listed above
>> +#define GPLR 0x0
>> +#define GPDR 0xc
>> +#define GPSR 0x18
>> +#define GPCR 0x24
>> +#define GRER 0x30
>> +#define GFER 0x3c
>> +#define GEDR 0x48
>> +#define GSDR 0x54
>> +#define GCDR 0x60
>> +#define GSRER 0x6c
>> +#define GCRER 0x78
>> +#define GSFER 0x84
>> +#define GCFER 0x90
>> +#define GAPMASK0x9c
>> +#define GCPMASK0xa8
>> +
>> +/* Bank will have 2^n GPIOes, and for mmp-gpio n = 5 */
>> +#define BANK_GPIO_ORDER5
>> +#define BANK_GPIO_NUMBER (1 << BANK_GPIO_ORDER)
>> +#define BANK_GPIO_MASK (BANK_GPIO_NUMBER - 1)
>> +
>> +#define mmp_gpio_to_bank_idx(gpio) ((gpio) >> BANK_GPIO_ORDER)
>> +#define mmp_gpio_to_bank_offset(gpio) ((gpio) & BANK_GPIO_MASK)
>> +#define mmp_bank_to_gpio(idx, offset) (((idx) << BANK_GPIO_ORDER) \
>> + | ((offset) &
>> BANK_GPIO_MASK))
>> +
>
>This looks convoluted. Why not just register each bank as a separate
>device instead of trying to figure out bank index like this?
>
There are the following reasons
1. There is only one IRQ for the whole GPIO, even there are 3 or more banks.
2. The registers are not formatted into group. They are interleaved.
For example, there are three banks, So for the registers order are
LEVEL_STATUS_BANK0, LEVEL_STASTUS_BANK1, LEVEL_STATUS_BANK2
DIRECTION_STATUS_BANK0, DIRECTION_STATUS_BANK1, DIRECTION_STATUS_BANK2
3. each bank has 32 bits. Formatting them into one driver will make other
driver easier.
For example, the MMC driver has GPIO detection for card. So it knows the
GPIO is GPIO56.
In the device tree of MMC driver, you can simple add as
cd-gpios = < 56 X>
if you format them into different devices, the mmc driver owner need to
know how much bits a bank is, and calculate out correct GPIOx and offset
cd-gpios = < 24 X>
>> +struct mmp_gpio_bank {
>> + void __iomem *reg_bank;
>> + u32 irq_mask;
>> + u32 irq_rising_edge;
>> + u32 irq_falling_edge;
>> +};
>> +
>> +struct mmp_gpio_chip {
>> + struct gpio_chip chip;
>
>That should then be
>struct bgpio_chip bgc;
>
>For generic GPIO.
>
>> + void __iomem *reg_base;
>> + int irq;
>> + struct irq_domain *domain;
>
>These two will not be necessary to keep around with
>GPIOLIB_IRQCHIP
>
>> + unsigned int ngpio;
>
>This is part of struct gpio_chip so do not duplicate it.
>
>> + unsigned int nbank;
>> + struct mmp_gpio_bank *banks;
>
>And those two I think you should get rid of by creating one
>chip per bank.
>
>> +};
>
>So merge these two into one struct and instantiate one device for
Re: [PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
On Wed, Jan 28, 2015 at 3:30 AM, Chao Xie wrote:
> From: Chao Xie
>
> For some old PXA series, they used PXA GPIO driver.
> The IP of GPIO changes since PXA988 which is Marvell MMP
> series.
> It will use new way to control the GPIO level, direction
> and edge status.
>
> Signed-off-by: Chao Xie
(...)
> +config GPIO_MMP
> + bool "MMP GPIO support"
> + depends on ARCH_MMP
All new simple drivers with IRQ should
select GPIOLIB_IRQCHIP
Since this looks like a basic MMIO driver I think
you should also use:
select GPIO_GENERIC
And set up simple getter/setter functions with a
> + help
> + Say yes here to support the MMP GPIO device at
> PXA1088/PXA1908/PXA1928.
> + Comparing with PXA GPIO device, the IP of MMP GPIO changes a lot.
> +
(...)
> +++ b/drivers/gpio/gpio-mmp.c
> +#include
> +#include
> +#include
You don't need this include with GPIOLIB_IRQCHIP
> +#include
> +#include
> +#include
#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
get rid of these two includes in favor of using GPIOLIB_IRQCHIP
> +#include
Add:
#include
And implement generic GPIO using bgpio_init()
(...)
> +#define GPLR 0x0
> +#define GPDR 0xc
> +#define GPSR 0x18
> +#define GPCR 0x24
> +#define GRER 0x30
> +#define GFER 0x3c
> +#define GEDR 0x48
> +#define GSDR 0x54
> +#define GCDR 0x60
> +#define GSRER 0x6c
> +#define GCRER 0x78
> +#define GSFER 0x84
> +#define GCFER 0x90
> +#define GAPMASK0x9c
> +#define GCPMASK0xa8
> +
> +/* Bank will have 2^n GPIOes, and for mmp-gpio n = 5 */
> +#define BANK_GPIO_ORDER5
> +#define BANK_GPIO_NUMBER (1 << BANK_GPIO_ORDER)
> +#define BANK_GPIO_MASK (BANK_GPIO_NUMBER - 1)
> +
> +#define mmp_gpio_to_bank_idx(gpio) ((gpio) >> BANK_GPIO_ORDER)
> +#define mmp_gpio_to_bank_offset(gpio) ((gpio) & BANK_GPIO_MASK)
> +#define mmp_bank_to_gpio(idx, offset) (((idx) << BANK_GPIO_ORDER) \
> + | ((offset) & BANK_GPIO_MASK))
> +
This looks convoluted. Why not just register each bank as a separate
device instead of trying to figure out bank index like this?
> +struct mmp_gpio_bank {
> + void __iomem *reg_bank;
> + u32 irq_mask;
> + u32 irq_rising_edge;
> + u32 irq_falling_edge;
> +};
> +
> +struct mmp_gpio_chip {
> + struct gpio_chip chip;
That should then be
struct bgpio_chip bgc;
For generic GPIO.
> + void __iomem *reg_base;
> + int irq;
> + struct irq_domain *domain;
These two will not be necessary to keep around with
GPIOLIB_IRQCHIP
> + unsigned int ngpio;
This is part of struct gpio_chip so do not duplicate it.
> + unsigned int nbank;
> + struct mmp_gpio_bank *banks;
And those two I think you should get rid of by creating one
chip per bank.
> +};
So merge these two into one struct and instantiate one device for
each bank.
> +static int mmp_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
> +{
> + struct mmp_gpio_chip *mmp_chip =
> + container_of(chip, struct mmp_gpio_chip, chip);
> +
> + return irq_create_mapping(mmp_chip->domain, offset);
> +}
This function goes away with GPIOLIB_IRQCHIP.
Just leave it unassigned and let the core handle this translation.
> +static int mmp_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
> +{
> + struct mmp_gpio_chip *mmp_chip =
> + container_of(chip, struct mmp_gpio_chip, chip);
Create a static inline to cast the gpio_chip to a mmp_chip like
this:
static inline struct mmp_gpio_chip *to_mmp(struct gpio_chip *gc)
{
return container_of(chip, struct mmp_gpio_chip, chip);
}
Use that everywhere to simplify.
> + struct mmp_gpio_bank *bank =
> + _chip->banks[mmp_gpio_to_bank_idx(offset)];
> + u32 bit = (1 << mmp_gpio_to_bank_offset(offset));
And get rid of this by using one device per bank.
> +static int mmp_gpio_direction_output(struct gpio_chip *chip,
> +static int mmp_gpio_get(struct gpio_chip *chip, unsigned offset)
> +static void mmp_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
Looks like generic GPIO will do the job.
> +#ifdef CONFIG_OF_GPIO
> +static int mmp_gpio_of_xlate(struct gpio_chip *chip,
> +const struct of_phandle_args *gpiospec,
> +u32 *flags)
> +{
> + struct mmp_gpio_chip *mmp_chip =
> + container_of(chip, struct mmp_gpio_chip, chip);
> +
> + /* GPIO index start from 0. */
> + if (gpiospec->args[0] >= mmp_chip->ngpio)
> + return -EINVAL;
> +
> + if (flags)
> + *flags = gpiospec->args[1];
> +
> + return gpiospec->args[0];
> +}
> +#endif
This also goes to the generic xlate
Re:Re: [PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
At 2015-02-03 21:21:43, Linus Walleij linus.wall...@linaro.org wrote:
On Wed, Jan 28, 2015 at 3:30 AM, Chao Xie chao@marvell.com wrote:
From: Chao Xie chao@marvell.com
For some old PXA series, they used PXA GPIO driver.
The IP of GPIO changes since PXA988 which is Marvell MMP
series.
It will use new way to control the GPIO level, direction
and edge status.
Signed-off-by: Chao Xie chao@marvell.com
(...)
+config GPIO_MMP
+ bool MMP GPIO support
+ depends on ARCH_MMP
All new simple drivers with IRQ should
select GPIOLIB_IRQCHIP
Thanks for review.
I will check GPIOLIB_IRQCHIP and make use of it.
Since this looks like a basic MMIO driver I think
you should also use:
select GPIO_GENERIC
I think the gpio-mmp is not same as gpio-generic.
gpio-mmp need control the level/direction/rising edge detect enable/falling
edge detect enable.
For each of them, there are three registers: status register, setting register
and clear register.
For example, for direction, if you configure it as output.
You need SET the bit in setting register.
If you want to configure it as input
You need SET the bit in clear register.
The bits will be cleared by hardware if the operation is completed by hardware.
It is same for level/rising edege detect enable/falling edge detect enable.
If you want to read the status of the pin. For example, the current level of
the pin.
You CAN NOT read the setting/clear register. You need read the level status
register.
And set up simple getter/setter functions with a
+ help
+ Say yes here to support the MMP GPIO device at
PXA1088/PXA1908/PXA1928.
+ Comparing with PXA GPIO device, the IP of MMP GPIO changes a lot.
+
(...)
+++ b/drivers/gpio/gpio-mmp.c
+#include linux/err.h
+#include linux/init.h
+#include linux/irq.h
You don't need this include with GPIOLIB_IRQCHIP
I will fix it.
+#include linux/interrupt.h
+#include linux/io.h
+#include linux/gpio.h
#include linux/gpio/driver.h
+#include linux/clk.h
+#include linux/of_device.h
+#include linux/platform_device.h
+#include linux/module.h
+#include linux/irqdomain.h
+#include linux/irqchip/chained_irq.h
get rid of these two includes in favor of using GPIOLIB_IRQCHIP
I will fix it.
+#include linux/platform_data/gpio-mmp.h
Add:
#include linux/basic_mmio_gpio.h
And implement generic GPIO using bgpio_init()
(...)
The reasons are listed above
+#define GPLR 0x0
+#define GPDR 0xc
+#define GPSR 0x18
+#define GPCR 0x24
+#define GRER 0x30
+#define GFER 0x3c
+#define GEDR 0x48
+#define GSDR 0x54
+#define GCDR 0x60
+#define GSRER 0x6c
+#define GCRER 0x78
+#define GSFER 0x84
+#define GCFER 0x90
+#define GAPMASK0x9c
+#define GCPMASK0xa8
+
+/* Bank will have 2^n GPIOes, and for mmp-gpio n = 5 */
+#define BANK_GPIO_ORDER5
+#define BANK_GPIO_NUMBER (1 BANK_GPIO_ORDER)
+#define BANK_GPIO_MASK (BANK_GPIO_NUMBER - 1)
+
+#define mmp_gpio_to_bank_idx(gpio) ((gpio) BANK_GPIO_ORDER)
+#define mmp_gpio_to_bank_offset(gpio) ((gpio) BANK_GPIO_MASK)
+#define mmp_bank_to_gpio(idx, offset) (((idx) BANK_GPIO_ORDER) \
+ | ((offset)
BANK_GPIO_MASK))
+
This looks convoluted. Why not just register each bank as a separate
device instead of trying to figure out bank index like this?
There are the following reasons
1. There is only one IRQ for the whole GPIO, even there are 3 or more banks.
2. The registers are not formatted into group. They are interleaved.
For example, there are three banks, So for the registers order are
LEVEL_STATUS_BANK0, LEVEL_STASTUS_BANK1, LEVEL_STATUS_BANK2
DIRECTION_STATUS_BANK0, DIRECTION_STATUS_BANK1, DIRECTION_STATUS_BANK2
3. each bank has 32 bits. Formatting them into one driver will make other
driver easier.
For example, the MMC driver has GPIO detection for card. So it knows the
GPIO is GPIO56.
In the device tree of MMC driver, you can simple add as
cd-gpios = gpio 56 X
if you format them into different devices, the mmc driver owner need to
know how much bits a bank is, and calculate out correct GPIOx and offset
cd-gpios = gpio1 24 X
+struct mmp_gpio_bank {
+ void __iomem *reg_bank;
+ u32 irq_mask;
+ u32 irq_rising_edge;
+ u32 irq_falling_edge;
+};
+
+struct mmp_gpio_chip {
+ struct gpio_chip chip;
That should then be
struct bgpio_chip bgc;
For generic GPIO.
+ void __iomem *reg_base;
+ int irq;
+ struct irq_domain *domain;
These two will not be necessary to keep around with
GPIOLIB_IRQCHIP
+ unsigned int ngpio;
This is part of struct gpio_chip so do not duplicate it.
+ unsigned int nbank;
+ struct mmp_gpio_bank *banks;
And those two I think you
Re: [PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
On Wed, Jan 28, 2015 at 3:30 AM, Chao Xie chao@marvell.com wrote:
From: Chao Xie chao@marvell.com
For some old PXA series, they used PXA GPIO driver.
The IP of GPIO changes since PXA988 which is Marvell MMP
series.
It will use new way to control the GPIO level, direction
and edge status.
Signed-off-by: Chao Xie chao@marvell.com
(...)
+config GPIO_MMP
+ bool MMP GPIO support
+ depends on ARCH_MMP
All new simple drivers with IRQ should
select GPIOLIB_IRQCHIP
Since this looks like a basic MMIO driver I think
you should also use:
select GPIO_GENERIC
And set up simple getter/setter functions with a
+ help
+ Say yes here to support the MMP GPIO device at
PXA1088/PXA1908/PXA1928.
+ Comparing with PXA GPIO device, the IP of MMP GPIO changes a lot.
+
(...)
+++ b/drivers/gpio/gpio-mmp.c
+#include linux/err.h
+#include linux/init.h
+#include linux/irq.h
You don't need this include with GPIOLIB_IRQCHIP
+#include linux/interrupt.h
+#include linux/io.h
+#include linux/gpio.h
#include linux/gpio/driver.h
+#include linux/clk.h
+#include linux/of_device.h
+#include linux/platform_device.h
+#include linux/module.h
+#include linux/irqdomain.h
+#include linux/irqchip/chained_irq.h
get rid of these two includes in favor of using GPIOLIB_IRQCHIP
+#include linux/platform_data/gpio-mmp.h
Add:
#include linux/basic_mmio_gpio.h
And implement generic GPIO using bgpio_init()
(...)
+#define GPLR 0x0
+#define GPDR 0xc
+#define GPSR 0x18
+#define GPCR 0x24
+#define GRER 0x30
+#define GFER 0x3c
+#define GEDR 0x48
+#define GSDR 0x54
+#define GCDR 0x60
+#define GSRER 0x6c
+#define GCRER 0x78
+#define GSFER 0x84
+#define GCFER 0x90
+#define GAPMASK0x9c
+#define GCPMASK0xa8
+
+/* Bank will have 2^n GPIOes, and for mmp-gpio n = 5 */
+#define BANK_GPIO_ORDER5
+#define BANK_GPIO_NUMBER (1 BANK_GPIO_ORDER)
+#define BANK_GPIO_MASK (BANK_GPIO_NUMBER - 1)
+
+#define mmp_gpio_to_bank_idx(gpio) ((gpio) BANK_GPIO_ORDER)
+#define mmp_gpio_to_bank_offset(gpio) ((gpio) BANK_GPIO_MASK)
+#define mmp_bank_to_gpio(idx, offset) (((idx) BANK_GPIO_ORDER) \
+ | ((offset) BANK_GPIO_MASK))
+
This looks convoluted. Why not just register each bank as a separate
device instead of trying to figure out bank index like this?
+struct mmp_gpio_bank {
+ void __iomem *reg_bank;
+ u32 irq_mask;
+ u32 irq_rising_edge;
+ u32 irq_falling_edge;
+};
+
+struct mmp_gpio_chip {
+ struct gpio_chip chip;
That should then be
struct bgpio_chip bgc;
For generic GPIO.
+ void __iomem *reg_base;
+ int irq;
+ struct irq_domain *domain;
These two will not be necessary to keep around with
GPIOLIB_IRQCHIP
+ unsigned int ngpio;
This is part of struct gpio_chip so do not duplicate it.
+ unsigned int nbank;
+ struct mmp_gpio_bank *banks;
And those two I think you should get rid of by creating one
chip per bank.
+};
So merge these two into one struct and instantiate one device for
each bank.
+static int mmp_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+ struct mmp_gpio_chip *mmp_chip =
+ container_of(chip, struct mmp_gpio_chip, chip);
+
+ return irq_create_mapping(mmp_chip-domain, offset);
+}
This function goes away with GPIOLIB_IRQCHIP.
Just leave it unassigned and let the core handle this translation.
+static int mmp_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ struct mmp_gpio_chip *mmp_chip =
+ container_of(chip, struct mmp_gpio_chip, chip);
Create a static inline to cast the gpio_chip to a mmp_chip like
this:
static inline struct mmp_gpio_chip *to_mmp(struct gpio_chip *gc)
{
return container_of(chip, struct mmp_gpio_chip, chip);
}
Use that everywhere to simplify.
+ struct mmp_gpio_bank *bank =
+ mmp_chip-banks[mmp_gpio_to_bank_idx(offset)];
+ u32 bit = (1 mmp_gpio_to_bank_offset(offset));
And get rid of this by using one device per bank.
+static int mmp_gpio_direction_output(struct gpio_chip *chip,
+static int mmp_gpio_get(struct gpio_chip *chip, unsigned offset)
+static void mmp_gpio_set(struct gpio_chip *chip, unsigned offset, int value)
Looks like generic GPIO will do the job.
+#ifdef CONFIG_OF_GPIO
+static int mmp_gpio_of_xlate(struct gpio_chip *chip,
+const struct of_phandle_args *gpiospec,
+u32 *flags)
+{
+ struct mmp_gpio_chip *mmp_chip =
+ container_of(chip, struct mmp_gpio_chip, chip);
+
+ /* GPIO index start from 0. */
+ if (gpiospec-args[0] =
Re: [PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
On Wed, Jan 28, 2015 at 8:00 AM, Chao Xie wrote:
> From: Chao Xie
>
> For some old PXA series, they used PXA GPIO driver.
> The IP of GPIO changes since PXA988 which is Marvell MMP
> series.
> It will use new way to control the GPIO level, direction
> and edge status.
>
> Signed-off-by: Chao Xie
> ---
> drivers/gpio/Kconfig| 7 +
> drivers/gpio/Makefile | 1 +
> drivers/gpio/gpio-mmp.c | 444
>
> 3 files changed, 452 insertions(+)
> create mode 100644 drivers/gpio/gpio-mmp.c
(...)
> +static int mmp_gpio_probe(struct platform_device *pdev)
> +{
> + struct device *dev = >dev;
> + struct mmp_gpio_platform_data *pdata;
> + struct device_node *np;
> + struct mmp_gpio_chip *mmp_chip;
> + struct mmp_gpio_bank *bank;
> + struct resource *res;
> + struct irq_domain *domain;
> + struct clk *clk;
> + int irq, i, ret;
> + void __iomem *base;
> +
> + pdata = dev_get_platdata(dev);
> + np = pdev->dev.of_node;
> + if (!np && !pdata)
> + return -EINVAL;
> +
> + mmp_chip = devm_kzalloc(dev, sizeof(*mmp_chip), GFP_KERNEL);
> + if (mmp_chip == NULL)
> + return -ENOMEM;
> +
Using ! operator preffred instead of comparing with NULL.
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0)
> + return irq;
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + if (!res)
> + return -EINVAL;
This check is not required.. Check on resource happenning with
devm_ioremap_resource.
> + base = devm_ioremap_resource(dev, res);
> + if (!base)
> + return -EINVAL;
I dont think this the correct return value.. return PTR_ERR(base)
> +
> + mmp_chip->irq = irq;
> + mmp_chip->reg_base = base;
> +
> + if (pdata)
> + ret = mmp_gpio_probe_pdata(pdev, mmp_chip, pdata);
> + else
> + ret = mmp_gpio_probe_dt(pdev, mmp_chip);
> +
> + if (ret) {
> + dev_err(dev, "Fail to initialize gpio unit, error %d.\n",
> ret);
> + return ret;
> + }
> +
> + clk = devm_clk_get(dev, NULL);
> + if (IS_ERR(clk)) {
> + dev_err(dev, "Fail to get gpio clock, error %ld.\n",
> + PTR_ERR(clk));
> + return PTR_ERR(clk);
> + }
> + ret = clk_prepare_enable(clk);
> + if (ret) {
> + dev_err(dev, "Fail to enable gpio clock, error %d.\n", ret);
> + return ret;
> + }
> +
> + domain = irq_domain_add_linear(np, mmp_chip->ngpio,
> + _gpio_irq_domain_ops, mmp_chip);
> + if (domain == NULL)
Using ! operator preferred instead of comparing with NULL.
--
Thanks and Regards,
Varka Bhadram.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
From: Chao Xie
For some old PXA series, they used PXA GPIO driver.
The IP of GPIO changes since PXA988 which is Marvell MMP
series.
It will use new way to control the GPIO level, direction
and edge status.
Signed-off-by: Chao Xie
---
drivers/gpio/Kconfig| 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-mmp.c | 444
3 files changed, 452 insertions(+)
create mode 100644 drivers/gpio/gpio-mmp.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 633ec21..c1e4e27 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -197,6 +197,13 @@ config GPIO_F7188X
To compile this driver as a module, choose M here: the module will
be called f7188x-gpio.
+config GPIO_MMP
+ bool "MMP GPIO support"
+ depends on ARCH_MMP
+ help
+ Say yes here to support the MMP GPIO device at
PXA1088/PXA1908/PXA1928.
+ Comparing with PXA GPIO device, the IP of MMP GPIO changes a lot.
+
config GPIO_MOXART
bool "MOXART GPIO support"
depends on ARCH_MOXART
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 81755f1..60a7cf5 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
obj-$(CONFIG_GPIO_MCP23S08)+= gpio-mcp23s08.o
obj-$(CONFIG_GPIO_ML_IOH) += gpio-ml-ioh.o
obj-$(CONFIG_GPIO_MM_LANTIQ) += gpio-mm-lantiq.o
+obj-$(CONFIG_GPIO_MMP) += gpio-mmp.o
obj-$(CONFIG_GPIO_MOXART) += gpio-moxart.o
obj-$(CONFIG_GPIO_MPC5200) += gpio-mpc5200.o
obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o
diff --git a/drivers/gpio/gpio-mmp.c b/drivers/gpio/gpio-mmp.c
new file mode 100644
index 000..63e0766
--- /dev/null
+++ b/drivers/gpio/gpio-mmp.c
@@ -0,0 +1,444 @@
+/*
+ * Copyright (C) 2014 Marvell International Ltd.
+ *
+ * Generic MMP GPIO handling
+ *
+ * Chao Xie
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define GPLR 0x0
+#define GPDR 0xc
+#define GPSR 0x18
+#define GPCR 0x24
+#define GRER 0x30
+#define GFER 0x3c
+#define GEDR 0x48
+#define GSDR 0x54
+#define GCDR 0x60
+#define GSRER 0x6c
+#define GCRER 0x78
+#define GSFER 0x84
+#define GCFER 0x90
+#define GAPMASK0x9c
+#define GCPMASK0xa8
+
+/* Bank will have 2^n GPIOes, and for mmp-gpio n = 5 */
+#define BANK_GPIO_ORDER5
+#define BANK_GPIO_NUMBER (1 << BANK_GPIO_ORDER)
+#define BANK_GPIO_MASK (BANK_GPIO_NUMBER - 1)
+
+#define mmp_gpio_to_bank_idx(gpio) ((gpio) >> BANK_GPIO_ORDER)
+#define mmp_gpio_to_bank_offset(gpio) ((gpio) & BANK_GPIO_MASK)
+#define mmp_bank_to_gpio(idx, offset) (((idx) << BANK_GPIO_ORDER) \
+ | ((offset) & BANK_GPIO_MASK))
+
+struct mmp_gpio_bank {
+ void __iomem *reg_bank;
+ u32 irq_mask;
+ u32 irq_rising_edge;
+ u32 irq_falling_edge;
+};
+
+struct mmp_gpio_chip {
+ struct gpio_chip chip;
+ void __iomem *reg_base;
+ int irq;
+ struct irq_domain *domain;
+ unsigned int ngpio;
+ unsigned int nbank;
+ struct mmp_gpio_bank *banks;
+};
+
+static int mmp_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+ struct mmp_gpio_chip *mmp_chip =
+ container_of(chip, struct mmp_gpio_chip, chip);
+
+ return irq_create_mapping(mmp_chip->domain, offset);
+}
+
+static int mmp_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ struct mmp_gpio_chip *mmp_chip =
+ container_of(chip, struct mmp_gpio_chip, chip);
+ struct mmp_gpio_bank *bank =
+ _chip->banks[mmp_gpio_to_bank_idx(offset)];
+ u32 bit = (1 << mmp_gpio_to_bank_offset(offset));
+
+ writel(bit, bank->reg_bank + GCDR);
+
+ return 0;
+}
+
+static int mmp_gpio_direction_output(struct gpio_chip *chip,
+unsigned offset, int value)
+{
+ struct mmp_gpio_chip *mmp_chip =
+ container_of(chip, struct mmp_gpio_chip, chip);
+ struct mmp_gpio_bank *bank =
+ _chip->banks[mmp_gpio_to_bank_idx(offset)];
+ u32 bit = (1 << mmp_gpio_to_bank_offset(offset));
+
+ /* Set value first. */
+ writel(bit, bank->reg_bank + (value ? GPSR : GPCR));
+
+ writel(bit, bank->reg_bank + GSDR);
+
+ return 0;
+}
+
+static int mmp_gpio_get(struct gpio_chip *chip, unsigned offset)
+{
+ struct mmp_gpio_chip *mmp_chip =
+
[PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
From: Chao Xie chao@marvell.com
For some old PXA series, they used PXA GPIO driver.
The IP of GPIO changes since PXA988 which is Marvell MMP
series.
It will use new way to control the GPIO level, direction
and edge status.
Signed-off-by: Chao Xie chao@marvell.com
---
drivers/gpio/Kconfig| 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-mmp.c | 444
3 files changed, 452 insertions(+)
create mode 100644 drivers/gpio/gpio-mmp.c
diff --git a/drivers/gpio/Kconfig b/drivers/gpio/Kconfig
index 633ec21..c1e4e27 100644
--- a/drivers/gpio/Kconfig
+++ b/drivers/gpio/Kconfig
@@ -197,6 +197,13 @@ config GPIO_F7188X
To compile this driver as a module, choose M here: the module will
be called f7188x-gpio.
+config GPIO_MMP
+ bool MMP GPIO support
+ depends on ARCH_MMP
+ help
+ Say yes here to support the MMP GPIO device at
PXA1088/PXA1908/PXA1928.
+ Comparing with PXA GPIO device, the IP of MMP GPIO changes a lot.
+
config GPIO_MOXART
bool MOXART GPIO support
depends on ARCH_MOXART
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index 81755f1..60a7cf5 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -53,6 +53,7 @@ obj-$(CONFIG_GPIO_MC9S08DZ60) += gpio-mc9s08dz60.o
obj-$(CONFIG_GPIO_MCP23S08)+= gpio-mcp23s08.o
obj-$(CONFIG_GPIO_ML_IOH) += gpio-ml-ioh.o
obj-$(CONFIG_GPIO_MM_LANTIQ) += gpio-mm-lantiq.o
+obj-$(CONFIG_GPIO_MMP) += gpio-mmp.o
obj-$(CONFIG_GPIO_MOXART) += gpio-moxart.o
obj-$(CONFIG_GPIO_MPC5200) += gpio-mpc5200.o
obj-$(CONFIG_GPIO_MPC8XXX) += gpio-mpc8xxx.o
diff --git a/drivers/gpio/gpio-mmp.c b/drivers/gpio/gpio-mmp.c
new file mode 100644
index 000..63e0766
--- /dev/null
+++ b/drivers/gpio/gpio-mmp.c
@@ -0,0 +1,444 @@
+/*
+ * Copyright (C) 2014 Marvell International Ltd.
+ *
+ * Generic MMP GPIO handling
+ *
+ * Chao Xie chao@marvell.com
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include linux/err.h
+#include linux/init.h
+#include linux/irq.h
+#include linux/interrupt.h
+#include linux/io.h
+#include linux/gpio.h
+#include linux/clk.h
+#include linux/of_device.h
+#include linux/platform_device.h
+#include linux/module.h
+#include linux/irqdomain.h
+#include linux/irqchip/chained_irq.h
+#include linux/platform_data/gpio-mmp.h
+
+#define GPLR 0x0
+#define GPDR 0xc
+#define GPSR 0x18
+#define GPCR 0x24
+#define GRER 0x30
+#define GFER 0x3c
+#define GEDR 0x48
+#define GSDR 0x54
+#define GCDR 0x60
+#define GSRER 0x6c
+#define GCRER 0x78
+#define GSFER 0x84
+#define GCFER 0x90
+#define GAPMASK0x9c
+#define GCPMASK0xa8
+
+/* Bank will have 2^n GPIOes, and for mmp-gpio n = 5 */
+#define BANK_GPIO_ORDER5
+#define BANK_GPIO_NUMBER (1 BANK_GPIO_ORDER)
+#define BANK_GPIO_MASK (BANK_GPIO_NUMBER - 1)
+
+#define mmp_gpio_to_bank_idx(gpio) ((gpio) BANK_GPIO_ORDER)
+#define mmp_gpio_to_bank_offset(gpio) ((gpio) BANK_GPIO_MASK)
+#define mmp_bank_to_gpio(idx, offset) (((idx) BANK_GPIO_ORDER) \
+ | ((offset) BANK_GPIO_MASK))
+
+struct mmp_gpio_bank {
+ void __iomem *reg_bank;
+ u32 irq_mask;
+ u32 irq_rising_edge;
+ u32 irq_falling_edge;
+};
+
+struct mmp_gpio_chip {
+ struct gpio_chip chip;
+ void __iomem *reg_base;
+ int irq;
+ struct irq_domain *domain;
+ unsigned int ngpio;
+ unsigned int nbank;
+ struct mmp_gpio_bank *banks;
+};
+
+static int mmp_gpio_to_irq(struct gpio_chip *chip, unsigned offset)
+{
+ struct mmp_gpio_chip *mmp_chip =
+ container_of(chip, struct mmp_gpio_chip, chip);
+
+ return irq_create_mapping(mmp_chip-domain, offset);
+}
+
+static int mmp_gpio_direction_input(struct gpio_chip *chip, unsigned offset)
+{
+ struct mmp_gpio_chip *mmp_chip =
+ container_of(chip, struct mmp_gpio_chip, chip);
+ struct mmp_gpio_bank *bank =
+ mmp_chip-banks[mmp_gpio_to_bank_idx(offset)];
+ u32 bit = (1 mmp_gpio_to_bank_offset(offset));
+
+ writel(bit, bank-reg_bank + GCDR);
+
+ return 0;
+}
+
+static int mmp_gpio_direction_output(struct gpio_chip *chip,
+unsigned offset, int value)
+{
+ struct mmp_gpio_chip *mmp_chip =
+ container_of(chip, struct mmp_gpio_chip, chip);
+ struct mmp_gpio_bank *bank =
+ mmp_chip-banks[mmp_gpio_to_bank_idx(offset)];
+ u32 bit = (1 mmp_gpio_to_bank_offset(offset));
+
+ /* Set value first. */
+
Re: [PATCH] gpio: mmp: add GPIO driver for Marvell MMP series
On Wed, Jan 28, 2015 at 8:00 AM, Chao Xie chao@marvell.com wrote:
From: Chao Xie chao@marvell.com
For some old PXA series, they used PXA GPIO driver.
The IP of GPIO changes since PXA988 which is Marvell MMP
series.
It will use new way to control the GPIO level, direction
and edge status.
Signed-off-by: Chao Xie chao@marvell.com
---
drivers/gpio/Kconfig| 7 +
drivers/gpio/Makefile | 1 +
drivers/gpio/gpio-mmp.c | 444
3 files changed, 452 insertions(+)
create mode 100644 drivers/gpio/gpio-mmp.c
(...)
+static int mmp_gpio_probe(struct platform_device *pdev)
+{
+ struct device *dev = pdev-dev;
+ struct mmp_gpio_platform_data *pdata;
+ struct device_node *np;
+ struct mmp_gpio_chip *mmp_chip;
+ struct mmp_gpio_bank *bank;
+ struct resource *res;
+ struct irq_domain *domain;
+ struct clk *clk;
+ int irq, i, ret;
+ void __iomem *base;
+
+ pdata = dev_get_platdata(dev);
+ np = pdev-dev.of_node;
+ if (!np !pdata)
+ return -EINVAL;
+
+ mmp_chip = devm_kzalloc(dev, sizeof(*mmp_chip), GFP_KERNEL);
+ if (mmp_chip == NULL)
+ return -ENOMEM;
+
Using ! operator preffred instead of comparing with NULL.
+ irq = platform_get_irq(pdev, 0);
+ if (irq 0)
+ return irq;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ if (!res)
+ return -EINVAL;
This check is not required.. Check on resource happenning with
devm_ioremap_resource.
+ base = devm_ioremap_resource(dev, res);
+ if (!base)
+ return -EINVAL;
I dont think this the correct return value.. return PTR_ERR(base)
+
+ mmp_chip-irq = irq;
+ mmp_chip-reg_base = base;
+
+ if (pdata)
+ ret = mmp_gpio_probe_pdata(pdev, mmp_chip, pdata);
+ else
+ ret = mmp_gpio_probe_dt(pdev, mmp_chip);
+
+ if (ret) {
+ dev_err(dev, Fail to initialize gpio unit, error %d.\n,
ret);
+ return ret;
+ }
+
+ clk = devm_clk_get(dev, NULL);
+ if (IS_ERR(clk)) {
+ dev_err(dev, Fail to get gpio clock, error %ld.\n,
+ PTR_ERR(clk));
+ return PTR_ERR(clk);
+ }
+ ret = clk_prepare_enable(clk);
+ if (ret) {
+ dev_err(dev, Fail to enable gpio clock, error %d.\n, ret);
+ return ret;
+ }
+
+ domain = irq_domain_add_linear(np, mmp_chip-ngpio,
+ mmp_gpio_irq_domain_ops, mmp_chip);
+ if (domain == NULL)
Using ! operator preferred instead of comparing with NULL.
--
Thanks and Regards,
Varka Bhadram.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
