From mboxrd@z Thu Jan 1 00:00:00 1970 From: mkl@pengutronix.de (Marc Kleine-Budde) Date: Mon, 05 May 2014 14:21:33 +0200 Subject: [PATCH 1/2] hwinit support for non-TI devices In-Reply-To: <20140505120810.GB16461@amd.pavel.ucw.cz> References: <20140426203131.GA21832@amd.pavel.ucw.cz> <6bf951b6-51e7-4b90-b054-9ba5b6e93874@email.android.com> <20140427122510.GB12901@amd.pavel.ucw.cz> <1398716449.836.4.camel@dinh-ubuntu> <20140428211505.GA28242@amd.pavel.ucw.cz> <20140430215359.GA15148@amd.pavel.ucw.cz> <20140502084851.GA5730@amd.pavel.ucw.cz> <20140505120810.GB16461@amd.pavel.ucw.cz> Message-ID: <5367824D.2070406@pengutronix.de> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 05/05/2014 02:08 PM, Pavel Machek wrote: > Non-TI chips (including socfpga) needs different raminit > sequence. Implement it. > > Tested-by: Thor Thayer > Signed-off-by: Thor Thayer > Signed-off-by: Pavel Machek > > diff --git a/drivers/net/can/c_can/c_can_platform.c b/drivers/net/can/c_can/c_can_platform.c > index 1df0b32..476d1e0 100644 > --- a/drivers/net/can/c_can/c_can_platform.c > +++ b/drivers/net/can/c_can/c_can_platform.c > @@ -40,6 +40,7 @@ > #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); > /* > * 16-bit c_can registers can be arranged differently in the memory > @@ -80,7 +81,7 @@ static void c_can_hw_raminit_wait(const struct c_can_priv *priv, u32 mask, > udelay(1); > } > > -static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable) > +static void c_can_hw_raminit_ti(const struct c_can_priv *priv, bool enable) > { > u32 mask = CAN_RAMINIT_ALL_MASK(priv->instance); > u32 ctrl; > @@ -88,7 +89,8 @@ static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable) > spin_lock(&raminit_lock); > > ctrl = readl(priv->raminit_ctrlreg); > - /* We clear the done and start bit first. The start bit is > + /* > + * We clear the done and start bit first. The start bit is Please don't reformat comments. > * looking at the 0 -> transition, but is not self clearing; > * And we clear the init done bit as well. > */ > @@ -108,6 +110,54 @@ static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable) > spin_unlock(&raminit_lock); > } > > +static void c_can_hw_raminit(const struct c_can_priv *priv, bool enable) > +{ > + u32 ctrl; > + > + spin_lock(&raminit_lock); Why do you need this spinlock? > + > + ctrl = readl(priv->raminit_ctrlreg); > + ctrl &= ~DCAN_RAM_INIT_BIT; > + writel(ctrl, priv->raminit_ctrlreg); Why don't use use the reg directly? Have you read my previous review? > + c_can_hw_raminit_wait(priv, ctrl, 0); > + > + if (enable) { > + /* Set start bit. */ > + ctrl |= DCAN_RAM_INIT_BIT; > + writel(ctrl, priv->raminit_ctrlreg); > + c_can_hw_raminit_wait(priv, ctrl, 0); > + } > + spin_unlock(&raminit_lock); > +} > + > +static u32 c_can_plat_read_reg32(struct c_can_priv *priv, enum reg index) > +{ > + u32 val; > + > + val = priv->read_reg(priv, index); > + val |= ((u32) priv->read_reg(priv, index + 1)) << 16; > + > + return val; > +} Why are you adding the read32() support here? Your commit message doesn't mention it. Please move this into a different patch. > + > +static void c_can_plat_write_reg32(struct c_can_priv *priv, enum reg index, > + u32 val) > +{ > + priv->write_reg(priv, index + 1, val>>16); > + priv->write_reg(priv, index, val); > +} > + > +static u32 d_can_plat_read_reg32(struct c_can_priv *priv, enum reg index) > +{ > + return readl(priv->base + priv->regs[index]); > +} > + > +static void d_can_plat_write_reg32(struct c_can_priv *priv, enum reg index, > + u32 val) > +{ > + writel(val, priv->base + priv->regs[index]); > +} > + > static struct platform_device_id c_can_id_table[] = { > [BOSCH_C_CAN_PLATFORM] = { > .name = KBUILD_MODNAME, > @@ -201,11 +251,15 @@ static int c_can_plat_probe(struct platform_device *pdev) > case IORESOURCE_MEM_32BIT: > priv->read_reg = c_can_plat_read_reg_aligned_to_32bit; > priv->write_reg = c_can_plat_write_reg_aligned_to_32bit; > + priv->read_reg32 = c_can_plat_read_reg32; > + priv->write_reg32 = c_can_plat_write_reg32; > break; > case IORESOURCE_MEM_16BIT: > default: > priv->read_reg = c_can_plat_read_reg_aligned_to_16bit; > priv->write_reg = c_can_plat_write_reg_aligned_to_16bit; > + priv->read_reg32 = c_can_plat_read_reg32; > + priv->write_reg32 = c_can_plat_write_reg32; > break; > } > break; > @@ -214,6 +268,8 @@ static int c_can_plat_probe(struct platform_device *pdev) > priv->can.ctrlmode_supported |= CAN_CTRLMODE_3_SAMPLES; > priv->read_reg = c_can_plat_read_reg_aligned_to_16bit; > priv->write_reg = c_can_plat_write_reg_aligned_to_16bit; > + 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"); > @@ -221,11 +277,22 @@ static int c_can_plat_probe(struct platform_device *pdev) > priv->instance = pdev->id; > > res = platform_get_resource(pdev, IORESOURCE_MEM, 1); > + /* > + * Not all D_CAN module have a separate register for the D_CAN > + * RAM initialization. Use default RAM init bit in D_CAN module > + * if not specified in DT. > + */ > + if (!res) { > + priv->raminit = c_can_hw_raminit; > + priv->raminit_ctrlreg = addr + priv->regs[C_CAN_FUNCTION_REG]; > + break; > + } > + > priv->raminit_ctrlreg = devm_ioremap_resource(&pdev->dev, res); > if (IS_ERR(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; > + priv->raminit = c_can_hw_raminit_ti; > break; > default: > ret = -EINVAL; > 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 |