diff mbox series

[v7,5/9] serial: sc16is7xx: fix regression with GPIO configuration

Message ID 20230602152626.284324-6-hugo@hugovil.com
State Superseded
Headers show
Series serial: sc16is7xx: fix GPIO regression and rs485 improvements | expand

Commit Message

Hugo Villeneuve June 2, 2023, 3:26 p.m. UTC
From: Hugo Villeneuve <hvilleneuve@dimonoff.com>

Commit 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
and commit 21144bab4f11 ("sc16is7xx: Handle modem status lines")
changed the function of the GPIOs pins to act as modem control
lines without any possibility of selecting GPIO function.

As a consequence, applications that depends on GPIO lines configured
by default as GPIO pins no longer work as expected.

Also, the change to select modem control lines function was done only
for channel A of dual UART variants (752/762). This was not documented
in the log message.

Allow to specify GPIO or modem control line function in the device
tree, and for each of the ports (A or B).

Do so by using the new device-tree property named
"modem-control-line-ports" (property added in separate patch).

When registering GPIO chip controller, mask-out GPIO pins declared as
modem control lines according to this new "modem-control-line-ports"
DT property.

Boards that need to have GPIOS configured as modem control lines
should add that property to their device tree. Here is a list of
boards using the sc16is7xx driver in their device tree and that may
need to be modified:
    arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
    mips/boot/dts/ingenic/cu1830-neo.dts
    mips/boot/dts/ingenic/cu1000-neo.dts

Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
Fixes: 21144bab4f11 ("sc16is7xx: Handle modem status lines")
Cc: <stable@vger.kernel.org> # 6.1.x: 35210b22 dt-bindings: sc16is7xx: Add property to change GPIO function
Cc: <stable@vger.kernel.org> # 6.1.x: 7d61ca47 serial: sc16is7xx: refactor GPIO controller registration
Cc: <stable@vger.kernel.org> # 6.1.x: 322470ed serial: sc16is7xx: mark IOCONTROL register as volatile
Cc: <stable@vger.kernel.org> # 6.1.x: a0077362 serial: sc16is7xx: fix broken port 0 uart init
Cc: <stable@vger.kernel.org> # 6.1.x
Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
---
 drivers/tty/serial/sc16is7xx.c | 103 ++++++++++++++++++++++++++-------
 1 file changed, 82 insertions(+), 21 deletions(-)

Comments

Hugo Villeneuve June 4, 2023, 5:43 p.m. UTC | #1
On Sun, 4 Jun 2023 09:47:44 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Fri, Jun 02, 2023 at 11:26:21AM -0400, Hugo Villeneuve wrote:
> > From: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > 
> > Commit 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
> > and commit 21144bab4f11 ("sc16is7xx: Handle modem status lines")
> > changed the function of the GPIOs pins to act as modem control
> > lines without any possibility of selecting GPIO function.
> > 
> > As a consequence, applications that depends on GPIO lines configured
> > by default as GPIO pins no longer work as expected.
> > 
> > Also, the change to select modem control lines function was done only
> > for channel A of dual UART variants (752/762). This was not documented
> > in the log message.
> > 
> > Allow to specify GPIO or modem control line function in the device
> > tree, and for each of the ports (A or B).
> > 
> > Do so by using the new device-tree property named
> > "modem-control-line-ports" (property added in separate patch).
> > 
> > When registering GPIO chip controller, mask-out GPIO pins declared as
> > modem control lines according to this new "modem-control-line-ports"
> > DT property.
> > 
> > Boards that need to have GPIOS configured as modem control lines
> > should add that property to their device tree. Here is a list of
> > boards using the sc16is7xx driver in their device tree and that may
> > need to be modified:
> >     arm64/boot/dts/freescale/fsl-ls1012a-frdm.dts
> >     mips/boot/dts/ingenic/cu1830-neo.dts
> >     mips/boot/dts/ingenic/cu1000-neo.dts
> > 
> > Fixes: 679875d1d880 ("sc16is7xx: Separate GPIOs from modem control lines")
> > Fixes: 21144bab4f11 ("sc16is7xx: Handle modem status lines")
> > Cc: <stable@vger.kernel.org> # 6.1.x: 35210b22 dt-bindings: sc16is7xx: Add property to change GPIO function
> > Cc: <stable@vger.kernel.org> # 6.1.x: 7d61ca47 serial: sc16is7xx: refactor GPIO controller registration
> > Cc: <stable@vger.kernel.org> # 6.1.x: 322470ed serial: sc16is7xx: mark IOCONTROL register as volatile
> > Cc: <stable@vger.kernel.org> # 6.1.x: a0077362 serial: sc16is7xx: fix broken port 0 uart init
> > Cc: <stable@vger.kernel.org> # 6.1.x
> > Signed-off-by: Hugo Villeneuve <hvilleneuve@dimonoff.com>
> > Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
> > ---
> >  drivers/tty/serial/sc16is7xx.c | 103 ++++++++++++++++++++++++++-------
> >  1 file changed, 82 insertions(+), 21 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
> > index 7d50674d2d0e..edc83f5f6340 100644
> > --- a/drivers/tty/serial/sc16is7xx.c
> > +++ b/drivers/tty/serial/sc16is7xx.c
> > @@ -236,7 +236,8 @@
> >  
> >  /* IOControl register bits (Only 750/760) */
> >  #define SC16IS7XX_IOCONTROL_LATCH_BIT	(1 << 0) /* Enable input latching */
> > -#define SC16IS7XX_IOCONTROL_MODEM_BIT	(1 << 1) /* Enable GPIO[7:4] as modem pins */
> > +#define SC16IS7XX_IOCONTROL_MODEM_A_BIT	(1 << 1) /* Enable GPIO[7:4] as modem A pins */
> > +#define SC16IS7XX_IOCONTROL_MODEM_B_BIT	(1 << 2) /* Enable GPIO[3:0] as modem B pins */
> >  #define SC16IS7XX_IOCONTROL_SRESET_BIT	(1 << 3) /* Software Reset */
> >  
> >  /* EFCR register bits */
> > @@ -301,12 +302,12 @@
> >  /* Misc definitions */
> >  #define SC16IS7XX_FIFO_SIZE		(64)
> >  #define SC16IS7XX_REG_SHIFT		2
> > +#define SC16IS7XX_GPIOS_PER_BANK	4
> >  
> >  struct sc16is7xx_devtype {
> >  	char	name[10];
> >  	int	nr_gpio;
> >  	int	nr_uart;
> > -	int	has_mctrl;
> >  };
> >  
> >  #define SC16IS7XX_RECONF_MD		(1 << 0)
> > @@ -336,6 +337,7 @@ struct sc16is7xx_port {
> >  	struct clk			*clk;
> >  #ifdef CONFIG_GPIOLIB
> >  	struct gpio_chip		gpio;
> > +	unsigned long			gpio_valid_mask;
> >  #endif
> >  	unsigned char			buf[SC16IS7XX_FIFO_SIZE];
> >  	struct kthread_worker		kworker;
> > @@ -447,35 +449,30 @@ static const struct sc16is7xx_devtype sc16is74x_devtype = {
> >  	.name		= "SC16IS74X",
> >  	.nr_gpio	= 0,
> >  	.nr_uart	= 1,
> > -	.has_mctrl	= 0,
> >  };
> >  
> >  static const struct sc16is7xx_devtype sc16is750_devtype = {
> >  	.name		= "SC16IS750",
> > -	.nr_gpio	= 4,
> > +	.nr_gpio	= 8,
> >  	.nr_uart	= 1,
> > -	.has_mctrl	= 1,
> >  };
> >  
> >  static const struct sc16is7xx_devtype sc16is752_devtype = {
> >  	.name		= "SC16IS752",
> > -	.nr_gpio	= 0,
> > +	.nr_gpio	= 8,
> >  	.nr_uart	= 2,
> > -	.has_mctrl	= 1,
> >  };
> >  
> >  static const struct sc16is7xx_devtype sc16is760_devtype = {
> >  	.name		= "SC16IS760",
> > -	.nr_gpio	= 4,
> > +	.nr_gpio	= 8,
> >  	.nr_uart	= 1,
> > -	.has_mctrl	= 1,
> >  };
> >  
> >  static const struct sc16is7xx_devtype sc16is762_devtype = {
> >  	.name		= "SC16IS762",
> > -	.nr_gpio	= 0,
> > +	.nr_gpio	= 8,
> >  	.nr_uart	= 2,
> > -	.has_mctrl	= 1,
> >  };
> >  
> >  static bool sc16is7xx_regmap_volatile(struct device *dev, unsigned int reg)
> > @@ -1350,16 +1347,45 @@ static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip,
> >  	return 0;
> >  }
> >  
> > -static int sc16is7xx_setup_gpio_chip(struct device *dev)
> > +static int sc16is7xx_gpio_init_valid_mask(struct gpio_chip *chip,
> > +					  unsigned long *valid_mask,
> > +					  unsigned int ngpios)
> > +{
> > +	struct sc16is7xx_port *s = gpiochip_get_data(chip);
> > +
> > +	*valid_mask = s->gpio_valid_mask;
> > +
> > +	return 0;
> > +}
> > +
> > +static int sc16is7xx_setup_gpio_chip(struct device *dev, u8 mctrl_mask)
> >  {
> >  	struct sc16is7xx_port *s = dev_get_drvdata(dev);
> >  
> >  	if (!s->devtype->nr_gpio)
> >  		return 0;
> >  
> > +	switch (mctrl_mask) {
> > +	case 0:
> > +		s->gpio_valid_mask = GENMASK(7, 0);
> > +		break;
> > +	case SC16IS7XX_IOCONTROL_MODEM_A_BIT:
> > +		s->gpio_valid_mask = GENMASK(3, 0);
> > +		break;
> > +	case SC16IS7XX_IOCONTROL_MODEM_B_BIT:
> > +		s->gpio_valid_mask = GENMASK(7, 4);
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> > +	if (s->gpio_valid_mask == 0)
> > +		return 0;
> > +
> >  	s->gpio.owner		 = THIS_MODULE;
> >  	s->gpio.parent		 = dev;
> >  	s->gpio.label		 = dev_name(dev);
> > +	s->gpio.init_valid_mask	 = sc16is7xx_gpio_init_valid_mask;
> >  	s->gpio.direction_input	 = sc16is7xx_gpio_direction_input;
> >  	s->gpio.get		 = sc16is7xx_gpio_get;
> >  	s->gpio.direction_output = sc16is7xx_gpio_direction_output;
> > @@ -1371,6 +1397,44 @@ static int sc16is7xx_setup_gpio_chip(struct device *dev)
> >  }
> >  #endif
> >  
> > +static u8 sc16is7xx_setup_mctrl_ports(struct device *dev)
> 
> This returns what, mctrl?  If so, please document that, it doesn't look
> obvious.  And as the kernel test robot reported, you do nothing with the
> return value so why compute it?

Hi Greg,
I will  the following comment to document return value:

/*
 * Configure ports designated to operate as modem control lines.
 * Return mask of ports configured as modem control lines.
 */
static u8 sc16is7xx_setup_mctrl_ports(struct device *dev)


The kernel test robot identified a case, when CONFIG_GPIOLIB is not defined, where the value returned by sc16is7xx_setup_mctrl_ports() is not used. But the function sc16is7xx_setup_mctrl_ports() still need to be called to configure the modem status line ports correctly in that case.

And if CONFIG_GPIOLIB is defined, the value is definitely used (and needed).

Here is what I suggest to silence the warning:

	mctrl_mask = sc16is7xx_setup_mctrl_ports(dev);

#ifdef CONFIG_GPIOLIB
	ret = sc16is7xx_setup_gpio_chip(dev, mctrl_mask);
	if (ret)
		goto out_thread;
#else
	(void) mctrl_mask;
#endif

I could also store (define new variable) mctrl_mask directly inside struct sc16is7xx_port...


> And you have a real port here, no need to pass in a "raw" struct device,
> right?

The function operates globally on both ports (or nr_uart), not just a single port. That is why I pass the "raw" struct device, in order to extract the 
struct sc16is7xx_port from it:

    struct sc16is7xx_port *s = dev_get_drvdata(dev);

Inside the function, I also need the "raw" struc device. If we pass a struct sc16is7xx_port to the function, then I can get the "raw" struc device with this:

static u8 sc16is7xx_setup_mctrl_ports(struct sc16is7xx_port *s)
{
	struct device *dev = &s->p[0].port.dev;

But I find this more obfuscated and hard to understand than to simply pass a "raw" struct device...

Hugo.
Greg Kroah-Hartman June 4, 2023, 6:29 p.m. UTC | #2
On Sun, Jun 04, 2023 at 01:43:44PM -0400, Hugo Villeneuve wrote:
> Here is what I suggest to silence the warning:
> 
> 	mctrl_mask = sc16is7xx_setup_mctrl_ports(dev);
> 
> #ifdef CONFIG_GPIOLIB
> 	ret = sc16is7xx_setup_gpio_chip(dev, mctrl_mask);
> 	if (ret)
> 		goto out_thread;
> #else
> 	(void) mctrl_mask;
> #endif

Eeek,  no, please no...

First off, please don't put #ifdef in .c files if at all possible.
Secondly, that (void) craziness is just that.  Rework this to not be an
issue some other way please.

> I could also store (define new variable) mctrl_mask directly inside struct sc16is7xx_port...

Sure, that sounds best.

> > And you have a real port here, no need to pass in a "raw" struct device,
> > right?
> 
> The function operates globally on both ports (or nr_uart), not just a single port. That is why I pass the "raw" struct device, in order to extract the 
> struct sc16is7xx_port from it:
> 
>     struct sc16is7xx_port *s = dev_get_drvdata(dev);
> 
> Inside the function, I also need the "raw" struc device. If we pass a struct sc16is7xx_port to the function, then I can get the "raw" struc device with this:
> 
> static u8 sc16is7xx_setup_mctrl_ports(struct sc16is7xx_port *s)
> {
> 	struct device *dev = &s->p[0].port.dev;
> 
> But I find this more obfuscated and hard to understand than to simply pass a "raw" struct device...

You should never need a "raw" struct device for stuff (if so, something
is really odd).  Except for error messages, but that's not really a big
deal, right?

Don't pass around struct device in a driver, use the real types as you
know you have it and it saves odd casting around and it just doesn't
look safe at all to do so.

And if you have that crazy s->p.... stuff in multiple places, then
perhaps you might want to rethink the structure somehow?  Or at the very
least, write an inline function to get it when needed.

Also, meta comment, you might want to use some \n characters in your
emails, your lines are really long :)

thanks,

greg k-h
Hugo Villeneuve June 4, 2023, 11:16 p.m. UTC | #3
On Sun, 4 Jun 2023 20:29:58 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Sun, Jun 04, 2023 at 01:43:44PM -0400, Hugo Villeneuve wrote:
> > Here is what I suggest to silence the warning:
> > 
> > 	mctrl_mask = sc16is7xx_setup_mctrl_ports(dev);
> > 
> > #ifdef CONFIG_GPIOLIB
> > 	ret = sc16is7xx_setup_gpio_chip(dev, mctrl_mask);
> > 	if (ret)
> > 		goto out_thread;
> > #else
> > 	(void) mctrl_mask;
> > #endif
> 
> Eeek,  no, please no...
> 
> First off, please don't put #ifdef in .c files if at all possible.

Hi Greg,
Andy also made a similar comment, but couldn't suggest a valid
alternative when I asked him what to do about that.

Just as a sidenote, I didn't add those #ifdef, they were already
present in the driver in multiple places.

What would be your suggestion to get rid of those #ifdef, simply delete
them all?

If you suggest me what to do, I will be happy to submit a
future patch after this series is finalized to clean that aspect.


> Secondly, that (void) craziness is just that.  Rework this to not be an
> issue some other way please.
> 
> > I could also store (define new variable) mctrl_mask directly inside struct sc16is7xx_port...
> 
> Sure, that sounds best.

Ok, I will do that.


> > > And you have a real port here, no need to pass in a "raw" struct device,
> > > right?
> > 
> > The function operates globally on both ports (or nr_uart), not just a single port. That is why I pass the "raw" struct device, in order to extract the 
> > struct sc16is7xx_port from it:
> > 
> >     struct sc16is7xx_port *s = dev_get_drvdata(dev);
> > 
> > Inside the function, I also need the "raw" struc device. If we pass a struct sc16is7xx_port to the function, then I can get the "raw" struc device with this:
> > 
> > static u8 sc16is7xx_setup_mctrl_ports(struct sc16is7xx_port *s)
> > {
> > 	struct device *dev = &s->p[0].port.dev;
> > 
> > But I find this more obfuscated and hard to understand than to simply pass a "raw" struct device...
> 
> You should never need a "raw" struct device for stuff (if so, something
> is really odd).  Except for error messages, but that's not really a big
> deal, right?

> Don't pass around struct device in a driver, use the real types as you
> know you have it and it saves odd casting around and it just doesn't
> look safe at all to do so.

If you look at the patch, you will see that I need "struct device *dev"
at two places in the sc16is7xx_setup_mctrl_ports() function to read the
device properties:

...
+static u8 sc16is7xx_setup_mctrl_ports(struct device *dev)
...
+	count = device_property_count_u32(dev,...
...
+	ret = device_property_read_u32_array(dev,
...

I do not understand why this is odd?


> And if you have that crazy s->p.... stuff in multiple places, the
> perhaps you might want to rethink the structure somehow?  Or at the very
> least, write an inline function to get it when needed.

I am not sure what you mean by that, since again that "crazy" stuff is
already used everywhere in this driver?


> Also, meta comment, you might want to use some \n characters in your
> emails, your lines are really long :)

Strange, I use sylpheed as a mail client, and the option "Wrap lines at
72 characters" is enabled by default, but somehow you must also check
the box "Wrap on input" for it to work, not very intuitive :) Thanks for
pointing that to me.

Hugo.
Hugo Villeneuve June 5, 2023, 5:53 p.m. UTC | #4
On Sun, 4 Jun 2023 22:31:04 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sun, Jun 4, 2023 at 8:45 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> >
> > On Sun, 4 Jun 2023 14:57:31 +0300
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> >
> > > On Sun, Jun 4, 2023 at 10:47 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > On Fri, Jun 02, 2023 at 11:26:21AM -0400, Hugo Villeneuve wrote:
> > >
> > > ...
> > >
> > > > > +static u8 sc16is7xx_setup_mctrl_ports(struct device *dev)
> > > >
> > > > This returns what, mctrl?  If so, please document that, it doesn't look
> > > > obvious.
> > >
> > > Good suggestion. Because I also stumbled over the returned type.
> > >
> > > >  And as the kernel test robot reported, you do nothing with the
> > > > return value so why compute it?
> > >
> > > It seems that the entire function and respective call has to be moved
> > > under #ifdef CONFIG_GPIOLIB.
> >
> > Hi,
> > it cannot. See my explanations in response to Greg's comments.
> 
> Then as Greg suggested, store in the structure and make this function
> to return an error code (with int), with this amendment you don't need
> to add a comment about the returned variable anymore.

Hi,
Yes, that is what I have done for V8. Simplifies/clean things a lot.

Hugo.
Hugo Villeneuve June 20, 2023, 2:08 p.m. UTC | #5
On Sun, 4 Jun 2023 22:31:04 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Sun, Jun 4, 2023 at 8:45 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> >
> > On Sun, 4 Jun 2023 14:57:31 +0300
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> >
> > > On Sun, Jun 4, 2023 at 10:47 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > > On Fri, Jun 02, 2023 at 11:26:21AM -0400, Hugo Villeneuve wrote:
> > >
> > > ...
> > >
> > > > > +static u8 sc16is7xx_setup_mctrl_ports(struct device *dev)
> > > >
> > > > This returns what, mctrl?  If so, please document that, it doesn't look
> > > > obvious.
> > >
> > > Good suggestion. Because I also stumbled over the returned type.
> > >
> > > >  And as the kernel test robot reported, you do nothing with the
> > > > return value so why compute it?
> > >
> > > It seems that the entire function and respective call has to be moved
> > > under #ifdef CONFIG_GPIOLIB.
> >
> > Hi,
> > it cannot. See my explanations in response to Greg's comments.
> 
> Then as Greg suggested, store in the structure and make this function
> to return an error code (with int), with this amendment you don't need
> to add a comment about the returned variable anymore.

Hi Andy,
did you have a chance to look at V8 (sent two weks ago) which fixed all
of what we discussed?

Thank you,
Hugo.
Andy Shevchenko June 20, 2023, 3:18 p.m. UTC | #6
On Tue, Jun 20, 2023 at 5:08 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> On Sun, 4 Jun 2023 22:31:04 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

...

> did you have a chance to look at V8 (sent two weks ago) which fixed all
> of what we discussed?

The patch 6 already has my tag, anything specific you want me to do?
Hugo Villeneuve June 20, 2023, 3:33 p.m. UTC | #7
On Tue, 20 Jun 2023 18:18:12 +0300
Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

> On Tue, Jun 20, 2023 at 5:08 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > On Sun, 4 Jun 2023 22:31:04 +0300
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> ...
> 
> > did you have a chance to look at V8 (sent two weks ago) which fixed all
> > of what we discussed?
> 
> The patch 6 already has my tag, anything specific you want me to do?

Hi Andy,
I forgot to remove your "Reviewed-by: Andy..." tag before sending V8
since there were some changes involved in patch 6 and I wanted you to
review them. Can you confirm if the changes are correct?

I also added a new patch "remove obsolete out_thread label". It has no 
real impact on the code generation itself, but maybe you can review and
confirm if tags are ok or not, based on commit message and also
additional commit message.

Thank you, Hugo.
Andy Shevchenko June 20, 2023, 3:35 p.m. UTC | #8
On Tue, Jun 20, 2023 at 6:33 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> On Tue, 20 Jun 2023 18:18:12 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Tue, Jun 20, 2023 at 5:08 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > On Sun, 4 Jun 2023 22:31:04 +0300
> > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

...

> > > did you have a chance to look at V8 (sent two weks ago) which fixed all
> > > of what we discussed?
> >
> > The patch 6 already has my tag, anything specific you want me to do?
>
> Hi Andy,
> I forgot to remove your "Reviewed-by: Andy..." tag before sending V8
> since there were some changes involved in patch 6 and I wanted you to
> review them. Can you confirm if the changes are correct?
>
> I also added a new patch "remove obsolete out_thread label". It has no
> real impact on the code generation itself, but maybe you can review and
> confirm if tags are ok or not, based on commit message and also
> additional commit message.

Both are fine to me.
Andy Shevchenko June 20, 2023, 3:45 p.m. UTC | #9
On Tue, Jun 20, 2023 at 6:42 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> On Tue, 20 Jun 2023 18:35:48 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > On Tue, Jun 20, 2023 at 6:33 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > On Tue, 20 Jun 2023 18:18:12 +0300
> > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > On Tue, Jun 20, 2023 at 5:08 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > On Sun, 4 Jun 2023 22:31:04 +0300
> > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:

...

> > > > > did you have a chance to look at V8 (sent two weks ago) which fixed all
> > > > > of what we discussed?
> > > >
> > > > The patch 6 already has my tag, anything specific you want me to do?
> > >
> > > Hi Andy,
> > > I forgot to remove your "Reviewed-by: Andy..." tag before sending V8
> > > since there were some changes involved in patch 6 and I wanted you to
> > > review them. Can you confirm if the changes are correct?
> > >
> > > I also added a new patch "remove obsolete out_thread label". It has no
> > > real impact on the code generation itself, but maybe you can review and
> > > confirm if tags are ok or not, based on commit message and also
> > > additional commit message.
> >
> > Both are fine to me.
>
> Hi,
> Ok, thank you for reviewing this.
>
> I guess now we are good to go with this series if the stable tags and
> patches order are good after Greg's review?

Taking into account that we are at rc7, and even with Fixes tags in
your series I think Greg might take this after v6.5-0rc1 is out. It's
up to him how to proceed with that. Note, he usually has thousands of
patches in backlog, you might need to respin it after the above
mentioned rc1.
Hugo Villeneuve July 19, 2023, 6:40 p.m. UTC | #10
On Tue, 20 Jun 2023 12:16:45 -0400
Hugo Villeneuve <hugo@hugovil.com> wrote:

> On Tue, 20 Jun 2023 18:45:51 +0300
> Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> 
> > On Tue, Jun 20, 2023 at 6:42 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > On Tue, 20 Jun 2023 18:35:48 +0300
> > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > On Tue, Jun 20, 2023 at 6:33 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > On Tue, 20 Jun 2023 18:18:12 +0300
> > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > On Tue, Jun 20, 2023 at 5:08 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > On Sun, 4 Jun 2023 22:31:04 +0300
> > > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > 
> > ...
> > 
> > > > > > > did you have a chance to look at V8 (sent two weks ago) which fixed all
> > > > > > > of what we discussed?
> > > > > >
> > > > > > The patch 6 already has my tag, anything specific you want me to do?
> > > > >
> > > > > Hi Andy,
> > > > > I forgot to remove your "Reviewed-by: Andy..." tag before sending V8
> > > > > since there were some changes involved in patch 6 and I wanted you to
> > > > > review them. Can you confirm if the changes are correct?
> > > > >
> > > > > I also added a new patch "remove obsolete out_thread label". It has no
> > > > > real impact on the code generation itself, but maybe you can review and
> > > > > confirm if tags are ok or not, based on commit message and also
> > > > > additional commit message.
> > > >
> > > > Both are fine to me.
> > >
> > > Hi,
> > > Ok, thank you for reviewing this.
> > >
> > > I guess now we are good to go with this series if the stable tags and
> > > patches order are good after Greg's review?
> > 
> > Taking into account that we are at rc7, and even with Fixes tags in
> > your series I think Greg might take this after v6.5-0rc1 is out. It's
> > up to him how to proceed with that. Note, he usually has thousands of
> > patches in backlog, you might need to respin it after the above
> > mentioned rc1.
> 
> Ok, understood.
> 
> Let's wait then.

Hi Andy/Greg,
we are now at v6.5-rc2 and I still do not see any of our patches in
linus or gregkh_tty repos.

Is there something missing from my part (or someone else) to go forward
with integrating these patches (v8) for v6.5?

Thank you,
Hugo.
Greg Kroah-Hartman July 19, 2023, 7:14 p.m. UTC | #11
On Wed, Jul 19, 2023 at 02:40:48PM -0400, Hugo Villeneuve wrote:
> On Tue, 20 Jun 2023 12:16:45 -0400
> Hugo Villeneuve <hugo@hugovil.com> wrote:
> 
> > On Tue, 20 Jun 2023 18:45:51 +0300
> > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > 
> > > On Tue, Jun 20, 2023 at 6:42 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > On Tue, 20 Jun 2023 18:35:48 +0300
> > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > On Tue, Jun 20, 2023 at 6:33 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > On Tue, 20 Jun 2023 18:18:12 +0300
> > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > > On Tue, Jun 20, 2023 at 5:08 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > > On Sun, 4 Jun 2023 22:31:04 +0300
> > > > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > 
> > > ...
> > > 
> > > > > > > > did you have a chance to look at V8 (sent two weks ago) which fixed all
> > > > > > > > of what we discussed?
> > > > > > >
> > > > > > > The patch 6 already has my tag, anything specific you want me to do?
> > > > > >
> > > > > > Hi Andy,
> > > > > > I forgot to remove your "Reviewed-by: Andy..." tag before sending V8
> > > > > > since there were some changes involved in patch 6 and I wanted you to
> > > > > > review them. Can you confirm if the changes are correct?
> > > > > >
> > > > > > I also added a new patch "remove obsolete out_thread label". It has no
> > > > > > real impact on the code generation itself, but maybe you can review and
> > > > > > confirm if tags are ok or not, based on commit message and also
> > > > > > additional commit message.
> > > > >
> > > > > Both are fine to me.
> > > >
> > > > Hi,
> > > > Ok, thank you for reviewing this.
> > > >
> > > > I guess now we are good to go with this series if the stable tags and
> > > > patches order are good after Greg's review?
> > > 
> > > Taking into account that we are at rc7, and even with Fixes tags in
> > > your series I think Greg might take this after v6.5-0rc1 is out. It's
> > > up to him how to proceed with that. Note, he usually has thousands of
> > > patches in backlog, you might need to respin it after the above
> > > mentioned rc1.
> > 
> > Ok, understood.
> > 
> > Let's wait then.
> 
> Hi Andy/Greg,
> we are now at v6.5-rc2 and I still do not see any of our patches in
> linus or gregkh_tty repos.
> 
> Is there something missing from my part (or someone else) to go forward
> with integrating these patches (v8) for v6.5?

My queue is huge right now, please be patient, I want to have them all
handled by the end of next week...

You can always help out by reviewing other patches on the mailing list
to reduce my review load.

thanks,

greg k-h
Greg Kroah-Hartman July 20, 2023, 7:38 p.m. UTC | #12
On Wed, Jul 19, 2023 at 09:14:23PM +0200, Greg KH wrote:
> On Wed, Jul 19, 2023 at 02:40:48PM -0400, Hugo Villeneuve wrote:
> > On Tue, 20 Jun 2023 12:16:45 -0400
> > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > 
> > > On Tue, 20 Jun 2023 18:45:51 +0300
> > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > 
> > > > On Tue, Jun 20, 2023 at 6:42 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > On Tue, 20 Jun 2023 18:35:48 +0300
> > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > On Tue, Jun 20, 2023 at 6:33 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > On Tue, 20 Jun 2023 18:18:12 +0300
> > > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > > > On Tue, Jun 20, 2023 at 5:08 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > > > On Sun, 4 Jun 2023 22:31:04 +0300
> > > > > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > 
> > > > ...
> > > > 
> > > > > > > > > did you have a chance to look at V8 (sent two weks ago) which fixed all
> > > > > > > > > of what we discussed?
> > > > > > > >
> > > > > > > > The patch 6 already has my tag, anything specific you want me to do?
> > > > > > >
> > > > > > > Hi Andy,
> > > > > > > I forgot to remove your "Reviewed-by: Andy..." tag before sending V8
> > > > > > > since there were some changes involved in patch 6 and I wanted you to
> > > > > > > review them. Can you confirm if the changes are correct?
> > > > > > >
> > > > > > > I also added a new patch "remove obsolete out_thread label". It has no
> > > > > > > real impact on the code generation itself, but maybe you can review and
> > > > > > > confirm if tags are ok or not, based on commit message and also
> > > > > > > additional commit message.
> > > > > >
> > > > > > Both are fine to me.
> > > > >
> > > > > Hi,
> > > > > Ok, thank you for reviewing this.
> > > > >
> > > > > I guess now we are good to go with this series if the stable tags and
> > > > > patches order are good after Greg's review?
> > > > 
> > > > Taking into account that we are at rc7, and even with Fixes tags in
> > > > your series I think Greg might take this after v6.5-0rc1 is out. It's
> > > > up to him how to proceed with that. Note, he usually has thousands of
> > > > patches in backlog, you might need to respin it after the above
> > > > mentioned rc1.
> > > 
> > > Ok, understood.
> > > 
> > > Let's wait then.
> > 
> > Hi Andy/Greg,
> > we are now at v6.5-rc2 and I still do not see any of our patches in
> > linus or gregkh_tty repos.
> > 
> > Is there something missing from my part (or someone else) to go forward
> > with integrating these patches (v8) for v6.5?
> 
> My queue is huge right now, please be patient, I want to have them all
> handled by the end of next week...
> 
> You can always help out by reviewing other patches on the mailing list
> to reduce my review load.

Wait, no, this series was superseeded by v8, and in there you said you
were going to send a new series.  So please, fix it up and send the
updated version of the series, this one isn't going to be applied for
obvious reasons.

thanks,

greg k-h
Hugo Villeneuve July 21, 2023, 3:25 p.m. UTC | #13
On Thu, 20 Jul 2023 21:38:21 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Wed, Jul 19, 2023 at 09:14:23PM +0200, Greg KH wrote:
> > On Wed, Jul 19, 2023 at 02:40:48PM -0400, Hugo Villeneuve wrote:
> > > On Tue, 20 Jun 2023 12:16:45 -0400
> > > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > 
> > > > On Tue, 20 Jun 2023 18:45:51 +0300
> > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > 
> > > > > On Tue, Jun 20, 2023 at 6:42 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > On Tue, 20 Jun 2023 18:35:48 +0300
> > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > > On Tue, Jun 20, 2023 at 6:33 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > > On Tue, 20 Jun 2023 18:18:12 +0300
> > > > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > > > > On Tue, Jun 20, 2023 at 5:08 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > > > > On Sun, 4 Jun 2023 22:31:04 +0300
> > > > > > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > 
> > > > > ...
> > > > > 
> > > > > > > > > > did you have a chance to look at V8 (sent two weks ago) which fixed all
> > > > > > > > > > of what we discussed?
> > > > > > > > >
> > > > > > > > > The patch 6 already has my tag, anything specific you want me to do?
> > > > > > > >
> > > > > > > > Hi Andy,
> > > > > > > > I forgot to remove your "Reviewed-by: Andy..." tag before sending V8
> > > > > > > > since there were some changes involved in patch 6 and I wanted you to
> > > > > > > > review them. Can you confirm if the changes are correct?
> > > > > > > >
> > > > > > > > I also added a new patch "remove obsolete out_thread label". It has no
> > > > > > > > real impact on the code generation itself, but maybe you can review and
> > > > > > > > confirm if tags are ok or not, based on commit message and also
> > > > > > > > additional commit message.
> > > > > > >
> > > > > > > Both are fine to me.
> > > > > >
> > > > > > Hi,
> > > > > > Ok, thank you for reviewing this.
> > > > > >
> > > > > > I guess now we are good to go with this series if the stable tags and
> > > > > > patches order are good after Greg's review?
> > > > > 
> > > > > Taking into account that we are at rc7, and even with Fixes tags in
> > > > > your series I think Greg might take this after v6.5-0rc1 is out. It's
> > > > > up to him how to proceed with that. Note, he usually has thousands of
> > > > > patches in backlog, you might need to respin it after the above
> > > > > mentioned rc1.
> > > > 
> > > > Ok, understood.
> > > > 
> > > > Let's wait then.
> > > 
> > > Hi Andy/Greg,
> > > we are now at v6.5-rc2 and I still do not see any of our patches in
> > > linus or gregkh_tty repos.
> > > 
> > > Is there something missing from my part (or someone else) to go forward
> > > with integrating these patches (v8) for v6.5?
> > 
> > My queue is huge right now, please be patient, I want to have them all
> > handled by the end of next week...
> > 
> > You can always help out by reviewing other patches on the mailing list
> > to reduce my review load.
> 
> Wait, no, this series was superseeded by v8, and in there you said you
> were going to send a new series.  So please, fix it up and send the
> updated version of the series, this one isn't going to be applied for
> obvious reasons.

Hi Greg,
I never said that I would resend another update for this current
serie (unless of course if it was to address a new comment). Re-reading
that email made me realise that it was maybe not perfectly clear the
way I wrote it.

What I said was that, once V8 was finally applied and
incorporated in the kernel, then I would send a completely new and
different serie to address issues/concerns/improvements/suggestions
noted during the review of this serie (example: conversion of bindings
to YAML and improve DTS node names, etc). We already agreed with some
maintainers (ex: Conor Dooley) that it was reasonnable to do so.

That is why I asked Andy if we were good to go with V8 and he
confirmed that, and that it was now up to you to integrate it if your
review was satisfactory.

Hope this clears things and we can integrate it soon.

Thank you, Hugo.
Greg Kroah-Hartman July 21, 2023, 3:40 p.m. UTC | #14
On Fri, Jul 21, 2023 at 11:25:17AM -0400, Hugo Villeneuve wrote:
> On Thu, 20 Jul 2023 21:38:21 +0200
> Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > On Wed, Jul 19, 2023 at 09:14:23PM +0200, Greg KH wrote:
> > > On Wed, Jul 19, 2023 at 02:40:48PM -0400, Hugo Villeneuve wrote:
> > > > On Tue, 20 Jun 2023 12:16:45 -0400
> > > > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > 
> > > > > On Tue, 20 Jun 2023 18:45:51 +0300
> > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > 
> > > > > > On Tue, Jun 20, 2023 at 6:42 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > On Tue, 20 Jun 2023 18:35:48 +0300
> > > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > > > On Tue, Jun 20, 2023 at 6:33 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > > > On Tue, 20 Jun 2023 18:18:12 +0300
> > > > > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > > > > > On Tue, Jun 20, 2023 at 5:08 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > > > > > On Sun, 4 Jun 2023 22:31:04 +0300
> > > > > > > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > 
> > > > > > ...
> > > > > > 
> > > > > > > > > > > did you have a chance to look at V8 (sent two weks ago) which fixed all
> > > > > > > > > > > of what we discussed?
> > > > > > > > > >
> > > > > > > > > > The patch 6 already has my tag, anything specific you want me to do?
> > > > > > > > >
> > > > > > > > > Hi Andy,
> > > > > > > > > I forgot to remove your "Reviewed-by: Andy..." tag before sending V8
> > > > > > > > > since there were some changes involved in patch 6 and I wanted you to
> > > > > > > > > review them. Can you confirm if the changes are correct?
> > > > > > > > >
> > > > > > > > > I also added a new patch "remove obsolete out_thread label". It has no
> > > > > > > > > real impact on the code generation itself, but maybe you can review and
> > > > > > > > > confirm if tags are ok or not, based on commit message and also
> > > > > > > > > additional commit message.
> > > > > > > >
> > > > > > > > Both are fine to me.
> > > > > > >
> > > > > > > Hi,
> > > > > > > Ok, thank you for reviewing this.
> > > > > > >
> > > > > > > I guess now we are good to go with this series if the stable tags and
> > > > > > > patches order are good after Greg's review?
> > > > > > 
> > > > > > Taking into account that we are at rc7, and even with Fixes tags in
> > > > > > your series I think Greg might take this after v6.5-0rc1 is out. It's
> > > > > > up to him how to proceed with that. Note, he usually has thousands of
> > > > > > patches in backlog, you might need to respin it after the above
> > > > > > mentioned rc1.
> > > > > 
> > > > > Ok, understood.
> > > > > 
> > > > > Let's wait then.
> > > > 
> > > > Hi Andy/Greg,
> > > > we are now at v6.5-rc2 and I still do not see any of our patches in
> > > > linus or gregkh_tty repos.
> > > > 
> > > > Is there something missing from my part (or someone else) to go forward
> > > > with integrating these patches (v8) for v6.5?
> > > 
> > > My queue is huge right now, please be patient, I want to have them all
> > > handled by the end of next week...
> > > 
> > > You can always help out by reviewing other patches on the mailing list
> > > to reduce my review load.
> > 
> > Wait, no, this series was superseeded by v8, and in there you said you
> > were going to send a new series.  So please, fix it up and send the
> > updated version of the series, this one isn't going to be applied for
> > obvious reasons.
> 
> Hi Greg,
> I never said that I would resend another update for this current
> serie (unless of course if it was to address a new comment). Re-reading
> that email made me realise that it was maybe not perfectly clear the
> way I wrote it.
> 
> What I said was that, once V8 was finally applied and
> incorporated in the kernel, then I would send a completely new and
> different serie to address issues/concerns/improvements/suggestions
> noted during the review of this serie (example: conversion of bindings
> to YAML and improve DTS node names, etc). We already agreed with some
> maintainers (ex: Conor Dooley) that it was reasonnable to do so.
> 
> That is why I asked Andy if we were good to go with V8 and he
> confirmed that, and that it was now up to you to integrate it if your
> review was satisfactory.
> 
> Hope this clears things and we can integrate it soon.

I don't have any of your series in my review queue at all, so please
resend them.

thanks,

greg k-h
Hugo Villeneuve July 21, 2023, 3:46 p.m. UTC | #15
On Fri, 21 Jul 2023 17:40:18 +0200
Greg KH <gregkh@linuxfoundation.org> wrote:

> On Fri, Jul 21, 2023 at 11:25:17AM -0400, Hugo Villeneuve wrote:
> > On Thu, 20 Jul 2023 21:38:21 +0200
> > Greg KH <gregkh@linuxfoundation.org> wrote:
> > 
> > > On Wed, Jul 19, 2023 at 09:14:23PM +0200, Greg KH wrote:
> > > > On Wed, Jul 19, 2023 at 02:40:48PM -0400, Hugo Villeneuve wrote:
> > > > > On Tue, 20 Jun 2023 12:16:45 -0400
> > > > > Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > 
> > > > > > On Tue, 20 Jun 2023 18:45:51 +0300
> > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > 
> > > > > > > On Tue, Jun 20, 2023 at 6:42 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > > On Tue, 20 Jun 2023 18:35:48 +0300
> > > > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > > > > On Tue, Jun 20, 2023 at 6:33 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > > > > On Tue, 20 Jun 2023 18:18:12 +0300
> > > > > > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > > > > > > On Tue, Jun 20, 2023 at 5:08 PM Hugo Villeneuve <hugo@hugovil.com> wrote:
> > > > > > > > > > > > On Sun, 4 Jun 2023 22:31:04 +0300
> > > > > > > > > > > > Andy Shevchenko <andy.shevchenko@gmail.com> wrote:
> > > > > > > 
> > > > > > > ...
> > > > > > > 
> > > > > > > > > > > > did you have a chance to look at V8 (sent two weks ago) which fixed all
> > > > > > > > > > > > of what we discussed?
> > > > > > > > > > >
> > > > > > > > > > > The patch 6 already has my tag, anything specific you want me to do?
> > > > > > > > > >
> > > > > > > > > > Hi Andy,
> > > > > > > > > > I forgot to remove your "Reviewed-by: Andy..." tag before sending V8
> > > > > > > > > > since there were some changes involved in patch 6 and I wanted you to
> > > > > > > > > > review them. Can you confirm if the changes are correct?
> > > > > > > > > >
> > > > > > > > > > I also added a new patch "remove obsolete out_thread label". It has no
> > > > > > > > > > real impact on the code generation itself, but maybe you can review and
> > > > > > > > > > confirm if tags are ok or not, based on commit message and also
> > > > > > > > > > additional commit message.
> > > > > > > > >
> > > > > > > > > Both are fine to me.
> > > > > > > >
> > > > > > > > Hi,
> > > > > > > > Ok, thank you for reviewing this.
> > > > > > > >
> > > > > > > > I guess now we are good to go with this series if the stable tags and
> > > > > > > > patches order are good after Greg's review?
> > > > > > > 
> > > > > > > Taking into account that we are at rc7, and even with Fixes tags in
> > > > > > > your series I think Greg might take this after v6.5-0rc1 is out. It's
> > > > > > > up to him how to proceed with that. Note, he usually has thousands of
> > > > > > > patches in backlog, you might need to respin it after the above
> > > > > > > mentioned rc1.
> > > > > > 
> > > > > > Ok, understood.
> > > > > > 
> > > > > > Let's wait then.
> > > > > 
> > > > > Hi Andy/Greg,
> > > > > we are now at v6.5-rc2 and I still do not see any of our patches in
> > > > > linus or gregkh_tty repos.
> > > > > 
> > > > > Is there something missing from my part (or someone else) to go forward
> > > > > with integrating these patches (v8) for v6.5?
> > > > 
> > > > My queue is huge right now, please be patient, I want to have them all
> > > > handled by the end of next week...
> > > > 
> > > > You can always help out by reviewing other patches on the mailing list
> > > > to reduce my review load.
> > > 
> > > Wait, no, this series was superseeded by v8, and in there you said you
> > > were going to send a new series.  So please, fix it up and send the
> > > updated version of the series, this one isn't going to be applied for
> > > obvious reasons.
> > 
> > Hi Greg,
> > I never said that I would resend another update for this current
> > serie (unless of course if it was to address a new comment). Re-reading
> > that email made me realise that it was maybe not perfectly clear the
> > way I wrote it.
> > 
> > What I said was that, once V8 was finally applied and
> > incorporated in the kernel, then I would send a completely new and
> > different serie to address issues/concerns/improvements/suggestions
> > noted during the review of this serie (example: conversion of bindings
> > to YAML and improve DTS node names, etc). We already agreed with some
> > maintainers (ex: Conor Dooley) that it was reasonnable to do so.
> > 
> > That is why I asked Andy if we were good to go with V8 and he
> > confirmed that, and that it was now up to you to integrate it if your
> > review was satisfactory.
> > 
> > Hope this clears things and we can integrate it soon.
> 
> I don't have any of your series in my review queue at all, so please
> resend them.

OK, I will resend V8 then.

Hugo.
diff mbox series

Patch

diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 7d50674d2d0e..edc83f5f6340 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -236,7 +236,8 @@ 
 
 /* IOControl register bits (Only 750/760) */
 #define SC16IS7XX_IOCONTROL_LATCH_BIT	(1 << 0) /* Enable input latching */
-#define SC16IS7XX_IOCONTROL_MODEM_BIT	(1 << 1) /* Enable GPIO[7:4] as modem pins */
+#define SC16IS7XX_IOCONTROL_MODEM_A_BIT	(1 << 1) /* Enable GPIO[7:4] as modem A pins */
+#define SC16IS7XX_IOCONTROL_MODEM_B_BIT	(1 << 2) /* Enable GPIO[3:0] as modem B pins */
 #define SC16IS7XX_IOCONTROL_SRESET_BIT	(1 << 3) /* Software Reset */
 
 /* EFCR register bits */
@@ -301,12 +302,12 @@ 
 /* Misc definitions */
 #define SC16IS7XX_FIFO_SIZE		(64)
 #define SC16IS7XX_REG_SHIFT		2
+#define SC16IS7XX_GPIOS_PER_BANK	4
 
 struct sc16is7xx_devtype {
 	char	name[10];
 	int	nr_gpio;
 	int	nr_uart;
-	int	has_mctrl;
 };
 
 #define SC16IS7XX_RECONF_MD		(1 << 0)
@@ -336,6 +337,7 @@  struct sc16is7xx_port {
 	struct clk			*clk;
 #ifdef CONFIG_GPIOLIB
 	struct gpio_chip		gpio;
+	unsigned long			gpio_valid_mask;
 #endif
 	unsigned char			buf[SC16IS7XX_FIFO_SIZE];
 	struct kthread_worker		kworker;
@@ -447,35 +449,30 @@  static const struct sc16is7xx_devtype sc16is74x_devtype = {
 	.name		= "SC16IS74X",
 	.nr_gpio	= 0,
 	.nr_uart	= 1,
-	.has_mctrl	= 0,
 };
 
 static const struct sc16is7xx_devtype sc16is750_devtype = {
 	.name		= "SC16IS750",
-	.nr_gpio	= 4,
+	.nr_gpio	= 8,
 	.nr_uart	= 1,
-	.has_mctrl	= 1,
 };
 
 static const struct sc16is7xx_devtype sc16is752_devtype = {
 	.name		= "SC16IS752",
-	.nr_gpio	= 0,
+	.nr_gpio	= 8,
 	.nr_uart	= 2,
-	.has_mctrl	= 1,
 };
 
 static const struct sc16is7xx_devtype sc16is760_devtype = {
 	.name		= "SC16IS760",
-	.nr_gpio	= 4,
+	.nr_gpio	= 8,
 	.nr_uart	= 1,
-	.has_mctrl	= 1,
 };
 
 static const struct sc16is7xx_devtype sc16is762_devtype = {
 	.name		= "SC16IS762",
-	.nr_gpio	= 0,
+	.nr_gpio	= 8,
 	.nr_uart	= 2,
-	.has_mctrl	= 1,
 };
 
 static bool sc16is7xx_regmap_volatile(struct device *dev, unsigned int reg)
@@ -1350,16 +1347,45 @@  static int sc16is7xx_gpio_direction_output(struct gpio_chip *chip,
 	return 0;
 }
 
-static int sc16is7xx_setup_gpio_chip(struct device *dev)
+static int sc16is7xx_gpio_init_valid_mask(struct gpio_chip *chip,
+					  unsigned long *valid_mask,
+					  unsigned int ngpios)
+{
+	struct sc16is7xx_port *s = gpiochip_get_data(chip);
+
+	*valid_mask = s->gpio_valid_mask;
+
+	return 0;
+}
+
+static int sc16is7xx_setup_gpio_chip(struct device *dev, u8 mctrl_mask)
 {
 	struct sc16is7xx_port *s = dev_get_drvdata(dev);
 
 	if (!s->devtype->nr_gpio)
 		return 0;
 
+	switch (mctrl_mask) {
+	case 0:
+		s->gpio_valid_mask = GENMASK(7, 0);
+		break;
+	case SC16IS7XX_IOCONTROL_MODEM_A_BIT:
+		s->gpio_valid_mask = GENMASK(3, 0);
+		break;
+	case SC16IS7XX_IOCONTROL_MODEM_B_BIT:
+		s->gpio_valid_mask = GENMASK(7, 4);
+		break;
+	default:
+		break;
+	}
+
+	if (s->gpio_valid_mask == 0)
+		return 0;
+
 	s->gpio.owner		 = THIS_MODULE;
 	s->gpio.parent		 = dev;
 	s->gpio.label		 = dev_name(dev);
+	s->gpio.init_valid_mask	 = sc16is7xx_gpio_init_valid_mask;
 	s->gpio.direction_input	 = sc16is7xx_gpio_direction_input;
 	s->gpio.get		 = sc16is7xx_gpio_get;
 	s->gpio.direction_output = sc16is7xx_gpio_direction_output;
@@ -1371,6 +1397,44 @@  static int sc16is7xx_setup_gpio_chip(struct device *dev)
 }
 #endif
 
+static u8 sc16is7xx_setup_mctrl_ports(struct device *dev)
+{
+	struct sc16is7xx_port *s = dev_get_drvdata(dev);
+	int i;
+	int ret;
+	int count;
+	u32 mctrl_port[2];
+	u8 mctrl_mask;
+
+	count = device_property_count_u32(dev, "nxp,modem-control-line-ports");
+	if (count < 0 || count > ARRAY_SIZE(mctrl_port))
+		return 0;
+
+	ret = device_property_read_u32_array(dev, "nxp,modem-control-line-ports",
+					     mctrl_port, count);
+	if (ret)
+		return 0;
+
+	mctrl_mask = 0;
+
+	for (i = 0; i < count; i++) {
+		/* Use GPIO lines as modem control lines */
+		if (mctrl_port[i] == 0)
+			mctrl_mask |= SC16IS7XX_IOCONTROL_MODEM_A_BIT;
+		else if (mctrl_port[i] == 1)
+			mctrl_mask |= SC16IS7XX_IOCONTROL_MODEM_B_BIT;
+	}
+
+	if (mctrl_mask)
+		regmap_update_bits(
+			s->regmap,
+			SC16IS7XX_IOCONTROL_REG << SC16IS7XX_REG_SHIFT,
+			SC16IS7XX_IOCONTROL_MODEM_A_BIT |
+			SC16IS7XX_IOCONTROL_MODEM_B_BIT, mctrl_mask);
+
+	return mctrl_mask;
+}
+
 static const struct serial_rs485 sc16is7xx_rs485_supported = {
 	.flags = SER_RS485_ENABLED | SER_RS485_RTS_AFTER_SEND,
 	.delay_rts_before_send = 1,
@@ -1383,6 +1447,7 @@  static int sc16is7xx_probe(struct device *dev,
 {
 	unsigned long freq = 0, *pfreq = dev_get_platdata(dev);
 	unsigned int val;
+	u8 mctrl_mask;
 	u32 uartclk = 0;
 	int i, ret;
 	struct sc16is7xx_port *s;
@@ -1478,12 +1543,6 @@  static int sc16is7xx_probe(struct device *dev,
 				     SC16IS7XX_EFCR_RXDISABLE_BIT |
 				     SC16IS7XX_EFCR_TXDISABLE_BIT);
 
-		/* Use GPIO lines as modem status registers */
-		if (devtype->has_mctrl)
-			sc16is7xx_port_write(&s->p[i].port,
-					     SC16IS7XX_IOCONTROL_REG,
-					     SC16IS7XX_IOCONTROL_MODEM_BIT);
-
 		/* Initialize kthread work structs */
 		kthread_init_work(&s->p[i].tx_work, sc16is7xx_tx_proc);
 		kthread_init_work(&s->p[i].reg_work, sc16is7xx_reg_proc);
@@ -1521,8 +1580,10 @@  static int sc16is7xx_probe(struct device *dev,
 				s->p[u].irda_mode = true;
 	}
 
+	mctrl_mask = sc16is7xx_setup_mctrl_ports(dev);
+
 #ifdef CONFIG_GPIOLIB
-	ret = sc16is7xx_setup_gpio_chip(dev);
+	ret = sc16is7xx_setup_gpio_chip(dev, mctrl_mask);
 	if (ret)
 		goto out_thread;
 #endif
@@ -1547,7 +1608,7 @@  static int sc16is7xx_probe(struct device *dev,
 		return 0;
 
 #ifdef CONFIG_GPIOLIB
-	if (devtype->nr_gpio)
+	if (s->gpio_valid_mask)
 		gpiochip_remove(&s->gpio);
 
 out_thread:
@@ -1573,7 +1634,7 @@  static void sc16is7xx_remove(struct device *dev)
 	int i;
 
 #ifdef CONFIG_GPIOLIB
-	if (s->devtype->nr_gpio)
+	if (s->gpio_valid_mask)
 		gpiochip_remove(&s->gpio);
 #endif