gpio: ep93xx: remove unused inline function
---
drivers/gpio/gpio-ep93xx.c +0 -5
@@ -62,11 +62,6 @@ static void ep93xx_gpio_update_int_params(unsigned port) EP93XX_GPIO_REG(int_en_register_offset[port])); } -static inline void ep93xx_gpio_int_mask (unsigned line) -{ - gpio_int_unmasked[line >>3 ] &= ~(1 << (line &7 )); -} -static void ep93xx_gpio_int_debounce (unsigned int irq,bool enable) {int line =irq_to_gpio (irq);
Ryan Mallon
Acked-byQuoted 12 lines · H Hartley Sweeten
Signed-off-by H Hartley Sweeten<[email protected]>Cc Ryan Mallon<[email protected]>Cc Grant Likely<[email protected]>---
drivers/gpio/gpio-ep93xx.c +0
-0@@ -62,11 +62,6 @@ static void ep93xx_gpio_update_int_params(unsigned port) EP93XX_GPIO_REG(int_en_register_offset[port]));
drivers/gpio/gpio-ep93xx.c static void ep93xx_gpio_update_int_params(unsigned port) · line 67
Quoted 10 lines
drivers/gpio/gpio-ep93xx.c +0
-5} -static inline void ep93xx_gpio_int_mask (unsigned line) -{ - gpio_int_unmasked[line>>3 ]&= ~(1 << (line&7 )); -} -static void ep93xx_gpio_int_debounce (unsigned int irq,bool enable) {int line =irq_to_gpio (irq);
Yup. Looks like it got factored directly into ep93xx_gpio_irq_mask.
Note that we have a lot of line >> 3 and (1 << (line & 7)) scattered through this file. We could do something like this (completely untested):
----
gpio-ep93xx: Add helper functions for irq_port and irq_port_mask
---
drivers/gpio/gpio-ep93xx.c +24 -18
@@ -45,6 +45,16 @@ static const u8 eoi_register_offset[3] = { 0x98, 0xb4, 0x54 };static const u8 int_en_register_offset[3 ] = {0x9c ,0xb8 ,0x58 };static const u8 int_debounce_register_offset[3 ] = {0xa8 ,0xc4 ,0x64 }; +static inline int irq_port_mask (unsigned int irq) +{ +return 1 << (irq_to_gpio (irq)&7 ); +} + +static inline int irq_port (unsigned int irq) +{ +return irq_to_gpio (irq)>>3 ; +} +static void ep93xx_gpio_update_int_params (unsigned port) {BUG_ON (port>2 );
@@ -68,9 +78,8 @@ static inline void ep93xx_gpio_int_mask(unsigned line)static void ep93xx_gpio_int_debounce (unsigned int irq,bool enable) { -int line =irq_to_gpio (irq); -int port = line>>3 ; -int port_mask =1 << (line&7 ); +int port =irq_port (irq); +int port_mask =irq_port_mask (irq);if (enable) gpio_int_debounce[port] |= port_mask;
@@ -118,9 +127,8 @@ static void ep93xx_gpio_f_irq_handler(unsigned int irq, struct irq_desc *desc)static void ep93xx_gpio_irq_ack (struct irq_data *d) { -int line =irq_to_gpio (d->irq); -int port = line>>3 ; -int port_mask =1 << (line&7 ); +int port =irq_port (irq); +int port_mask =irq_port_mask (irq);if (irqd_get_trigger_type (d) ==IRQ_TYPE_EDGE_BOTH ) { gpio_int_type2[port] ^= port_mask;/* switch edge direction */
@@ -132,9 +140,8 @@ static void ep93xx_gpio_irq_ack(struct irq_data *d)static void ep93xx_gpio_irq_mask_ack (struct irq_data *d) { -int line =irq_to_gpio (d->irq); -int port = line>>3 ; -int port_mask =1 << (line&7 ); +int port =irq_port (irq); +int port_mask =irq_port_mask (irq);if (irqd_get_trigger_type (d) ==IRQ_TYPE_EDGE_BOTH ) gpio_int_type2[port] ^= port_mask;/* switch edge direction */
@@ -147,19 +154,19 @@ static void ep93xx_gpio_irq_mask_ack(struct irq_data *d)static void ep93xx_gpio_irq_mask (struct irq_data *d) { -int line =irq_to_gpio (d->irq); -int port = line>>3 ; +int port =irq_port (irq); +int port_mask =irq_port_mask (irq); - gpio_int_unmasked[port]&= ~(1 << (line&7 )); + gpio_int_unmasked[port]&= ~port_mask;ep93xx_gpio_update_int_params (port); }static void ep93xx_gpio_irq_unmask (struct irq_data *d) { -int line =irq_to_gpio (d->irq); -int port = line>>3 ; +int port =irq_port (irq); +int port_mask =irq_port_mask (irq); - gpio_int_unmasked[port] |=1 << (line&7 ); + gpio_int_unmasked[port] |= port_mask;ep93xx_gpio_update_int_params (port); }
@@ -170,9 +177,8 @@ static void ep93xx_gpio_irq_unmask(struct irq_data *d)*/ static int ep93xx_gpio_irq_type (struct irq_data *d,unsigned int type) { -const int gpio =irq_to_gpio (d->irq); -const int port = gpio>>3 ; -const int port_mask =1 << (gpio&7 ); +const int port =irq_port (d->irq); +const int port_mask =irq_port_mask (d->irq);irq_flow_handler_t handler;gpio_direction_input (gpio);
Quoted 26 lines · Ryan Mallon
Quoted 12 lines · H Hartley Sweeten
Signed-off-by H Hartley Sweeten<[email protected]>Cc Ryan Mallon<[email protected]>Cc Grant Likely<[email protected]>---
drivers/gpio/gpio-ep93xx.c +0
-0@@ -62,11 +62,6 @@ static void ep93xx_gpio_update_int_params(unsigned port) EP93XX_GPIO_REG(int_en_register_offset[port]));drivers/gpio/gpio-ep93xx.c static void ep93xx_gpio_update_int_params(unsigned port) · line 67
Quoted 10 lines
drivers/gpio/gpio-ep93xx.c +0
-5} -static inline void ep93xx_gpio_int_mask (unsigned line) -{ - gpio_int_unmasked[line>>3 ]&= ~(1 << (line&7 )); -} -static void ep93xx_gpio_int_debounce (unsigned int irq,bool enable) {int line =irq_to_gpio (irq);Yup. Looks like it got factored directly into
ep93xx_gpio_irq_mask.
Quoted 1 line
Acked-by Ryan Mallon <[email protected]>
Thanks.
Quoted 2 lines
Note that we have a lot of line >> 3 and (1 << (line & 7)) scattered through this file. We could do something like this (completely untested):
Actually, I'm trying to figure out a way to use the generic-chip stuff to handle the gpio interrupts. I just haven't worked it out yet.
There seems to be a push to deprecate irq_to_gpio() so something will eventually have to be done...
Regards,
Hartley ���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m����������?�I�
Quoted 17 lines · Ryan Mallon
Note that we have a lot of line >> 3 and (1 << (line & 7)) scattered through this file. We could do something like this (completely untested):
----
gpio-ep93xx: Add helper functions forirq_portandirq_port_maskSigned-off-by Ryan Mallon <[email protected]>---
drivers/gpio/gpio-ep93xx.c +0
-0@@ -45,6 +45,16 @@ static const u8 eoi_register_offset[3] = { 0x98, 0xb4, 0x54 };static const u8 int_en_register_offset[3 ] = {0x9c ,0xb8 ,0x58 };static const u8 int_debounce_register_offset[3 ] = {0xa8 ,0xc4 ,0x64 };
drivers/gpio/gpio-ep93xx.c static const u8 eoi_register_offset[3] = { 0x98, 0xb4, 0x54 }; · line 57
Quoted 10 lines
drivers/gpio/gpio-ep93xx.c +10
-0+static inline int irq_port_mask (unsigned int irq) +{ +return 1 << (irq_to_gpio (irq)&7 ); +} + +static inline int irq_port (unsigned int irq) +{ +return irq_to_gpio (irq)>>3 ; +} +
Ryan,
If you _do_ want to consolidate the port/mask stuff how about something like this instead? This at least gets the irq_to_gpio() call in one place.
Regards,
Hartley
---
gpio: ep93xx: Add helper function to convert irq to gpio, port, and mask.
---
drivers/gpio/gpio-ep93xx.c +50 -37
@@ -46,6 +46,17 @@ static const u8 eoi_register_offset[3] = { 0x98, 0xb4, 0x54 };static const u8 int_en_register_offset[3 ] = {0x9c ,0xb8 ,0x58 };static const u8 int_debounce_register_offset[3 ] = {0xa8 ,0xc4 ,0x64 }; +static int ep93xx_gpio_irq_to_gpio_port_mask (unsigned int irq, +int *port,int *mask) +{ +int gpio =irq_to_gpio (irq); + + *port = gpio >>3 ; + *mask =1 << (gpio &7 ); + +return gpio; +} +static void ep93xx_gpio_update_int_params (unsigned port) {BUG_ON (port >2 );
@@ -69,14 +80,14 @@ static inline void ep93xx_gpio_int_mask(unsigned line)static void ep93xx_gpio_int_debounce (unsigned int irq,bool enable) { -int line =irq_to_gpio (irq); -int port = line >>3 ; -int port_mask =1 << (line &7 ); +int port, mask; + +ep93xx_gpio_irq_to_gpio_port_mask (irq, &port, &mask);if (enable) - gpio_int_debounce[port] |= port_mask; + gpio_int_debounce[port] |= mask;else - gpio_int_debounce[port] &= ~port_mask; + gpio_int_debounce[port] &= ~mask;__raw_writeb (gpio_int_debounce[port],EP93XX_GPIO_REG (int_debounce_register_offset[port]));
@@ -119,48 +130,50 @@ static void ep93xx_gpio_f_irq_handler(unsigned int irq, struct irq_desc *desc)static void ep93xx_gpio_irq_ack (struct irq_data *d) { -int line =irq_to_gpio (d->irq); -int port = line >>3 ; -int port_mask =1 << (line &7 ); +int port, mask; + +ep93xx_gpio_irq_to_gpio_port_mask (d->irq, &port, &mask);if (irqd_get_trigger_type (d) ==IRQ_TYPE_EDGE_BOTH ) { - gpio_int_type2[port] ^= port_mask;/* switch edge direction */ + gpio_int_type2[port] ^= mask;/* switch edge direction */ ep93xx_gpio_update_int_params (port); } -__raw_writeb (port_mask,EP93XX_GPIO_REG (eoi_register_offset[port])); +__raw_writeb (mask,EP93XX_GPIO_REG (eoi_register_offset[port])); }static void ep93xx_gpio_irq_mask_ack (struct irq_data *d) { -int line =irq_to_gpio (d->irq); -int port = line >>3 ; -int port_mask =1 << (line &7 ); +int port, mask; + +ep93xx_gpio_irq_to_gpio_port_mask (d->irq, &port, &mask);if (irqd_get_trigger_type (d) ==IRQ_TYPE_EDGE_BOTH ) - gpio_int_type2[port] ^= port_mask;/* switch edge direction */ + gpio_int_type2[port] ^= mask;/* switch edge direction */ - gpio_int_unmasked[port] &= ~port_mask; + gpio_int_unmasked[port] &= ~mask;ep93xx_gpio_update_int_params (port); -__raw_writeb (port_mask,EP93XX_GPIO_REG (eoi_register_offset[port])); +__raw_writeb (mask,EP93XX_GPIO_REG (eoi_register_offset[port])); }static void ep93xx_gpio_irq_mask (struct irq_data *d) { -int line =irq_to_gpio (d->irq); -int port = line >>3 ; +int port, mask; + +ep93xx_gpio_irq_to_gpio_port_mask (d->irq, &port, &mask); - gpio_int_unmasked[port] &= ~(1 << (line &7 )); + gpio_int_unmasked[port] &= ~mask;ep93xx_gpio_update_int_params (port); }static void ep93xx_gpio_irq_unmask (struct irq_data *d) { -int line =irq_to_gpio (d->irq); -int port = line >>3 ; +int port, mask; - gpio_int_unmasked[port] |=1 << (line &7 ); +ep93xx_gpio_irq_to_gpio_port_mask (d->irq, &port, &mask); + + gpio_int_unmasked[port] |= mask;ep93xx_gpio_update_int_params (port); }
@@ -171,41 +184,41 @@ static void ep93xx_gpio_irq_unmask(struct irq_data *d)*/ static int ep93xx_gpio_irq_type (struct irq_data *d,unsigned int type) { -const int gpio =irq_to_gpio (d->irq); -const int port = gpio >>3 ; -const int port_mask =1 << (gpio &7 ); +int gpio, port, mask;irq_flow_handler_t handler; + gpio =ep93xx_gpio_irq_to_gpio_port_mask (d->irq, &port, &mask); +gpio_direction_input (gpio);switch (type) {case IRQ_TYPE_EDGE_RISING : - gpio_int_type1[port] |= port_mask; - gpio_int_type2[port] |= port_mask; + gpio_int_type1[port] |= mask; + gpio_int_type2[port] |= mask; handler = handle_edge_irq;break ;case IRQ_TYPE_EDGE_FALLING : - gpio_int_type1[port] |= port_mask; - gpio_int_type2[port] &= ~port_mask; + gpio_int_type1[port] |= mask; + gpio_int_type2[port] &= ~mask; handler = handle_edge_irq;break ;case IRQ_TYPE_LEVEL_HIGH : - gpio_int_type1[port] &= ~port_mask; - gpio_int_type2[port] |= port_mask; + gpio_int_type1[port] &= ~mask; + gpio_int_type2[port] |= mask; handler = handle_level_irq;break ;case IRQ_TYPE_LEVEL_LOW : - gpio_int_type1[port] &= ~port_mask; - gpio_int_type2[port] &= ~port_mask; + gpio_int_type1[port] &= ~mask; + gpio_int_type2[port] &= ~mask; handler = handle_level_irq;break ;case IRQ_TYPE_EDGE_BOTH : - gpio_int_type1[port] |= port_mask; + gpio_int_type1[port] |= mask;/* set initial polarity based on current input level */ if (gpio_get_value (gpio)) - gpio_int_type2[port] &= ~port_mask;/* falling */ + gpio_int_type2[port] &= ~mask;/* falling */ else - gpio_int_type2[port] |= port_mask;/* rising */ + gpio_int_type2[port] |= mask;/* rising */ handler = handle_edge_irq;break ;default :
@@ -215,7 +228,7 @@ static int ep93xx_gpio_irq_type(struct irq_data *d, unsigned int type)__irq_set_handler_locked (d->irq, handler); - gpio_int_enabled[port] |= port_mask; + gpio_int_enabled[port] |= mask;ep93xx_gpio_update_int_params (port);
���{.n�+�������+%�����ݶ��w��{.n�+����{��G�����{ay�ʇڙ�,j��f���h��������z_�(�階�ݢj"���m������G������?����&���~��iO��z��v�^�m����������?�I�
Quoted 30 lines · H Hartley Sweeten
Quoted 17 lines · Ryan Mallon
Note that we have a lot of line>> 3 and (1<< (line& 7)) scattered through this file. We could do something like this (completely untested):
----
gpio-ep93xx: Add helper functions forirq_portandirq_port_maskSigned-off-by Ryan Mallon<[email protected]>---
drivers/gpio/gpio-ep93xx.c +0
-0@@ -45,6 +45,16 @@ static const u8 eoi_register_offset[3] = { 0x98, 0xb4, 0x54 };static const u8 int_en_register_offset[3 ] = {0x9c ,0xb8 ,0x58 };static const u8 int_debounce_register_offset[3 ] = {0xa8 ,0xc4 ,0x64 };drivers/gpio/gpio-ep93xx.c static const u8 eoi_register_offset[3] = { 0x98, 0xb4, 0x54 }; · line 57
Quoted 10 lines
drivers/gpio/gpio-ep93xx.c +10
-0+static inline int irq_port_mask (unsigned int irq) +{ +return 1 << (irq_to_gpio (irq)&7 ); +} + +static inline int irq_port (unsigned int irq) +{ +return irq_to_gpio (irq)>>3 ; +} +Ryan,
Quoted 3 lines
If you _do_ want to consolidate the port/mask stuff how about something like this instead? This at least gets the
irq_to_gpio()call in one place.
Either approach should be fine. Since the helper functions are inline the compiler should be smart enough to realise that it only needs to call irq_to_gpio once. You don't really need to rename port_mask to mask either, it just makes the patch more verbose than it needs to be.
If you want to put this through can you please add:
Thanks,
~Ryan
Quoted 186 lines
Regards,
Hartley---
gpio: ep93xx: Add helper function to convert irq to gpio, port, and mask.
Signed-off-by H Hartley Sweeten<[email protected]>---
drivers/gpio/gpio-ep93xx.c +50
-37@@ -46,6 +46,17 @@ static const u8 eoi_register_offset[3] = { 0x98, 0xb4, 0x54 };static const u8 int_en_register_offset[3 ] = {0x9c ,0xb8 ,0x58 };static const u8 int_debounce_register_offset[3 ] = {0xa8 ,0xc4 ,0x64 }; +static int ep93xx_gpio_irq_to_gpio_port_mask (unsigned int irq, +int *port,int *mask) +{ +int gpio =irq_to_gpio (irq); + + *port = gpio>>3 ; + *mask =1 << (gpio&7 ); + +return gpio; +} +static void ep93xx_gpio_update_int_params (unsigned port) {BUG_ON (port>2 );@@ -69,14 +80,14 @@ static inline void ep93xx_gpio_int_mask(unsigned line)static void ep93xx_gpio_int_debounce (unsigned int irq,bool enable) { -int line =irq_to_gpio (irq); -int port = line>>3 ; -int port_mask =1 << (line&7 ); +int port, mask; + +ep93xx_gpio_irq_to_gpio_port_mask (irq,&port,&mask);if (enable) - gpio_int_debounce[port] |= port_mask; + gpio_int_debounce[port] |= mask;else - gpio_int_debounce[port]&= ~port_mask; + gpio_int_debounce[port]&= ~mask;__raw_writeb (gpio_int_debounce[port],EP93XX_GPIO_REG (int_debounce_register_offset[port]));@@ -119,48 +130,50 @@ static void ep93xx_gpio_f_irq_handler(unsigned int irq, struct irq_desc *desc)static void ep93xx_gpio_irq_ack (struct irq_data *d) { -int line =irq_to_gpio (d->irq); -int port = line>>3 ; -int port_mask =1 << (line&7 ); +int port, mask; + +ep93xx_gpio_irq_to_gpio_port_mask (d->irq,&port,&mask);if (irqd_get_trigger_type (d) ==IRQ_TYPE_EDGE_BOTH ) { - gpio_int_type2[port] ^= port_mask;/* switch edge direction */ + gpio_int_type2[port] ^= mask;/* switch edge direction */ ep93xx_gpio_update_int_params (port); } -__raw_writeb (port_mask,EP93XX_GPIO_REG (eoi_register_offset[port])); +__raw_writeb (mask,EP93XX_GPIO_REG (eoi_register_offset[port])); }static void ep93xx_gpio_irq_mask_ack (struct irq_data *d) { -int line =irq_to_gpio (d->irq); -int port = line>>3 ; -int port_mask =1 << (line&7 ); +int port, mask; + +ep93xx_gpio_irq_to_gpio_port_mask (d->irq,&port,&mask);if (irqd_get_trigger_type (d) ==IRQ_TYPE_EDGE_BOTH ) - gpio_int_type2[port] ^= port_mask;/* switch edge direction */ + gpio_int_type2[port] ^= mask;/* switch edge direction */ - gpio_int_unmasked[port]&= ~port_mask; + gpio_int_unmasked[port]&= ~mask;ep93xx_gpio_update_int_params (port); -__raw_writeb (port_mask,EP93XX_GPIO_REG (eoi_register_offset[port])); +__raw_writeb (mask,EP93XX_GPIO_REG (eoi_register_offset[port])); }static void ep93xx_gpio_irq_mask (struct irq_data *d) { -int line =irq_to_gpio (d->irq); -int port = line>>3 ; +int port, mask; + +ep93xx_gpio_irq_to_gpio_port_mask (d->irq,&port,&mask); - gpio_int_unmasked[port]&= ~(1 << (line&7 )); + gpio_int_unmasked[port]&= ~mask;ep93xx_gpio_update_int_params (port); }static void ep93xx_gpio_irq_unmask (struct irq_data *d) { -int line =irq_to_gpio (d->irq); -int port = line>>3 ; +int port, mask; - gpio_int_unmasked[port] |=1 << (line&7 ); +ep93xx_gpio_irq_to_gpio_port_mask (d->irq,&port,&mask); + + gpio_int_unmasked[port] |= mask;ep93xx_gpio_update_int_params (port); }@@ -171,41 +184,41 @@ static void ep93xx_gpio_irq_unmask(struct irq_data *d)*/ static int ep93xx_gpio_irq_type (struct irq_data *d,unsigned int type) { -const int gpio =irq_to_gpio (d->irq); -const int port = gpio>>3 ; -const int port_mask =1 << (gpio&7 ); +int gpio, port, mask;irq_flow_handler_t handler; + gpio =ep93xx_gpio_irq_to_gpio_port_mask (d->irq,&port,&mask); +gpio_direction_input (gpio);switch (type) {case IRQ_TYPE_EDGE_RISING : - gpio_int_type1[port] |= port_mask; - gpio_int_type2[port] |= port_mask; + gpio_int_type1[port] |= mask; + gpio_int_type2[port] |= mask; handler = handle_edge_irq;break ;case IRQ_TYPE_EDGE_FALLING : - gpio_int_type1[port] |= port_mask; - gpio_int_type2[port]&= ~port_mask; + gpio_int_type1[port] |= mask; + gpio_int_type2[port]&= ~mask; handler = handle_edge_irq;break ;case IRQ_TYPE_LEVEL_HIGH : - gpio_int_type1[port]&= ~port_mask; - gpio_int_type2[port] |= port_mask; + gpio_int_type1[port]&= ~mask; + gpio_int_type2[port] |= mask; handler = handle_level_irq;break ;case IRQ_TYPE_LEVEL_LOW : - gpio_int_type1[port]&= ~port_mask; - gpio_int_type2[port]&= ~port_mask; + gpio_int_type1[port]&= ~mask; + gpio_int_type2[port]&= ~mask; handler = handle_level_irq;break ;case IRQ_TYPE_EDGE_BOTH : - gpio_int_type1[port] |= port_mask; + gpio_int_type1[port] |= mask;/* set initial polarity based on current input level */ if (gpio_get_value (gpio)) - gpio_int_type2[port]&= ~port_mask;/* falling */ + gpio_int_type2[port]&= ~mask;/* falling */ else - gpio_int_type2[port] |= port_mask;/* rising */ + gpio_int_type2[port] |= mask;/* rising */ handler = handle_edge_irq;break ;default :@@ -215,7 +228,7 @@ static int ep93xx_gpio_irq_type(struct irq_data *d, unsigned int type)__irq_set_handler_locked (d->irq, handler); - gpio_int_enabled[port] |= port_mask; + gpio_int_enabled[port] |= mask;ep93xx_gpio_update_int_params (port);