* [PATCH v2 0/3] net: can: Use syscon regmap for TI specific RAMINIT register @ 2014-09-09 14:31 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:31 UTC (permalink / raw) To: wg, mkl; +Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar Hi, Some hardware (TI am43xx) has a buggy RAMINIT DONE mechanism and it might not always set the DONE bit. This will result in a lockup in c_can_hw_raminit_wait_ti(), so patch 1 adds a timeout mechanism there. There is a non compliancy within TI platforms with respect to the layout of the RAMINIT register. The patches 2 and 3 address this issue and make a flexible but standard way of defining the RAMINIT hardware register layout in the device tree. The RAMINIT register is accessed using the syscon regmap framework. Patches available at git@github.com:rogerq/linux.git [for-v3.18/can] Changelog: v2: - added "ti" vendor prefix to TI specific raminit properties. - split DTS changes into a separate series cheers, -roger --- Roger Quadros (3): can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout net: can: c_can: Add syscon/regmap RAMINIT mechanism net: can: c_can: Add support for START pulse in RAMINIT sequence .../devicetree/bindings/net/can/c_can.txt | 10 ++ drivers/net/can/c_can/c_can.h | 12 +- drivers/net/can/c_can/c_can_platform.c | 123 ++++++++++++++++----- 3 files changed, 114 insertions(+), 31 deletions(-) -- 1.8.3.2 ^ permalink raw reply [flat|nested] 70+ messages in thread
* [PATCH v2 0/3] net: can: Use syscon regmap for TI specific RAMINIT register @ 2014-09-09 14:31 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:31 UTC (permalink / raw) To: wg, mkl Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros Hi, Some hardware (TI am43xx) has a buggy RAMINIT DONE mechanism and it might not always set the DONE bit. This will result in a lockup in c_can_hw_raminit_wait_ti(), so patch 1 adds a timeout mechanism there. There is a non compliancy within TI platforms with respect to the layout of the RAMINIT register. The patches 2 and 3 address this issue and make a flexible but standard way of defining the RAMINIT hardware register layout in the device tree. The RAMINIT register is accessed using the syscon regmap framework. Patches available at git@github.com:rogerq/linux.git [for-v3.18/can] Changelog: v2: - added "ti" vendor prefix to TI specific raminit properties. - split DTS changes into a separate series cheers, -roger --- Roger Quadros (3): can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout net: can: c_can: Add syscon/regmap RAMINIT mechanism net: can: c_can: Add support for START pulse in RAMINIT sequence .../devicetree/bindings/net/can/c_can.txt | 10 ++ drivers/net/can/c_can/c_can.h | 12 +- drivers/net/can/c_can/c_can_platform.c | 123 ++++++++++++++++----- 3 files changed, 114 insertions(+), 31 deletions(-) -- 1.8.3.2 ^ permalink raw reply [flat|nested] 70+ messages in thread
* [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout 2014-09-09 14:31 ` Roger Quadros @ 2014-09-09 14:31 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:31 UTC (permalink / raw) To: wg, mkl; +Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). They seem to have been swapped in the usage instances. TI's RAMINIT DONE mechanism is buggy and may not always be set after the START bit is set. So add a timeout mechanism to c_can_hw_raminit_wait_ti(). Signed-off-by: Roger Quadros <rogerq@ti.com> --- drivers/net/can/c_can/c_can_platform.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c index 109cb44..b144e71 100644 --- a/drivers/net/can/c_can/c_can_platform.c +++ b/drivers/net/can/c_can/c_can_platform.c @@ -75,10 +75,18 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv, static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask, u32 val) { + int timeout = 0; /* We look only at the bits of our instance. */ val &= mask; - while ((readl(priv->raminit_ctrlreg) & mask) != val) + while ((readl(priv->raminit_ctrlreg) & mask) != val) { udelay(1); + timeout++; + + if (timeout == 1000) { + dev_err(&priv->dev->dev, "%s: time out\n", __func__); + break; + } + } } static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) @@ -97,14 +105,14 @@ static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); writel(ctrl, priv->raminit_ctrlreg); ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance); - c_can_hw_raminit_wait_ti(priv, ctrl, mask); + c_can_hw_raminit_wait_ti(priv, mask, ctrl); if (enable) { /* Set start bit and wait for the done bit. */ ctrl |= CAN_RAMINIT_START_MASK(priv->instance); writel(ctrl, priv->raminit_ctrlreg); ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); - c_can_hw_raminit_wait_ti(priv, ctrl, mask); + c_can_hw_raminit_wait_ti(priv, mask, ctrl); } spin_unlock(&raminit_lock); } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 70+ messages in thread
* [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout @ 2014-09-09 14:31 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:31 UTC (permalink / raw) To: wg, mkl Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). They seem to have been swapped in the usage instances. TI's RAMINIT DONE mechanism is buggy and may not always be set after the START bit is set. So add a timeout mechanism to c_can_hw_raminit_wait_ti(). Signed-off-by: Roger Quadros <rogerq@ti.com> --- drivers/net/can/c_can/c_can_platform.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c index 109cb44..b144e71 100644 --- a/drivers/net/can/c_can/c_can_platform.c +++ b/drivers/net/can/c_can/c_can_platform.c @@ -75,10 +75,18 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv, static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask, u32 val) { + int timeout = 0; /* We look only at the bits of our instance. */ val &= mask; - while ((readl(priv->raminit_ctrlreg) & mask) != val) + while ((readl(priv->raminit_ctrlreg) & mask) != val) { udelay(1); + timeout++; + + if (timeout == 1000) { + dev_err(&priv->dev->dev, "%s: time out\n", __func__); + break; + } + } } static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) @@ -97,14 +105,14 @@ static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); writel(ctrl, priv->raminit_ctrlreg); ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance); - c_can_hw_raminit_wait_ti(priv, ctrl, mask); + c_can_hw_raminit_wait_ti(priv, mask, ctrl); if (enable) { /* Set start bit and wait for the done bit. */ ctrl |= CAN_RAMINIT_START_MASK(priv->instance); writel(ctrl, priv->raminit_ctrlreg); ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); - c_can_hw_raminit_wait_ti(priv, ctrl, mask); + c_can_hw_raminit_wait_ti(priv, mask, ctrl); } spin_unlock(&raminit_lock); } -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout 2014-09-09 14:31 ` Roger Quadros @ 2014-09-09 14:34 ` Marc Kleine-Budde -1 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-09 14:34 UTC (permalink / raw) To: Roger Quadros, wg Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 724 bytes --] On 09/09/2014 04:31 PM, Roger Quadros wrote: > Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). > They seem to have been swapped in the usage instances. Can you split this fix into a seperate patch, please. > TI's RAMINIT DONE mechanism is buggy and may not always be > set after the START bit is set. So add a timeout mechanism to > c_can_hw_raminit_wait_ti(). What should happen if the timeout occurs? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout @ 2014-09-09 14:34 ` Marc Kleine-Budde 0 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-09 14:34 UTC (permalink / raw) To: Roger Quadros, wg Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 724 bytes --] On 09/09/2014 04:31 PM, Roger Quadros wrote: > Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). > They seem to have been swapped in the usage instances. Can you split this fix into a seperate patch, please. > TI's RAMINIT DONE mechanism is buggy and may not always be > set after the START bit is set. So add a timeout mechanism to > c_can_hw_raminit_wait_ti(). What should happen if the timeout occurs? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout 2014-09-09 14:34 ` Marc Kleine-Budde @ 2014-09-09 14:39 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:39 UTC (permalink / raw) To: Marc Kleine-Budde, wg Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar On 09/09/2014 05:34 PM, Marc Kleine-Budde wrote: > On 09/09/2014 04:31 PM, Roger Quadros wrote: >> Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). >> They seem to have been swapped in the usage instances. > > Can you split this fix into a seperate patch, please. OK. > >> TI's RAMINIT DONE mechanism is buggy and may not always be >> set after the START bit is set. So add a timeout mechanism to >> c_can_hw_raminit_wait_ti(). > > What should happen if the timeout occurs? I'm not sure yet. I will verify if the hardware still works or not in that case. If it doesn't work then we should fail. cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout @ 2014-09-09 14:39 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:39 UTC (permalink / raw) To: Marc Kleine-Budde, wg Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev On 09/09/2014 05:34 PM, Marc Kleine-Budde wrote: > On 09/09/2014 04:31 PM, Roger Quadros wrote: >> Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). >> They seem to have been swapped in the usage instances. > > Can you split this fix into a seperate patch, please. OK. > >> TI's RAMINIT DONE mechanism is buggy and may not always be >> set after the START bit is set. So add a timeout mechanism to >> c_can_hw_raminit_wait_ti(). > > What should happen if the timeout occurs? I'm not sure yet. I will verify if the hardware still works or not in that case. If it doesn't work then we should fail. cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout 2014-09-09 14:39 ` Roger Quadros @ 2014-09-16 14:13 ` Marc Kleine-Budde -1 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-16 14:13 UTC (permalink / raw) To: Roger Quadros, wg Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 694 bytes --] On 09/09/2014 04:39 PM, Roger Quadros wrote: > On 09/09/2014 05:34 PM, Marc Kleine-Budde wrote: >> On 09/09/2014 04:31 PM, Roger Quadros wrote: >>> Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). >>> They seem to have been swapped in the usage instances. >> >> Can you split this fix into a seperate patch, please. > > OK. I've added this fix only to the can-tree. Thanks, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout @ 2014-09-16 14:13 ` Marc Kleine-Budde 0 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-16 14:13 UTC (permalink / raw) To: Roger Quadros, wg Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 694 bytes --] On 09/09/2014 04:39 PM, Roger Quadros wrote: > On 09/09/2014 05:34 PM, Marc Kleine-Budde wrote: >> On 09/09/2014 04:31 PM, Roger Quadros wrote: >>> Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). >>> They seem to have been swapped in the usage instances. >> >> Can you split this fix into a seperate patch, please. > > OK. I've added this fix only to the can-tree. Thanks, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout 2014-09-09 14:31 ` Roger Quadros @ 2014-09-09 14:34 ` Nishanth Menon -1 siblings, 0 replies; 70+ messages in thread From: Nishanth Menon @ 2014-09-09 14:34 UTC (permalink / raw) To: Roger Quadros, wg, mkl Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, sergei.shtylyov, linux-omap, linux-can, netdev On 09/09/2014 09:31 AM, Roger Quadros wrote: > Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). > They seem to have been swapped in the usage instances. > > TI's RAMINIT DONE mechanism is buggy and may not always be > set after the START bit is set. So add a timeout mechanism to > c_can_hw_raminit_wait_ti(). > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > drivers/net/can/c_can/c_can_platform.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c > index 109cb44..b144e71 100644 > --- a/drivers/net/can/c_can/c_can_platform.c > +++ b/drivers/net/can/c_can/c_can_platform.c > @@ -75,10 +75,18 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv, > static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask, > u32 val) > { > + int timeout = 0; > /* We look only at the bits of our instance. */ > val &= mask; > - while ((readl(priv->raminit_ctrlreg) & mask) != val) > + while ((readl(priv->raminit_ctrlreg) & mask) != val) { > udelay(1); > + timeout++; > + > + if (timeout == 1000) { How did we come up with this number? > + dev_err(&priv->dev->dev, "%s: time out\n", __func__); > + break; lets say we did timeout.. see below: > + } > + } > } > > static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) > @@ -97,14 +105,14 @@ static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) > ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); > writel(ctrl, priv->raminit_ctrlreg); > ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance); > - c_can_hw_raminit_wait_ti(priv, ctrl, mask); > + c_can_hw_raminit_wait_ti(priv, mask, ctrl); > > if (enable) { > /* Set start bit and wait for the done bit. */ > ctrl |= CAN_RAMINIT_START_MASK(priv->instance); > writel(ctrl, priv->raminit_ctrlreg); > ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); > - c_can_hw_raminit_wait_ti(priv, ctrl, mask); > + c_can_hw_raminit_wait_ti(priv, mask, ctrl); is it possible for us to continue? does it make sense for us to change that void to a int and handle error cascading? > } > spin_unlock(&raminit_lock); > } > -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout @ 2014-09-09 14:34 ` Nishanth Menon 0 siblings, 0 replies; 70+ messages in thread From: Nishanth Menon @ 2014-09-09 14:34 UTC (permalink / raw) To: Roger Quadros, wg, mkl Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, sergei.shtylyov, linux-omap, linux-can, netdev On 09/09/2014 09:31 AM, Roger Quadros wrote: > Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). > They seem to have been swapped in the usage instances. > > TI's RAMINIT DONE mechanism is buggy and may not always be > set after the START bit is set. So add a timeout mechanism to > c_can_hw_raminit_wait_ti(). > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > drivers/net/can/c_can/c_can_platform.c | 14 +++++++++++--- > 1 file changed, 11 insertions(+), 3 deletions(-) > > diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c > index 109cb44..b144e71 100644 > --- a/drivers/net/can/c_can/c_can_platform.c > +++ b/drivers/net/can/c_can/c_can_platform.c > @@ -75,10 +75,18 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv, > static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask, > u32 val) > { > + int timeout = 0; > /* We look only at the bits of our instance. */ > val &= mask; > - while ((readl(priv->raminit_ctrlreg) & mask) != val) > + while ((readl(priv->raminit_ctrlreg) & mask) != val) { > udelay(1); > + timeout++; > + > + if (timeout == 1000) { How did we come up with this number? > + dev_err(&priv->dev->dev, "%s: time out\n", __func__); > + break; lets say we did timeout.. see below: > + } > + } > } > > static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) > @@ -97,14 +105,14 @@ static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) > ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); > writel(ctrl, priv->raminit_ctrlreg); > ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance); > - c_can_hw_raminit_wait_ti(priv, ctrl, mask); > + c_can_hw_raminit_wait_ti(priv, mask, ctrl); > > if (enable) { > /* Set start bit and wait for the done bit. */ > ctrl |= CAN_RAMINIT_START_MASK(priv->instance); > writel(ctrl, priv->raminit_ctrlreg); > ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); > - c_can_hw_raminit_wait_ti(priv, ctrl, mask); > + c_can_hw_raminit_wait_ti(priv, mask, ctrl); is it possible for us to continue? does it make sense for us to change that void to a int and handle error cascading? > } > spin_unlock(&raminit_lock); > } > -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout 2014-09-09 14:34 ` Nishanth Menon @ 2014-09-09 14:45 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:45 UTC (permalink / raw) To: wg, mkl Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, sergei.shtylyov, linux-omap, linux-can, netdev On 09/09/2014 05:34 PM, Nishanth Menon wrote: > On 09/09/2014 09:31 AM, Roger Quadros wrote: >> Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). >> They seem to have been swapped in the usage instances. >> >> TI's RAMINIT DONE mechanism is buggy and may not always be >> set after the START bit is set. So add a timeout mechanism to >> c_can_hw_raminit_wait_ti(). >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> drivers/net/can/c_can/c_can_platform.c | 14 +++++++++++--- >> 1 file changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c >> index 109cb44..b144e71 100644 >> --- a/drivers/net/can/c_can/c_can_platform.c >> +++ b/drivers/net/can/c_can/c_can_platform.c >> @@ -75,10 +75,18 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv, >> static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask, >> u32 val) >> { >> + int timeout = 0; >> /* We look only at the bits of our instance. */ >> val &= mask; >> - while ((readl(priv->raminit_ctrlreg) & mask) != val) >> + while ((readl(priv->raminit_ctrlreg) & mask) != val) { >> udelay(1); >> + timeout++; >> + >> + if (timeout == 1000) { > > How did we come up with this number? wild guess ;), that it should be set in a few microseconds and the delay is not too large. Till I don't hear from hardware guys, it will remain a guess. > >> + dev_err(&priv->dev->dev, "%s: time out\n", __func__); >> + break; > lets say we did timeout.. > see below: >> + } >> + } >> } >> >> static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) >> @@ -97,14 +105,14 @@ static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) >> ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); >> writel(ctrl, priv->raminit_ctrlreg); >> ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance); >> - c_can_hw_raminit_wait_ti(priv, ctrl, mask); >> + c_can_hw_raminit_wait_ti(priv, mask, ctrl); >> >> if (enable) { >> /* Set start bit and wait for the done bit. */ >> ctrl |= CAN_RAMINIT_START_MASK(priv->instance); >> writel(ctrl, priv->raminit_ctrlreg); >> ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); >> - c_can_hw_raminit_wait_ti(priv, ctrl, mask); >> + c_can_hw_raminit_wait_ti(priv, mask, ctrl); > > is it possible for us to continue? does it make sense for us to change > that void to a int and handle error cascading? I will verify this first to check if the hardware works or not in the failing case. Considering we never checked for the DONE bit in our earlier TI releases maybe it works. But I'll verify and get back. > >> } >> spin_unlock(&raminit_lock); >> } >> > > cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout @ 2014-09-09 14:45 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:45 UTC (permalink / raw) To: Nishanth Menon, wg, mkl Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, sergei.shtylyov, linux-omap, linux-can, netdev On 09/09/2014 05:34 PM, Nishanth Menon wrote: > On 09/09/2014 09:31 AM, Roger Quadros wrote: >> Pass the correct 'mask' and 'value' bits to c_can_hw_raminit_wait_ti(). >> They seem to have been swapped in the usage instances. >> >> TI's RAMINIT DONE mechanism is buggy and may not always be >> set after the START bit is set. So add a timeout mechanism to >> c_can_hw_raminit_wait_ti(). >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> drivers/net/can/c_can/c_can_platform.c | 14 +++++++++++--- >> 1 file changed, 11 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c >> index 109cb44..b144e71 100644 >> --- a/drivers/net/can/c_can/c_can_platform.c >> +++ b/drivers/net/can/c_can/c_can_platform.c >> @@ -75,10 +75,18 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv, >> static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask, >> u32 val) >> { >> + int timeout = 0; >> /* We look only at the bits of our instance. */ >> val &= mask; >> - while ((readl(priv->raminit_ctrlreg) & mask) != val) >> + while ((readl(priv->raminit_ctrlreg) & mask) != val) { >> udelay(1); >> + timeout++; >> + >> + if (timeout == 1000) { > > How did we come up with this number? wild guess ;), that it should be set in a few microseconds and the delay is not too large. Till I don't hear from hardware guys, it will remain a guess. > >> + dev_err(&priv->dev->dev, "%s: time out\n", __func__); >> + break; > lets say we did timeout.. > see below: >> + } >> + } >> } >> >> static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) >> @@ -97,14 +105,14 @@ static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) >> ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); >> writel(ctrl, priv->raminit_ctrlreg); >> ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance); >> - c_can_hw_raminit_wait_ti(priv, ctrl, mask); >> + c_can_hw_raminit_wait_ti(priv, mask, ctrl); >> >> if (enable) { >> /* Set start bit and wait for the done bit. */ >> ctrl |= CAN_RAMINIT_START_MASK(priv->instance); >> writel(ctrl, priv->raminit_ctrlreg); >> ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); >> - c_can_hw_raminit_wait_ti(priv, ctrl, mask); >> + c_can_hw_raminit_wait_ti(priv, mask, ctrl); > > is it possible for us to continue? does it make sense for us to change > that void to a int and handle error cascading? I will verify this first to check if the hardware works or not in the failing case. Considering we never checked for the DONE bit in our earlier TI releases maybe it works. But I'll verify and get back. > >> } >> spin_unlock(&raminit_lock); >> } >> > > cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout 2014-09-09 14:45 ` Roger Quadros @ 2014-09-09 14:51 ` Nishanth Menon -1 siblings, 0 replies; 70+ messages in thread From: Nishanth Menon @ 2014-09-09 14:51 UTC (permalink / raw) To: Roger Quadros, wg, mkl Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, sergei.shtylyov, linux-omap, linux-can, netdev On 09/09/2014 09:45 AM, Roger Quadros wrote: [...] >>> /* We look only at the bits of our instance. */ >>> val &= mask; >>> - while ((readl(priv->raminit_ctrlreg) & mask) != val) >>> + while ((readl(priv->raminit_ctrlreg) & mask) != val) { >>> udelay(1); >>> + timeout++; >>> + >>> + if (timeout == 1000) { >> >> How did we come up with this number? > > wild guess ;), that it should be set in a few microseconds and the delay is not too > large. > > Till I don't hear from hardware guys, it will remain a guess. > in cases like these, I suggest using emperical data as point -> example doing some 10,000 iterations of the operation and picking up the worse case number and double it. Either way, you need to document the same, else a few years down the line, when that number is in question, no one will know what it's basis was.. -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout @ 2014-09-09 14:51 ` Nishanth Menon 0 siblings, 0 replies; 70+ messages in thread From: Nishanth Menon @ 2014-09-09 14:51 UTC (permalink / raw) To: Roger Quadros, wg, mkl Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, sergei.shtylyov, linux-omap, linux-can, netdev On 09/09/2014 09:45 AM, Roger Quadros wrote: [...] >>> /* We look only at the bits of our instance. */ >>> val &= mask; >>> - while ((readl(priv->raminit_ctrlreg) & mask) != val) >>> + while ((readl(priv->raminit_ctrlreg) & mask) != val) { >>> udelay(1); >>> + timeout++; >>> + >>> + if (timeout == 1000) { >> >> How did we come up with this number? > > wild guess ;), that it should be set in a few microseconds and the delay is not too > large. > > Till I don't hear from hardware guys, it will remain a guess. > in cases like these, I suggest using emperical data as point -> example doing some 10,000 iterations of the operation and picking up the worse case number and double it. Either way, you need to document the same, else a few years down the line, when that number is in question, no one will know what it's basis was.. -- Regards, Nishanth Menon ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout 2014-09-09 14:51 ` Nishanth Menon @ 2014-09-09 14:54 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:54 UTC (permalink / raw) To: wg, mkl Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, sergei.shtylyov, linux-omap, linux-can, netdev On 09/09/2014 05:51 PM, Nishanth Menon wrote: > On 09/09/2014 09:45 AM, Roger Quadros wrote: > [...] >>>> /* We look only at the bits of our instance. */ >>>> val &= mask; >>>> - while ((readl(priv->raminit_ctrlreg) & mask) != val) >>>> + while ((readl(priv->raminit_ctrlreg) & mask) != val) { >>>> udelay(1); >>>> + timeout++; >>>> + >>>> + if (timeout == 1000) { >>> >>> How did we come up with this number? >> >> wild guess ;), that it should be set in a few microseconds and the delay is not too >> large. >> >> Till I don't hear from hardware guys, it will remain a guess. >> > > in cases like these, I suggest using emperical data as point -> > example doing some 10,000 iterations of the operation and picking up > the worse case number and double it. In my tests the bit was either set immediately or never at all. Not sure if we should increase it further. > > Either way, you need to document the same, else a few years down the > line, when that number is in question, no one will know what it's > basis was.. > OK. I'll add a comment there. cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout @ 2014-09-09 14:54 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:54 UTC (permalink / raw) To: Nishanth Menon, wg, mkl Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, sergei.shtylyov, linux-omap, linux-can, netdev On 09/09/2014 05:51 PM, Nishanth Menon wrote: > On 09/09/2014 09:45 AM, Roger Quadros wrote: > [...] >>>> /* We look only at the bits of our instance. */ >>>> val &= mask; >>>> - while ((readl(priv->raminit_ctrlreg) & mask) != val) >>>> + while ((readl(priv->raminit_ctrlreg) & mask) != val) { >>>> udelay(1); >>>> + timeout++; >>>> + >>>> + if (timeout == 1000) { >>> >>> How did we come up with this number? >> >> wild guess ;), that it should be set in a few microseconds and the delay is not too >> large. >> >> Till I don't hear from hardware guys, it will remain a guess. >> > > in cases like these, I suggest using emperical data as point -> > example doing some 10,000 iterations of the operation and picking up > the worse case number and double it. In my tests the bit was either set immediately or never at all. Not sure if we should increase it further. > > Either way, you need to document the same, else a few years down the > line, when that number is in question, no one will know what it's > basis was.. > OK. I'll add a comment there. cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-09 14:31 ` Roger Quadros @ 2014-09-09 14:31 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:31 UTC (permalink / raw) To: wg, mkl; +Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar Some TI SoCs like DRA7 have a RAMINIT register specification different from the other AMxx SoCs and as expected by the existing driver. To add more insanity, this register is shared with other IPs like DSS, PCIe and PWM. Provides a more generic mechanism to specify the RAMINIT register location and START/DONE bit position and use the syscon/regmap framework to access the register. Signed-off-by: Roger Quadros <rogerq@ti.com> --- .../devicetree/bindings/net/can/c_can.txt | 7 ++ drivers/net/can/c_can/c_can.h | 11 ++- drivers/net/can/c_can/c_can_platform.c | 109 +++++++++++++++------ 3 files changed, 95 insertions(+), 32 deletions(-) diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt index 8f1ae81..e12d1a1 100644 --- a/Documentation/devicetree/bindings/net/can/c_can.txt +++ b/Documentation/devicetree/bindings/net/can/c_can.txt @@ -13,6 +13,13 @@ Optional properties: - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the instance number +- ti,raminit-syscon : Handle to system control region that contains the + RAMINIT register. If specified, the second memory resource + in the reg property must index into the RAMINIT + register within the syscon region +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register + Note: "ti,hwmods" field is used to fetch the base address and irq resources from TI, omap hwmod data base during device registration. Future plan is to migrate hwmod data base contents into device tree diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h index 99ad1aa..bf68822 100644 --- a/drivers/net/can/c_can/c_can.h +++ b/drivers/net/can/c_can/c_can.h @@ -169,6 +169,14 @@ enum c_can_dev_id { BOSCH_D_CAN, }; +/* Out of band RAMINIT register access via syscon regmap */ +struct c_can_raminit { + struct regmap *syscon; /* for raminit ctrl. reg. access */ + unsigned int reg; /* register index within syscon */ + u8 start_bit; /* START bit position in raminit reg. */ + u8 done_bit; /* DONE bit position in raminit reg. */ +}; + /* c_can private data structure */ struct c_can_priv { struct can_priv can; /* must be the first member */ @@ -186,8 +194,7 @@ struct c_can_priv { const u16 *regs; void *priv; /* for board-specific data */ enum c_can_dev_id type; - u32 __iomem *raminit_ctrlreg; - int instance; + struct c_can_raminit raminit_sys; /* RAMINIT via syscon regmap */ void (*raminit) (const struct c_can_priv *priv, bool enable); u32 comm_rcv_high; u32 rxmasked; diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c index b144e71..fb0c35b 100644 --- a/drivers/net/can/c_can/c_can_platform.c +++ b/drivers/net/can/c_can/c_can_platform.c @@ -32,14 +32,13 @@ #include <linux/clk.h> #include <linux/of.h> #include <linux/of_device.h> +#include <linux/mfd/syscon.h> +#include <linux/regmap.h> #include <linux/can/dev.h> #include "c_can.h" -#define CAN_RAMINIT_START_MASK(i) (0x001 << (i)) -#define CAN_RAMINIT_DONE_MASK(i) (0x100 << (i)) -#define CAN_RAMINIT_ALL_MASK(i) (0x101 << (i)) #define DCAN_RAM_INIT_BIT (1 << 3) static DEFINE_SPINLOCK(raminit_lock); /* @@ -72,48 +71,59 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv, writew(val, priv->base + 2 * priv->regs[index]); } -static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask, - u32 val) +static void c_can_hw_raminit_wait_syscon(const struct c_can_priv *priv, + u32 mask, u32 val) { int timeout = 0; + const struct c_can_raminit *raminit = &priv->raminit_sys; + u32 ctrl; + /* We look only at the bits of our instance. */ val &= mask; - while ((readl(priv->raminit_ctrlreg) & mask) != val) { + do { udelay(1); timeout++; + regmap_read(raminit->syscon, raminit->reg, &ctrl); if (timeout == 1000) { dev_err(&priv->dev->dev, "%s: time out\n", __func__); break; } - } + } while ((ctrl & mask) != val); } -static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) +static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable) { - u32 mask = CAN_RAMINIT_ALL_MASK(priv->instance); + u32 mask; u32 ctrl; + const struct c_can_raminit *raminit = &priv->raminit_sys; spin_lock(&raminit_lock); - ctrl = readl(priv->raminit_ctrlreg); + mask = 1 << raminit->start_bit | 1 << raminit->done_bit; + regmap_read(raminit->syscon, raminit->reg, &ctrl); + /* We clear the done and start bit first. The start bit is * looking at the 0 -> transition, but is not self clearing; * And we clear the init done bit as well. + * NOTE: DONE must be written with 1 to clear it. */ - ctrl &= ~CAN_RAMINIT_START_MASK(priv->instance); - ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); - writel(ctrl, priv->raminit_ctrlreg); - ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance); - c_can_hw_raminit_wait_ti(priv, mask, ctrl); + ctrl &= ~(1 << raminit->start_bit); + ctrl |= 1 << raminit->done_bit; + regmap_write(raminit->syscon, raminit->reg, ctrl); + + ctrl &= ~(1 << raminit->done_bit); + c_can_hw_raminit_wait_syscon(priv, mask, ctrl); if (enable) { /* Set start bit and wait for the done bit. */ - ctrl |= CAN_RAMINIT_START_MASK(priv->instance); - writel(ctrl, priv->raminit_ctrlreg); - ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); - c_can_hw_raminit_wait_ti(priv, mask, ctrl); + ctrl |= 1 << raminit->start_bit; + regmap_write(raminit->syscon, raminit->reg, ctrl); + + ctrl |= 1 << raminit->done_bit; + c_can_hw_raminit_wait_syscon(priv, mask, ctrl); } + spin_unlock(&raminit_lock); } @@ -202,6 +212,8 @@ static int c_can_plat_probe(struct platform_device *pdev) struct resource *mem, *res; int irq; struct clk *clk; + struct device_node *np = pdev->dev.of_node; + u32 val; if (pdev->dev.of_node) { match = of_match_device(c_can_of_table, &pdev->dev); @@ -271,11 +283,6 @@ static int c_can_plat_probe(struct platform_device *pdev) priv->read_reg32 = d_can_plat_read_reg32; priv->write_reg32 = d_can_plat_write_reg32; - if (pdev->dev.of_node) - priv->instance = of_alias_get_id(pdev->dev.of_node, "d_can"); - else - priv->instance = pdev->id; - res = platform_get_resource(pdev, IORESOURCE_MEM, 1); /* Not all D_CAN modules have a separate register for the D_CAN * RAM initialization. Use default RAM init bit in D_CAN module @@ -286,12 +293,54 @@ static int c_can_plat_probe(struct platform_device *pdev) break; } - priv->raminit_ctrlreg = devm_ioremap(&pdev->dev, res->start, - resource_size(res)); - if (!priv->raminit_ctrlreg || priv->instance < 0) - dev_info(&pdev->dev, "control memory is not used for raminit\n"); - else - priv->raminit = c_can_hw_raminit_ti; + /* If separate RAMINIT register is specified access + * it using syscon regmap. Mostly for TI platforms. + */ + ret = -EINVAL; + if (!np) { + dev_err(&pdev->dev, + "separate RAMINIT reg. not supported on non DT\n"); + goto exit_free_device; + } + + priv->raminit_sys.syscon = syscon_regmap_lookup_by_phandle(np, + "ti,raminit-syscon"); + if (IS_ERR(priv->raminit_sys.syscon)) { + dev_err(&pdev->dev, + "couldn't get syscon regmap for RAMINIT reg.\n"); + goto exit_free_device; + } + + priv->raminit_sys.reg = res->start; + if (of_property_read_u32(np, "ti,raminit-start-bit", + &val)) { + dev_err(&pdev->dev, + "missing ti,raminit-start-bit property\n"); + goto exit_free_device; + } + + if (val > 31) { + dev_err(&pdev->dev, + "invalid ti,raminit-start-bit property\n"); + goto exit_free_device; + } + + priv->raminit_sys.start_bit = val; + if (of_property_read_u32(np, "ti,raminit-done-bit", + &val)) { + dev_err(&pdev->dev, + "missing ti,raminit-done-bit property\n"); + goto exit_free_device; + } + + if (val > 31) { + dev_err(&pdev->dev, + "invalid ti,raminit-done-bit property\n"); + goto exit_free_device; + } + + priv->raminit_sys.done_bit = val; + priv->raminit = c_can_hw_raminit_syscon; break; default: ret = -EINVAL; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 70+ messages in thread
* [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-09 14:31 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:31 UTC (permalink / raw) To: wg, mkl Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros Some TI SoCs like DRA7 have a RAMINIT register specification different from the other AMxx SoCs and as expected by the existing driver. To add more insanity, this register is shared with other IPs like DSS, PCIe and PWM. Provides a more generic mechanism to specify the RAMINIT register location and START/DONE bit position and use the syscon/regmap framework to access the register. Signed-off-by: Roger Quadros <rogerq@ti.com> --- .../devicetree/bindings/net/can/c_can.txt | 7 ++ drivers/net/can/c_can/c_can.h | 11 ++- drivers/net/can/c_can/c_can_platform.c | 109 +++++++++++++++------ 3 files changed, 95 insertions(+), 32 deletions(-) diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt index 8f1ae81..e12d1a1 100644 --- a/Documentation/devicetree/bindings/net/can/c_can.txt +++ b/Documentation/devicetree/bindings/net/can/c_can.txt @@ -13,6 +13,13 @@ Optional properties: - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the instance number +- ti,raminit-syscon : Handle to system control region that contains the + RAMINIT register. If specified, the second memory resource + in the reg property must index into the RAMINIT + register within the syscon region +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register + Note: "ti,hwmods" field is used to fetch the base address and irq resources from TI, omap hwmod data base during device registration. Future plan is to migrate hwmod data base contents into device tree diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h index 99ad1aa..bf68822 100644 --- a/drivers/net/can/c_can/c_can.h +++ b/drivers/net/can/c_can/c_can.h @@ -169,6 +169,14 @@ enum c_can_dev_id { BOSCH_D_CAN, }; +/* Out of band RAMINIT register access via syscon regmap */ +struct c_can_raminit { + struct regmap *syscon; /* for raminit ctrl. reg. access */ + unsigned int reg; /* register index within syscon */ + u8 start_bit; /* START bit position in raminit reg. */ + u8 done_bit; /* DONE bit position in raminit reg. */ +}; + /* c_can private data structure */ struct c_can_priv { struct can_priv can; /* must be the first member */ @@ -186,8 +194,7 @@ struct c_can_priv { const u16 *regs; void *priv; /* for board-specific data */ enum c_can_dev_id type; - u32 __iomem *raminit_ctrlreg; - int instance; + struct c_can_raminit raminit_sys; /* RAMINIT via syscon regmap */ void (*raminit) (const struct c_can_priv *priv, bool enable); u32 comm_rcv_high; u32 rxmasked; diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c index b144e71..fb0c35b 100644 --- a/drivers/net/can/c_can/c_can_platform.c +++ b/drivers/net/can/c_can/c_can_platform.c @@ -32,14 +32,13 @@ #include <linux/clk.h> #include <linux/of.h> #include <linux/of_device.h> +#include <linux/mfd/syscon.h> +#include <linux/regmap.h> #include <linux/can/dev.h> #include "c_can.h" -#define CAN_RAMINIT_START_MASK(i) (0x001 << (i)) -#define CAN_RAMINIT_DONE_MASK(i) (0x100 << (i)) -#define CAN_RAMINIT_ALL_MASK(i) (0x101 << (i)) #define DCAN_RAM_INIT_BIT (1 << 3) static DEFINE_SPINLOCK(raminit_lock); /* @@ -72,48 +71,59 @@ static void c_can_plat_write_reg_aligned_to_32bit(const struct c_can_priv *priv, writew(val, priv->base + 2 * priv->regs[index]); } -static void c_can_hw_raminit_wait_ti(const struct c_can_priv *priv, u32 mask, - u32 val) +static void c_can_hw_raminit_wait_syscon(const struct c_can_priv *priv, + u32 mask, u32 val) { int timeout = 0; + const struct c_can_raminit *raminit = &priv->raminit_sys; + u32 ctrl; + /* We look only at the bits of our instance. */ val &= mask; - while ((readl(priv->raminit_ctrlreg) & mask) != val) { + do { udelay(1); timeout++; + regmap_read(raminit->syscon, raminit->reg, &ctrl); if (timeout == 1000) { dev_err(&priv->dev->dev, "%s: time out\n", __func__); break; } - } + } while ((ctrl & mask) != val); } -static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) +static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable) { - u32 mask = CAN_RAMINIT_ALL_MASK(priv->instance); + u32 mask; u32 ctrl; + const struct c_can_raminit *raminit = &priv->raminit_sys; spin_lock(&raminit_lock); - ctrl = readl(priv->raminit_ctrlreg); + mask = 1 << raminit->start_bit | 1 << raminit->done_bit; + regmap_read(raminit->syscon, raminit->reg, &ctrl); + /* We clear the done and start bit first. The start bit is * looking at the 0 -> transition, but is not self clearing; * And we clear the init done bit as well. + * NOTE: DONE must be written with 1 to clear it. */ - ctrl &= ~CAN_RAMINIT_START_MASK(priv->instance); - ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); - writel(ctrl, priv->raminit_ctrlreg); - ctrl &= ~CAN_RAMINIT_DONE_MASK(priv->instance); - c_can_hw_raminit_wait_ti(priv, mask, ctrl); + ctrl &= ~(1 << raminit->start_bit); + ctrl |= 1 << raminit->done_bit; + regmap_write(raminit->syscon, raminit->reg, ctrl); + + ctrl &= ~(1 << raminit->done_bit); + c_can_hw_raminit_wait_syscon(priv, mask, ctrl); if (enable) { /* Set start bit and wait for the done bit. */ - ctrl |= CAN_RAMINIT_START_MASK(priv->instance); - writel(ctrl, priv->raminit_ctrlreg); - ctrl |= CAN_RAMINIT_DONE_MASK(priv->instance); - c_can_hw_raminit_wait_ti(priv, mask, ctrl); + ctrl |= 1 << raminit->start_bit; + regmap_write(raminit->syscon, raminit->reg, ctrl); + + ctrl |= 1 << raminit->done_bit; + c_can_hw_raminit_wait_syscon(priv, mask, ctrl); } + spin_unlock(&raminit_lock); } @@ -202,6 +212,8 @@ static int c_can_plat_probe(struct platform_device *pdev) struct resource *mem, *res; int irq; struct clk *clk; + struct device_node *np = pdev->dev.of_node; + u32 val; if (pdev->dev.of_node) { match = of_match_device(c_can_of_table, &pdev->dev); @@ -271,11 +283,6 @@ static int c_can_plat_probe(struct platform_device *pdev) priv->read_reg32 = d_can_plat_read_reg32; priv->write_reg32 = d_can_plat_write_reg32; - if (pdev->dev.of_node) - priv->instance = of_alias_get_id(pdev->dev.of_node, "d_can"); - else - priv->instance = pdev->id; - res = platform_get_resource(pdev, IORESOURCE_MEM, 1); /* Not all D_CAN modules have a separate register for the D_CAN * RAM initialization. Use default RAM init bit in D_CAN module @@ -286,12 +293,54 @@ static int c_can_plat_probe(struct platform_device *pdev) break; } - priv->raminit_ctrlreg = devm_ioremap(&pdev->dev, res->start, - resource_size(res)); - if (!priv->raminit_ctrlreg || priv->instance < 0) - dev_info(&pdev->dev, "control memory is not used for raminit\n"); - else - priv->raminit = c_can_hw_raminit_ti; + /* If separate RAMINIT register is specified access + * it using syscon regmap. Mostly for TI platforms. + */ + ret = -EINVAL; + if (!np) { + dev_err(&pdev->dev, + "separate RAMINIT reg. not supported on non DT\n"); + goto exit_free_device; + } + + priv->raminit_sys.syscon = syscon_regmap_lookup_by_phandle(np, + "ti,raminit-syscon"); + if (IS_ERR(priv->raminit_sys.syscon)) { + dev_err(&pdev->dev, + "couldn't get syscon regmap for RAMINIT reg.\n"); + goto exit_free_device; + } + + priv->raminit_sys.reg = res->start; + if (of_property_read_u32(np, "ti,raminit-start-bit", + &val)) { + dev_err(&pdev->dev, + "missing ti,raminit-start-bit property\n"); + goto exit_free_device; + } + + if (val > 31) { + dev_err(&pdev->dev, + "invalid ti,raminit-start-bit property\n"); + goto exit_free_device; + } + + priv->raminit_sys.start_bit = val; + if (of_property_read_u32(np, "ti,raminit-done-bit", + &val)) { + dev_err(&pdev->dev, + "missing ti,raminit-done-bit property\n"); + goto exit_free_device; + } + + if (val > 31) { + dev_err(&pdev->dev, + "invalid ti,raminit-done-bit property\n"); + goto exit_free_device; + } + + priv->raminit_sys.done_bit = val; + priv->raminit = c_can_hw_raminit_syscon; break; default: ret = -EINVAL; -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-09 14:31 ` Roger Quadros @ 2014-09-30 13:26 ` Wolfram Sang -1 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-09-30 13:26 UTC (permalink / raw) To: Roger Quadros Cc: wg, mkl, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 1868 bytes --] On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: > Some TI SoCs like DRA7 have a RAMINIT register specification > different from the other AMxx SoCs and as expected by the > existing driver. > > To add more insanity, this register is shared with other > IPs like DSS, PCIe and PWM. > > Provides a more generic mechanism to specify the RAMINIT > register location and START/DONE bit position and use the > syscon/regmap framework to access the register. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > .../devicetree/bindings/net/can/c_can.txt | 7 ++ > drivers/net/can/c_can/c_can.h | 11 ++- > drivers/net/can/c_can/c_can_platform.c | 109 +++++++++++++++------ > 3 files changed, 95 insertions(+), 32 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt > index 8f1ae81..e12d1a1 100644 > --- a/Documentation/devicetree/bindings/net/can/c_can.txt > +++ b/Documentation/devicetree/bindings/net/can/c_can.txt > @@ -13,6 +13,13 @@ Optional properties: > - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the > instance number > > +- ti,raminit-syscon : Handle to system control region that contains the > + RAMINIT register. If specified, the second memory resource > + in the reg property must index into the RAMINIT > + register within the syscon region There seems to be a simple "syscon" property these days. > +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register > +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register This should not be encoded in DT! This is not describing hardware setup. The driver should know where the bits are for the syscon phandle, depending on which SoC it runs... [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 13:26 ` Wolfram Sang 0 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-09-30 13:26 UTC (permalink / raw) To: Roger Quadros Cc: wg, mkl, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 1868 bytes --] On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: > Some TI SoCs like DRA7 have a RAMINIT register specification > different from the other AMxx SoCs and as expected by the > existing driver. > > To add more insanity, this register is shared with other > IPs like DSS, PCIe and PWM. > > Provides a more generic mechanism to specify the RAMINIT > register location and START/DONE bit position and use the > syscon/regmap framework to access the register. > > Signed-off-by: Roger Quadros <rogerq@ti.com> > --- > .../devicetree/bindings/net/can/c_can.txt | 7 ++ > drivers/net/can/c_can/c_can.h | 11 ++- > drivers/net/can/c_can/c_can_platform.c | 109 +++++++++++++++------ > 3 files changed, 95 insertions(+), 32 deletions(-) > > diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt > index 8f1ae81..e12d1a1 100644 > --- a/Documentation/devicetree/bindings/net/can/c_can.txt > +++ b/Documentation/devicetree/bindings/net/can/c_can.txt > @@ -13,6 +13,13 @@ Optional properties: > - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the > instance number > > +- ti,raminit-syscon : Handle to system control region that contains the > + RAMINIT register. If specified, the second memory resource > + in the reg property must index into the RAMINIT > + register within the syscon region There seems to be a simple "syscon" property these days. > +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register > +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register This should not be encoded in DT! This is not describing hardware setup. The driver should know where the bits are for the syscon phandle, depending on which SoC it runs... [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 13:26 ` Wolfram Sang @ 2014-09-30 13:33 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-30 13:33 UTC (permalink / raw) To: Wolfram Sang Cc: wg, mkl, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar On 09/30/2014 04:26 PM, Wolfram Sang wrote: > On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: >> Some TI SoCs like DRA7 have a RAMINIT register specification >> different from the other AMxx SoCs and as expected by the >> existing driver. >> >> To add more insanity, this register is shared with other >> IPs like DSS, PCIe and PWM. >> >> Provides a more generic mechanism to specify the RAMINIT >> register location and START/DONE bit position and use the >> syscon/regmap framework to access the register. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> .../devicetree/bindings/net/can/c_can.txt | 7 ++ >> drivers/net/can/c_can/c_can.h | 11 ++- >> drivers/net/can/c_can/c_can_platform.c | 109 +++++++++++++++------ >> 3 files changed, 95 insertions(+), 32 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt >> index 8f1ae81..e12d1a1 100644 >> --- a/Documentation/devicetree/bindings/net/can/c_can.txt >> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt >> @@ -13,6 +13,13 @@ Optional properties: >> - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the >> instance number >> >> +- ti,raminit-syscon : Handle to system control region that contains the >> + RAMINIT register. If specified, the second memory resource >> + in the reg property must index into the RAMINIT >> + register within the syscon region > > There seems to be a simple "syscon" property these days. I had used plain "syscon" in the earlier revisions but was asked to make it a TI specific property since only TI uses this mechanism. > >> +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register >> +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register > > This should not be encoded in DT! This is not describing hardware setup. > The driver should know where the bits are for the syscon phandle, > depending on which SoC it runs... > OK. I'll think of matching the compatible ID with SOC specific data in the driver. cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 13:33 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-30 13:33 UTC (permalink / raw) To: Wolfram Sang Cc: wg, mkl, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev On 09/30/2014 04:26 PM, Wolfram Sang wrote: > On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: >> Some TI SoCs like DRA7 have a RAMINIT register specification >> different from the other AMxx SoCs and as expected by the >> existing driver. >> >> To add more insanity, this register is shared with other >> IPs like DSS, PCIe and PWM. >> >> Provides a more generic mechanism to specify the RAMINIT >> register location and START/DONE bit position and use the >> syscon/regmap framework to access the register. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> .../devicetree/bindings/net/can/c_can.txt | 7 ++ >> drivers/net/can/c_can/c_can.h | 11 ++- >> drivers/net/can/c_can/c_can_platform.c | 109 +++++++++++++++------ >> 3 files changed, 95 insertions(+), 32 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt >> index 8f1ae81..e12d1a1 100644 >> --- a/Documentation/devicetree/bindings/net/can/c_can.txt >> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt >> @@ -13,6 +13,13 @@ Optional properties: >> - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the >> instance number >> >> +- ti,raminit-syscon : Handle to system control region that contains the >> + RAMINIT register. If specified, the second memory resource >> + in the reg property must index into the RAMINIT >> + register within the syscon region > > There seems to be a simple "syscon" property these days. I had used plain "syscon" in the earlier revisions but was asked to make it a TI specific property since only TI uses this mechanism. > >> +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register >> +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register > > This should not be encoded in DT! This is not describing hardware setup. > The driver should know where the bits are for the syscon phandle, > depending on which SoC it runs... > OK. I'll think of matching the compatible ID with SOC specific data in the driver. cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 13:33 ` Roger Quadros @ 2014-09-30 13:52 ` Wolfram Sang -1 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-09-30 13:52 UTC (permalink / raw) To: Roger Quadros Cc: wg, mkl, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 1414 bytes --] > >> +- ti,raminit-syscon : Handle to system control region that contains the > >> + RAMINIT register. If specified, the second memory resource > >> + in the reg property must index into the RAMINIT > >> + register within the syscon region > > > > There seems to be a simple "syscon" property these days. > > I had used plain "syscon" in the earlier revisions but was asked to make it a TI specific > property since only TI uses this mechanism. I see, when was that? Currently, it looks messy :( Grepping through the dts files in 3.17-rc7, I see that bcm7445 uses "syscon" and variants with prefixes, STE uses it, too. Samsung uses "samsung,syscon-phandle", st uses "st,syscon"... The MACID readout patches for AM335x (just applied right now) also use "syscon": https://patchwork.ozlabs.org/patch/394289/ I'd vote for a generic "syscon" to be OK, yet I guess the DT maintainers will have the final word. > >> +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register > >> +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register > > > > This should not be encoded in DT! This is not describing hardware setup. > > The driver should know where the bits are for the syscon phandle, > > depending on which SoC it runs... > > > OK. I'll think of matching the compatible ID with SOC specific data in the driver. Great, thanks! [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 13:52 ` Wolfram Sang 0 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-09-30 13:52 UTC (permalink / raw) To: Roger Quadros Cc: wg, mkl, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 1414 bytes --] > >> +- ti,raminit-syscon : Handle to system control region that contains the > >> + RAMINIT register. If specified, the second memory resource > >> + in the reg property must index into the RAMINIT > >> + register within the syscon region > > > > There seems to be a simple "syscon" property these days. > > I had used plain "syscon" in the earlier revisions but was asked to make it a TI specific > property since only TI uses this mechanism. I see, when was that? Currently, it looks messy :( Grepping through the dts files in 3.17-rc7, I see that bcm7445 uses "syscon" and variants with prefixes, STE uses it, too. Samsung uses "samsung,syscon-phandle", st uses "st,syscon"... The MACID readout patches for AM335x (just applied right now) also use "syscon": https://patchwork.ozlabs.org/patch/394289/ I'd vote for a generic "syscon" to be OK, yet I guess the DT maintainers will have the final word. > >> +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register > >> +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register > > > > This should not be encoded in DT! This is not describing hardware setup. > > The driver should know where the bits are for the syscon phandle, > > depending on which SoC it runs... > > > OK. I'll think of matching the compatible ID with SOC specific data in the driver. Great, thanks! [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 13:52 ` Wolfram Sang @ 2014-09-30 13:58 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-30 13:58 UTC (permalink / raw) To: Wolfram Sang Cc: wg, mkl, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar On 09/30/2014 04:52 PM, Wolfram Sang wrote: > >>>> +- ti,raminit-syscon : Handle to system control region that contains the >>>> + RAMINIT register. If specified, the second memory resource >>>> + in the reg property must index into the RAMINIT >>>> + register within the syscon region >>> >>> There seems to be a simple "syscon" property these days. >> >> I had used plain "syscon" in the earlier revisions but was asked to make it a TI specific >> property since only TI uses this mechanism. > > I see, when was that? Currently, it looks messy :( Grepping through the > dts files in 3.17-rc7, I see that bcm7445 uses "syscon" and variants > with prefixes, STE uses it, too. Samsung uses "samsung,syscon-phandle", > st uses "st,syscon"... > > The MACID readout patches for AM335x (just applied right now) also use > "syscon": https://patchwork.ozlabs.org/patch/394289/ > > I'd vote for a generic "syscon" to be OK, yet I guess the DT maintainers > will have the final word. Now that I actually checked, it was never just "syscon" but just without the "ti" vendor prefix. Sorry for the misinformation. As just TI is using this out of band RAMINIT mechanism, should it be "ti,syscon" or just "syscon"? cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 13:58 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-30 13:58 UTC (permalink / raw) To: Wolfram Sang Cc: wg, mkl, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev On 09/30/2014 04:52 PM, Wolfram Sang wrote: > >>>> +- ti,raminit-syscon : Handle to system control region that contains the >>>> + RAMINIT register. If specified, the second memory resource >>>> + in the reg property must index into the RAMINIT >>>> + register within the syscon region >>> >>> There seems to be a simple "syscon" property these days. >> >> I had used plain "syscon" in the earlier revisions but was asked to make it a TI specific >> property since only TI uses this mechanism. > > I see, when was that? Currently, it looks messy :( Grepping through the > dts files in 3.17-rc7, I see that bcm7445 uses "syscon" and variants > with prefixes, STE uses it, too. Samsung uses "samsung,syscon-phandle", > st uses "st,syscon"... > > The MACID readout patches for AM335x (just applied right now) also use > "syscon": https://patchwork.ozlabs.org/patch/394289/ > > I'd vote for a generic "syscon" to be OK, yet I guess the DT maintainers > will have the final word. Now that I actually checked, it was never just "syscon" but just without the "ti" vendor prefix. Sorry for the misinformation. As just TI is using this out of band RAMINIT mechanism, should it be "ti,syscon" or just "syscon"? cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 13:58 ` Roger Quadros @ 2014-09-30 14:19 ` Wolfram Sang -1 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-09-30 14:19 UTC (permalink / raw) To: Roger Quadros Cc: wg, mkl, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 578 bytes --] > As just TI is using this out of band RAMINIT mechanism, should it be "ti,syscon" or just "syscon"? Yes, only TI uses this out-of-band RAMINIT (currently, at least). So, we need an (optional) way to describe that. However, accessing syscon registers in general is not TI specific and a generic way to do this should be used. Which looks to me like the "syscon" property to allow access to the register. Still, we should ask DT maintainers about it, maybe they prefer a more precise property like "syscon-raminit" to allow for further syscon extensions later or something... [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 14:19 ` Wolfram Sang 0 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-09-30 14:19 UTC (permalink / raw) To: Roger Quadros Cc: wg, mkl, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 578 bytes --] > As just TI is using this out of band RAMINIT mechanism, should it be "ti,syscon" or just "syscon"? Yes, only TI uses this out-of-band RAMINIT (currently, at least). So, we need an (optional) way to describe that. However, accessing syscon registers in general is not TI specific and a generic way to do this should be used. Which looks to me like the "syscon" property to allow access to the register. Still, we should ask DT maintainers about it, maybe they prefer a more precise property like "syscon-raminit" to allow for further syscon extensions later or something... [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 14:19 ` Wolfram Sang @ 2014-09-30 14:22 ` Marc Kleine-Budde -1 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-30 14:22 UTC (permalink / raw) To: Wolfram Sang, Roger Quadros Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 1047 bytes --] On 09/30/2014 04:19 PM, Wolfram Sang wrote: > >> As just TI is using this out of band RAMINIT mechanism, should it be "ti,syscon" or just "syscon"? > > Yes, only TI uses this out-of-band RAMINIT (currently, at least). So, we > need an (optional) way to describe that. However, accessing syscon > registers in general is not TI specific and a generic way to do this > should be used. Which looks to me like the "syscon" property to allow > access to the register. Still, we should ask DT maintainers about it, > maybe they prefer a more precise property like "syscon-raminit" to allow > for further syscon extensions later or something... What do you think about putting the bit information in the syscon-raminit phandle as additional arguments? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 14:22 ` Marc Kleine-Budde 0 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-30 14:22 UTC (permalink / raw) To: Wolfram Sang, Roger Quadros Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 1047 bytes --] On 09/30/2014 04:19 PM, Wolfram Sang wrote: > >> As just TI is using this out of band RAMINIT mechanism, should it be "ti,syscon" or just "syscon"? > > Yes, only TI uses this out-of-band RAMINIT (currently, at least). So, we > need an (optional) way to describe that. However, accessing syscon > registers in general is not TI specific and a generic way to do this > should be used. Which looks to me like the "syscon" property to allow > access to the register. Still, we should ask DT maintainers about it, > maybe they prefer a more precise property like "syscon-raminit" to allow > for further syscon extensions later or something... What do you think about putting the bit information in the syscon-raminit phandle as additional arguments? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 14:22 ` Marc Kleine-Budde @ 2014-09-30 14:49 ` Wolfram Sang -1 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-09-30 14:49 UTC (permalink / raw) To: Marc Kleine-Budde Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 1199 bytes --] On Tue, Sep 30, 2014 at 04:22:08PM +0200, Marc Kleine-Budde wrote: > On 09/30/2014 04:19 PM, Wolfram Sang wrote: > > > >> As just TI is using this out of band RAMINIT mechanism, should it be "ti,syscon" or just "syscon"? > > > > Yes, only TI uses this out-of-band RAMINIT (currently, at least). So, we > > need an (optional) way to describe that. However, accessing syscon > > registers in general is not TI specific and a generic way to do this > > should be used. Which looks to me like the "syscon" property to allow > > access to the register. Still, we should ask DT maintainers about it, > > maybe they prefer a more precise property like "syscon-raminit" to allow > > for further syscon extensions later or something... > > What do you think about putting the bit information in the > syscon-raminit phandle as additional arguments? Then we'd have <n> syscon phandles for <n> instances? Also, judging from Markus patch [1] there is already some infrastructure, namely syscon_regmap_lookup_by_phandle(). From a glimpse, it doesn't look viable to add such a support to it. So, I'd rather drop additional arguments. Why would you like to have it encoded in DT? [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 14:49 ` Wolfram Sang 0 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-09-30 14:49 UTC (permalink / raw) To: Marc Kleine-Budde Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 1199 bytes --] On Tue, Sep 30, 2014 at 04:22:08PM +0200, Marc Kleine-Budde wrote: > On 09/30/2014 04:19 PM, Wolfram Sang wrote: > > > >> As just TI is using this out of band RAMINIT mechanism, should it be "ti,syscon" or just "syscon"? > > > > Yes, only TI uses this out-of-band RAMINIT (currently, at least). So, we > > need an (optional) way to describe that. However, accessing syscon > > registers in general is not TI specific and a generic way to do this > > should be used. Which looks to me like the "syscon" property to allow > > access to the register. Still, we should ask DT maintainers about it, > > maybe they prefer a more precise property like "syscon-raminit" to allow > > for further syscon extensions later or something... > > What do you think about putting the bit information in the > syscon-raminit phandle as additional arguments? Then we'd have <n> syscon phandles for <n> instances? Also, judging from Markus patch [1] there is already some infrastructure, namely syscon_regmap_lookup_by_phandle(). From a glimpse, it doesn't look viable to add such a support to it. So, I'd rather drop additional arguments. Why would you like to have it encoded in DT? [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 14:49 ` Wolfram Sang @ 2014-09-30 15:01 ` Marc Kleine-Budde -1 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-30 15:01 UTC (permalink / raw) To: Wolfram Sang Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 1997 bytes --] On 09/30/2014 04:49 PM, Wolfram Sang wrote: > On Tue, Sep 30, 2014 at 04:22:08PM +0200, Marc Kleine-Budde wrote: >> On 09/30/2014 04:19 PM, Wolfram Sang wrote: >>> >>>> As just TI is using this out of band RAMINIT mechanism, should it be "ti,syscon" or just "syscon"? >>> >>> Yes, only TI uses this out-of-band RAMINIT (currently, at least). So, we >>> need an (optional) way to describe that. However, accessing syscon >>> registers in general is not TI specific and a generic way to do this >>> should be used. Which looks to me like the "syscon" property to allow >>> access to the register. Still, we should ask DT maintainers about it, >>> maybe they prefer a more precise property like "syscon-raminit" to allow >>> for further syscon extensions later or something... >> >> What do you think about putting the bit information in the >> syscon-raminit phandle as additional arguments? > > Then we'd have <n> syscon phandles for <n> instances? > > Also, judging from Markus patch [1] there is already some > infrastructure, namely syscon_regmap_lookup_by_phandle(). From a > glimpse, it doesn't look viable to add such a support to it. Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for additional parameters. Have a look at: drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c First get the regmap, then the 1st argument is the offset in the regmap, the 2nd and 3rd could be the bits. > So, I'd rather drop additional arguments. > > Why would you like to have it encoded in DT? Where put the information then? Into the driver, but where do you get the reference which instance of the DCAN you are, so that you can look up the correct bits? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 15:01 ` Marc Kleine-Budde 0 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-30 15:01 UTC (permalink / raw) To: Wolfram Sang Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 1997 bytes --] On 09/30/2014 04:49 PM, Wolfram Sang wrote: > On Tue, Sep 30, 2014 at 04:22:08PM +0200, Marc Kleine-Budde wrote: >> On 09/30/2014 04:19 PM, Wolfram Sang wrote: >>> >>>> As just TI is using this out of band RAMINIT mechanism, should it be "ti,syscon" or just "syscon"? >>> >>> Yes, only TI uses this out-of-band RAMINIT (currently, at least). So, we >>> need an (optional) way to describe that. However, accessing syscon >>> registers in general is not TI specific and a generic way to do this >>> should be used. Which looks to me like the "syscon" property to allow >>> access to the register. Still, we should ask DT maintainers about it, >>> maybe they prefer a more precise property like "syscon-raminit" to allow >>> for further syscon extensions later or something... >> >> What do you think about putting the bit information in the >> syscon-raminit phandle as additional arguments? > > Then we'd have <n> syscon phandles for <n> instances? > > Also, judging from Markus patch [1] there is already some > infrastructure, namely syscon_regmap_lookup_by_phandle(). From a > glimpse, it doesn't look viable to add such a support to it. Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for additional parameters. Have a look at: drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c First get the regmap, then the 1st argument is the offset in the regmap, the 2nd and 3rd could be the bits. > So, I'd rather drop additional arguments. > > Why would you like to have it encoded in DT? Where put the information then? Into the driver, but where do you get the reference which instance of the DCAN you are, so that you can look up the correct bits? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 15:01 ` Marc Kleine-Budde @ 2014-09-30 15:25 ` Wolfram Sang -1 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-09-30 15:25 UTC (permalink / raw) To: Marc Kleine-Budde Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 946 bytes --] > Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for > additional parameters. Have a look at: > > drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c > > First get the regmap, then the 1st argument is the offset in the regmap, > the 2nd and 3rd could be the bits. So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> For another driver (the stmmac example): <reg_offset> <reg_shift> Phew... Then we should really have a "syscon-raminit" property probably, so that at least plain "syscon" has a consistent syntax? > > > So, I'd rather drop additional arguments. > > > > Why would you like to have it encoded in DT? > > Where put the information then? Into the driver, but where do you get > the reference which instance of the DCAN you are, so that you can look > up the correct bits? Agreed. I thought we had this information in the driver already, but we haven't... [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 15:25 ` Wolfram Sang 0 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-09-30 15:25 UTC (permalink / raw) To: Marc Kleine-Budde Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 946 bytes --] > Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for > additional parameters. Have a look at: > > drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c > > First get the regmap, then the 1st argument is the offset in the regmap, > the 2nd and 3rd could be the bits. So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> For another driver (the stmmac example): <reg_offset> <reg_shift> Phew... Then we should really have a "syscon-raminit" property probably, so that at least plain "syscon" has a consistent syntax? > > > So, I'd rather drop additional arguments. > > > > Why would you like to have it encoded in DT? > > Where put the information then? Into the driver, but where do you get > the reference which instance of the DCAN you are, so that you can look > up the correct bits? Agreed. I thought we had this information in the driver already, but we haven't... [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 15:25 ` Wolfram Sang @ 2014-09-30 16:04 ` Marc Kleine-Budde -1 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-30 16:04 UTC (permalink / raw) To: Wolfram Sang Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 1640 bytes --] On 09/30/2014 05:25 PM, Wolfram Sang wrote: > >> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >> additional parameters. Have a look at: >> >> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >> >> First get the regmap, then the 1st argument is the offset in the regmap, >> the 2nd and 3rd could be the bits. > > So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> > For another driver (the stmmac example): <reg_offset> <reg_shift> The DCAN's "reg" is a "reg_offset" as in the stmmc. Roger, can we derive both start and done bit from a common reg_shift? > Phew... Then we should really have a "syscon-raminit" property probably, > so that at least plain "syscon" has a consistent syntax? I think^whope we can have the same syntax as the stmmc :D >>> So, I'd rather drop additional arguments. >>> >>> Why would you like to have it encoded in DT? >> >> Where put the information then? Into the driver, but where do you get >> the reference which instance of the DCAN you are, so that you can look >> up the correct bits? > > Agreed. I thought we had this information in the driver already, but we > haven't... > The current driver relies on the of_alias_get_id(), which isn't considered best practice, is it? So I want to avoid this when switching to syscon. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 16:04 ` Marc Kleine-Budde 0 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-30 16:04 UTC (permalink / raw) To: Wolfram Sang Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 1640 bytes --] On 09/30/2014 05:25 PM, Wolfram Sang wrote: > >> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >> additional parameters. Have a look at: >> >> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >> >> First get the regmap, then the 1st argument is the offset in the regmap, >> the 2nd and 3rd could be the bits. > > So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> > For another driver (the stmmac example): <reg_offset> <reg_shift> The DCAN's "reg" is a "reg_offset" as in the stmmc. Roger, can we derive both start and done bit from a common reg_shift? > Phew... Then we should really have a "syscon-raminit" property probably, > so that at least plain "syscon" has a consistent syntax? I think^whope we can have the same syntax as the stmmc :D >>> So, I'd rather drop additional arguments. >>> >>> Why would you like to have it encoded in DT? >> >> Where put the information then? Into the driver, but where do you get >> the reference which instance of the DCAN you are, so that you can look >> up the correct bits? > > Agreed. I thought we had this information in the driver already, but we > haven't... > The current driver relies on the of_alias_get_id(), which isn't considered best practice, is it? So I want to avoid this when switching to syscon. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 16:04 ` Marc Kleine-Budde @ 2014-10-01 8:45 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-10-01 8:45 UTC (permalink / raw) To: Marc Kleine-Budde, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote: > On 09/30/2014 05:25 PM, Wolfram Sang wrote: >> >>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >>> additional parameters. Have a look at: >>> >>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>> >>> First get the regmap, then the 1st argument is the offset in the regmap, >>> the 2nd and 3rd could be the bits. >> >> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> >> For another driver (the stmmac example): <reg_offset> <reg_shift> > > The DCAN's "reg" is a "reg_offset" as in the stmmc. > > Roger, can we derive both start and done bit from a common reg_shift? I'm sorry I didn't understand what you meant. <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us. Even though reg offset is the same for both the DCAN instances. > >> Phew... Then we should really have a "syscon-raminit" property probably, >> so that at least plain "syscon" has a consistent syntax? > > I think^whope we can have the same syntax as the stmmc :D I agree too. > >>>> So, I'd rather drop additional arguments. >>>> >>>> Why would you like to have it encoded in DT? >>> >>> Where put the information then? Into the driver, but where do you get >>> the reference which instance of the DCAN you are, so that you can look >>> up the correct bits? >> >> Agreed. I thought we had this information in the driver already, but we >> haven't... >> > > The current driver relies on the of_alias_get_id(), which isn't > considered best practice, is it? So I want to avoid this when switching > to syscon. cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-10-01 8:45 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-10-01 8:45 UTC (permalink / raw) To: Marc Kleine-Budde, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote: > On 09/30/2014 05:25 PM, Wolfram Sang wrote: >> >>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >>> additional parameters. Have a look at: >>> >>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>> >>> First get the regmap, then the 1st argument is the offset in the regmap, >>> the 2nd and 3rd could be the bits. >> >> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> >> For another driver (the stmmac example): <reg_offset> <reg_shift> > > The DCAN's "reg" is a "reg_offset" as in the stmmc. > > Roger, can we derive both start and done bit from a common reg_shift? I'm sorry I didn't understand what you meant. <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us. Even though reg offset is the same for both the DCAN instances. > >> Phew... Then we should really have a "syscon-raminit" property probably, >> so that at least plain "syscon" has a consistent syntax? > > I think^whope we can have the same syntax as the stmmc :D I agree too. > >>>> So, I'd rather drop additional arguments. >>>> >>>> Why would you like to have it encoded in DT? >>> >>> Where put the information then? Into the driver, but where do you get >>> the reference which instance of the DCAN you are, so that you can look >>> up the correct bits? >> >> Agreed. I thought we had this information in the driver already, but we >> haven't... >> > > The current driver relies on the of_alias_get_id(), which isn't > considered best practice, is it? So I want to avoid this when switching > to syscon. cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-10-01 8:45 ` Roger Quadros @ 2014-10-01 8:47 ` Marc Kleine-Budde -1 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-10-01 8:47 UTC (permalink / raw) To: Roger Quadros, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 1338 bytes --] On 10/01/2014 10:45 AM, Roger Quadros wrote: > On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote: >> On 09/30/2014 05:25 PM, Wolfram Sang wrote: >>> >>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >>>> additional parameters. Have a look at: >>>> >>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>> >>>> First get the regmap, then the 1st argument is the offset in the regmap, >>>> the 2nd and 3rd could be the bits. >>> >>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> >>> For another driver (the stmmac example): <reg_offset> <reg_shift> >> >> The DCAN's "reg" is a "reg_offset" as in the stmmc. >> >> Roger, can we derive both start and done bit from a common reg_shift? > > I'm sorry I didn't understand what you meant. > > <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us. > Even though reg offset is the same for both the DCAN instances. What's start bit and stop bit for instance 0 and 1 on that SoC that has two instances?? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-10-01 8:47 ` Marc Kleine-Budde 0 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-10-01 8:47 UTC (permalink / raw) To: Roger Quadros, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 1338 bytes --] On 10/01/2014 10:45 AM, Roger Quadros wrote: > On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote: >> On 09/30/2014 05:25 PM, Wolfram Sang wrote: >>> >>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >>>> additional parameters. Have a look at: >>>> >>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>> >>>> First get the regmap, then the 1st argument is the offset in the regmap, >>>> the 2nd and 3rd could be the bits. >>> >>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> >>> For another driver (the stmmac example): <reg_offset> <reg_shift> >> >> The DCAN's "reg" is a "reg_offset" as in the stmmc. >> >> Roger, can we derive both start and done bit from a common reg_shift? > > I'm sorry I didn't understand what you meant. > > <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us. > Even though reg offset is the same for both the DCAN instances. What's start bit and stop bit for instance 0 and 1 on that SoC that has two instances?? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-10-01 8:47 ` Marc Kleine-Budde @ 2014-10-01 9:06 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-10-01 9:06 UTC (permalink / raw) To: Marc Kleine-Budde, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote: > On 10/01/2014 10:45 AM, Roger Quadros wrote: >> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote: >>> On 09/30/2014 05:25 PM, Wolfram Sang wrote: >>>> >>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >>>>> additional parameters. Have a look at: >>>>> >>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>>> >>>>> First get the regmap, then the 1st argument is the offset in the regmap, >>>>> the 2nd and 3rd could be the bits. >>>> >>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> >>>> For another driver (the stmmac example): <reg_offset> <reg_shift> >>> >>> The DCAN's "reg" is a "reg_offset" as in the stmmc. >>> >>> Roger, can we derive both start and done bit from a common reg_shift? >> >> I'm sorry I didn't understand what you meant. >> >> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us. >> Even though reg offset is the same for both the DCAN instances. > > What's start bit and stop bit for instance 0 and 1 on that SoC that has > two instances?? > we have 3 SoCs at the moment, all have 2 DCAN instances. AM33xx & AM43xx instance start stop 1 0 8 2 1 9 DRA7xx instance start stop 1 3 1 2 5 2 cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-10-01 9:06 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-10-01 9:06 UTC (permalink / raw) To: Marc Kleine-Budde, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote: > On 10/01/2014 10:45 AM, Roger Quadros wrote: >> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote: >>> On 09/30/2014 05:25 PM, Wolfram Sang wrote: >>>> >>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >>>>> additional parameters. Have a look at: >>>>> >>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>>> >>>>> First get the regmap, then the 1st argument is the offset in the regmap, >>>>> the 2nd and 3rd could be the bits. >>>> >>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> >>>> For another driver (the stmmac example): <reg_offset> <reg_shift> >>> >>> The DCAN's "reg" is a "reg_offset" as in the stmmc. >>> >>> Roger, can we derive both start and done bit from a common reg_shift? >> >> I'm sorry I didn't understand what you meant. >> >> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us. >> Even though reg offset is the same for both the DCAN instances. > > What's start bit and stop bit for instance 0 and 1 on that SoC that has > two instances?? > we have 3 SoCs at the moment, all have 2 DCAN instances. AM33xx & AM43xx instance start stop 1 0 8 2 1 9 DRA7xx instance start stop 1 3 1 2 5 2 cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-10-01 9:06 ` Roger Quadros @ 2014-10-01 10:01 ` Marc Kleine-Budde -1 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-10-01 10:01 UTC (permalink / raw) To: Roger Quadros, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 1852 bytes --] On 10/01/2014 11:06 AM, Roger Quadros wrote: > On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote: >> On 10/01/2014 10:45 AM, Roger Quadros wrote: >>> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote: >>>> On 09/30/2014 05:25 PM, Wolfram Sang wrote: >>>>> >>>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >>>>>> additional parameters. Have a look at: >>>>>> >>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>>>> >>>>>> First get the regmap, then the 1st argument is the offset in the regmap, >>>>>> the 2nd and 3rd could be the bits. >>>>> >>>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> >>>>> For another driver (the stmmac example): <reg_offset> <reg_shift> >>>> >>>> The DCAN's "reg" is a "reg_offset" as in the stmmc. >>>> >>>> Roger, can we derive both start and done bit from a common reg_shift? >>> >>> I'm sorry I didn't understand what you meant. >>> >>> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us. >>> Even though reg offset is the same for both the DCAN instances. >> >> What's start bit and stop bit for instance 0 and 1 on that SoC that has >> two instances? >> > > we have 3 SoCs at the moment, all have 2 DCAN instances. > > AM33xx & AM43xx > instance start stop > 1 0 8 > 2 1 9 If we use a 0-based numbering for the instances: instance start stop 0 (0 << instance) (8 << instance) 1 (0 << instance) (8 << instance) > DRA7xx > instance start stop > 1 3 1 > 2 5 2 ^ 5 or 4? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-10-01 10:01 ` Marc Kleine-Budde 0 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-10-01 10:01 UTC (permalink / raw) To: Roger Quadros, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 1852 bytes --] On 10/01/2014 11:06 AM, Roger Quadros wrote: > On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote: >> On 10/01/2014 10:45 AM, Roger Quadros wrote: >>> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote: >>>> On 09/30/2014 05:25 PM, Wolfram Sang wrote: >>>>> >>>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >>>>>> additional parameters. Have a look at: >>>>>> >>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>>>> >>>>>> First get the regmap, then the 1st argument is the offset in the regmap, >>>>>> the 2nd and 3rd could be the bits. >>>>> >>>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> >>>>> For another driver (the stmmac example): <reg_offset> <reg_shift> >>>> >>>> The DCAN's "reg" is a "reg_offset" as in the stmmc. >>>> >>>> Roger, can we derive both start and done bit from a common reg_shift? >>> >>> I'm sorry I didn't understand what you meant. >>> >>> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us. >>> Even though reg offset is the same for both the DCAN instances. >> >> What's start bit and stop bit for instance 0 and 1 on that SoC that has >> two instances? >> > > we have 3 SoCs at the moment, all have 2 DCAN instances. > > AM33xx & AM43xx > instance start stop > 1 0 8 > 2 1 9 If we use a 0-based numbering for the instances: instance start stop 0 (0 << instance) (8 << instance) 1 (0 << instance) (8 << instance) > DRA7xx > instance start stop > 1 3 1 > 2 5 2 ^ 5 or 4? Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-10-01 10:01 ` Marc Kleine-Budde @ 2014-10-01 10:12 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-10-01 10:12 UTC (permalink / raw) To: Marc Kleine-Budde, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar On 10/01/2014 01:01 PM, Marc Kleine-Budde wrote: > On 10/01/2014 11:06 AM, Roger Quadros wrote: >> On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote: >>> On 10/01/2014 10:45 AM, Roger Quadros wrote: >>>> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote: >>>>> On 09/30/2014 05:25 PM, Wolfram Sang wrote: >>>>>> >>>>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >>>>>>> additional parameters. Have a look at: >>>>>>> >>>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>>>>> >>>>>>> First get the regmap, then the 1st argument is the offset in the regmap, >>>>>>> the 2nd and 3rd could be the bits. >>>>>> >>>>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> >>>>>> For another driver (the stmmac example): <reg_offset> <reg_shift> >>>>> >>>>> The DCAN's "reg" is a "reg_offset" as in the stmmc. >>>>> >>>>> Roger, can we derive both start and done bit from a common reg_shift? >>>> >>>> I'm sorry I didn't understand what you meant. >>>> >>>> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us. >>>> Even though reg offset is the same for both the DCAN instances. >>> >>> What's start bit and stop bit for instance 0 and 1 on that SoC that has >>> two instances? >>> >> >> we have 3 SoCs at the moment, all have 2 DCAN instances. >> >> AM33xx & AM43xx >> instance start stop >> 1 0 8 >> 2 1 9 > > If we use a 0-based numbering for the instances: > instance start stop > 0 (0 << instance) (8 << instance) > 1 (0 << instance) (8 << instance) > How does the instance number get set? What happens on boards where the first instance is unused while the second one is in use? >> DRA7xx >> instance start stop >> 1 3 1 >> 2 5 2 > ^ > 5 or 4? Unfortunately it is 5 ;) We have display IP related bit in between 3 and 5 :P cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-10-01 10:12 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-10-01 10:12 UTC (permalink / raw) To: Marc Kleine-Budde, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev On 10/01/2014 01:01 PM, Marc Kleine-Budde wrote: > On 10/01/2014 11:06 AM, Roger Quadros wrote: >> On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote: >>> On 10/01/2014 10:45 AM, Roger Quadros wrote: >>>> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote: >>>>> On 09/30/2014 05:25 PM, Wolfram Sang wrote: >>>>>> >>>>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >>>>>>> additional parameters. Have a look at: >>>>>>> >>>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>>>>> >>>>>>> First get the regmap, then the 1st argument is the offset in the regmap, >>>>>>> the 2nd and 3rd could be the bits. >>>>>> >>>>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> >>>>>> For another driver (the stmmac example): <reg_offset> <reg_shift> >>>>> >>>>> The DCAN's "reg" is a "reg_offset" as in the stmmc. >>>>> >>>>> Roger, can we derive both start and done bit from a common reg_shift? >>>> >>>> I'm sorry I didn't understand what you meant. >>>> >>>> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us. >>>> Even though reg offset is the same for both the DCAN instances. >>> >>> What's start bit and stop bit for instance 0 and 1 on that SoC that has >>> two instances? >>> >> >> we have 3 SoCs at the moment, all have 2 DCAN instances. >> >> AM33xx & AM43xx >> instance start stop >> 1 0 8 >> 2 1 9 > > If we use a 0-based numbering for the instances: > instance start stop > 0 (0 << instance) (8 << instance) > 1 (0 << instance) (8 << instance) > How does the instance number get set? What happens on boards where the first instance is unused while the second one is in use? >> DRA7xx >> instance start stop >> 1 3 1 >> 2 5 2 > ^ > 5 or 4? Unfortunately it is 5 ;) We have display IP related bit in between 3 and 5 :P cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-10-01 10:12 ` Roger Quadros @ 2014-10-01 10:26 ` Marc Kleine-Budde -1 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-10-01 10:26 UTC (permalink / raw) To: Roger Quadros, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 3071 bytes --] On 10/01/2014 12:12 PM, Roger Quadros wrote: > On 10/01/2014 01:01 PM, Marc Kleine-Budde wrote: >> On 10/01/2014 11:06 AM, Roger Quadros wrote: >>> On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote: >>>> On 10/01/2014 10:45 AM, Roger Quadros wrote: >>>>> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote: >>>>>> On 09/30/2014 05:25 PM, Wolfram Sang wrote: >>>>>>> >>>>>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >>>>>>>> additional parameters. Have a look at: >>>>>>>> >>>>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>>>>>> >>>>>>>> First get the regmap, then the 1st argument is the offset in the regmap, >>>>>>>> the 2nd and 3rd could be the bits. >>>>>>> >>>>>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> >>>>>>> For another driver (the stmmac example): <reg_offset> <reg_shift> >>>>>> >>>>>> The DCAN's "reg" is a "reg_offset" as in the stmmc. >>>>>> >>>>>> Roger, can we derive both start and done bit from a common reg_shift? >>>>> >>>>> I'm sorry I didn't understand what you meant. >>>>> >>>>> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us. >>>>> Even though reg offset is the same for both the DCAN instances. >>>> >>>> What's start bit and stop bit for instance 0 and 1 on that SoC that has >>>> two instances? >>>> >>> >>> we have 3 SoCs at the moment, all have 2 DCAN instances. >>> >>> AM33xx & AM43xx >>> instance start stop >>> 1 0 8 >>> 2 1 9 >> >> If we use a 0-based numbering for the instances: >> instance start stop >> 0 (0 << instance) (8 << instance) >> 1 (0 << instance) (8 << instance) >> > > How does the instance number get set? What happens on boards where > the first instance is unused while the second one is in use? Via a single "instance" parameter of the syscon phandle.... > >>> DRA7xx >>> instance start stop >>> 1 3 1 >>> 2 5 2 >> ^ >> 5 or 4? > > Unfortunately it is 5 ;) > We have display IP related bit in between 3 and 5 :P What on earth were the HW engineers thinking???????????? ...if we just have the instance parameter in the syscon phandle, we have to put the mapping into the driver, which makes IMHO no sense, because you have to touch the driver, if there is another SoC with the DCAN core. AFAICS we have these options: 1) syscon phandle with three parameters: reg offset, start bit shift, stop bit shift (the name of the syscon phandle is a different topic) 2) a single ti,start-stop-bit option with two parameter 3) two ti,* entries with a single parameter each (as in Roger's initial patch) Wolfram, which solution do you prefer? I'm in favour of 3 (+ plus a phandle with a reg offset parameter). puzzled, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-10-01 10:26 ` Marc Kleine-Budde 0 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-10-01 10:26 UTC (permalink / raw) To: Roger Quadros, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 3071 bytes --] On 10/01/2014 12:12 PM, Roger Quadros wrote: > On 10/01/2014 01:01 PM, Marc Kleine-Budde wrote: >> On 10/01/2014 11:06 AM, Roger Quadros wrote: >>> On 10/01/2014 11:47 AM, Marc Kleine-Budde wrote: >>>> On 10/01/2014 10:45 AM, Roger Quadros wrote: >>>>> On 09/30/2014 07:04 PM, Marc Kleine-Budde wrote: >>>>>> On 09/30/2014 05:25 PM, Wolfram Sang wrote: >>>>>>> >>>>>>>> Yes, but syscon_regmap_lookup_by_phandle() doesn't need any support for >>>>>>>> additional parameters. Have a look at: >>>>>>>> >>>>>>>> drivers/net/ethernet/stmicro/stmmac/dwmac-socfpga.c >>>>>>>> >>>>>>>> First get the regmap, then the 1st argument is the offset in the regmap, >>>>>>>> the 2nd and 3rd could be the bits. >>>>>>> >>>>>>> So, for one driver the extra arguments are: <reg> <start_bit> <stop_bit> >>>>>>> For another driver (the stmmac example): <reg_offset> <reg_shift> >>>>>> >>>>>> The DCAN's "reg" is a "reg_offset" as in the stmmc. >>>>>> >>>>>> Roger, can we derive both start and done bit from a common reg_shift? >>>>> >>>>> I'm sorry I didn't understand what you meant. >>>>> >>>>> <&syscon_phandl> <reg offset> <start bit> <stop bit> should work well for us. >>>>> Even though reg offset is the same for both the DCAN instances. >>>> >>>> What's start bit and stop bit for instance 0 and 1 on that SoC that has >>>> two instances? >>>> >>> >>> we have 3 SoCs at the moment, all have 2 DCAN instances. >>> >>> AM33xx & AM43xx >>> instance start stop >>> 1 0 8 >>> 2 1 9 >> >> If we use a 0-based numbering for the instances: >> instance start stop >> 0 (0 << instance) (8 << instance) >> 1 (0 << instance) (8 << instance) >> > > How does the instance number get set? What happens on boards where > the first instance is unused while the second one is in use? Via a single "instance" parameter of the syscon phandle.... > >>> DRA7xx >>> instance start stop >>> 1 3 1 >>> 2 5 2 >> ^ >> 5 or 4? > > Unfortunately it is 5 ;) > We have display IP related bit in between 3 and 5 :P What on earth were the HW engineers thinking???????????? ...if we just have the instance parameter in the syscon phandle, we have to put the mapping into the driver, which makes IMHO no sense, because you have to touch the driver, if there is another SoC with the DCAN core. AFAICS we have these options: 1) syscon phandle with three parameters: reg offset, start bit shift, stop bit shift (the name of the syscon phandle is a different topic) 2) a single ti,start-stop-bit option with two parameter 3) two ti,* entries with a single parameter each (as in Roger's initial patch) Wolfram, which solution do you prefer? I'm in favour of 3 (+ plus a phandle with a reg offset parameter). puzzled, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-10-01 10:26 ` Marc Kleine-Budde @ 2014-10-01 10:43 ` Wolfram Sang -1 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-10-01 10:43 UTC (permalink / raw) To: Marc Kleine-Budde Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 752 bytes --] > > Unfortunately it is 5 ;) > > We have display IP related bit in between 3 and 5 :P > > What on earth were the HW engineers thinking???????????? "Let's test my RNG on the bit-placement of this register" :) > ...if we just have the instance parameter in the syscon phandle, we have > to put the mapping into the driver, which makes IMHO no sense, because > you have to touch the driver, if there is another SoC with the DCAN core. ... which would be my preferred solution. I think new SoCs should have some kind of: compatible = "commodore,c64ultra", "bosch,d_can"; in the DT anyhow to allow for SoC specific quirks/adjustments. And custom raminit belongs to that IMO (see the ti routine getting more and more specific). [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-10-01 10:43 ` Wolfram Sang 0 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-10-01 10:43 UTC (permalink / raw) To: Marc Kleine-Budde Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 752 bytes --] > > Unfortunately it is 5 ;) > > We have display IP related bit in between 3 and 5 :P > > What on earth were the HW engineers thinking???????????? "Let's test my RNG on the bit-placement of this register" :) > ...if we just have the instance parameter in the syscon phandle, we have > to put the mapping into the driver, which makes IMHO no sense, because > you have to touch the driver, if there is another SoC with the DCAN core. ... which would be my preferred solution. I think new SoCs should have some kind of: compatible = "commodore,c64ultra", "bosch,d_can"; in the DT anyhow to allow for SoC specific quirks/adjustments. And custom raminit belongs to that IMO (see the ti routine getting more and more specific). [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-10-01 10:43 ` Wolfram Sang @ 2014-10-01 10:57 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-10-01 10:57 UTC (permalink / raw) To: Wolfram Sang, Marc Kleine-Budde Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar On 10/01/2014 01:43 PM, Wolfram Sang wrote: > >>> Unfortunately it is 5 ;) >>> We have display IP related bit in between 3 and 5 :P >> >> What on earth were the HW engineers thinking???????????? > > "Let's test my RNG on the bit-placement of this register" :) :D > >> ...if we just have the instance parameter in the syscon phandle, we have >> to put the mapping into the driver, which makes IMHO no sense, because >> you have to touch the driver, if there is another SoC with the DCAN core. My guess is that TI won't come up with a 3rd variant so we won't have to touch the driver, but you never know for sure. > > ... which would be my preferred solution. I think new SoCs should have > some kind of: > > compatible = "commodore,c64ultra", "bosch,d_can"; > > in the DT anyhow to allow for SoC specific quirks/adjustments. And > custom raminit belongs to that IMO (see the ti routine getting more and > more specific). > Right. For now we need 2 start/stop definations for the existing TI Socs. but where to store the raminit start/stop bits? The driver_data currently seems to contain the CAN type C_CAN vs D_CAN without containing it in a platform_data structure. Is it OK to create a new platform_data structure for CAN and put the type and raminit start/stop bits there? cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-10-01 10:57 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-10-01 10:57 UTC (permalink / raw) To: Wolfram Sang, Marc Kleine-Budde Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev On 10/01/2014 01:43 PM, Wolfram Sang wrote: > >>> Unfortunately it is 5 ;) >>> We have display IP related bit in between 3 and 5 :P >> >> What on earth were the HW engineers thinking???????????? > > "Let's test my RNG on the bit-placement of this register" :) :D > >> ...if we just have the instance parameter in the syscon phandle, we have >> to put the mapping into the driver, which makes IMHO no sense, because >> you have to touch the driver, if there is another SoC with the DCAN core. My guess is that TI won't come up with a 3rd variant so we won't have to touch the driver, but you never know for sure. > > ... which would be my preferred solution. I think new SoCs should have > some kind of: > > compatible = "commodore,c64ultra", "bosch,d_can"; > > in the DT anyhow to allow for SoC specific quirks/adjustments. And > custom raminit belongs to that IMO (see the ti routine getting more and > more specific). > Right. For now we need 2 start/stop definations for the existing TI Socs. but where to store the raminit start/stop bits? The driver_data currently seems to contain the CAN type C_CAN vs D_CAN without containing it in a platform_data structure. Is it OK to create a new platform_data structure for CAN and put the type and raminit start/stop bits there? cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-10-01 10:57 ` Roger Quadros @ 2014-10-01 11:06 ` Marc Kleine-Budde -1 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-10-01 11:06 UTC (permalink / raw) To: Roger Quadros, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 1538 bytes --] On 10/01/2014 12:57 PM, Roger Quadros wrote: >>> ...if we just have the instance parameter in the syscon phandle, we have >>> to put the mapping into the driver, which makes IMHO no sense, because >>> you have to touch the driver, if there is another SoC with the DCAN core. > > My guess is that TI won't come up with a 3rd variant so we won't have > to touch the driver, but you never know for sure. When I comes to 99% compatible hardware.... I've seen some. >> ... which would be my preferred solution. I think new SoCs should have >> some kind of: >> >> compatible = "commodore,c64ultra", "bosch,d_can"; >> >> in the DT anyhow to allow for SoC specific quirks/adjustments. And >> custom raminit belongs to that IMO (see the ti routine getting more and >> more specific). >> > > Right. For now we need 2 start/stop definations for the existing TI Socs. > > but where to store the raminit start/stop bits? The driver_data currently seems to > contain the CAN type C_CAN vs D_CAN without containing it in a platform_data structure. > > Is it OK to create a new platform_data structure for CAN and put the type and raminit start/stop > bits there? Yes, have a look how it's handled in the flexcan driver. regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-10-01 11:06 ` Marc Kleine-Budde 0 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-10-01 11:06 UTC (permalink / raw) To: Roger Quadros, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 1538 bytes --] On 10/01/2014 12:57 PM, Roger Quadros wrote: >>> ...if we just have the instance parameter in the syscon phandle, we have >>> to put the mapping into the driver, which makes IMHO no sense, because >>> you have to touch the driver, if there is another SoC with the DCAN core. > > My guess is that TI won't come up with a 3rd variant so we won't have > to touch the driver, but you never know for sure. When I comes to 99% compatible hardware.... I've seen some. >> ... which would be my preferred solution. I think new SoCs should have >> some kind of: >> >> compatible = "commodore,c64ultra", "bosch,d_can"; >> >> in the DT anyhow to allow for SoC specific quirks/adjustments. And >> custom raminit belongs to that IMO (see the ti routine getting more and >> more specific). >> > > Right. For now we need 2 start/stop definations for the existing TI Socs. > > but where to store the raminit start/stop bits? The driver_data currently seems to > contain the CAN type C_CAN vs D_CAN without containing it in a platform_data structure. > > Is it OK to create a new platform_data structure for CAN and put the type and raminit start/stop > bits there? Yes, have a look how it's handled in the flexcan driver. regards, Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-10-01 10:57 ` Roger Quadros @ 2014-10-01 11:10 ` Wolfram Sang -1 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-10-01 11:10 UTC (permalink / raw) To: Roger Quadros Cc: Marc Kleine-Budde, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 156 bytes --] > Is it OK to create a new platform_data structure for CAN and put the type and raminit start/stop > bits there? I'd say yes, don't see a reason not to. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-10-01 11:10 ` Wolfram Sang 0 siblings, 0 replies; 70+ messages in thread From: Wolfram Sang @ 2014-10-01 11:10 UTC (permalink / raw) To: Roger Quadros Cc: Marc Kleine-Budde, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 156 bytes --] > Is it OK to create a new platform_data structure for CAN and put the type and raminit start/stop > bits there? I'd say yes, don't see a reason not to. [-- Attachment #2: Digital signature --] [-- Type: application/pgp-signature, Size: 819 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-10-01 10:43 ` Wolfram Sang @ 2014-10-01 11:11 ` Marc Kleine-Budde -1 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-10-01 11:11 UTC (permalink / raw) To: Wolfram Sang Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 383 bytes --] On 10/01/2014 12:43 PM, Wolfram Sang wrote: > compatible = "commodore,c64ultra", "bosch,d_can"; \o/ -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-10-01 11:11 ` Marc Kleine-Budde 0 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-10-01 11:11 UTC (permalink / raw) To: Wolfram Sang Cc: Roger Quadros, wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 383 bytes --] On 10/01/2014 12:43 PM, Wolfram Sang wrote: > compatible = "commodore,c64ultra", "bosch,d_can"; \o/ -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 13:26 ` Wolfram Sang @ 2014-09-30 13:45 ` Marc Kleine-Budde -1 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-30 13:45 UTC (permalink / raw) To: Wolfram Sang, Roger Quadros Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 2390 bytes --] On 09/30/2014 03:26 PM, Wolfram Sang wrote: > On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: >> Some TI SoCs like DRA7 have a RAMINIT register specification >> different from the other AMxx SoCs and as expected by the >> existing driver. >> >> To add more insanity, this register is shared with other >> IPs like DSS, PCIe and PWM. >> >> Provides a more generic mechanism to specify the RAMINIT >> register location and START/DONE bit position and use the >> syscon/regmap framework to access the register. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> .../devicetree/bindings/net/can/c_can.txt | 7 ++ >> drivers/net/can/c_can/c_can.h | 11 ++- >> drivers/net/can/c_can/c_can_platform.c | 109 +++++++++++++++------ >> 3 files changed, 95 insertions(+), 32 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt >> index 8f1ae81..e12d1a1 100644 >> --- a/Documentation/devicetree/bindings/net/can/c_can.txt >> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt >> @@ -13,6 +13,13 @@ Optional properties: >> - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the >> instance number >> >> +- ti,raminit-syscon : Handle to system control region that contains the >> + RAMINIT register. If specified, the second memory resource >> + in the reg property must index into the RAMINIT >> + register within the syscon region > > There seems to be a simple "syscon" property these days. > >> +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register >> +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register > > This should not be encoded in DT! This is not describing hardware setup. > The driver should know where the bits are for the syscon phandle, > depending on which SoC it runs... Is the register shared by more than one core? If so the information has to go somewhere. Using an alias in the DT is probably the wrong approach. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 13:45 ` Marc Kleine-Budde 0 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-30 13:45 UTC (permalink / raw) To: Wolfram Sang, Roger Quadros Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 2390 bytes --] On 09/30/2014 03:26 PM, Wolfram Sang wrote: > On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: >> Some TI SoCs like DRA7 have a RAMINIT register specification >> different from the other AMxx SoCs and as expected by the >> existing driver. >> >> To add more insanity, this register is shared with other >> IPs like DSS, PCIe and PWM. >> >> Provides a more generic mechanism to specify the RAMINIT >> register location and START/DONE bit position and use the >> syscon/regmap framework to access the register. >> >> Signed-off-by: Roger Quadros <rogerq@ti.com> >> --- >> .../devicetree/bindings/net/can/c_can.txt | 7 ++ >> drivers/net/can/c_can/c_can.h | 11 ++- >> drivers/net/can/c_can/c_can_platform.c | 109 +++++++++++++++------ >> 3 files changed, 95 insertions(+), 32 deletions(-) >> >> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt >> index 8f1ae81..e12d1a1 100644 >> --- a/Documentation/devicetree/bindings/net/can/c_can.txt >> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt >> @@ -13,6 +13,13 @@ Optional properties: >> - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the >> instance number >> >> +- ti,raminit-syscon : Handle to system control region that contains the >> + RAMINIT register. If specified, the second memory resource >> + in the reg property must index into the RAMINIT >> + register within the syscon region > > There seems to be a simple "syscon" property these days. > >> +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register >> +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register > > This should not be encoded in DT! This is not describing hardware setup. > The driver should know where the bits are for the syscon phandle, > depending on which SoC it runs... Is the register shared by more than one core? If so the information has to go somewhere. Using an alias in the DT is probably the wrong approach. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 13:45 ` Marc Kleine-Budde @ 2014-09-30 14:02 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-30 14:02 UTC (permalink / raw) To: Marc Kleine-Budde, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar On 09/30/2014 04:45 PM, Marc Kleine-Budde wrote: > On 09/30/2014 03:26 PM, Wolfram Sang wrote: >> On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: >>> Some TI SoCs like DRA7 have a RAMINIT register specification >>> different from the other AMxx SoCs and as expected by the >>> existing driver. >>> >>> To add more insanity, this register is shared with other >>> IPs like DSS, PCIe and PWM. >>> >>> Provides a more generic mechanism to specify the RAMINIT >>> register location and START/DONE bit position and use the >>> syscon/regmap framework to access the register. >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> --- >>> .../devicetree/bindings/net/can/c_can.txt | 7 ++ >>> drivers/net/can/c_can/c_can.h | 11 ++- >>> drivers/net/can/c_can/c_can_platform.c | 109 +++++++++++++++------ >>> 3 files changed, 95 insertions(+), 32 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt >>> index 8f1ae81..e12d1a1 100644 >>> --- a/Documentation/devicetree/bindings/net/can/c_can.txt >>> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt >>> @@ -13,6 +13,13 @@ Optional properties: >>> - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the >>> instance number >>> >>> +- ti,raminit-syscon : Handle to system control region that contains the >>> + RAMINIT register. If specified, the second memory resource >>> + in the reg property must index into the RAMINIT >>> + register within the syscon region >> >> There seems to be a simple "syscon" property these days. >> >>> +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register >>> +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register >> >> This should not be encoded in DT! This is not describing hardware setup. >> The driver should know where the bits are for the syscon phandle, >> depending on which SoC it runs... > > Is the register shared by more than one core? If so the information has > to go somewhere. Using an alias in the DT is probably the wrong approach. In case of the DRA7xx Soc, this syscon register is shared by multiple cores. (CAN, Display, etc), sigh!! For AM335x and AM43xx SoCs, the register is used only for DCAN. but shared by 2 DCAN instances. Shouldn't the information for the CAN specific bits lie in the CAN driver? cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 14:02 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-30 14:02 UTC (permalink / raw) To: Marc Kleine-Budde, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev On 09/30/2014 04:45 PM, Marc Kleine-Budde wrote: > On 09/30/2014 03:26 PM, Wolfram Sang wrote: >> On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: >>> Some TI SoCs like DRA7 have a RAMINIT register specification >>> different from the other AMxx SoCs and as expected by the >>> existing driver. >>> >>> To add more insanity, this register is shared with other >>> IPs like DSS, PCIe and PWM. >>> >>> Provides a more generic mechanism to specify the RAMINIT >>> register location and START/DONE bit position and use the >>> syscon/regmap framework to access the register. >>> >>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>> --- >>> .../devicetree/bindings/net/can/c_can.txt | 7 ++ >>> drivers/net/can/c_can/c_can.h | 11 ++- >>> drivers/net/can/c_can/c_can_platform.c | 109 +++++++++++++++------ >>> 3 files changed, 95 insertions(+), 32 deletions(-) >>> >>> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt >>> index 8f1ae81..e12d1a1 100644 >>> --- a/Documentation/devicetree/bindings/net/can/c_can.txt >>> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt >>> @@ -13,6 +13,13 @@ Optional properties: >>> - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the >>> instance number >>> >>> +- ti,raminit-syscon : Handle to system control region that contains the >>> + RAMINIT register. If specified, the second memory resource >>> + in the reg property must index into the RAMINIT >>> + register within the syscon region >> >> There seems to be a simple "syscon" property these days. >> >>> +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register >>> +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register >> >> This should not be encoded in DT! This is not describing hardware setup. >> The driver should know where the bits are for the syscon phandle, >> depending on which SoC it runs... > > Is the register shared by more than one core? If so the information has > to go somewhere. Using an alias in the DT is probably the wrong approach. In case of the DRA7xx Soc, this syscon register is shared by multiple cores. (CAN, Display, etc), sigh!! For AM335x and AM43xx SoCs, the register is used only for DCAN. but shared by 2 DCAN instances. Shouldn't the information for the CAN specific bits lie in the CAN driver? cheers, -roger ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism 2014-09-30 14:02 ` Roger Quadros @ 2014-09-30 14:11 ` Marc Kleine-Budde -1 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-30 14:11 UTC (permalink / raw) To: Roger Quadros, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar [-- Attachment #1: Type: text/plain, Size: 3096 bytes --] On 09/30/2014 04:02 PM, Roger Quadros wrote: > On 09/30/2014 04:45 PM, Marc Kleine-Budde wrote: >> On 09/30/2014 03:26 PM, Wolfram Sang wrote: >>> On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: >>>> Some TI SoCs like DRA7 have a RAMINIT register specification >>>> different from the other AMxx SoCs and as expected by the >>>> existing driver. >>>> >>>> To add more insanity, this register is shared with other >>>> IPs like DSS, PCIe and PWM. >>>> >>>> Provides a more generic mechanism to specify the RAMINIT >>>> register location and START/DONE bit position and use the >>>> syscon/regmap framework to access the register. >>>> >>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>> --- >>>> .../devicetree/bindings/net/can/c_can.txt | 7 ++ >>>> drivers/net/can/c_can/c_can.h | 11 ++- >>>> drivers/net/can/c_can/c_can_platform.c | 109 +++++++++++++++------ >>>> 3 files changed, 95 insertions(+), 32 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt >>>> index 8f1ae81..e12d1a1 100644 >>>> --- a/Documentation/devicetree/bindings/net/can/c_can.txt >>>> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt >>>> @@ -13,6 +13,13 @@ Optional properties: >>>> - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the >>>> instance number >>>> >>>> +- ti,raminit-syscon : Handle to system control region that contains the >>>> + RAMINIT register. If specified, the second memory resource >>>> + in the reg property must index into the RAMINIT >>>> + register within the syscon region >>> >>> There seems to be a simple "syscon" property these days. >>> >>>> +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register >>>> +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register >>> >>> This should not be encoded in DT! This is not describing hardware setup. >>> The driver should know where the bits are for the syscon phandle, >>> depending on which SoC it runs... >> >> Is the register shared by more than one core? If so the information has >> to go somewhere. Using an alias in the DT is probably the wrong approach. > > In case of the DRA7xx Soc, this syscon register is shared by multiple cores. (CAN, Display, etc), sigh!! > For AM335x and AM43xx SoCs, the register is used only for DCAN. but shared by 2 DCAN instances. > > Shouldn't the information for the CAN specific bits lie in the CAN driver? As there are more than one DCAN instance on the same SOC the information which DCAN instance uses which bits in the register has to go somewhere. We might add these bits as the 3rd and 4th parameter to the syscon phandle. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* Re: [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism @ 2014-09-30 14:11 ` Marc Kleine-Budde 0 siblings, 0 replies; 70+ messages in thread From: Marc Kleine-Budde @ 2014-09-30 14:11 UTC (permalink / raw) To: Roger Quadros, Wolfram Sang Cc: wg, tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev [-- Attachment #1: Type: text/plain, Size: 3096 bytes --] On 09/30/2014 04:02 PM, Roger Quadros wrote: > On 09/30/2014 04:45 PM, Marc Kleine-Budde wrote: >> On 09/30/2014 03:26 PM, Wolfram Sang wrote: >>> On Tue, Sep 09, 2014 at 05:31:09PM +0300, Roger Quadros wrote: >>>> Some TI SoCs like DRA7 have a RAMINIT register specification >>>> different from the other AMxx SoCs and as expected by the >>>> existing driver. >>>> >>>> To add more insanity, this register is shared with other >>>> IPs like DSS, PCIe and PWM. >>>> >>>> Provides a more generic mechanism to specify the RAMINIT >>>> register location and START/DONE bit position and use the >>>> syscon/regmap framework to access the register. >>>> >>>> Signed-off-by: Roger Quadros <rogerq@ti.com> >>>> --- >>>> .../devicetree/bindings/net/can/c_can.txt | 7 ++ >>>> drivers/net/can/c_can/c_can.h | 11 ++- >>>> drivers/net/can/c_can/c_can_platform.c | 109 +++++++++++++++------ >>>> 3 files changed, 95 insertions(+), 32 deletions(-) >>>> >>>> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt >>>> index 8f1ae81..e12d1a1 100644 >>>> --- a/Documentation/devicetree/bindings/net/can/c_can.txt >>>> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt >>>> @@ -13,6 +13,13 @@ Optional properties: >>>> - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the >>>> instance number >>>> >>>> +- ti,raminit-syscon : Handle to system control region that contains the >>>> + RAMINIT register. If specified, the second memory resource >>>> + in the reg property must index into the RAMINIT >>>> + register within the syscon region >>> >>> There seems to be a simple "syscon" property these days. >>> >>>> +- ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register >>>> +- ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register >>> >>> This should not be encoded in DT! This is not describing hardware setup. >>> The driver should know where the bits are for the syscon phandle, >>> depending on which SoC it runs... >> >> Is the register shared by more than one core? If so the information has >> to go somewhere. Using an alias in the DT is probably the wrong approach. > > In case of the DRA7xx Soc, this syscon register is shared by multiple cores. (CAN, Display, etc), sigh!! > For AM335x and AM43xx SoCs, the register is used only for DCAN. but shared by 2 DCAN instances. > > Shouldn't the information for the CAN specific bits lie in the CAN driver? As there are more than one DCAN instance on the same SOC the information which DCAN instance uses which bits in the register has to go somewhere. We might add these bits as the 3rd and 4th parameter to the syscon phandle. Marc -- Pengutronix e.K. | Marc Kleine-Budde | Industrial Linux Solutions | Phone: +49-231-2826-924 | Vertretung West/Dortmund | Fax: +49-5121-206917-5555 | Amtsgericht Hildesheim, HRA 2686 | http://www.pengutronix.de | [-- Attachment #2: OpenPGP digital signature --] [-- Type: application/pgp-signature, Size: 181 bytes --] ^ permalink raw reply [flat|nested] 70+ messages in thread
* [PATCH v2 3/3] net: can: c_can: Add support for START pulse in RAMINIT sequence 2014-09-09 14:31 ` Roger Quadros @ 2014-09-09 14:31 ` Roger Quadros -1 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:31 UTC (permalink / raw) To: wg, mkl; +Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar Some SoCs e.g. (TI DRA7xx) need a START pulse to start the RAMINIT sequence i.e. START bit must be set and cleared before checking for the DONE bit status. Add a new DT property "raminit-pulse" to specify if this mechanism must be used for RAMINIT. Signed-off-by: Roger Quadros <rogerq@ti.com> --- Documentation/devicetree/bindings/net/can/c_can.txt | 3 +++ drivers/net/can/c_can/c_can.h | 1 + drivers/net/can/c_can/c_can_platform.c | 8 ++++++++ 3 files changed, 12 insertions(+) diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt index e12d1a1..705411f 100644 --- a/Documentation/devicetree/bindings/net/can/c_can.txt +++ b/Documentation/devicetree/bindings/net/can/c_can.txt @@ -19,6 +19,9 @@ Optional properties: register within the syscon region - ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register - ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register +- ti,raminit-pulse : Property must exist if START pulse is needed for RAMINIT + sequence i.e. START bit will be set and cleared before + checking for DONE bit. Note: "ti,hwmods" field is used to fetch the base address and irq resources from TI, omap hwmod data base during device registration. diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h index bf68822..85b5ad0 100644 --- a/drivers/net/can/c_can/c_can.h +++ b/drivers/net/can/c_can/c_can.h @@ -175,6 +175,7 @@ struct c_can_raminit { unsigned int reg; /* register index within syscon */ u8 start_bit; /* START bit position in raminit reg. */ u8 done_bit; /* DONE bit position in raminit reg. */ + bool needs_pulse; /* If set, sets and clears START bit (pulse) */ }; /* c_can private data structure */ diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c index fb0c35b..5ae9eb3 100644 --- a/drivers/net/can/c_can/c_can_platform.c +++ b/drivers/net/can/c_can/c_can_platform.c @@ -120,6 +120,12 @@ static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable) ctrl |= 1 << raminit->start_bit; regmap_write(raminit->syscon, raminit->reg, ctrl); + /* clear START bit if start pulse is needed */ + if (raminit->needs_pulse) { + ctrl &= ~(1 << raminit->start_bit); + regmap_write(raminit->syscon, raminit->reg, ctrl); + } + ctrl |= 1 << raminit->done_bit; c_can_hw_raminit_wait_syscon(priv, mask, ctrl); } @@ -340,6 +346,8 @@ static int c_can_plat_probe(struct platform_device *pdev) } priv->raminit_sys.done_bit = val; + priv->raminit_sys.needs_pulse = of_property_read_bool(np, + "ti,raminit-pulse"); priv->raminit = c_can_hw_raminit_syscon; break; default: -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 70+ messages in thread
* [PATCH v2 3/3] net: can: c_can: Add support for START pulse in RAMINIT sequence @ 2014-09-09 14:31 ` Roger Quadros 0 siblings, 0 replies; 70+ messages in thread From: Roger Quadros @ 2014-09-09 14:31 UTC (permalink / raw) To: wg, mkl Cc: tony, tglx, mugunthanvnm, george.cherian, balbi, nsekhar, nm, sergei.shtylyov, linux-omap, linux-can, netdev, Roger Quadros Some SoCs e.g. (TI DRA7xx) need a START pulse to start the RAMINIT sequence i.e. START bit must be set and cleared before checking for the DONE bit status. Add a new DT property "raminit-pulse" to specify if this mechanism must be used for RAMINIT. Signed-off-by: Roger Quadros <rogerq@ti.com> --- Documentation/devicetree/bindings/net/can/c_can.txt | 3 +++ drivers/net/can/c_can/c_can.h | 1 + drivers/net/can/c_can/c_can_platform.c | 8 ++++++++ 3 files changed, 12 insertions(+) diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt index e12d1a1..705411f 100644 --- a/Documentation/devicetree/bindings/net/can/c_can.txt +++ b/Documentation/devicetree/bindings/net/can/c_can.txt @@ -19,6 +19,9 @@ Optional properties: register within the syscon region - ti,raminit-start-bit : Bit posistion of START bit in the RAMINIT register - ti,raminit-done-bit : Bit position of DONE bit in the RAMINIT register +- ti,raminit-pulse : Property must exist if START pulse is needed for RAMINIT + sequence i.e. START bit will be set and cleared before + checking for DONE bit. Note: "ti,hwmods" field is used to fetch the base address and irq resources from TI, omap hwmod data base during device registration. diff --git a/drivers/net/can/c_can/c_can.h b/drivers/net/can/c_can/c_can.h index bf68822..85b5ad0 100644 --- a/drivers/net/can/c_can/c_can.h +++ b/drivers/net/can/c_can/c_can.h @@ -175,6 +175,7 @@ struct c_can_raminit { unsigned int reg; /* register index within syscon */ u8 start_bit; /* START bit position in raminit reg. */ u8 done_bit; /* DONE bit position in raminit reg. */ + bool needs_pulse; /* If set, sets and clears START bit (pulse) */ }; /* c_can private data structure */ diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c index fb0c35b..5ae9eb3 100644 --- a/drivers/net/can/c_can/c_can_platform.c +++ b/drivers/net/can/c_can/c_can_platform.c @@ -120,6 +120,12 @@ static void c_can_hw_raminit_syscon(const struct c_can_priv *priv, bool enable) ctrl |= 1 << raminit->start_bit; regmap_write(raminit->syscon, raminit->reg, ctrl); + /* clear START bit if start pulse is needed */ + if (raminit->needs_pulse) { + ctrl &= ~(1 << raminit->start_bit); + regmap_write(raminit->syscon, raminit->reg, ctrl); + } + ctrl |= 1 << raminit->done_bit; c_can_hw_raminit_wait_syscon(priv, mask, ctrl); } @@ -340,6 +346,8 @@ static int c_can_plat_probe(struct platform_device *pdev) } priv->raminit_sys.done_bit = val; + priv->raminit_sys.needs_pulse = of_property_read_bool(np, + "ti,raminit-pulse"); priv->raminit = c_can_hw_raminit_syscon; break; default: -- 1.8.3.2 ^ permalink raw reply related [flat|nested] 70+ messages in thread
end of thread, other threads:[~2014-10-01 11:12 UTC | newest] Thread overview: 70+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2014-09-09 14:31 [PATCH v2 0/3] net: can: Use syscon regmap for TI specific RAMINIT register Roger Quadros 2014-09-09 14:31 ` Roger Quadros 2014-09-09 14:31 ` [PATCH v2 1/3] can: c_can_platform: Fix c_can_hw_raminit_ti() and add timeout Roger Quadros 2014-09-09 14:31 ` Roger Quadros 2014-09-09 14:34 ` Marc Kleine-Budde 2014-09-09 14:34 ` Marc Kleine-Budde 2014-09-09 14:39 ` Roger Quadros 2014-09-09 14:39 ` Roger Quadros 2014-09-16 14:13 ` Marc Kleine-Budde 2014-09-16 14:13 ` Marc Kleine-Budde 2014-09-09 14:34 ` Nishanth Menon 2014-09-09 14:34 ` Nishanth Menon 2014-09-09 14:45 ` Roger Quadros 2014-09-09 14:45 ` Roger Quadros 2014-09-09 14:51 ` Nishanth Menon 2014-09-09 14:51 ` Nishanth Menon 2014-09-09 14:54 ` Roger Quadros 2014-09-09 14:54 ` Roger Quadros 2014-09-09 14:31 ` [PATCH v2 2/3] net: can: c_can: Add syscon/regmap RAMINIT mechanism Roger Quadros 2014-09-09 14:31 ` Roger Quadros 2014-09-30 13:26 ` Wolfram Sang 2014-09-30 13:26 ` Wolfram Sang 2014-09-30 13:33 ` Roger Quadros 2014-09-30 13:33 ` Roger Quadros 2014-09-30 13:52 ` Wolfram Sang 2014-09-30 13:52 ` Wolfram Sang 2014-09-30 13:58 ` Roger Quadros 2014-09-30 13:58 ` Roger Quadros 2014-09-30 14:19 ` Wolfram Sang 2014-09-30 14:19 ` Wolfram Sang 2014-09-30 14:22 ` Marc Kleine-Budde 2014-09-30 14:22 ` Marc Kleine-Budde 2014-09-30 14:49 ` Wolfram Sang 2014-09-30 14:49 ` Wolfram Sang 2014-09-30 15:01 ` Marc Kleine-Budde 2014-09-30 15:01 ` Marc Kleine-Budde 2014-09-30 15:25 ` Wolfram Sang 2014-09-30 15:25 ` Wolfram Sang 2014-09-30 16:04 ` Marc Kleine-Budde 2014-09-30 16:04 ` Marc Kleine-Budde 2014-10-01 8:45 ` Roger Quadros 2014-10-01 8:45 ` Roger Quadros 2014-10-01 8:47 ` Marc Kleine-Budde 2014-10-01 8:47 ` Marc Kleine-Budde 2014-10-01 9:06 ` Roger Quadros 2014-10-01 9:06 ` Roger Quadros 2014-10-01 10:01 ` Marc Kleine-Budde 2014-10-01 10:01 ` Marc Kleine-Budde 2014-10-01 10:12 ` Roger Quadros 2014-10-01 10:12 ` Roger Quadros 2014-10-01 10:26 ` Marc Kleine-Budde 2014-10-01 10:26 ` Marc Kleine-Budde 2014-10-01 10:43 ` Wolfram Sang 2014-10-01 10:43 ` Wolfram Sang 2014-10-01 10:57 ` Roger Quadros 2014-10-01 10:57 ` Roger Quadros 2014-10-01 11:06 ` Marc Kleine-Budde 2014-10-01 11:06 ` Marc Kleine-Budde 2014-10-01 11:10 ` Wolfram Sang 2014-10-01 11:10 ` Wolfram Sang 2014-10-01 11:11 ` Marc Kleine-Budde 2014-10-01 11:11 ` Marc Kleine-Budde 2014-09-30 13:45 ` Marc Kleine-Budde 2014-09-30 13:45 ` Marc Kleine-Budde 2014-09-30 14:02 ` Roger Quadros 2014-09-30 14:02 ` Roger Quadros 2014-09-30 14:11 ` Marc Kleine-Budde 2014-09-30 14:11 ` Marc Kleine-Budde 2014-09-09 14:31 ` [PATCH v2 3/3] net: can: c_can: Add support for START pulse in RAMINIT sequence Roger Quadros 2014-09-09 14:31 ` Roger Quadros
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.