[v2,11/11] can: tcan4x5x: Specify separate read/write ranges

Message ID 20221206115728.1056014-12-msp@baylibre.com
State New
Headers
Series can: m_can: Optimizations for tcan and peripheral chips |

Commit Message

Markus Schneider-Pargmann Dec. 6, 2022, 11:57 a.m. UTC
  Specify exactly which registers are read/writeable in the chip. This
is supposed to help detect any violations in the future.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/net/can/m_can/tcan4x5x-regmap.c | 43 +++++++++++++++++++++----
 1 file changed, 37 insertions(+), 6 deletions(-)
  

Comments

Marc Kleine-Budde Dec. 6, 2022, 4:20 p.m. UTC | #1
On 06.12.2022 12:57:28, Markus Schneider-Pargmann wrote:
> Specify exactly which registers are read/writeable in the chip. This
> is supposed to help detect any violations in the future.
> 
> Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> ---
>  drivers/net/can/m_can/tcan4x5x-regmap.c | 43 +++++++++++++++++++++----
>  1 file changed, 37 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
> index 33aed989e42a..2b218ce04e9f 100644
> --- a/drivers/net/can/m_can/tcan4x5x-regmap.c
> +++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
> @@ -90,16 +90,47 @@ static int tcan4x5x_regmap_read(void *context,
>  	return 0;
>  }
>  
> -static const struct regmap_range tcan4x5x_reg_table_yes_range[] = {
> +static const struct regmap_range tcan4x5x_reg_table_wr_range[] = {
> +	/* Device ID and SPI Registers */
> +	regmap_reg_range(0x000c, 0x0010),

According to "Table 8-8" 0xc is RO, but in "8.6.1.4 Status (address =
h000C) [reset = h0000000U]" it clearly says it has write 1 to clear bits
:/.

> +	/* Device configuration registers and Interrupt Flags*/
> +	regmap_reg_range(0x0800, 0x080c),
> +	regmap_reg_range(0x0814, 0x0814),

0x814 is marked as reserved in "SLLSEZ5D – JANUARY 2018 – REVISED JUNE
2022"?

Marc
  
Marc Kleine-Budde Dec. 12, 2022, 10:54 a.m. UTC | #2
On 06.12.2022 17:20:01, Marc Kleine-Budde wrote:
> On 06.12.2022 12:57:28, Markus Schneider-Pargmann wrote:
> > Specify exactly which registers are read/writeable in the chip. This
> > is supposed to help detect any violations in the future.
> > 
> > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > ---
> >  drivers/net/can/m_can/tcan4x5x-regmap.c | 43 +++++++++++++++++++++----
> >  1 file changed, 37 insertions(+), 6 deletions(-)
> > 
> > diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
> > index 33aed989e42a..2b218ce04e9f 100644
> > --- a/drivers/net/can/m_can/tcan4x5x-regmap.c
> > +++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
> > @@ -90,16 +90,47 @@ static int tcan4x5x_regmap_read(void *context,
> >  	return 0;
> >  }
> >  
> > -static const struct regmap_range tcan4x5x_reg_table_yes_range[] = {
> > +static const struct regmap_range tcan4x5x_reg_table_wr_range[] = {
> > +	/* Device ID and SPI Registers */
> > +	regmap_reg_range(0x000c, 0x0010),
> 
> According to "Table 8-8" 0xc is RO, but in "8.6.1.4 Status (address =
> h000C) [reset = h0000000U]" it clearly says it has write 1 to clear bits
> :/.
> 
> > +	/* Device configuration registers and Interrupt Flags*/
> > +	regmap_reg_range(0x0800, 0x080c),
> > +	regmap_reg_range(0x0814, 0x0814),
> 
> 0x814 is marked as reserved in "SLLSEZ5D – JANUARY 2018 – REVISED JUNE
> 2022"?

I'll take the series as is, that can be fixed later.

regards,
Marc
  
Markus Schneider-Pargmann Dec. 13, 2022, 5:10 p.m. UTC | #3
Hi Marc,

sorry for the delay.

On Mon, Dec 12, 2022 at 11:54:44AM +0100, Marc Kleine-Budde wrote:
> On 06.12.2022 17:20:01, Marc Kleine-Budde wrote:
> > On 06.12.2022 12:57:28, Markus Schneider-Pargmann wrote:
> > > Specify exactly which registers are read/writeable in the chip. This
> > > is supposed to help detect any violations in the future.
> > > 
> > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > > ---
> > >  drivers/net/can/m_can/tcan4x5x-regmap.c | 43 +++++++++++++++++++++----
> > >  1 file changed, 37 insertions(+), 6 deletions(-)
> > > 
> > > diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
> > > index 33aed989e42a..2b218ce04e9f 100644
> > > --- a/drivers/net/can/m_can/tcan4x5x-regmap.c
> > > +++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
> > > @@ -90,16 +90,47 @@ static int tcan4x5x_regmap_read(void *context,
> > >  	return 0;
> > >  }
> > >  
> > > -static const struct regmap_range tcan4x5x_reg_table_yes_range[] = {
> > > +static const struct regmap_range tcan4x5x_reg_table_wr_range[] = {
> > > +	/* Device ID and SPI Registers */
> > > +	regmap_reg_range(0x000c, 0x0010),
> > 
> > According to "Table 8-8" 0xc is RO, but in "8.6.1.4 Status (address =
> > h000C) [reset = h0000000U]" it clearly says it has write 1 to clear bits
> > :/.

I am trying to clarify this. I guess table 8-8 is not correct, but we
will see.

> > 
> > > +	/* Device configuration registers and Interrupt Flags*/
> > > +	regmap_reg_range(0x0800, 0x080c),
> > > +	regmap_reg_range(0x0814, 0x0814),
> > 
> > 0x814 is marked as reserved in "SLLSEZ5D – JANUARY 2018 – REVISED JUNE
> > 2022"?
> 

Yes that's a mistake, sorry. I will add a fixup for the upcoming series.

> I'll take the series as is, that can be fixed later.

Thank you!.

Best,
Markus
  
Markus Schneider-Pargmann Dec. 13, 2022, 7:10 p.m. UTC | #4
Hi Marc,

On Tue, Dec 13, 2022 at 06:10:36PM +0100, Markus Schneider-Pargmann wrote:
> Hi Marc,
> 
> sorry for the delay.
> 
> On Mon, Dec 12, 2022 at 11:54:44AM +0100, Marc Kleine-Budde wrote:
> > On 06.12.2022 17:20:01, Marc Kleine-Budde wrote:
> > > On 06.12.2022 12:57:28, Markus Schneider-Pargmann wrote:
> > > > Specify exactly which registers are read/writeable in the chip. This
> > > > is supposed to help detect any violations in the future.
> > > > 
> > > > Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
> > > > ---
> > > >  drivers/net/can/m_can/tcan4x5x-regmap.c | 43 +++++++++++++++++++++----
> > > >  1 file changed, 37 insertions(+), 6 deletions(-)
> > > > 
> > > > diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
> > > > index 33aed989e42a..2b218ce04e9f 100644
> > > > --- a/drivers/net/can/m_can/tcan4x5x-regmap.c
> > > > +++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
> > > > @@ -90,16 +90,47 @@ static int tcan4x5x_regmap_read(void *context,
> > > >  	return 0;
> > > >  }
> > > >  
> > > > -static const struct regmap_range tcan4x5x_reg_table_yes_range[] = {
> > > > +static const struct regmap_range tcan4x5x_reg_table_wr_range[] = {
> > > > +	/* Device ID and SPI Registers */
> > > > +	regmap_reg_range(0x000c, 0x0010),
> > > 
> > > According to "Table 8-8" 0xc is RO, but in "8.6.1.4 Status (address =
> > > h000C) [reset = h0000000U]" it clearly says it has write 1 to clear bits
> > > :/.
> 
> I am trying to clarify this. I guess table 8-8 is not correct, but we
> will see.

So it is indeed a typo in table 8-8. The register is R/W.

Best,
Markus
  
Marc Kleine-Budde Dec. 13, 2022, 7:13 p.m. UTC | #5
On 13.12.2022 20:10:25, Markus Schneider-Pargmann wrote:
> > > > According to "Table 8-8" 0xc is RO, but in "8.6.1.4 Status (address =
> > > > h000C) [reset = h0000000U]" it clearly says it has write 1 to clear bits
> > > > :/.
> > 
> > I am trying to clarify this. I guess table 8-8 is not correct, but we
> > will see.
> 
> So it is indeed a typo in table 8-8. The register is R/W.

Do you have a contact to TI to fix this?

Marc
  

Patch

diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
index 33aed989e42a..2b218ce04e9f 100644
--- a/drivers/net/can/m_can/tcan4x5x-regmap.c
+++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
@@ -90,16 +90,47 @@  static int tcan4x5x_regmap_read(void *context,
 	return 0;
 }
 
-static const struct regmap_range tcan4x5x_reg_table_yes_range[] = {
+static const struct regmap_range tcan4x5x_reg_table_wr_range[] = {
+	/* Device ID and SPI Registers */
+	regmap_reg_range(0x000c, 0x0010),
+	/* Device configuration registers and Interrupt Flags*/
+	regmap_reg_range(0x0800, 0x080c),
+	regmap_reg_range(0x0814, 0x0814),
+	regmap_reg_range(0x0820, 0x0820),
+	regmap_reg_range(0x0830, 0x0830),
+	/* M_CAN */
+	regmap_reg_range(0x100c, 0x102c),
+	regmap_reg_range(0x1048, 0x1048),
+	regmap_reg_range(0x1050, 0x105c),
+	regmap_reg_range(0x1080, 0x1088),
+	regmap_reg_range(0x1090, 0x1090),
+	regmap_reg_range(0x1098, 0x10a0),
+	regmap_reg_range(0x10a8, 0x10b0),
+	regmap_reg_range(0x10b8, 0x10c0),
+	regmap_reg_range(0x10c8, 0x10c8),
+	regmap_reg_range(0x10d0, 0x10d4),
+	regmap_reg_range(0x10e0, 0x10e4),
+	regmap_reg_range(0x10f0, 0x10f0),
+	regmap_reg_range(0x10f8, 0x10f8),
+	/* MRAM */
+	regmap_reg_range(0x8000, 0x87fc),
+};
+
+static const struct regmap_range tcan4x5x_reg_table_rd_range[] = {
 	regmap_reg_range(0x0000, 0x0010),	/* Device ID and SPI Registers */
 	regmap_reg_range(0x0800, 0x0830),	/* Device configuration registers and Interrupt Flags*/
 	regmap_reg_range(0x1000, 0x10fc),	/* M_CAN */
 	regmap_reg_range(0x8000, 0x87fc),	/* MRAM */
 };
 
-static const struct regmap_access_table tcan4x5x_reg_table = {
-	.yes_ranges = tcan4x5x_reg_table_yes_range,
-	.n_yes_ranges = ARRAY_SIZE(tcan4x5x_reg_table_yes_range),
+static const struct regmap_access_table tcan4x5x_reg_table_wr = {
+	.yes_ranges = tcan4x5x_reg_table_wr_range,
+	.n_yes_ranges = ARRAY_SIZE(tcan4x5x_reg_table_wr_range),
+};
+
+static const struct regmap_access_table tcan4x5x_reg_table_rd = {
+	.yes_ranges = tcan4x5x_reg_table_rd_range,
+	.n_yes_ranges = ARRAY_SIZE(tcan4x5x_reg_table_rd_range),
 };
 
 static const struct regmap_config tcan4x5x_regmap = {
@@ -107,8 +138,8 @@  static const struct regmap_config tcan4x5x_regmap = {
 	.reg_stride = 4,
 	.pad_bits = 8,
 	.val_bits = 32,
-	.wr_table = &tcan4x5x_reg_table,
-	.rd_table = &tcan4x5x_reg_table,
+	.wr_table = &tcan4x5x_reg_table_wr,
+	.rd_table = &tcan4x5x_reg_table_rd,
 	.max_register = TCAN4X5X_MAX_REGISTER,
 	.cache_type = REGCACHE_NONE,
 	.read_flag_mask = (__force unsigned long)