Re: [PATCH v2 4/6] [POWERPC] QE: implement support for the GPIO LIB API
Anton Vorontsov wrote: No. The spec indeed says that 0 for low, non-0 for high. Fair enough. -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2 4/6] [POWERPC] QE: implement support for the GPIO LIB API
Anton Vorontsov wrote: This is needed to access QE GPIOs via Linux GPIO API. Signed-off-by: Anton Vorontsov [EMAIL PROTECTED] Acked-By: Timur Tabi [EMAIL PROTECTED] -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2 4/6] [POWERPC] QE: implement support for the GPIO LIB API
Anton Vorontsov wrote:
+ Example:
+ qe_pio_a: [EMAIL PROTECTED] {
+ #gpio-cells = 2;
+ compatible = fsl,mpc8360-qe-pario-bank,
+ fsl,mpc8323-qe-pario-bank;
I know this is an example, but would we ever include both compatible strings in
one DTS? Isn't the norm something like:
fsl,mpc8360-qe-pario-bank, fsl,qe-pario-bank;
I.e. specific version, then generic version?
Otherwise, every time we introduce a new QE part, we'll have to update *every*
QE-enabled device tree *and* the QE gpio driver.
+static int qe_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct qe_pio_regs __iomem *regs = mm_gc-regs;
+ u32 pin_mask = 1 (QE_PIO_PINS - 1 - gpio);
+
+ return in_be32(regs-cpdata) pin_mask;
+}
Return value should be u32, not int.
--
Timur Tabi
Linux kernel developer at Freescale
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2 4/6] [POWERPC] QE: implement support for the GPIO LIB API
On Wed, Apr 30, 2008 at 02:42:03PM -0500, Timur Tabi wrote:
Anton Vorontsov wrote:
+ Example:
+ qe_pio_a: [EMAIL PROTECTED] {
+ #gpio-cells = 2;
+ compatible = fsl,mpc8360-qe-pario-bank,
+fsl,mpc8323-qe-pario-bank;
I know this is an example, but would we ever include both compatible strings
in
one DTS? Isn't the norm something like:
fsl,mpc8360-qe-pario-bank, fsl,qe-pario-bank;
I.e. specific version, then generic version?
Per Grant Likely's comments, fsl,qe-pario-bank considered as a made up
stuff. MPC8323 is the first QE chip, so every next QE chips should be
compatible.
Though, for fsl,gtm we can't use this, since we really want generic
name.
Otherwise, every time we introduce a new QE part, we'll have to update *every*
QE-enabled device tree *and* the QE gpio driver.
Nope. If the the QE is compatible with the previous, just write
fsl,newchip-qe-pario-bank, fsl,mpc8323-qe-pario-bank.
Btw, the same we do for the 8349 compatible PCI.
+static int qe_gpio_get(struct gpio_chip *gc, unsigned int gpio)
+{
+ struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc);
+ struct qe_pio_regs __iomem *regs = mm_gc-regs;
+ u32 pin_mask = 1 (QE_PIO_PINS - 1 - gpio);
+
+ return in_be32(regs-cpdata) pin_mask;
+}
Return value should be u32, not int.
gpio.h disagree
$ cat include/asm-generic/gpio.h | grep \*get
int (*get)(struct gpio_chip *chip,
--
Anton Vorontsov
email: [EMAIL PROTECTED]
irc://irc.freenode.net/bd2
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2 4/6] [POWERPC] QE: implement support for the GPIO LIB API
Anton Vorontsov wrote: Per Grant Likely's comments, fsl,qe-pario-bank considered as a made up stuff. MPC8323 is the first QE chip, so every next QE chips should be compatible. Ok. Return value should be u32, not int. gpio.h disagree $ cat include/asm-generic/gpio.h | grep \*get int (*get)(struct gpio_chip *chip, In that case, perhaps you should return this: return in_be32(regs-cpdata) pin_mask ? 1 : 0; -- Timur Tabi Linux kernel developer at Freescale ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2 4/6] [POWERPC] QE: implement support for the GPIO LIB API
On Wed, Apr 30, 2008 at 05:50:42PM -0500, Timur Tabi wrote: Anton Vorontsov wrote: Per Grant Likely's comments, fsl,qe-pario-bank considered as a made up stuff. MPC8323 is the first QE chip, so every next QE chips should be compatible. Ok. Return value should be u32, not int. gpio.h disagree $ cat include/asm-generic/gpio.h | grep \*get int (*get)(struct gpio_chip *chip, In that case, perhaps you should return this: return in_be32(regs-cpdata) pin_mask ? 1 : 0; What is the problem with returning (int)(u32 u32) value? You've asked to remove !! stuff and now purposing exactly the same... :-? -- Anton Vorontsov email: [EMAIL PROTECTED] irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v2 4/6] [POWERPC] QE: implement support for the GPIO LIB API
On Wed, Apr 30, 2008 at 06:03:45PM -0500, Timur Tabi wrote: Anton Vorontsov wrote: What is the problem with returning (int)(u32 u32) value? Technically, a signed int is smaller than an unsigned int, so a value of 0x8000 won't fit in an 'int'. They're of the same size. And because of that, we'll not lose information, so (int)(0x8000U) still != 0, it is just negative. You've asked to remove !! stuff and now purposing exactly the same... :-? The !! stuff was because the function was returning a boolean value, so non-zero == TRUE. I don't think the return value from 'get' is a technically a boolean, so I'm assuming that the spec says the return value should be 0 or 1, to reflect the value of that GPIO pin. No. The spec indeed says that 0 for low, non-0 for high. -- Anton Vorontsov email: [EMAIL PROTECTED] irc://irc.freenode.net/bd2 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
