date:20180519

Re: [PATCH v3 05/11] staging: mt7621-gpio: avoid use of globals and use platform_data instead

2018-05-19 Thread NeilBrown

On Sat, May 19 2018, Sergio Paracuellos wrote:

> On Sat, May 19, 2018 at 08:51:43PM +1000, NeilBrown wrote:
>> On Wed, May 16 2018, Sergio Paracuellos wrote:
>> 
>> > Gpio driver have a some globals which can be avoided just
>> > using platform_data in a proper form. This commit adds a new
>> > struct mtk_data which includes all of those globals setting them
>> > using platform_set_drvdata and devm_gpiochip_add_data functions.
>> > With this properly set we are able to retrieve driver data along
>> > the code using kernel api's so globals are not needed anymore.
>> >
>> > Signed-off-by: Sergio Paracuellos 
>> > ---
>> >  drivers/staging/mt7621-gpio/gpio-mt7621.c | 85 
>> > +--
>> >  1 file changed, 59 insertions(+), 26 deletions(-)
>> >
>> > diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c 
>> > b/drivers/staging/mt7621-gpio/gpio-mt7621.c
>> > index 7d17949..c701259 100644
>> > --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
>> > +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
>> > @@ -31,17 +31,20 @@ enum mediatek_gpio_reg {
>> >GPIO_REG_EDGE,
>> >  };
>> >  
>> > -static void __iomem *mediatek_gpio_membase;
>> > -static int mediatek_gpio_irq;
>> > -static struct irq_domain *mediatek_gpio_irq_domain;
>> > -
>> > -static struct mtk_gc {
>> > +struct mtk_gc {
>> >struct gpio_chip chip;
>> >spinlock_t lock;
>> >int bank;
>> >u32 rising;
>> >u32 falling;
>> > -} *gc_map[MTK_MAX_BANK];
>> > +};
>> > +
>> > +struct mtk_data {
>> > +  void __iomem *gpio_membase;
>> > +  int gpio_irq;
>> > +  struct irq_domain *gpio_irq_domain;
>> > +  struct mtk_gc *gc_map[MTK_MAX_BANK];
>> > +};
>> >  
>> >  static inline struct mtk_gc
>> >  *to_mediatek_gpio(struct gpio_chip *chip)
>> > @@ -56,15 +59,19 @@ static inline struct mtk_gc
>> >  static inline void
>> >  mtk_gpio_w32(struct mtk_gc *rg, u8 reg, u32 val)
>> >  {
>> > -  iowrite32(val, mediatek_gpio_membase + (reg * 0x10) + (rg->bank * 0x4));
>> > +  struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
>> > +  u32 offset = (reg * 0x10) + (rg->bank * 0x4);
>> > +
>> > +  iowrite32(val, gpio_data->gpio_membase + offset);
>> >  }
>> >  
>> >  static inline u32
>> >  mtk_gpio_r32(struct mtk_gc *rg, u8 reg)
>> >  {
>> > +  struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
>> >u32 offset = (reg * 0x10) + (rg->bank * 0x4);
>> >  
>> > -  return ioread32(mediatek_gpio_membase + offset);
>> > +  return ioread32(gpio_data->gpio_membase + offset);
>> >  }
>> >  
>> >  static void
>> > @@ -137,23 +144,26 @@ mediatek_gpio_get_direction(struct gpio_chip *chip, 
>> > unsigned int offset)
>> >  static int
>> >  mediatek_gpio_to_irq(struct gpio_chip *chip, unsigned int pin)
>> >  {
>> > +  struct mtk_data *gpio_data = gpiochip_get_data(chip);
>> >struct mtk_gc *rg = to_mediatek_gpio(chip);
>> >  
>> > -  return irq_create_mapping(mediatek_gpio_irq_domain,
>> > +  return irq_create_mapping(gpio_data->gpio_irq_domain,
>> >  pin + (rg->bank * MTK_BANK_WIDTH));
>> >  }
>> >  
>> >  static int
>> >  mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node 
>> > *bank)
>> >  {
>> > +  struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
>> >const __be32 *id = of_get_property(bank, "reg", NULL);
>> >struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
>> >sizeof(struct mtk_gc), GFP_KERNEL);
>> > +  int ret;
>> >  
>> >if (!rg || !id || be32_to_cpu(*id) > MTK_MAX_BANK)
>> >return -ENOMEM;
>> >  
>> > -  gc_map[be32_to_cpu(*id)] = rg;
>> > +  gpio_data->gc_map[be32_to_cpu(*id)] = rg;
>> >  
>> >memset(rg, 0, sizeof(struct mtk_gc));
>> >  
>> > @@ -169,10 +179,17 @@ mediatek_gpio_bank_probe(struct platform_device 
>> > *pdev, struct device_node *bank)
>> >rg->chip.get_direction = mediatek_gpio_get_direction;
>> >rg->chip.get = mediatek_gpio_get;
>> >rg->chip.set = mediatek_gpio_set;
>> > -  if (mediatek_gpio_irq_domain)
>> > +  if (gpio_data->gpio_irq_domain)
>> >rg->chip.to_irq = mediatek_gpio_to_irq;
>> >rg->bank = be32_to_cpu(*id);
>> >  
>> > +  ret = devm_gpiochip_add_data(&pdev->dev, &rg->chip, gpio_data);
>> > +  if (ret < 0) {
>> > +  dev_err(&pdev->dev, "Could not register gpio %d, ret=%d\n",
>> > +  rg->chip.ngpio, ret);
>> > +  return ret;
>> > +  }
>> > +
>> 
>> Calling devm_gpiochip_add_data() here looks good.
>> Not removing
>>  return gpiochip_add(&rg->chip);
>> from the end of the function isn't so good :-(
>
> True, thanks for pointing this out.
>
>> 
>> BTW interrupt definitely don't work yet.  The calls
>>  struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
>> get NULL.  I think some sort of irq_set_chip_data(irq,...) is needed
>> in mediatek_gpio_gpio_map(), but it is too late at night for it to
>> all make sense to me :-)
>
> You are right. It seems irq_set_chip_data must be called in 
> mediatek_gpio_gpio_map
> function.
>

I th

[PATCH] Staging: rtl8192e: rtllib_tx: fix spelling issue.

2018-05-19 Thread Davide spataro

Fix a spelling warning from checkpatch.pl.

Signed-off-by: Davide Spataro 
---
 drivers/staging/rtl8192e/rtllib_tx.c | 14 +++---
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/drivers/staging/rtl8192e/rtllib_tx.c 
b/drivers/staging/rtl8192e/rtllib_tx.c
index fc88d47dea43..d314b2f602e4 100644
--- a/drivers/staging/rtl8192e/rtllib_tx.c
+++ b/drivers/staging/rtl8192e/rtllib_tx.c
@@ -578,7 +578,7 @@ static int rtllib_xmit_inter(struct sk_buff *skb, struct 
net_device *dev)
.seq_ctl = 0,
.qos_ctl = 0
};
-   int qos_actived = ieee->current_network.qos_data.active;
+   int qos_activated = ieee->current_network.qos_data.active;
u8 dest[ETH_ALEN];
u8 src[ETH_ALEN];
struct lib80211_crypt_data *crypt = NULL;
@@ -684,7 +684,7 @@ static int rtllib_xmit_inter(struct sk_buff *skb, struct 
net_device *dev)
else
fc = RTLLIB_FTYPE_DATA;
 
-   if (qos_actived)
+   if (qos_activated)
fc |= RTLLIB_STYPE_QOS_DATA;
else
fc |= RTLLIB_STYPE_DATA;
@@ -727,7 +727,7 @@ static int rtllib_xmit_inter(struct sk_buff *skb, struct 
net_device *dev)
qos_ctl = 0;
}
 
-   if (qos_actived) {
+   if (qos_activated) {
hdr_len = RTLLIB_3ADDR_LEN + 2;
 
/* in case we are a client verify acm is not set for 
this ac */
@@ -788,7 +788,7 @@ static int rtllib_xmit_inter(struct sk_buff *skb, struct 
net_device *dev)
txb->encrypted = encrypt;
txb->payload_size = cpu_to_le16(bytes);
 
-   if (qos_actived)
+   if (qos_activated)
txb->queue_index = UP2AC(skb->priority);
else
txb->queue_index = WME_AC_BE;
@@ -797,7 +797,7 @@ static int rtllib_xmit_inter(struct sk_buff *skb, struct 
net_device *dev)
skb_frag = txb->fragments[i];
tcb_desc = (struct cb_desc *)(skb_frag->cb +
MAX_DEV_ADDR_SIZE);
-   if (qos_actived) {
+   if (qos_activated) {
skb_frag->priority = skb->priority;
tcb_desc->queue_index =  UP2AC(skb->priority);
} else {
@@ -831,7 +831,7 @@ static int rtllib_xmit_inter(struct sk_buff *skb, struct 
net_device *dev)
/* The last fragment has the remaining length */
bytes = bytes_last_frag;
}
-   if ((qos_actived) && (!bIsMulticast)) {
+   if ((qos_activated) && (!bIsMulticast)) {
frag_hdr->seq_ctl =
 cpu_to_le16(rtllib_query_seqnum(ieee, 
skb_frag,
 header.addr1));
@@ -866,7 +866,7 @@ static int rtllib_xmit_inter(struct sk_buff *skb, struct 
net_device *dev)
skb_put(skb_frag, 4);
}
 
-   if ((qos_actived) && (!bIsMulticast)) {
+   if ((qos_activated) && (!bIsMulticast)) {
if (ieee->seq_ctrl[UP2AC(skb->priority) + 1] == 0xFFF)
ieee->seq_ctrl[UP2AC(skb->priority) + 1] = 0;
else
-- 
2.17.0

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 05/11] staging: mt7621-gpio: avoid use of globals and use platform_data instead

2018-05-19 Thread Sergio Paracuellos

On Sat, May 19, 2018 at 08:51:43PM +1000, NeilBrown wrote:
> On Wed, May 16 2018, Sergio Paracuellos wrote:
> 
> > Gpio driver have a some globals which can be avoided just
> > using platform_data in a proper form. This commit adds a new
> > struct mtk_data which includes all of those globals setting them
> > using platform_set_drvdata and devm_gpiochip_add_data functions.
> > With this properly set we are able to retrieve driver data along
> > the code using kernel api's so globals are not needed anymore.
> >
> > Signed-off-by: Sergio Paracuellos 
> > ---
> >  drivers/staging/mt7621-gpio/gpio-mt7621.c | 85 
> > +--
> >  1 file changed, 59 insertions(+), 26 deletions(-)
> >
> > diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c 
> > b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > index 7d17949..c701259 100644
> > --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> > @@ -31,17 +31,20 @@ enum mediatek_gpio_reg {
> > GPIO_REG_EDGE,
> >  };
> >  
> > -static void __iomem *mediatek_gpio_membase;
> > -static int mediatek_gpio_irq;
> > -static struct irq_domain *mediatek_gpio_irq_domain;
> > -
> > -static struct mtk_gc {
> > +struct mtk_gc {
> > struct gpio_chip chip;
> > spinlock_t lock;
> > int bank;
> > u32 rising;
> > u32 falling;
> > -} *gc_map[MTK_MAX_BANK];
> > +};
> > +
> > +struct mtk_data {
> > +   void __iomem *gpio_membase;
> > +   int gpio_irq;
> > +   struct irq_domain *gpio_irq_domain;
> > +   struct mtk_gc *gc_map[MTK_MAX_BANK];
> > +};
> >  
> >  static inline struct mtk_gc
> >  *to_mediatek_gpio(struct gpio_chip *chip)
> > @@ -56,15 +59,19 @@ static inline struct mtk_gc
> >  static inline void
> >  mtk_gpio_w32(struct mtk_gc *rg, u8 reg, u32 val)
> >  {
> > -   iowrite32(val, mediatek_gpio_membase + (reg * 0x10) + (rg->bank * 0x4));
> > +   struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
> > +   u32 offset = (reg * 0x10) + (rg->bank * 0x4);
> > +
> > +   iowrite32(val, gpio_data->gpio_membase + offset);
> >  }
> >  
> >  static inline u32
> >  mtk_gpio_r32(struct mtk_gc *rg, u8 reg)
> >  {
> > +   struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
> > u32 offset = (reg * 0x10) + (rg->bank * 0x4);
> >  
> > -   return ioread32(mediatek_gpio_membase + offset);
> > +   return ioread32(gpio_data->gpio_membase + offset);
> >  }
> >  
> >  static void
> > @@ -137,23 +144,26 @@ mediatek_gpio_get_direction(struct gpio_chip *chip, 
> > unsigned int offset)
> >  static int
> >  mediatek_gpio_to_irq(struct gpio_chip *chip, unsigned int pin)
> >  {
> > +   struct mtk_data *gpio_data = gpiochip_get_data(chip);
> > struct mtk_gc *rg = to_mediatek_gpio(chip);
> >  
> > -   return irq_create_mapping(mediatek_gpio_irq_domain,
> > +   return irq_create_mapping(gpio_data->gpio_irq_domain,
> >   pin + (rg->bank * MTK_BANK_WIDTH));
> >  }
> >  
> >  static int
> >  mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node 
> > *bank)
> >  {
> > +   struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
> > const __be32 *id = of_get_property(bank, "reg", NULL);
> > struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
> > sizeof(struct mtk_gc), GFP_KERNEL);
> > +   int ret;
> >  
> > if (!rg || !id || be32_to_cpu(*id) > MTK_MAX_BANK)
> > return -ENOMEM;
> >  
> > -   gc_map[be32_to_cpu(*id)] = rg;
> > +   gpio_data->gc_map[be32_to_cpu(*id)] = rg;
> >  
> > memset(rg, 0, sizeof(struct mtk_gc));
> >  
> > @@ -169,10 +179,17 @@ mediatek_gpio_bank_probe(struct platform_device 
> > *pdev, struct device_node *bank)
> > rg->chip.get_direction = mediatek_gpio_get_direction;
> > rg->chip.get = mediatek_gpio_get;
> > rg->chip.set = mediatek_gpio_set;
> > -   if (mediatek_gpio_irq_domain)
> > +   if (gpio_data->gpio_irq_domain)
> > rg->chip.to_irq = mediatek_gpio_to_irq;
> > rg->bank = be32_to_cpu(*id);
> >  
> > +   ret = devm_gpiochip_add_data(&pdev->dev, &rg->chip, gpio_data);
> > +   if (ret < 0) {
> > +   dev_err(&pdev->dev, "Could not register gpio %d, ret=%d\n",
> > +   rg->chip.ngpio, ret);
> > +   return ret;
> > +   }
> > +
> 
> Calling devm_gpiochip_add_data() here looks good.
> Not removing
>   return gpiochip_add(&rg->chip);
> from the end of the function isn't so good :-(

True, thanks for pointing this out.

> 
> BTW interrupt definitely don't work yet.  The calls
>   struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
> get NULL.  I think some sort of irq_set_chip_data(irq,...) is needed
> in mediatek_gpio_gpio_map(), but it is too late at night for it to
> all make sense to me :-)

You are right. It seems irq_set_chip_data must be called in 
mediatek_gpio_gpio_map
function.

> 
> Thanks,
> NeilBrown

Sent v4 with this two changes.

Hope this helps.

Best regards,
Sergio Paracuellos

[PATCH v4 02/11] staging: mt7621-gpio: update TODO file

2018-05-19 Thread Sergio Paracuellos

This commit updates TODO file with missing things to
get this driver ready to be mainlined.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-gpio/TODO | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/mt7621-gpio/TODO b/drivers/staging/mt7621-gpio/TODO
index 7143905..9077b16 100644
--- a/drivers/staging/mt7621-gpio/TODO
+++ b/drivers/staging/mt7621-gpio/TODO
@@ -1,5 +1,7 @@
 
-- general code review and clean up
+- general code review and clean up 
+- avoid global variables and use drvdata allocated instead
 - ensure device-tree requirements are documented
+- make sure interrupts work
 
 Cc:  NeilBrown 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 04/11] staging: mt7621-gpio: replace 'mtk' to use correct one 'mediatek'

2018-05-19 Thread Sergio Paracuellos

Gpio driver is using mtk and there is already 'mediatek' binding
defined for this maker. Update driver to use it instead the custom
one 'mtk'.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c 
b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index a577381..7d17949 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -323,7 +323,7 @@ mediatek_gpio_probe(struct platform_device *pdev)
}
 
for_each_child_of_node(np, bank)
-   if (of_device_is_compatible(bank, "mtk,mt7621-gpio-bank"))
+   if (of_device_is_compatible(bank, "mediatek,mt7621-gpio-bank"))
mediatek_gpio_bank_probe(pdev, bank);
 
if (mediatek_gpio_irq_domain)
@@ -334,7 +334,7 @@ mediatek_gpio_probe(struct platform_device *pdev)
 }
 
 static const struct of_device_id mediatek_gpio_match[] = {
-   { .compatible = "mtk,mt7621-gpio" },
+   { .compatible = "mediatek,mt7621-gpio" },
{},
 };
 MODULE_DEVICE_TABLE(of, mediatek_gpio_match);
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 09/11] staging: mt7621-gpio: use ternary operator in return in mediatek_gpio_get_direction

2018-05-19 Thread Sergio Paracuellos

This commits replaces if statement and two returns in favour
of a only one return using a ternary operator.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c 
b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 05eab5a..a7b698b 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -135,10 +135,7 @@ mediatek_gpio_get_direction(struct gpio_chip *chip, 
unsigned int offset)
t = mtk_gpio_r32(rg, GPIO_REG_CTRL);
spin_unlock_irqrestore(&rg->lock, flags);
 
-   if (t & BIT(offset))
-   return 0;
-
-   return 1;
+   return (t & BIT(offset)) ? 0 : 1;
 }
 
 static int
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 01/11] staging: mt7621-gpio: dt-bindings: add documentation for mt7621-gpio

2018-05-19 Thread Sergio Paracuellos

This commit add missing dt bindings documentation for mt7621-gpio
driver. There is some missing stuff here about interrupts with is
not also being used in the mt7621.dtsi file. So just include in
staging a incomplete version before moving this to kernel's dt-bindings
place.

Signed-off-by: Sergio Paracuellos 
---
 .../staging/mt7621-gpio/mediatek,mt7621-gpio.txt   | 51 ++
 1 file changed, 51 insertions(+)
 create mode 100644 drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt

diff --git a/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt 
b/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
new file mode 100644
index 000..54de9f7
--- /dev/null
+++ b/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
@@ -0,0 +1,51 @@
+Mediatek SoC GPIO controller bindings
+
+The IP core used inside these SoCs has 3 banks of 32 GPIOs each.
+The registers of all the banks are interwoven inside one single IO range.
+We load one GPIO controller instance per bank. To make this possible
+we support 2 types of nodes. The parent node defines the memory I/O range and
+has 3 children each describing a single bank.
+
+Required properties for the top level node:
+- compatible:
+  - "mediatek,mt7621-gpio" for Mediatek controllers
+- reg : Physical base address and length of the controller's registers
+
+Required properties for the GPIO bank node:
+- compatible:
+  - "mediatek,mt7621-gpio-bank" for Mediatek banks
+- #gpio-cells : Should be two.
+  - first cell is the pin number
+  - second cell is used to specify optional parameters (unused)
+- gpio-controller : Marks the device node as a GPIO controller
+- reg : The id of the bank that the node describes.
+
+Example:
+   gpio@600 {
+   #address-cells = <1>;
+   #size-cells = <0>;
+
+   compatible = "mediatek,mt7621-gpio";
+   reg = <0x600 0x100>;
+
+   gpio0: bank@0 {
+   reg = <0>;
+   compatible = "mediatek,mt7621-gpio-bank";
+   gpio-controller;
+   #gpio-cells = <2>;
+   };
+
+   gpio1: bank@1 {
+   reg = <1>;
+   compatible = "mediatek,mt7621-gpio-bank";
+   gpio-controller;
+   #gpio-cells = <2>;
+   };
+
+   gpio2: bank@2 {
+   reg = <2>;
+   compatible = "mediatek,mt7621-gpio-bank";
+   gpio-controller;
+   #gpio-cells = <2>;
+   };
+   };
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 08/11] staging: mt7621-gpio: avoid devm_kzalloc() hidden inside declarations and refactor function a bit

2018-05-19 Thread Sergio Paracuellos

Driver probe function includes an allocation using devm_kzalloc
which is "hidden" a bit inside the declarations. Extract this
to a better place to increase readability. Also because we are
allocating zeroed memory 'memset' statement is not needed at all.
Condition for checking for a valid gpio id is wrong and it should
be greater or equal instead of only greater so update to be the
good one.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 11 ++-
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c 
b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 650286df..05eab5a 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -156,17 +156,18 @@ mediatek_gpio_bank_probe(struct platform_device *pdev, 
struct device_node *bank)
 {
struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
const __be32 *id = of_get_property(bank, "reg", NULL);
-   struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
-   sizeof(struct mtk_gc), GFP_KERNEL);
+   struct mtk_gc *rg;
int ret;
 
-   if (!rg || !id || be32_to_cpu(*id) > MTK_MAX_BANK)
+   if (!id || be32_to_cpu(*id) >= MTK_MAX_BANK)
+   return -EINVAL;
+
+   rg = devm_kzalloc(&pdev->dev, sizeof(struct mtk_gc), GFP_KERNEL);
+   if (!rg)
return -ENOMEM;
 
gpio_data->gc_map[be32_to_cpu(*id)] = rg;
 
-   memset(rg, 0, sizeof(struct mtk_gc));
-
spin_lock_init(&rg->lock);
 
rg->chip.parent = &pdev->dev;
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 00/11] staging: mt7621-gpio: use mediatek as binding instead of custom mtk

2018-05-19 Thread Sergio Paracuellos

The following series updated mt7621-gpio driver to use 'mediatek'
which is already defined in kernel bindings documentation instead
of add a new custom one 'mtk' for this company. mt7621-gpio device-tree
documentation has been added also to the staging driver directory.

v4:
 - Only PATCH 5 ("staging: mt7621-gpio: avoid use of globals and use 
   platform_data instead") has changes:
+ avoid call to gpiochip_add in probe function because function
  devm_gpiochip_add_data is being used instead.
+ Call irq_set_chip_data in mediatek_gpio_gpio_map to make data
  properly passed to interrupts related functions.

v3:
 - Fix condition for check for a valid gpio id in driver probe function
   included in the PATCH 8 ("staging: mt7621-gpio: avoid devm_kzalloc()
   hidden inside declarations and refactor function a bit") of the series.
 - Rest of patches are not modified at all.

v2:
 - Join some patch ("staging: mt7621-gpio: avoid use of globals
   and use platform_data instead") with the needed fixes to make
   it work pointed out by NeilBrown
 - Add gpio interrupts to documentation and device tree in order to
   can try them
 - Other minor code cleanups have been added also.

After this changes only testing interrupts should be remaining to
get this out of staging.

Hope this helps.

Best regards,
 Sergio

Sergio Paracuellos (11):
  staging: mt7621-gpio: dt-bindings: add documentation for mt7621-gpio
  staging: mt7621-gpio: update TODO file
  staging: mt7621-dts: update gpios related entries to use 'mediatek'
  staging: mt7621-gpio: replace 'mtk' to use correct one 'mediatek'
  staging: mt7621-gpio: avoid use of globals and use platform_data
instead
  staging: mt7621-dts: add interrupt device tree nodes for the gpio
controller
  staging: mt7621-gpio: dt-bindings: add interrupt nodes to bindings doc
  staging: mt7621-gpio: avoid devm_kzalloc() hidden inside declarations
and refactor function a bit
  staging: mt7621-gpio: use ternary operator in return in
mediatek_gpio_get_direction
  staging: mt7621-gpio: use MTK_BANK_WIDTH instead of magic number
  staging: mt7621-gpio: update TODO list

 drivers/staging/mt7621-dts/mt7621.dtsi |  11 +-
 drivers/staging/mt7621-gpio/TODO   |   3 +-
 drivers/staging/mt7621-gpio/gpio-mt7621.c  | 118 ++---
 .../staging/mt7621-gpio/mediatek,mt7621-gpio.txt   |  58 ++
 4 files changed, 143 insertions(+), 47 deletions(-)
 create mode 100644 drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 06/11] staging: mt7621-dts: add interrupt device tree nodes for the gpio controller

2018-05-19 Thread Sergio Paracuellos

The GPIO controller of mt7621 can receive interrupts on any
of the GPIOs, either edge or level. It then interrupts the CPU using
GIC INT12. Update device tree accordly.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-dts/mt7621.dtsi | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi 
b/drivers/staging/mt7621-dts/mt7621.dtsi
index 115eb04..240d396 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -67,6 +67,9 @@
compatible = "mediatek,mt7621-gpio";
reg = <0x600 0x100>;
 
+   interrupt-parent = <&gic>;
+   interrupts = ;
+
gpio0: bank@0 {
reg = <0>;
compatible = "mediatek,mt7621-gpio-bank";
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 10/11] staging: mt7621-gpio: use MTK_BANK_WIDTH instead of magic number

2018-05-19 Thread Sergio Paracuellos

There are some places where magic number '32' is being used to get
the gpio bank. There already exist a definition MTK_BANK_WIDTH
with this value, so just use it instead.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c 
b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index a7b698b..e412860 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -229,7 +229,7 @@ mediatek_gpio_irq_unmask(struct irq_data *d)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct mtk_data *gpio_data = gpiochip_get_data(gc);
int pin = d->hwirq;
-   int bank = pin / 32;
+   int bank = pin / MTK_BANK_WIDTH;
struct mtk_gc *rg = gpio_data->gc_map[bank];
unsigned long flags;
u32 rise, fall;
@@ -252,7 +252,7 @@ mediatek_gpio_irq_mask(struct irq_data *d)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct mtk_data *gpio_data = gpiochip_get_data(gc);
int pin = d->hwirq;
-   int bank = pin / 32;
+   int bank = pin / MTK_BANK_WIDTH;
struct mtk_gc *rg = gpio_data->gc_map[bank];
unsigned long flags;
u32 rise, fall;
@@ -275,7 +275,7 @@ mediatek_gpio_irq_type(struct irq_data *d, unsigned int 
type)
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
struct mtk_data *gpio_data = gpiochip_get_data(gc);
int pin = d->hwirq;
-   int bank = pin / 32;
+   int bank = pin / MTK_BANK_WIDTH;
struct mtk_gc *rg = gpio_data->gc_map[bank];
u32 mask = BIT(d->hwirq);
 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 03/11] staging: mt7621-dts: update gpios related entries to use 'mediatek'

2018-05-19 Thread Sergio Paracuellos

Gpio driver for mt7621 is using 'mtk' as binding but in the kernel
is already defined one for this maker which is 'mediatek'. Update
device tree to use the correct one.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-dts/mt7621.dtsi | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/staging/mt7621-dts/mt7621.dtsi 
b/drivers/staging/mt7621-dts/mt7621.dtsi
index 9d941b5..115eb04 100644
--- a/drivers/staging/mt7621-dts/mt7621.dtsi
+++ b/drivers/staging/mt7621-dts/mt7621.dtsi
@@ -64,26 +64,26 @@
#address-cells = <1>;
#size-cells = <0>;
 
-   compatible = "mtk,mt7621-gpio";
+   compatible = "mediatek,mt7621-gpio";
reg = <0x600 0x100>;
 
gpio0: bank@0 {
reg = <0>;
-   compatible = "mtk,mt7621-gpio-bank";
+   compatible = "mediatek,mt7621-gpio-bank";
gpio-controller;
#gpio-cells = <2>;
};
 
gpio1: bank@1 {
reg = <1>;
-   compatible = "mtk,mt7621-gpio-bank";
+   compatible = "mediatek,mt7621-gpio-bank";
gpio-controller;
#gpio-cells = <2>;
};
 
gpio2: bank@2 {
reg = <2>;
-   compatible = "mtk,mt7621-gpio-bank";
+   compatible = "mediatek,mt7621-gpio-bank";
gpio-controller;
#gpio-cells = <2>;
};
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 05/11] staging: mt7621-gpio: avoid use of globals and use platform_data instead

2018-05-19 Thread Sergio Paracuellos

Gpio driver have a some globals which can be avoided just
using platform_data in a proper form. This commit adds a new
struct mtk_data which includes all of those globals setting them
using platform_set_drvdata and devm_gpiochip_add_data functions.
With this properly set we are able to retrieve driver data along
the code using kernel api's so globals are not needed anymore.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-gpio/gpio-mt7621.c | 92 ++-
 1 file changed, 65 insertions(+), 27 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c 
b/drivers/staging/mt7621-gpio/gpio-mt7621.c
index 7d17949..650286df 100644
--- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
+++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
@@ -31,17 +31,20 @@ enum mediatek_gpio_reg {
GPIO_REG_EDGE,
 };
 
-static void __iomem *mediatek_gpio_membase;
-static int mediatek_gpio_irq;
-static struct irq_domain *mediatek_gpio_irq_domain;
-
-static struct mtk_gc {
+struct mtk_gc {
struct gpio_chip chip;
spinlock_t lock;
int bank;
u32 rising;
u32 falling;
-} *gc_map[MTK_MAX_BANK];
+};
+
+struct mtk_data {
+   void __iomem *gpio_membase;
+   int gpio_irq;
+   struct irq_domain *gpio_irq_domain;
+   struct mtk_gc *gc_map[MTK_MAX_BANK];
+};
 
 static inline struct mtk_gc
 *to_mediatek_gpio(struct gpio_chip *chip)
@@ -56,15 +59,19 @@ static inline struct mtk_gc
 static inline void
 mtk_gpio_w32(struct mtk_gc *rg, u8 reg, u32 val)
 {
-   iowrite32(val, mediatek_gpio_membase + (reg * 0x10) + (rg->bank * 0x4));
+   struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
+   u32 offset = (reg * 0x10) + (rg->bank * 0x4);
+
+   iowrite32(val, gpio_data->gpio_membase + offset);
 }
 
 static inline u32
 mtk_gpio_r32(struct mtk_gc *rg, u8 reg)
 {
+   struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
u32 offset = (reg * 0x10) + (rg->bank * 0x4);
 
-   return ioread32(mediatek_gpio_membase + offset);
+   return ioread32(gpio_data->gpio_membase + offset);
 }
 
 static void
@@ -137,23 +144,26 @@ mediatek_gpio_get_direction(struct gpio_chip *chip, 
unsigned int offset)
 static int
 mediatek_gpio_to_irq(struct gpio_chip *chip, unsigned int pin)
 {
+   struct mtk_data *gpio_data = gpiochip_get_data(chip);
struct mtk_gc *rg = to_mediatek_gpio(chip);
 
-   return irq_create_mapping(mediatek_gpio_irq_domain,
+   return irq_create_mapping(gpio_data->gpio_irq_domain,
  pin + (rg->bank * MTK_BANK_WIDTH));
 }
 
 static int
 mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node 
*bank)
 {
+   struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
const __be32 *id = of_get_property(bank, "reg", NULL);
struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
sizeof(struct mtk_gc), GFP_KERNEL);
+   int ret;
 
if (!rg || !id || be32_to_cpu(*id) > MTK_MAX_BANK)
return -ENOMEM;
 
-   gc_map[be32_to_cpu(*id)] = rg;
+   gpio_data->gc_map[be32_to_cpu(*id)] = rg;
 
memset(rg, 0, sizeof(struct mtk_gc));
 
@@ -169,25 +179,34 @@ mediatek_gpio_bank_probe(struct platform_device *pdev, 
struct device_node *bank)
rg->chip.get_direction = mediatek_gpio_get_direction;
rg->chip.get = mediatek_gpio_get;
rg->chip.set = mediatek_gpio_set;
-   if (mediatek_gpio_irq_domain)
+   if (gpio_data->gpio_irq_domain)
rg->chip.to_irq = mediatek_gpio_to_irq;
rg->bank = be32_to_cpu(*id);
 
+   ret = devm_gpiochip_add_data(&pdev->dev, &rg->chip, gpio_data);
+   if (ret < 0) {
+   dev_err(&pdev->dev, "Could not register gpio %d, ret=%d\n",
+   rg->chip.ngpio, ret);
+   return ret;
+   }
+
/* set polarity to low for all gpios */
mtk_gpio_w32(rg, GPIO_REG_POL, 0);
 
dev_info(&pdev->dev, "registering %d gpios\n", rg->chip.ngpio);
 
-   return gpiochip_add(&rg->chip);
+   return 0;
 }
 
 static void
 mediatek_gpio_irq_handler(struct irq_desc *desc)
 {
+   struct gpio_chip *gc = irq_desc_get_handler_data(desc);
+   struct mtk_data *gpio_data = gpiochip_get_data(gc);
int i;
 
for (i = 0; i < MTK_MAX_BANK; i++) {
-   struct mtk_gc *rg = gc_map[i];
+   struct mtk_gc *rg = gpio_data->gc_map[i];
unsigned long pending;
int bit;
 
@@ -197,7 +216,7 @@ mediatek_gpio_irq_handler(struct irq_desc *desc)
pending = mtk_gpio_r32(rg, GPIO_REG_STAT);
 
for_each_set_bit(bit, &pending, MTK_BANK_WIDTH) {
-   u32 map = irq_find_mapping(mediatek_gpio_irq_domain,
+   u32 map = irq_find_mapping(gpio_data->gpio_irq_domain,
   (MTK_BANK_WIDTH * i) + bit);

[PATCH v4 11/11] staging: mt7621-gpio: update TODO list

2018-05-19 Thread Sergio Paracuellos

Some of the remaining stuff included in TODO list
have been complete. So update this file accordly.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-gpio/TODO | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/staging/mt7621-gpio/TODO b/drivers/staging/mt7621-gpio/TODO
index 9077b16..9089833 100644
--- a/drivers/staging/mt7621-gpio/TODO
+++ b/drivers/staging/mt7621-gpio/TODO
@@ -1,7 +1,4 @@
 
-- general code review and clean up 
-- avoid global variables and use drvdata allocated instead
-- ensure device-tree requirements are documented
 - make sure interrupts work
 
 Cc:  NeilBrown 
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 07/11] staging: mt7621-gpio: dt-bindings: add interrupt nodes to bindings doc

2018-05-19 Thread Sergio Paracuellos

Interrupt related stuff for gpio controller in mt7621 was missing
in device tree documentation. Add it to complete documentation for
this driver.

Signed-off-by: Sergio Paracuellos 
---
 drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt | 9 -
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt 
b/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
index 54de9f7..af64092 100644
--- a/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
+++ b/drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt
@@ -4,12 +4,16 @@ The IP core used inside these SoCs has 3 banks of 32 GPIOs 
each.
 The registers of all the banks are interwoven inside one single IO range.
 We load one GPIO controller instance per bank. To make this possible
 we support 2 types of nodes. The parent node defines the memory I/O range and
-has 3 children each describing a single bank.
+has 3 children each describing a single bank. Also the GPIO controller can 
receive
+interrupts on any of the GPIOs, either edge or level. It then interrupts the 
CPU
+using GIC INT12.
 
 Required properties for the top level node:
 - compatible:
   - "mediatek,mt7621-gpio" for Mediatek controllers
 - reg : Physical base address and length of the controller's registers
+- interrupt-parent : phandle of the parent interrupt controller.
+- interrupts = Interrupt specifier for the controllers interrupt
 
 Required properties for the GPIO bank node:
 - compatible:
@@ -28,6 +32,9 @@ Example:
compatible = "mediatek,mt7621-gpio";
reg = <0x600 0x100>;
 
+   interrupt-parent = <&gic>;
+   interrupts = ;
+
gpio0: bank@0 {
reg = <0>;
compatible = "mediatek,mt7621-gpio-bank";
-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

[PATCH v4 00/11] staging: mt7621-gpio: use mediatek as binding instead of custom mtk

2018-05-19 Thread Sergio Paracuellos

The following series updated mt7621-gpio driver to use 'mediatek'
which is already defined in kernel bindings documentation instead
of add a new custom one 'mtk' for this company. mt7621-gpio device-tree
documentation has been added also to the staging driver directory.

v4:
 - Only PATCH 5 ("staging: mt7621-gpio: avoid use of globals and use 
   platform_data instead") has changes:
+ avoid call to gpiochip_add in probe function because function
  devm_gpiochip_add_data is being used instead.
+ Call irq_set_chip_data in mediatek_gpio_gpio_map to make data
  properly passed to interrupts related functions.

v3:
 - Fix condition for check for a valid gpio id in driver probe function
   included in the PATCH 8 ("staging: mt7621-gpio: avoid devm_kzalloc()
   hidden inside declarations and refactor function a bit") of the series.
 - Rest of patches are not modified at all.

v2:
 - Join some patch ("staging: mt7621-gpio: avoid use of globals
   and use platform_data instead") with the needed fixes to make
   it work pointed out by NeilBrown
 - Add gpio interrupts to documentation and device tree in order to
   can try them
 - Other minor code cleanups have been added also.

After this changes only testing interrupts should be remaining to
get this out of staging.

Hope this helps.

Best regards,
 Sergio

Sergio Paracuellos (11):
  staging: mt7621-gpio: dt-bindings: add documentation for mt7621-gpio
  staging: mt7621-gpio: update TODO file
  staging: mt7621-dts: update gpios related entries to use 'mediatek'
  staging: mt7621-gpio: replace 'mtk' to use correct one 'mediatek'
  staging: mt7621-gpio: avoid use of globals and use platform_data
instead
  staging: mt7621-dts: add interrupt device tree nodes for the gpio
controller
  staging: mt7621-gpio: dt-bindings: add interrupt nodes to bindings doc
  staging: mt7621-gpio: avoid devm_kzalloc() hidden inside declarations
and refactor function a bit
  staging: mt7621-gpio: use ternary operator in return in
mediatek_gpio_get_direction
  staging: mt7621-gpio: use MTK_BANK_WIDTH instead of magic number
  staging: mt7621-gpio: update TODO list

 drivers/staging/mt7621-dts/mt7621.dtsi |  11 +-
 drivers/staging/mt7621-gpio/TODO   |   3 +-
 drivers/staging/mt7621-gpio/gpio-mt7621.c  | 118 ++---
 .../staging/mt7621-gpio/mediatek,mt7621-gpio.txt   |  58 ++
 4 files changed, 143 insertions(+), 47 deletions(-)
 create mode 100644 drivers/staging/mt7621-gpio/mediatek,mt7621-gpio.txt

-- 
2.7.4

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v3 05/11] staging: mt7621-gpio: avoid use of globals and use platform_data instead

2018-05-19 Thread NeilBrown

On Wed, May 16 2018, Sergio Paracuellos wrote:

> Gpio driver have a some globals which can be avoided just
> using platform_data in a proper form. This commit adds a new
> struct mtk_data which includes all of those globals setting them
> using platform_set_drvdata and devm_gpiochip_add_data functions.
> With this properly set we are able to retrieve driver data along
> the code using kernel api's so globals are not needed anymore.
>
> Signed-off-by: Sergio Paracuellos 
> ---
>  drivers/staging/mt7621-gpio/gpio-mt7621.c | 85 
> +--
>  1 file changed, 59 insertions(+), 26 deletions(-)
>
> diff --git a/drivers/staging/mt7621-gpio/gpio-mt7621.c 
> b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> index 7d17949..c701259 100644
> --- a/drivers/staging/mt7621-gpio/gpio-mt7621.c
> +++ b/drivers/staging/mt7621-gpio/gpio-mt7621.c
> @@ -31,17 +31,20 @@ enum mediatek_gpio_reg {
>   GPIO_REG_EDGE,
>  };
>  
> -static void __iomem *mediatek_gpio_membase;
> -static int mediatek_gpio_irq;
> -static struct irq_domain *mediatek_gpio_irq_domain;
> -
> -static struct mtk_gc {
> +struct mtk_gc {
>   struct gpio_chip chip;
>   spinlock_t lock;
>   int bank;
>   u32 rising;
>   u32 falling;
> -} *gc_map[MTK_MAX_BANK];
> +};
> +
> +struct mtk_data {
> + void __iomem *gpio_membase;
> + int gpio_irq;
> + struct irq_domain *gpio_irq_domain;
> + struct mtk_gc *gc_map[MTK_MAX_BANK];
> +};
>  
>  static inline struct mtk_gc
>  *to_mediatek_gpio(struct gpio_chip *chip)
> @@ -56,15 +59,19 @@ static inline struct mtk_gc
>  static inline void
>  mtk_gpio_w32(struct mtk_gc *rg, u8 reg, u32 val)
>  {
> - iowrite32(val, mediatek_gpio_membase + (reg * 0x10) + (rg->bank * 0x4));
> + struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
> + u32 offset = (reg * 0x10) + (rg->bank * 0x4);
> +
> + iowrite32(val, gpio_data->gpio_membase + offset);
>  }
>  
>  static inline u32
>  mtk_gpio_r32(struct mtk_gc *rg, u8 reg)
>  {
> + struct mtk_data *gpio_data = gpiochip_get_data(&rg->chip);
>   u32 offset = (reg * 0x10) + (rg->bank * 0x4);
>  
> - return ioread32(mediatek_gpio_membase + offset);
> + return ioread32(gpio_data->gpio_membase + offset);
>  }
>  
>  static void
> @@ -137,23 +144,26 @@ mediatek_gpio_get_direction(struct gpio_chip *chip, 
> unsigned int offset)
>  static int
>  mediatek_gpio_to_irq(struct gpio_chip *chip, unsigned int pin)
>  {
> + struct mtk_data *gpio_data = gpiochip_get_data(chip);
>   struct mtk_gc *rg = to_mediatek_gpio(chip);
>  
> - return irq_create_mapping(mediatek_gpio_irq_domain,
> + return irq_create_mapping(gpio_data->gpio_irq_domain,
> pin + (rg->bank * MTK_BANK_WIDTH));
>  }
>  
>  static int
>  mediatek_gpio_bank_probe(struct platform_device *pdev, struct device_node 
> *bank)
>  {
> + struct mtk_data *gpio_data = dev_get_drvdata(&pdev->dev);
>   const __be32 *id = of_get_property(bank, "reg", NULL);
>   struct mtk_gc *rg = devm_kzalloc(&pdev->dev,
>   sizeof(struct mtk_gc), GFP_KERNEL);
> + int ret;
>  
>   if (!rg || !id || be32_to_cpu(*id) > MTK_MAX_BANK)
>   return -ENOMEM;
>  
> - gc_map[be32_to_cpu(*id)] = rg;
> + gpio_data->gc_map[be32_to_cpu(*id)] = rg;
>  
>   memset(rg, 0, sizeof(struct mtk_gc));
>  
> @@ -169,10 +179,17 @@ mediatek_gpio_bank_probe(struct platform_device *pdev, 
> struct device_node *bank)
>   rg->chip.get_direction = mediatek_gpio_get_direction;
>   rg->chip.get = mediatek_gpio_get;
>   rg->chip.set = mediatek_gpio_set;
> - if (mediatek_gpio_irq_domain)
> + if (gpio_data->gpio_irq_domain)
>   rg->chip.to_irq = mediatek_gpio_to_irq;
>   rg->bank = be32_to_cpu(*id);
>  
> + ret = devm_gpiochip_add_data(&pdev->dev, &rg->chip, gpio_data);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Could not register gpio %d, ret=%d\n",
> + rg->chip.ngpio, ret);
> + return ret;
> + }
> +

Calling devm_gpiochip_add_data() here looks good.
Not removing
return gpiochip_add(&rg->chip);
from the end of the function isn't so good :-(

BTW interrupt definitely don't work yet.  The calls
struct gpio_chip *gc = irq_data_get_irq_chip_data(d);
get NULL.  I think some sort of irq_set_chip_data(irq,...) is needed
in mediatek_gpio_gpio_map(), but it is too late at night for it to
all make sense to me :-)

Thanks,
NeilBrown


signature.asc
Description: PGP signature
___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v1] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic

2018-05-19 Thread Greg KH

On Fri, May 18, 2018 at 09:00:51PM +, Sunil Muthuswamy wrote:
> Thanks, Greg.
> 
> My first patch to the Linux kernel. Still making mistakes, but, learning 
> through the documented process.
> 
> > -Original Message-
> > From: Greg KH 
> > Sent: Wednesday, May 9, 2018 11:51 PM
> > To: Sunil Muthuswamy 
> > Cc: Haiyang Zhang ;
> > de...@linuxdriverproject.org; Stephen Hemminger
> > 
> > Subject: Re: [PATCH v1] Drivers: HV: Send one page worth of kmsg dump
> > over Hyper-V during panic
> > 
> > On Wed, May 09, 2018 at 07:19:24PM +, Sunil Muthuswamy wrote:
> > > In the VM mode on Hyper-V, currently, when the kernel panics, an error
> > > code and few register values are populated in an MSR and the 
> > > Hypervisor
> > > notified. This information is collected on the host. The amount of
> > > information currently collected is found to be limited and not very
> > > actionable. To gather more actionable data, such as stack trace, the
> > > proposal is to write one page worth of kmsg data on an allocated page
> > > and the Hypervisor notified of the page address through the MSR.
> > 
> > Odd indentation, what editor made you do that?  Please move it all to the
> > left.
> I inserted them. Will fix.
> > 
> > >
> > > CC: k...@microsoft.com
> > > CC: sthem...@microsoft.com
> > > Signed-off-by: Sunil Muthuswamy 
> > > ---
> > >  arch/x86/hyperv/hv_init.c  | 28 +
> > >  arch/x86/include/asm/hyperv-tlfs.h |  5 ++--
> > >  arch/x86/include/asm/mshyperv.h|  1 +
> > >  drivers/hv/vmbus_drv.c | 61
> > ++
> > >  4 files changed, 93 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> > > index cfecc22..88ee90d 100644
> > > --- a/arch/x86/hyperv/hv_init.c
> > > +++ b/arch/x86/hyperv/hv_init.c
> > > @@ -395,6 +395,34 @@ void hyperv_report_panic(struct pt_regs *regs,
> > > long err)  }  EXPORT_SYMBOL_GPL(hyperv_report_panic);
> > >
> > > +void hyperv_report_panic_msg(phys_addr_t pa, size_t size) {
> > > + static bool panic_msg_reported;
> > > +
> > > + if (panic_msg_reported)
> > > + return;
> > > + panic_msg_reported = true;
> > 
> > Why do you only care about the first message?
> It is following the general direction from ' hyperv_report_panic', but, I 
> don't think it needs to. Will change.
> > 
> > > +
> > > + /*
> > > +  * P3 to contain the physical address of the panic page & P4 to
> > > +  * contain the size of the panic data in that page. Rest of the
> > > +  * registers are no-op when the NOTIFY_MSG flag is set.
> > > +  */
> > > + wrmsrl(HV_X64_MSR_CRASH_P0, 0);
> > > + wrmsrl(HV_X64_MSR_CRASH_P1, 0);
> > > + wrmsrl(HV_X64_MSR_CRASH_P2, 0);
> > > + wrmsrl(HV_X64_MSR_CRASH_P3, pa);
> > > + wrmsrl(HV_X64_MSR_CRASH_P4, size);
> > > +
> > > + /*
> > > +  * Let Hyper-V know there is crash data available along with
> > > +  * the panic message.
> > > +  */
> > > + wrmsrl(HV_X64_MSR_CRASH_CTL,
> > > +(HV_CRASH_CTL_CRASH_NOTIFY |
> > HV_CRASH_CTL_CRASH_NOTIFY_MSG));
> > > +} EXPORT_SYMBOL_GPL(hyperv_report_panic_msg);
> > > +
> > >  bool hv_is_hyperv_initialized(void)
> > >  {
> > >   union hv_x64_msr_hypercall_contents hypercall_msr; diff --git
> > > a/arch/x86/include/asm/hyperv-tlfs.h
> > > b/arch/x86/include/asm/hyperv-tlfs.h
> > > index 416cb0e..fc2932c 100644
> > > --- a/arch/x86/include/asm/hyperv-tlfs.h
> > > +++ b/arch/x86/include/asm/hyperv-tlfs.h
> > > @@ -171,9 +171,10 @@
> > >  #define HV_X64_ENLIGHTENED_VMCS_RECOMMENDED(1 << 14)
> > >
> > >  /*
> > > - * Crash notification flag.
> > > + * Crash notification flags.
> > >   */
> > > -#define HV_CRASH_CTL_CRASH_NOTIFY (1ULL << 63)
> > > +#define HV_CRASH_CTL_CRASH_NOTIFY (1ULL << 63)
> > > +#define HV_CRASH_CTL_CRASH_NOTIFY_MSG (1ULL << 62)
> > 
> > Not in numerical order?
> > 
> > And can you use the BIT() macro here instead?  Not a requirement, just a
> > general question.
> Will change in the next version.

You didn't do that :(

___
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel

Re: [PATCH v2] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic

2018-05-19 Thread Greg KH

On Fri, May 18, 2018 at 07:09:10PM +, Sunil Muthuswamy wrote:
> In the VM mode on Hyper-V, currently, when the kernel panics, an error
> code and few register values are populated in an MSR and the Hypervisor
> notified. This information is collected on the host. The amount of
> information currently collected is found to be limited and not very
> actionable. To gather more actionable data, such as stack trace, the
> proposal is to write one page worth of kmsg data on an allocated page
> and the Hypervisor notified of the page address through the MSR.
> 
> - Added a sysctl option to control the behavior, with ON by default.
> 
> CC: k...@microsoft.com
> CC: sthem...@microsoft.com
> Signed-off-by: Sunil Muthuswamy 
> ---
>  arch/x86/hyperv/hv_init.c  |  35 ++
>  arch/x86/include/asm/hyperv-tlfs.h |   5 +-
>  arch/x86/include/asm/mshyperv.h|   1 +
>  drivers/hv/vmbus_drv.c | 128 
> +
>  4 files changed, 167 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/hyperv/hv_init.c b/arch/x86/hyperv/hv_init.c
> index cfecc22..fc1e3cb 100644
> --- a/arch/x86/hyperv/hv_init.c
> +++ b/arch/x86/hyperv/hv_init.c
> @@ -395,6 +395,41 @@ void hyperv_report_panic(struct pt_regs *regs, long err)
>  }
>  EXPORT_SYMBOL_GPL(hyperv_report_panic);
>  
> +/**
> + * hyperv_report_panic_msg - report panic message to Hyper-V
> + * @pa: physical address of the panic page containing the message
> + * @size: size of the message in the page
> + *
> + */
> +

The blank lines are not needed, please drop.

> +void hyperv_report_panic_msg(phys_addr_t pa, size_t size)
> +{
> + static bool panic_msg_reported;
> +
> + if (panic_msg_reported)
> + return;
> + panic_msg_reported = true;
> +
> + /*
> +  * P3 to contain the physical address of the panic page & P4 to
> +  * contain the size of the panic data in that page. Rest of the
> +  * registers are no-op when the NOTIFY_MSG flag is set.
> +  */
> + wrmsrl(HV_X64_MSR_CRASH_P0, 0);
> + wrmsrl(HV_X64_MSR_CRASH_P1, 0);
> + wrmsrl(HV_X64_MSR_CRASH_P2, 0);
> + wrmsrl(HV_X64_MSR_CRASH_P3, pa);
> + wrmsrl(HV_X64_MSR_CRASH_P4, size);
> +
> + /*
> +  * Let Hyper-V know there is crash data available along with
> +  * the panic message.
> +  */
> + wrmsrl(HV_X64_MSR_CRASH_CTL,
> +(HV_CRASH_CTL_CRASH_NOTIFY | HV_CRASH_CTL_CRASH_NOTIFY_MSG));
> +}
> +EXPORT_SYMBOL_GPL(hyperv_report_panic_msg);
> +
>  bool hv_is_hyperv_initialized(void)
>  {
>   union hv_x64_msr_hypercall_contents hypercall_msr;
> diff --git a/arch/x86/include/asm/hyperv-tlfs.h 
> b/arch/x86/include/asm/hyperv-tlfs.h
> index 416cb0e..fc2932c 100644
> --- a/arch/x86/include/asm/hyperv-tlfs.h
> +++ b/arch/x86/include/asm/hyperv-tlfs.h
> @@ -171,9 +171,10 @@
>  #define HV_X64_ENLIGHTENED_VMCS_RECOMMENDED(1 << 14)
>  
>  /*
> - * Crash notification flag.
> + * Crash notification flags.
>   */
> -#define HV_CRASH_CTL_CRASH_NOTIFY (1ULL << 63)
> +#define HV_CRASH_CTL_CRASH_NOTIFY (1ULL << 63)
> +#define HV_CRASH_CTL_CRASH_NOTIFY_MSG (1ULL << 62)

BIT_ULL()?

And again, why are they not in numerical order?

>  /* MSR used to identify the guest OS. */
>  #define HV_X64_MSR_GUEST_OS_ID   0x4000
> diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
> index b90e796..ac83f2d 100644
> --- a/arch/x86/include/asm/mshyperv.h
> +++ b/arch/x86/include/asm/mshyperv.h
> @@ -262,6 +262,7 @@ void hyperv_init(void);
>  void hyperv_setup_mmu_ops(void);
>  void hyper_alloc_mmu(void);
>  void hyperv_report_panic(struct pt_regs *regs, long err);
> +void hyperv_report_panic_msg(phys_addr_t pa, size_t size);
>  bool hv_is_hyperv_initialized(void);
>  void hyperv_cleanup(void);
>  
> diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
> index b10fe26..7b04f7f 100644
> --- a/drivers/hv/vmbus_drv.c
> +++ b/drivers/hv/vmbus_drv.c
> @@ -56,6 +56,8 @@ static struct completion probe_event;
>  
>  static int hyperv_cpuhp_online;
>  
> +static void *hv_panic_page;
> +
>  static int hyperv_panic_event(struct notifier_block *nb, unsigned long val,
> void *args)
>  {
> @@ -1018,6 +1020,86 @@ static void vmbus_isr(void)
>   add_interrupt_randomness(HYPERVISOR_CALLBACK_VECTOR, 0);
>  }
>  
> +/*
> + * Boolean to control whether to report panic messages over Hyper-V.
> + *
> + * It can be set via /proc/sys/kernel/hyperv/record_panic_msg
> + */
> +int sysctl_record_panic_msg = 1;

Why was a sysctl chosen?  I'm not objecting, but you have to document
new user/kernel apis when you create them.

> +/*
> + * Callback from kmsg_dump. Grab as much as possible from the end of the kmsg
> + * buffer and call into Hyper-V to transfer the data.
> + */
> +static void hv_kmsg_dump(struct kmsg_dumper *dumper,
> +  enum kmsg_dump_reason reason)
> +{
> + size_t bytes_written;
> + phys_addr_t panic_pa;
>

Re: [PATCH v3 05/11] staging: mt7621-gpio: avoid use of globals and use platform_data instead

[PATCH] Staging: rtl8192e: rtllib_tx: fix spelling issue.

Re: [PATCH v3 05/11] staging: mt7621-gpio: avoid use of globals and use platform_data instead

[PATCH v4 02/11] staging: mt7621-gpio: update TODO file

[PATCH v4 04/11] staging: mt7621-gpio: replace 'mtk' to use correct one 'mediatek'

[PATCH v4 09/11] staging: mt7621-gpio: use ternary operator in return in mediatek_gpio_get_direction

[PATCH v4 01/11] staging: mt7621-gpio: dt-bindings: add documentation for mt7621-gpio

[PATCH v4 08/11] staging: mt7621-gpio: avoid devm_kzalloc() hidden inside declarations and refactor function a bit

[PATCH v4 00/11] staging: mt7621-gpio: use mediatek as binding instead of custom mtk

[PATCH v4 06/11] staging: mt7621-dts: add interrupt device tree nodes for the gpio controller

[PATCH v4 10/11] staging: mt7621-gpio: use MTK_BANK_WIDTH instead of magic number

[PATCH v4 03/11] staging: mt7621-dts: update gpios related entries to use 'mediatek'

[PATCH v4 05/11] staging: mt7621-gpio: avoid use of globals and use platform_data instead

[PATCH v4 11/11] staging: mt7621-gpio: update TODO list

[PATCH v4 07/11] staging: mt7621-gpio: dt-bindings: add interrupt nodes to bindings doc

[PATCH v4 00/11] staging: mt7621-gpio: use mediatek as binding instead of custom mtk

Re: [PATCH v3 05/11] staging: mt7621-gpio: avoid use of globals and use platform_data instead

Re: [PATCH v1] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic

Re: [PATCH v2] Drivers: HV: Send one page worth of kmsg dump over Hyper-V during panic

19 matches

Site Navigation

Mail list logo

Footer information