| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Thu, 13 Nov 2014 15:07:44 +0200
From: Roger Quadros <rogerq@...com>
To: Marc Kleine-Budde <mkl@...gutronix.de>, <wg@...ndegger.com>
CC: <wsa@...-dreams.de>, <tony@...mide.com>, <tglx@...utronix.de>,
<mugunthanvnm@...com>, <george.cherian@...com>, <balbi@...com>,
<nsekhar@...com>, <nm@...com>,
<sergei.shtylyov@...entembedded.com>, <linux-omap@...r.kernel.org>,
<linux-can@...r.kernel.org>, <netdev@...r.kernel.org>
Subject: Re: [PATCH v4 4/8] net: can: c_can: Add syscon/regmap RAMINIT mechanism
On 11/13/2014 02:44 PM, Marc Kleine-Budde wrote:
> On 11/07/2014 03:49 PM, 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@...com>
>> ---
>> .../devicetree/bindings/net/can/c_can.txt | 3 +
>> drivers/net/can/c_can/c_can.h | 11 +-
>> drivers/net/can/c_can/c_can_platform.c | 112 ++++++++++++++-------
>> 3 files changed, 86 insertions(+), 40 deletions(-)
>>
>> diff --git a/Documentation/devicetree/bindings/net/can/c_can.txt b/Documentation/devicetree/bindings/net/can/c_can.txt
>> index 8f1ae81..a3ca3ee 100644
>> --- a/Documentation/devicetree/bindings/net/can/c_can.txt
>> +++ b/Documentation/devicetree/bindings/net/can/c_can.txt
>> @@ -12,6 +12,9 @@ Required properties:
>> Optional properties:
>> - ti,hwmods : Must be "d_can<n>" or "c_can<n>", n being the
>> instance number
>> +- syscon-raminit : Handle to system control region that contains the
>> + RAMINIT register, register offset to the RAMINIT
>> + register and the CAN instance number (0 offset).
>>
>> 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 3c305a1..0e17c7b 100644
>> --- a/drivers/net/can/c_can/c_can.h
>> +++ b/drivers/net/can/c_can/c_can.h
>> @@ -179,6 +179,14 @@ struct c_can_driver_data {
>> bool raminit_pulse; /* If set, sets and clears START bit (pulse) */
>> };
>>
>> +/* 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;
>> + u8 done_bit;
>> +};
>> +
>> /* c_can private data structure */
>> struct c_can_priv {
>> struct can_priv can; /* must be the first member */
>> @@ -196,8 +204,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 20deb67..3776483 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,47 +71,61 @@ 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;
>> + u8 start_bit, done_bit;
>> +
>> + start_bit = raminit->start_bit;
>> + done_bit = raminit->done_bit;
>>
>> spin_lock(&raminit_lock);
>>
>> - ctrl = readl(priv->raminit_ctrlreg);
>> + mask = 1 << start_bit | 1 << 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 << start_bit);
>> + ctrl |= 1 << done_bit;
>> + regmap_write(raminit->syscon, raminit->reg, ctrl);
>> +
>> + ctrl &= ~(1 << 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 << start_bit;
>> + regmap_write(raminit->syscon, raminit->reg, ctrl);
>> +
>> + ctrl |= 1 << done_bit;
>> + c_can_hw_raminit_wait_syscon(priv, mask, ctrl);
>> }
>> spin_unlock(&raminit_lock);
>> }
>> @@ -206,10 +219,11 @@ static int c_can_plat_probe(struct platform_device *pdev)
>> struct net_device *dev;
>> struct c_can_priv *priv;
>> const struct of_device_id *match;
>> - struct resource *mem, *res;
>> + struct resource *mem;
>> int irq;
>> struct clk *clk;
>> const struct c_can_driver_data *drvdata;
>> + struct device_node *np = pdev->dev.of_node;
>>
>> match = of_match_device(c_can_of_table, &pdev->dev);
>> if (match) {
>> @@ -278,27 +292,49 @@ 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
>> - * if not specified in DT.
>> + /* Check if we need custom RAMINIT via syscon. Mostly for TI
>> + * platforms. Only supported with DT boot.
>> */
>> - if (!res) {
>> + if (np && of_property_read_bool(np, "syscon-raminit")) {
>> + u32 id;
>> + struct c_can_raminit *raminit = &priv->raminit_sys;
>> +
>> + ret = -EINVAL;
>> + raminit->syscon = syscon_regmap_lookup_by_phandle(np,
>> + "syscon-raminit");
>
> You should return PTR_ERR() here, as it it might be -EPROBE_DEFER
Good catch.
>
>> + if (IS_ERR(raminit->syscon)) {
>> + dev_err(&pdev->dev,
>> + "couldn't get syscon regmap for RAMINIT\n");
>> + goto exit_free_device;
>> + }
>
> ...and maybe remove this error message completely.
OK.
cheers,
-roger
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Powered by blists - more mailing lists