Re: [PATCH v3 2/2] tty/serial: atmel: add ISO7816 support

2018-09-05 Thread Ludovic Desroches
On Wed, Sep 05, 2018 at 03:53:02PM +0200, Richard Genoud wrote:
> [added Nicolas back in the thread, he was removed somehow]
> 
> Hi Ludovic !
> 
> On 05/09/2018 14:43, Ludovic Desroches wrote:
> > Hi Richard,
> > 
> > On Thu, Aug 09, 2018 at 01:30:35PM +0200, Ludovic Desroches wrote:
> >> Hi Richard,
> >>
> >> On Thu, Aug 09, 2018 at 10:47:17AM +0200, Richard Genoud wrote:
> >>> Hi !
> >>>
> >>> On 07/08/2018 15:00, Ludovic Desroches wrote:
>  From: Nicolas Ferre 
> 
>  When mode is set in atmel_config_iso7816() we backup last RS232 mode
>  for coming back to this mode if requested.
>  Also allow setup of T=0 and T=1 parameter and basic support in 
>  set_termios
>  function as well.
> 
>  Signed-off-by: Nicolas Ferre 
>  [ludovic.desroc...@microchip.com: rebase, add check on fidi ratio, 
>  checkpatch fixes]
>  Signed-off-by: Ludovic Desroches 
>  ---
>   drivers/tty/serial/atmel_serial.c | 211 
>  +++---
>   drivers/tty/serial/atmel_serial.h |   6 +-
>   2 files changed, 201 insertions(+), 16 deletions(-)
> 
>  diff --git a/drivers/tty/serial/atmel_serial.c 
>  b/drivers/tty/serial/atmel_serial.c
>  index 8e4428725848..4a7ec44b0ace 100644
>  --- a/drivers/tty/serial/atmel_serial.c
>  +++ b/drivers/tty/serial/atmel_serial.c
> >>
> >> [...]
> >>
>   #if defined(CONFIG_OF)
>  +static struct atmel_uart_pdata at91rm9200_pdata = {
>  +.fidi_min = 1,
>  +.fidi_max = 2047,
>  +};
>  +
>  +static struct atmel_uart_pdata at91sam9260_pdata = {
>  +.fidi_min = 1,
>  +.fidi_max = 2047,
>  +};
>  +
>  +static struct atmel_uart_pdata sama5d3_pdata = {
>  +.fidi_min = 3,
>  +.fidi_max = 65535,
> >>> Are you sure this is for sama5d3 ?
> >>> From the datasheets I have, 65535 is for sama5d4/sama5d2
> >>
> >> I checked it and I missed it. What a pity... In fact, it's a bit more
> >> tricky since the min value for d3 is 3 and no longer 1.
> >>
> >>> And also, you'll have to s/atmel,at91sam9260-usart/atmel,sama5d2-usart/g
> >>> in sama5d{2,4}.dtsi
> >>>
> >>
> >> Yes, I planed to send it later but I can add those patches within this
> >> set of patches. 
> >>
> >>> But I wonder if it could be detected via ATMEL_US_VERSION instead ?
> >>>
> >>
> >> I have not checked, I tend to prefer the compatible string for this kind
> >> of thing. But as we already use the version number, I can investigate
> >> this solution if it's the one you prefer.
> >>
> > 
> > ping about this question still in suspend in order to prepare a new version.
> Well, if we use the compatible string for this, we will have to add :
> - atmel,sama5d2-usart
> - atmel,sama5d3-usart
> - (maybe others ?)
> to the already existing :
> - atmel,at91sam9260-usart
> - atmel,at91rm9200-usart
> just for setting different limits on the fidi register.
> IMHO, it seems a bit overkill. Moreover, the ATMEL_US_VERSION has
> already been read, so...
> But if you think adding compatible strings is a better/cleaner solution, just 
> convince me ! :)

I won't try to convince you since we already use the ATMEL_US_VERSION.
IMHO we should use the compatible string from the beginning but we
didn't have the necessary DT experience at this time to figure out it
was probably better than relying on the version number which can be not
incrimented by error.

I'll check the values of the ATMEL_US_VERSION and use it if everything
is okay.

Regards

Ludovic


Re: [PATCH v3 2/2] tty/serial: atmel: add ISO7816 support

2018-09-05 Thread Richard Genoud
[added Nicolas back in the thread, he was removed somehow]

Hi Ludovic !

On 05/09/2018 14:43, Ludovic Desroches wrote:
> Hi Richard,
> 
> On Thu, Aug 09, 2018 at 01:30:35PM +0200, Ludovic Desroches wrote:
>> Hi Richard,
>>
>> On Thu, Aug 09, 2018 at 10:47:17AM +0200, Richard Genoud wrote:
>>> Hi !
>>>
>>> On 07/08/2018 15:00, Ludovic Desroches wrote:
 From: Nicolas Ferre 

 When mode is set in atmel_config_iso7816() we backup last RS232 mode
 for coming back to this mode if requested.
 Also allow setup of T=0 and T=1 parameter and basic support in set_termios
 function as well.

 Signed-off-by: Nicolas Ferre 
 [ludovic.desroc...@microchip.com: rebase, add check on fidi ratio, 
 checkpatch fixes]
 Signed-off-by: Ludovic Desroches 
 ---
  drivers/tty/serial/atmel_serial.c | 211 
 +++---
  drivers/tty/serial/atmel_serial.h |   6 +-
  2 files changed, 201 insertions(+), 16 deletions(-)

 diff --git a/drivers/tty/serial/atmel_serial.c 
 b/drivers/tty/serial/atmel_serial.c
 index 8e4428725848..4a7ec44b0ace 100644
 --- a/drivers/tty/serial/atmel_serial.c
 +++ b/drivers/tty/serial/atmel_serial.c
>>
>> [...]
>>
  #if defined(CONFIG_OF)
 +static struct atmel_uart_pdata at91rm9200_pdata = {
 +  .fidi_min = 1,
 +  .fidi_max = 2047,
 +};
 +
 +static struct atmel_uart_pdata at91sam9260_pdata = {
 +  .fidi_min = 1,
 +  .fidi_max = 2047,
 +};
 +
 +static struct atmel_uart_pdata sama5d3_pdata = {
 +  .fidi_min = 3,
 +  .fidi_max = 65535,
>>> Are you sure this is for sama5d3 ?
>>> From the datasheets I have, 65535 is for sama5d4/sama5d2
>>
>> I checked it and I missed it. What a pity... In fact, it's a bit more
>> tricky since the min value for d3 is 3 and no longer 1.
>>
>>> And also, you'll have to s/atmel,at91sam9260-usart/atmel,sama5d2-usart/g
>>> in sama5d{2,4}.dtsi
>>>
>>
>> Yes, I planed to send it later but I can add those patches within this
>> set of patches. 
>>
>>> But I wonder if it could be detected via ATMEL_US_VERSION instead ?
>>>
>>
>> I have not checked, I tend to prefer the compatible string for this kind
>> of thing. But as we already use the version number, I can investigate
>> this solution if it's the one you prefer.
>>
> 
> ping about this question still in suspend in order to prepare a new version.
Well, if we use the compatible string for this, we will have to add :
- atmel,sama5d2-usart
- atmel,sama5d3-usart
- (maybe others ?)
to the already existing :
- atmel,at91sam9260-usart
- atmel,at91rm9200-usart
just for setting different limits on the fidi register.
IMHO, it seems a bit overkill. Moreover, the ATMEL_US_VERSION has
already been read, so...
But if you think adding compatible strings is a better/cleaner solution, just 
convince me ! :)


Re: [PATCH v3 2/2] tty/serial: atmel: add ISO7816 support

2018-09-05 Thread Ludovic Desroches
Hi Richard,

On Thu, Aug 09, 2018 at 01:30:35PM +0200, Ludovic Desroches wrote:
> Hi Richard,
> 
> On Thu, Aug 09, 2018 at 10:47:17AM +0200, Richard Genoud wrote:
> > Hi !
> > 
> > On 07/08/2018 15:00, Ludovic Desroches wrote:
> > > From: Nicolas Ferre 
> > > 
> > > When mode is set in atmel_config_iso7816() we backup last RS232 mode
> > > for coming back to this mode if requested.
> > > Also allow setup of T=0 and T=1 parameter and basic support in set_termios
> > > function as well.
> > > 
> > > Signed-off-by: Nicolas Ferre 
> > > [ludovic.desroc...@microchip.com: rebase, add check on fidi ratio, 
> > > checkpatch fixes]
> > > Signed-off-by: Ludovic Desroches 
> > > ---
> > >  drivers/tty/serial/atmel_serial.c | 211 
> > > +++---
> > >  drivers/tty/serial/atmel_serial.h |   6 +-
> > >  2 files changed, 201 insertions(+), 16 deletions(-)
> > > 
> > > diff --git a/drivers/tty/serial/atmel_serial.c 
> > > b/drivers/tty/serial/atmel_serial.c
> > > index 8e4428725848..4a7ec44b0ace 100644
> > > --- a/drivers/tty/serial/atmel_serial.c
> > > +++ b/drivers/tty/serial/atmel_serial.c
> 
> [...]
> 
> > >  #if defined(CONFIG_OF)
> > > +static struct atmel_uart_pdata at91rm9200_pdata = {
> > > + .fidi_min = 1,
> > > + .fidi_max = 2047,
> > > +};
> > > +
> > > +static struct atmel_uart_pdata at91sam9260_pdata = {
> > > + .fidi_min = 1,
> > > + .fidi_max = 2047,
> > > +};
> > > +
> > > +static struct atmel_uart_pdata sama5d3_pdata = {
> > > + .fidi_min = 3,
> > > + .fidi_max = 65535,
> > Are you sure this is for sama5d3 ?
> > From the datasheets I have, 65535 is for sama5d4/sama5d2
> 
> I checked it and I missed it. What a pity... In fact, it's a bit more
> tricky since the min value for d3 is 3 and no longer 1.
> 
> > And also, you'll have to s/atmel,at91sam9260-usart/atmel,sama5d2-usart/g
> > in sama5d{2,4}.dtsi
> >
> 
> Yes, I planed to send it later but I can add those patches within this
> set of patches. 
> 
> > But I wonder if it could be detected via ATMEL_US_VERSION instead ?
> > 
> 
> I have not checked, I tend to prefer the compatible string for this kind
> of thing. But as we already use the version number, I can investigate
> this solution if it's the one you prefer.
> 

ping about this question still in suspend in order to prepare a new version.

Regards

Ludovic


Re: [PATCH v3 2/2] tty/serial: atmel: add ISO7816 support

2018-08-09 Thread Ludovic Desroches
Hi Richard,

On Thu, Aug 09, 2018 at 10:47:17AM +0200, Richard Genoud wrote:
> Hi !
> 
> On 07/08/2018 15:00, Ludovic Desroches wrote:
> > From: Nicolas Ferre 
> > 
> > When mode is set in atmel_config_iso7816() we backup last RS232 mode
> > for coming back to this mode if requested.
> > Also allow setup of T=0 and T=1 parameter and basic support in set_termios
> > function as well.
> > 
> > Signed-off-by: Nicolas Ferre 
> > [ludovic.desroc...@microchip.com: rebase, add check on fidi ratio, 
> > checkpatch fixes]
> > Signed-off-by: Ludovic Desroches 
> > ---
> >  drivers/tty/serial/atmel_serial.c | 211 
> > +++---
> >  drivers/tty/serial/atmel_serial.h |   6 +-
> >  2 files changed, 201 insertions(+), 16 deletions(-)
> > 
> > diff --git a/drivers/tty/serial/atmel_serial.c 
> > b/drivers/tty/serial/atmel_serial.c
> > index 8e4428725848..4a7ec44b0ace 100644
> > --- a/drivers/tty/serial/atmel_serial.c
> > +++ b/drivers/tty/serial/atmel_serial.c

[...]

> >  #if defined(CONFIG_OF)
> > +static struct atmel_uart_pdata at91rm9200_pdata = {
> > +   .fidi_min = 1,
> > +   .fidi_max = 2047,
> > +};
> > +
> > +static struct atmel_uart_pdata at91sam9260_pdata = {
> > +   .fidi_min = 1,
> > +   .fidi_max = 2047,
> > +};
> > +
> > +static struct atmel_uart_pdata sama5d3_pdata = {
> > +   .fidi_min = 3,
> > +   .fidi_max = 65535,
> Are you sure this is for sama5d3 ?
> From the datasheets I have, 65535 is for sama5d4/sama5d2

I checked it and I missed it. What a pity... In fact, it's a bit more
tricky since the min value for d3 is 3 and no longer 1.

> And also, you'll have to s/atmel,at91sam9260-usart/atmel,sama5d2-usart/g
> in sama5d{2,4}.dtsi
>

Yes, I planed to send it later but I can add those patches within this
set of patches. 

> But I wonder if it could be detected via ATMEL_US_VERSION instead ?
> 

I have not checked, I tend to prefer the compatible string for this kind
of thing. But as we already use the version number, I can investigate
this solution if it's the one you prefer.


Regards

Ludovic


Re: [PATCH v3 2/2] tty/serial: atmel: add ISO7816 support

2018-08-09 Thread Richard Genoud
Hi !

On 07/08/2018 15:00, Ludovic Desroches wrote:
> From: Nicolas Ferre 
> 
> When mode is set in atmel_config_iso7816() we backup last RS232 mode
> for coming back to this mode if requested.
> Also allow setup of T=0 and T=1 parameter and basic support in set_termios
> function as well.
> 
> Signed-off-by: Nicolas Ferre 
> [ludovic.desroc...@microchip.com: rebase, add check on fidi ratio, checkpatch 
> fixes]
> Signed-off-by: Ludovic Desroches 
> ---
>  drivers/tty/serial/atmel_serial.c | 211 
> +++---
>  drivers/tty/serial/atmel_serial.h |   6 +-
>  2 files changed, 201 insertions(+), 16 deletions(-)
> 
> diff --git a/drivers/tty/serial/atmel_serial.c 
> b/drivers/tty/serial/atmel_serial.c
> index 8e4428725848..4a7ec44b0ace 100644
> --- a/drivers/tty/serial/atmel_serial.c
> +++ b/drivers/tty/serial/atmel_serial.c
> @@ -34,6 +34,7 @@
>  #include 
>  #include 
>  
> +#include 
>  #include 
>  #include 
>  
> @@ -83,6 +84,11 @@ static void atmel_stop_rx(struct uart_port *port);
>  
>  #define ATMEL_ISR_PASS_LIMIT 256
>  
> +struct atmel_uart_pdata {
> + unsigned intfidi_min;
> + unsigned intfidi_max;
> +};
> +
>  struct atmel_dma_buffer {
>   unsigned char   *buf;
>   dma_addr_t  dma_addr;
> @@ -114,6 +120,7 @@ struct atmel_uart_char {
>   */
>  struct atmel_uart_port {
>   struct uart_portuart;   /* uart */
> + const struct atmel_uart_pdata *pdata;   /* SoC specific parameters */
>   struct clk  *clk;   /* uart clock */
>   int may_wakeup; /* cached value of 
> device_may_wakeup for times we need to disable it */
>   u32 backup_imr; /* IMR saved during suspend */
> @@ -147,6 +154,8 @@ struct atmel_uart_port {
>   struct circ_buf rx_ring;
>  
>   struct mctrl_gpios  *gpios;
> + u32 backup_mode;/* MR saved during iso7816 
> operations */
> + u32 backup_brgr;/* BRGR saved during iso7816 
> operations */
>   unsigned inttx_done_mask;
>   u32 fifo_size;
>   u32 rts_high;
> @@ -192,10 +201,34 @@ static struct console atmel_console;
>  #endif
>  
>  #if defined(CONFIG_OF)
> +static struct atmel_uart_pdata at91rm9200_pdata = {
> + .fidi_min = 1,
> + .fidi_max = 2047,
> +};
> +
> +static struct atmel_uart_pdata at91sam9260_pdata = {
> + .fidi_min = 1,
> + .fidi_max = 2047,
> +};
> +
> +static struct atmel_uart_pdata sama5d3_pdata = {
> + .fidi_min = 3,
> + .fidi_max = 65535,
Are you sure this is for sama5d3 ?
>From the datasheets I have, 65535 is for sama5d4/sama5d2
And also, you'll have to s/atmel,at91sam9260-usart/atmel,sama5d2-usart/g
in sama5d{2,4}.dtsi

But I wonder if it could be detected via ATMEL_US_VERSION instead ?


> +};
> +
>  static const struct of_device_id atmel_serial_dt_ids[] = {
> - { .compatible = "atmel,at91rm9200-usart" },
> - { .compatible = "atmel,at91sam9260-usart" },
> - { /* sentinel */ }
> + {
> + .compatible = "atmel,at91rm9200-usart",
> + .data = &at91rm9200_pdata,
> + }, {
> + .compatible = "atmel,at91sam9260-usart",
> + .data = &at91sam9260_pdata,
> + }, {
> + .compatible = "atmel,sama5d3-usart",
> + .data = &sama5d3_pdata,
> + }, {
> + /* sentinel */
> + }
>  };
>  #endif
>  
> @@ -362,6 +395,127 @@ static int atmel_config_rs485(struct uart_port *port,
>   return 0;
>  }
>  
> +static unsigned int atmel_calc_cd(struct uart_port *port,
> +   struct serial_iso7816 *iso7816conf)
> +{
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + unsigned int cd;
> + u64 mck_rate;
> +
> + mck_rate = (u64)clk_get_rate(atmel_port->clk);
> + do_div(mck_rate, iso7816conf->clk);
> + cd = mck_rate;
> + return cd;
> +}
> +
> +static unsigned int atmel_calc_fidi(struct uart_port *port,
> + struct serial_iso7816 *iso7816conf)
> +{
> + u64 fidi = 0;
> +
> + if (iso7816conf->sc_fi && iso7816conf->sc_di) {
> + fidi = (u64)iso7816conf->sc_fi;
> + do_div(fidi, iso7816conf->sc_di);
> + }
> + return (u32)fidi;
> +}
> +
> +/* Enable or disable the iso7816 support */
> +/* Called with interrupts disabled */
> +static int atmel_config_iso7816(struct uart_port *port,
> + struct serial_iso7816 *iso7816conf)
> +{
> + struct atmel_uart_port *atmel_port = to_atmel_uart_port(port);
> + unsigned int mode;
> + unsigned int cd, fidi;
> + int ret = 0;
> +
> + /* Disable interrupts */
> + atmel_uart_writel(port, ATMEL_US_IDR, atmel_port->tx_done_mask);
> +
> + mode = atmel_uart_readl(port, ATMEL_US_MR);
> +
> + if (iso7816conf->flags & SER_ISO7816_ENABLED) {
> + mode &=