[14/15] can: tcan4x5x: Fix register range of first block

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

Commit Message

Markus Schneider-Pargmann Nov. 16, 2022, 8:53 p.m. UTC
  According to the datasheet 0x1c is the last register in the first block,
not register 0x2c.

Signed-off-by: Markus Schneider-Pargmann <msp@baylibre.com>
---
 drivers/net/can/m_can/tcan4x5x-regmap.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Marc Kleine-Budde Dec. 2, 2022, 2:28 p.m. UTC | #1
On 16.11.2022 21:53:07, Markus Schneider-Pargmann wrote:
> According to the datasheet 0x1c is the last register in the first block,
> not register 0x2c.

The datasheet "SLLSF91A – DECEMBER 2018 – REVISED JANUARY 2020" says:

| 8.6.1 Device ID and Interrupt/Diagnostic Flag Registers: 16'h0000 to
| 16'h002F

While the last described register is at 0xc.

Marc
  
Markus Schneider-Pargmann Dec. 5, 2022, 9:30 a.m. UTC | #2
Hi Marc,

On Fri, Dec 02, 2022 at 03:28:10PM +0100, Marc Kleine-Budde wrote:
> On 16.11.2022 21:53:07, Markus Schneider-Pargmann wrote:
> > According to the datasheet 0x1c is the last register in the first block,
> > not register 0x2c.
> 
> The datasheet "SLLSF91A – DECEMBER 2018 – REVISED JANUARY 2020" says:
> 
> | 8.6.1 Device ID and Interrupt/Diagnostic Flag Registers: 16'h0000 to
> | 16'h002F
> 
> While the last described register is at 0xc.

Sorry, not sure what I looked up here. The last described register is
0x10 SPI Error status mask in my datasheet:
'SLLSEZ5D – JANUARY 2018 – REVISED JUNE 2022'

I would prefer using the actual registers if that is ok with you, so
0x10 here because I assume the remaining registers have internal use or
maybe don't exist at all?! If there is an undocumented register that
needs to be used at some point we can still modify the ranges.

Also it seems the existing ranges are following the same logic and don't
list the whole range, just the documented registers.

The second range is wrong as well. The last register is 0x830, will fix.

Best,
Markus
  
Marc Kleine-Budde Dec. 5, 2022, 9:44 a.m. UTC | #3
On 05.12.2022 10:30:13, Markus Schneider-Pargmann wrote:
> Hi Marc,
> 
> On Fri, Dec 02, 2022 at 03:28:10PM +0100, Marc Kleine-Budde wrote:
> > On 16.11.2022 21:53:07, Markus Schneider-Pargmann wrote:
> > > According to the datasheet 0x1c is the last register in the first block,
> > > not register 0x2c.
> > 
> > The datasheet "SLLSF91A – DECEMBER 2018 – REVISED JANUARY 2020" says:
> > 
> > | 8.6.1 Device ID and Interrupt/Diagnostic Flag Registers: 16'h0000 to
> > | 16'h002F
> > 
> > While the last described register is at 0xc.
> 
> Sorry, not sure what I looked up here. The last described register is
> 0x10 SPI Error status mask in my datasheet:
> 'SLLSEZ5D – JANUARY 2018 – REVISED JUNE 2022'

The TCAN4550-Q1 variant has the 0x10 register documented, while the
TCAN4550 (w/o -Q1) doesn't have.

> I would prefer using the actual registers if that is ok with you, so
> 0x10 here because I assume the remaining registers have internal use or
> maybe don't exist at all?! If there is an undocumented register that
> needs to be used at some point we can still modify the ranges.

I'm fine with using 0x10 as the last register.

> Also it seems the existing ranges are following the same logic and don't
> list the whole range, just the documented registers.
> 
> The second range is wrong as well. The last register is 0x830, will
> fix.

IIRC I used the register ranges from the section titles ("8.6.1 Device
ID and Interrupt/Diagnostic Flag Registers: 16'h0000 to 16'h002F") when
I added the {wr,rd}_table.

Marc
  
Markus Schneider-Pargmann Dec. 5, 2022, 9:55 a.m. UTC | #4
On Mon, Dec 05, 2022 at 10:44:58AM +0100, Marc Kleine-Budde wrote:
> On 05.12.2022 10:30:13, Markus Schneider-Pargmann wrote:
> > Hi Marc,
> > 
> > On Fri, Dec 02, 2022 at 03:28:10PM +0100, Marc Kleine-Budde wrote:
> > > On 16.11.2022 21:53:07, Markus Schneider-Pargmann wrote:
> > > > According to the datasheet 0x1c is the last register in the first block,
> > > > not register 0x2c.
> > > 
> > > The datasheet "SLLSF91A – DECEMBER 2018 – REVISED JANUARY 2020" says:
> > > 
> > > | 8.6.1 Device ID and Interrupt/Diagnostic Flag Registers: 16'h0000 to
> > > | 16'h002F
> > > 
> > > While the last described register is at 0xc.
> > 
> > Sorry, not sure what I looked up here. The last described register is
> > 0x10 SPI Error status mask in my datasheet:
> > 'SLLSEZ5D – JANUARY 2018 – REVISED JUNE 2022'
> 
> The TCAN4550-Q1 variant has the 0x10 register documented, while the
> TCAN4550 (w/o -Q1) doesn't have.

Ah haven't noticed, thank you.

> 
> > I would prefer using the actual registers if that is ok with you, so
> > 0x10 here because I assume the remaining registers have internal use or
> > maybe don't exist at all?! If there is an undocumented register that
> > needs to be used at some point we can still modify the ranges.
> 
> I'm fine with using 0x10 as the last register.
> 
> > Also it seems the existing ranges are following the same logic and don't
> > list the whole range, just the documented registers.
> > 
> > The second range is wrong as well. The last register is 0x830, will
> > fix.
> 
> IIRC I used the register ranges from the section titles ("8.6.1 Device
> ID and Interrupt/Diagnostic Flag Registers: 16'h0000 to 16'h002F") when
> I added the {wr,rd}_table.

The second range in the driver was specified as 0x800-0x83c in the
driver. The last documented register is 0x830 in both normal and Q1
versions while the range in the title is 0x800-0x8ff. That's why I
thought it was using the last register, just because it is closer.

Anyways not really important.

I can put in whatever you feel comfortable with or keep as it is.

Thanks,
Markus
  

Patch

diff --git a/drivers/net/can/m_can/tcan4x5x-regmap.c b/drivers/net/can/m_can/tcan4x5x-regmap.c
index 26e212b8ca7a..d4b79d2d4598 100644
--- a/drivers/net/can/m_can/tcan4x5x-regmap.c
+++ b/drivers/net/can/m_can/tcan4x5x-regmap.c
@@ -91,7 +91,7 @@  static int tcan4x5x_regmap_read(void *context,
 }
 
 static const struct regmap_range tcan4x5x_reg_table_yes_range[] = {
-	regmap_reg_range(0x0000, 0x002c),	/* Device ID and SPI Registers */
+	regmap_reg_range(0x0000, 0x001c),	/* Device ID and SPI Registers */
 	regmap_reg_range(0x0800, 0x083c),	/* Device configuration registers and Interrupt Flags*/
 	regmap_reg_range(0x1000, 0x10fc),	/* M_CAN */
 	regmap_reg_range(0x8000, 0x87fc),	/* MRAM */