[U-Boot] [PATCH 0/2] Add GPIO driver for BCM2835 SoC

[U-Boot] [PATCH 0/2] Add GPIO driver for BCM2835 SoC

[Vikram Narayanan]

Add a GPIO driver for the BCM2835 Soc.
Refer Datasheet http://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
Also, add the driver to the raspberrypi's default config


Vikram Narayanan (2):
  bcm: Add GPIO driver for BCM2835 SoC
  rbpi: Add BCM2835 GPIO driver for raspberry pi

 arch/arm/include/asm/arch-bcm2835/gpio.h |   49 +++++++++++
 drivers/gpio/Makefile                    |    1 +
 drivers/gpio/rpi_gpio.c                  |  128 ++++++++++++++++++++++++++++++
 include/configs/rpi_b.h                  |    4 +-
 4 files changed, 181 insertions(+), 1 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-bcm2835/gpio.h
 create mode 100644 drivers/gpio/rpi_gpio.c

-- 
1.7.4.1

[U-Boot] [PATCH 1/2] bcm: Add GPIO driver for BCM2835 SoC

[Vikram Narayanan]

Signed-off-by: Vikram Narayanan <vikram186@gmail.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
---
 arch/arm/include/asm/arch-bcm2835/gpio.h |   49 +++++++++++
 drivers/gpio/Makefile                    |    1 +
 drivers/gpio/rpi_gpio.c                  |  128 ++++++++++++++++++++++++++++++
 3 files changed, 178 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/arch-bcm2835/gpio.h
 create mode 100644 drivers/gpio/rpi_gpio.c

diff --git a/arch/arm/include/asm/arch-bcm2835/gpio.h b/arch/arm/include/asm/arch-bcm2835/gpio.h
new file mode 100644
index 0000000..3cba0fb
--- /dev/null
+++ b/arch/arm/include/asm/arch-bcm2835/gpio.h
@@ -0,0 +1,49 @@
+/*
+ * Copyright (C) 2012 Vikram Narayananan
+ * <vikram186@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#ifndef _BCM2835_GPIO_H_
+#define _BCM2835_GPIO_H_
+
+#define BCM2835_GPIO_BASE	(0x7E200000)
+#define BCM2835_MAX_GPIOS	53
+
+#define BCM2835_GPIO_FSEL_CLR_MASK	(0x7)
+#define BCM2835_GPIO_OUTPUT			(0x1)
+
+#define GPIO_TO_BANK(n)	(n / 10)
+#define GPIO_TO_PIN(n)	(n % 10)
+
+struct bcm_gpio_regs {
+	u32 gpfsel[6];
+	u32 reserved1;
+	u32 gpset0;
+	u32 gpset1;
+	u32 reserved2;
+	u32 gpclr0;
+	u32 gpclr1;
+	u32 reserved3;
+	u32 gplev0;
+	u32 gplev1;
+};
+
+#endif /* _BCM2835_GPIO_H_ */
diff --git a/drivers/gpio/Makefile b/drivers/gpio/Makefile
index fb3b09a..b042c46 100644
--- a/drivers/gpio/Makefile
+++ b/drivers/gpio/Makefile
@@ -39,6 +39,7 @@ COBJS-$(CONFIG_TEGRA2_GPIO)	+= tegra2_gpio.o
 COBJS-$(CONFIG_DA8XX_GPIO)	+= da8xx_gpio.o
 COBJS-$(CONFIG_ALTERA_PIO)	+= altera_pio.o
 COBJS-$(CONFIG_MPC83XX_GPIO)	+= mpc83xx_gpio.o
+COBJS-$(CONFIG_BCM2835_GPIO)	+= rpi_gpio.o
 
 COBJS	:= $(COBJS-y)
 SRCS 	:= $(COBJS:.o=.c)
diff --git a/drivers/gpio/rpi_gpio.c b/drivers/gpio/rpi_gpio.c
new file mode 100644
index 0000000..c2b547f
--- /dev/null
+++ b/drivers/gpio/rpi_gpio.c
@@ -0,0 +1,128 @@
+/*
+ * Copyright (C) 2012 Vikram Narayananan
+ * <vikram186@gmail.com>
+ *
+ * See file CREDITS for list of people who contributed to this
+ * project.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of
+ * the License, or (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston,
+ * MA 02111-1307 USA
+ */
+
+#include <common.h>
+#include <asm/gpio.h>
+#include <asm/io.h>
+
+struct bcm_gpio {
+	u32 bank;
+	u32 pin;
+};
+
+inline int gpio_is_valid(unsigned gpio)
+{
+	return (gpio > BCM2835_MAX_GPIOS) ? 0 : 1;
+}
+
+static int get_bank_pin(unsigned gpio, struct bcm_gpio *pio)
+{
+	int bank = GPIO_TO_BANK(gpio);
+	int pin = GPIO_TO_PIN(gpio);
+
+	if (!gpio_is_valid(gpio))
+		return -1;
+
+	pin &= 0x09;
+	pio->pin = pin;
+	pio->bank = bank;
+	return 0;
+}
+
+int gpio_request(unsigned gpio, const char *label)
+{
+	return (gpio_is_valid(gpio)) ? 1 : 0;
+}
+
+int gpio_free(unsigned gpio)
+{
+	return 0;
+}
+
+int gpio_direction_input(unsigned gpio)
+{
+	struct bcm_gpio pio;
+	struct bcm_gpio_regs *reg = (struct bcm_gpio_regs *)BCM2835_GPIO_BASE;
+	unsigned val = 0;
+
+	if (get_bank_pin(gpio, &pio))
+		return -1;
+
+	val = readl(&reg->gpfsel[pio.bank]);
+	val &= ~(BCM2835_GPIO_FSEL_CLR_MASK << (pio.pin * 3));
+	writel(val, reg->gpfsel[pio.bank]);
+}
+
+int gpio_direction_output(unsigned gpio, int value)
+{
+	struct bcm_gpio pio;
+	struct bcm_gpio_regs *reg = (struct bcm_gpio_regs *)BCM2835_GPIO_BASE;
+	unsigned val = 0;
+
+	if (get_bank_pin(gpio, &pio))
+		return -1;
+
+	val = readl(&reg->gpfsel[pio.bank]);
+	val &= ~(BCM2835_GPIO_FSEL_CLR_MASK << (pio.pin * 3));
+	val |= (BCM2835_GPIO_OUTPUT << (pio.pin * 3));
+	writel(val, reg->gpfsel[pio.bank]);
+
+	if (value)
+		gpio_set_value(gpio, value);
+}
+
+int gpio_get_value(unsigned gpio)
+{
+	struct bcm_gpio_regs *reg = (struct bcm_gpio_regs *)BCM2835_GPIO_BASE;
+	unsigned val = 0;
+
+	if (!gpio_is_valid(gpio))
+		return -1;
+
+	val = (gpio < 32) ? readl(&reg->gplev0) : readl(&reg->gplev1);
+	gpio &= 0x1f;
+
+	return (val & (1 << gpio)) >> gpio;
+}
+
+int gpio_set_value(unsigned gpio, int value)
+{
+	struct bcm_gpio_regs *reg = (struct bcm_gpio_regs *)BCM2835_GPIO_BASE;
+
+	if (!gpio_is_valid(gpio))
+		return -1;
+
+	if (gpio < 32) {
+		if (value)
+			writel((1 << gpio), reg->gpset0);
+		else
+			writel((1 << gpio), reg->gpclr0);
+	} else {
+		gpio &= 0x1f;
+		if (value)
+			writel((1 << pin), reg->gpset1);
+		else
+			writel((1 << pin), reg->gpclr1);
+	}
+	return 0;
+}
-- 
1.7.4.1

[U-Boot] [PATCH 1/2] bcm: Add GPIO driver for BCM2835 SoC

[Stephen Warren]

On 06/24/2012 11:21 AM, Vikram Narayanan wrote:

First off, it's great to see some patches for the chip. Thanks. Sorry
for being so nit-picky below; it's a tendency of mine...

It'd be nice to include a patch description here, so that something
shows up besides just the patch subject.

> diff --git a/arch/arm/include/asm/arch-bcm2835/gpio.h b/arch/arm/include/asm/arch-bcm2835/gpio.h

> +#define BCM2835_GPIO_BASE	(0x7E200000)

There's no need for brackets around simple numbers.

> +#define BCM2835_MAX_GPIOS	53

I think that should be named either BCM2835_NUM_GPIOS or
BCM2835_MAX_GPIO; you can have a number of GPIOs or a maximum GPIO
number, but not a maximum GPIOs.

> +#define BCM2835_GPIO_FSEL_CLR_MASK	(0x7)

The mask applies equally to set or clear; I'd rename this to just
BCM2835_GPIO_FSEL_MASK.

> +#define BCM2835_GPIO_OUTPUT			(0x1)

I'd like to see a matching BCM2835_GPIO_INPUT function select value too;
even if the value is actually 0 so it doesn't matter, it's still good to
document it.

It would also be useful to define all the values for the function
selector, in order to extend this driver to provide pinmuxing in the
future, e.g.

#define BCM2835_GPIO_FSEL_ALT0 4
etc.

> +#define GPIO_TO_BANK(n)	(n / 10)
> +#define GPIO_TO_PIN(n)	(n % 10)

There are two kinds of bank relevant to the current driver, and perhaps
more in the HW. The macros should also be prefixed with the module name
to avoid conflicts. PIN isn't really a good name, since the GPIOs
themselves /are/ pins; how about SHIFT? I'd like to see these renamed
BCM2835_GPIO_FSEL_BANK/SHIFT, and also to add
BCM2835_GPIO_OTHER_BANK/SHIFT macros.

(or perhaps GENERAL or 1BIT instead of OTHER?)

> +struct bcm_gpio_regs {
> +	u32 gpfsel[6];
> +	u32 reserved1;

> +	u32 gpset0;
> +	u32 gpset1;

You could replace those two with gpset[2], and then use
BCM2835_GPIO_OTHER_BANK() to index into the array. That will simplify
the driver code a bit. The same applies to gpclr* and gplev* below.

> +	u32 reserved2;
> +	u32 gpclr0;
> +	u32 gpclr1;
> +	u32 reserved3;
> +	u32 gplev0;
> +	u32 gplev1;

There are quite a few more registers defined in the datasheet after
this. Even if the driver doesn't use them yet, it'd still be good to add
them here.

> +};

> diff --git a/drivers/gpio/rpi_gpio.c b/drivers/gpio/rpi_gpio.c

I think this should be named bcm2835_gpio.c, since it's a driver for the
SoC in general, rather than a particular board that uses the SoC.

> +struct bcm_gpio {
> +	u32 bank;
> +	u32 pin;
> +};
...
> +static int get_bank_pin(unsigned gpio, struct bcm_gpio *pio)
> +{
> +	int bank = GPIO_TO_BANK(gpio);
> +	int pin = GPIO_TO_PIN(gpio);
> +
> +	if (!gpio_is_valid(gpio))
> +		return -1;
> +
> +	pin &= 0x09;

GPIO_TO_PIN already performs any required masking. Also, this line
doesn't work, because you want mod 9, not bitwise-and with 9.

> +	pio->pin = pin;
> +	pio->bank = bank;
> +	return 0;
> +}

I'm not really sure that structure or function are useful; you can just
use macros BCM2835_GPIO_*_BANK/SHIFT directly in the code.

> +int gpio_request(unsigned gpio, const char *label)
> +{
> +	return (gpio_is_valid(gpio)) ? 1 : 0;
> +}
> +
> +int gpio_free(unsigned gpio)
> +{
> +	return 0;
> +}

Hmmm. Don't you want to do something like save the label away so you
know who requested the pin for what, and mark it requested so you can't
request it twice? IIRC, there's some "gpio info" command that will list
out all the GPIO owners, at least for some pre-existing GPIO drivers.

> +int gpio_direction_input(unsigned gpio)
> +{
> +	struct bcm_gpio pio;
> +	struct bcm_gpio_regs *reg = (struct bcm_gpio_regs *)BCM2835_GPIO_BASE;
> +	unsigned val = 0;

There's no need to initialize val; it's unconditionally written to below
before it's used.

> +
> +	if (get_bank_pin(gpio, &pio))
> +		return -1;

I would drop that function call and the pio variable, and just use the
macros directly where needed.

> +	val = readl(&reg->gpfsel[pio.bank]);
> +	val &= ~(BCM2835_GPIO_FSEL_CLR_MASK << (pio.pin * 3));

That might be clearer as:

val &= ~(BCM2835_GPIO_FSEL_CLR_MASK << BCM2835_GPIO_FSEL_SHIFT(pin));
val |= ~(BCM2835_GPIO_OUTPUT << BCM2835_GPIO_FSEL_SHIFT(pin));

Similar for gpio_direction_output() below.

> +	writel(val, reg->gpfsel[pio.bank]);

> +int gpio_get_value(unsigned gpio)
> +{
> +	struct bcm_gpio_regs *reg = (struct bcm_gpio_regs *)BCM2835_GPIO_BASE;
> +	unsigned val = 0;
> +
> +	if (!gpio_is_valid(gpio))
> +		return -1;
> +
> +	val = (gpio < 32) ? readl(&reg->gplev0) : readl(&reg->gplev1);

How about:

val = readl(&reg->gplev[BCM2835_GPIO_OTHER_BANK(gpio)]);

> +	gpio &= 0x1f;

That's not needed, since we extract only the bits we care about below
anyway.

> +	return (val & (1 << gpio)) >> gpio;

That would be simpler as:

return (val >> BCM2835_GPIO_OTHER_SHIFT(gpio)) & 1;

> +int gpio_set_value(unsigned gpio, int value)
> +{
> +	struct bcm_gpio_regs *reg = (struct bcm_gpio_regs *)BCM2835_GPIO_BASE;
> +
> +	if (!gpio_is_valid(gpio))
> +		return -1;
> +
> +	if (gpio < 32) {
> +		if (value)
> +			writel((1 << gpio), reg->gpset0);
> +		else
> +			writel((1 << gpio), reg->gpclr0);
> +	} else {
> +		gpio &= 0x1f;
> +		if (value)
> +			writel((1 << pin), reg->gpset1);
> +		else
> +			writel((1 << pin), reg->gpclr1);
> +	}
> +	return 0;
> +}

How about:

u32 *reg = value ? reg->gpset : reg->gpclr;
writel(1 << BCM2835_GPIO_VALUE_SHIFT(gpio),
      reg[BCM2835_GPIO_VALUE_BANK(gpio)]);

BTW, I don't think this patch compiles; gpio_set_value() references
variable "pin", which doesn't exist.

[U-Boot] [PATCH 1/2] bcm: Add GPIO driver for BCM2835 SoC

[Vikram Narayanan]

Hello Stephen,

On 6/27/2012 7:09 AM, Stephen Warren wrote:
> On 06/24/2012 11:21 AM, Vikram Narayanan wrote:
>
> First off, it's great to see some patches for the chip. Thanks. Sorry
> for being so nit-picky below; it's a tendency of mine...

Thanks for the detailed review. I'd make a v2 for this.
And I might probably include you in the signed-off-by line.
Hope you don't mind that.

Do you have a hosted repo somewhere for this rpi_b stuff?
If so, please post it here or we shall have one, which has all the 
patches queued in for the mainline. What do you say? Share your opinions 
about this.

> BTW, I don't think this patch compiles; gpio_set_value() references
> variable "pin", which doesn't exist.

Sorry about that, Late night submission!

Thanks,
Vikram

[U-Boot] [PATCH 1/2] bcm: Add GPIO driver for BCM2835 SoC

[Stephen Warren]

On 06/27/2012 11:32 AM, Vikram Narayanan wrote:
> Hello Stephen,
> 
> On 6/27/2012 7:09 AM, Stephen Warren wrote:
>> On 06/24/2012 11:21 AM, Vikram Narayanan wrote:
>>
>> First off, it's great to see some patches for the chip. Thanks. Sorry
>> for being so nit-picky below; it's a tendency of mine...
> 
> Thanks for the detailed review. I'd make a v2 for this.
> And I might probably include you in the signed-off-by line.
> Hope you don't mind that.

No, you shouldn't add any tags to the patch that refer to other people,
except perhaps a Reported-By, without their explicitly giving those tags.

Also, Signed-off-by wouldn't make sense here since I'm not vouching for
the code or passing it along. Once V2 is posted, I may give an ack or
review tag.

> Do you have a hosted repo somewhere for this rpi_b stuff?
> If so, please post it here or we shall have one, which has all the
> patches queued in for the mainline. What do you say? Share your opinions
> about this.

I do have a repo. It's at:
https://github.com/swarren/u-boot

However, that's my personal work-space. The RPi patches should
eventually make it into the official U-Boot repositories through the
standard review process. They are:

ARM repo:
git://git.denx.de/u-boot-arm.git

Main repo:
git://git.denx.de/u-boot.git

[U-Boot] [PATCH 1/2] bcm: Add GPIO driver for BCM2835 SoC

[Vikram Narayanan]

On 6/27/2012 11:36 PM, Stephen Warren wrote:
> On 06/27/2012 11:32 AM, Vikram Narayanan wrote:
>> Hello Stephen,
>>
>> On 6/27/2012 7:09 AM, Stephen Warren wrote:
>>> On 06/24/2012 11:21 AM, Vikram Narayanan wrote:
>>>
>>> First off, it's great to see some patches for the chip. Thanks. Sorry
>>> for being so nit-picky below; it's a tendency of mine...
>>
>> Thanks for the detailed review. I'd make a v2 for this.
>> And I might probably include you in the signed-off-by line.
>> Hope you don't mind that.
>
> No, you shouldn't add any tags to the patch that refer to other people,
> except perhaps a Reported-By, without their explicitly giving those tags.
>
> Also, Signed-off-by wouldn't make sense here since I'm not vouching for
> the code or passing it along. Once V2 is posted, I may give an ack or
> review tag.

I'm aware of it. :)
Just for the level of details you said to change in the code I said so.
Don't mind that.

>> Do you have a hosted repo somewhere for this rpi_b stuff?
>> If so, please post it here or we shall have one, which has all the
>> patches queued in for the mainline. What do you say? Share your opinions
>> about this.
>
> I do have a repo. It's at:
> https://github.com/swarren/u-boot
>
> However, that's my personal work-space. The RPi patches should
> eventually make it into the official U-Boot repositories through the
> standard review process. They are:
>
> ARM repo:
> git://git.denx.de/u-boot-arm.git
>
> Main repo:
> git://git.denx.de/u-boot.git

I'm aware of this too. I'm referring to a public hosting of these RPi 
patches somewhere, so that it could easily be submitted to the mainline 
in _one_ shot. Since your initial SoC support patches aren't added to 
the u-boot-arm, I planned to have everything queued up for submission 
into the mainline *via* the mailing list. Hope you got my point.

[U-Boot] [PATCH 1/2] bcm: Add GPIO driver for BCM2835 SoC

[Stephen Warren]

On 06/27/2012 08:59 PM, Vikram Narayanan wrote:
> On 6/27/2012 11:36 PM, Stephen Warren wrote:
>> On 06/27/2012 11:32 AM, Vikram Narayanan wrote:
...
>>> Do you have a hosted repo somewhere for this rpi_b stuff?
>>> If so, please post it here or we shall have one, which has all the
>>> patches queued in for the mainline. What do you say? Share your opinions
>>> about this.
>>
>> I do have a repo. It's at:
>> https://github.com/swarren/u-boot
>>
>> However, that's my personal work-space. The RPi patches should
>> eventually make it into the official U-Boot repositories through the
>> standard review process. They are:
>>
>> ARM repo:
>> git://git.denx.de/u-boot-arm.git
>>
>> Main repo:
>> git://git.denx.de/u-boot.git
> 
> I'm aware of this too. I'm referring to a public hosting of these RPi
> patches somewhere, so that it could easily be submitted to the mainline
> in _one_ shot. Since your initial SoC support patches aren't added to
> the u-boot-arm, I planned to have everything queued up for submission
> into the mainline *via* the mailing list. Hope you got my point.

I can certainly apply the GPIO patch to my github tree too if that
helps. However, the U-Boot maintainers have indicated that BCM2835
support is too small to warrant their taking patches via git pull rather
than mailing list posts.

[U-Boot] [PATCH 2/2] rbpi: Add BCM2835 GPIO driver for raspberry pi

[Vikram Narayanan]

Add the driver to the rpi_b's default config

Signed-off-by: Vikram Narayanan <vikram186@gmail.com>
Cc: Stephen Warren <swarren@wwwdotorg.org>
Cc: Albert Aribaud <albert.u.boot@aribaud.net>
---
 include/configs/rpi_b.h |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/include/configs/rpi_b.h b/include/configs/rpi_b.h
index f547027..fb45a18 100644
--- a/include/configs/rpi_b.h
+++ b/include/configs/rpi_b.h
@@ -43,7 +43,9 @@
 #define CONFIG_SYS_NO_FLASH
 
 /* Devices */
-/* None yet */
+/* GPIO */
+#define CONFIG_BCM2835_GPIO
+
 
 /* Console UART */
 #define CONFIG_PL011_SERIAL
-- 
1.7.4.1

[U-Boot] [PATCH 2/2] rbpi: Add BCM2835 GPIO driver for raspberry pi

[Stephen Warren]

On 06/24/2012 11:22 AM, Vikram Narayanan wrote:
> 
> Add the driver to the rpi_b's default config

It looks like there's a blank line before the patch description there.

Aside from that and the nit below,
Acked-by: Stephen Warren <swarren@wwwdotorg.org>

> diff --git a/include/configs/rpi_b.h b/include/configs/rpi_b.h

>  /* Devices */
> -/* None yet */
> +/* GPIO */
> +#define CONFIG_BCM2835_GPIO
> +
>  

No need to add an extra blank line there.

[U-Boot] [PATCH 0/2] Add GPIO driver for BCM2835 SoC

[Stephen Warren]

On 06/24/2012 11:19 AM, Vikram Narayanan wrote:
> Add a GPIO driver for the BCM2835 Soc.
> Refer Datasheet http://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
> Also, add the driver to the raspberrypi's default config

Vikram,

Are you planning on revising this driver and re-posting? I was hoping to
enhance it to add support for the pin muxing functionality of the HW,
but obviously want to wait for any revised version before doing so.

Thanks.

[U-Boot] [PATCH 0/2] Add GPIO driver for BCM2835 SoC

[Vikram Narayanan]

Hello Stephen,

On 7/4/2012 7:37 AM, Stephen Warren wrote:
> On 06/24/2012 11:19 AM, Vikram Narayanan wrote:
>> Add a GPIO driver for the BCM2835 Soc.
>> Refer Datasheet http://www.raspberrypi.org/wp-content/uploads/2012/02/BCM2835-ARM-Peripherals.pdf
>> Also, add the driver to the raspberrypi's default config
>
> Vikram,
>
> Are you planning on revising this driver and re-posting? I was hoping to
> enhance it to add support for the pin muxing functionality of the HW,
> but obviously want to wait for any revised version before doing so.

Yes. I'm on it. I'm quite busy on other tasks.
I'll post the v2 by the end of this week.
Hope this should be ok for you.

Thanks,
Vikram