From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1753555Ab2DWM60 (ORCPT ); Mon, 23 Apr 2012 08:58:26 -0400 Received: from mga01.intel.com ([192.55.52.88]:6685 "EHLO mga01.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752193Ab2DWM6Z (ORCPT ); Mon, 23 Apr 2012 08:58:25 -0400 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="4.71,315,1320652800"; d="scan'208";a="157068759" Subject: Re: [PATCH 2/2 v3] dma/amba-pl08x: add support for the Nomadik variant From: Vinod Koul To: Linus Walleij Cc: linux-kernel@vger.kernel.org, Russell King , Alessandro Rubini , Dan Williams , Alim Akhtar , linux-arm-kernel@lists.infradead.org In-Reply-To: <1334214109-15644-1-git-send-email-linus.walleij@linaro.org> References: <1334214109-15644-1-git-send-email-linus.walleij@linaro.org> Content-Type: text/plain; charset="UTF-8" Date: Mon, 23 Apr 2012 18:24:58 +0530 Message-ID: <1335185698.31825.116.camel@vkoul-udesk3> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Thu, 2012-04-12 at 09:01 +0200, Linus Walleij wrote: > The Nomadik PL080 variant has some extra protection bits that > may be set, so we need to check these bits to see if the > channels are actually available for the DMAengine to use. > > Cc: Russell King > Cc: Alim Akhtar > Cc: Alessandro Rubini > Reviewed-by: Viresh Kumar > Signed-off-by: Linus Walleij Applied both thanks > --- > ChangeLog v1->v2: > - Fixed these review comments from Viresh Kumar > - Use kzalloc to allocate the physical channel structs so > we don't have to assign values to everything and can assume > booleans are default false > - unbreak long error message. > ChangeLog v2->v3: > - Remove an extraneous NULL assignment now that we have > kzalloc for the physical channel info. > --- > arch/arm/include/asm/hardware/pl080.h | 2 + > drivers/dma/amba-pl08x.c | 44 +++++++++++++++++++++++++++----- > include/linux/amba/pl08x.h | 3 ++ > 3 files changed, 42 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/include/asm/hardware/pl080.h b/arch/arm/include/asm/hardware/pl080.h > index 33c78d7..4eea210 100644 > --- a/arch/arm/include/asm/hardware/pl080.h > +++ b/arch/arm/include/asm/hardware/pl080.h > @@ -102,6 +102,8 @@ > #define PL080_WIDTH_16BIT (0x1) > #define PL080_WIDTH_32BIT (0x2) > > +#define PL080N_CONFIG_ITPROT (1 << 20) > +#define PL080N_CONFIG_SECPROT (1 << 19) > #define PL080_CONFIG_HALT (1 << 18) > #define PL080_CONFIG_ACTIVE (1 << 17) /* RO */ > #define PL080_CONFIG_LOCK (1 << 16) > diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c > index 08589c6..629250e 100644 > --- a/drivers/dma/amba-pl08x.c > +++ b/drivers/dma/amba-pl08x.c > @@ -95,10 +95,14 @@ static struct amba_driver pl08x_amba_driver; > * struct vendor_data - vendor-specific config parameters for PL08x derivatives > * @channels: the number of channels available in this variant > * @dualmaster: whether this version supports dual AHB masters or not. > + * @nomadik: whether the channels have Nomadik security extension bits > + * that need to be checked for permission before use and some registers are > + * missing > */ > struct vendor_data { > u8 channels; > bool dualmaster; > + bool nomadik; > }; > > /* > @@ -385,7 +389,7 @@ pl08x_get_phy_channel(struct pl08x_driver_data *pl08x, > > spin_lock_irqsave(&ch->lock, flags); > > - if (!ch->serving) { > + if (!ch->locked && !ch->serving) { > ch->serving = virt_chan; > ch->signal = -1; > spin_unlock_irqrestore(&ch->lock, flags); > @@ -1483,6 +1487,9 @@ bool pl08x_filter_id(struct dma_chan *chan, void *chan_id) > */ > static void pl08x_ensure_on(struct pl08x_driver_data *pl08x) > { > + /* The Nomadik variant does not have the config register */ > + if (pl08x->vd->nomadik) > + return; > writel(PL080_CONFIG_ENABLE, pl08x->base + PL080_CONFIG); > } > > @@ -1772,8 +1779,10 @@ static int pl08x_debugfs_show(struct seq_file *s, void *data) > spin_lock_irqsave(&ch->lock, flags); > virt_chan = ch->serving; > > - seq_printf(s, "%d\t\t%s\n", > - ch->id, virt_chan ? virt_chan->name : "(none)"); > + seq_printf(s, "%d\t\t%s%s\n", > + ch->id, > + virt_chan ? virt_chan->name : "(none)", > + ch->locked ? " LOCKED" : ""); > > spin_unlock_irqrestore(&ch->lock, flags); > } > @@ -1917,7 +1926,7 @@ static int pl08x_probe(struct amba_device *adev, const struct amba_id *id) > } > > /* Initialize physical channels */ > - pl08x->phy_chans = kmalloc((vd->channels * sizeof(*pl08x->phy_chans)), > + pl08x->phy_chans = kzalloc((vd->channels * sizeof(*pl08x->phy_chans)), > GFP_KERNEL); > if (!pl08x->phy_chans) { > dev_err(&adev->dev, "%s failed to allocate " > @@ -1932,8 +1941,23 @@ static int pl08x_probe(struct amba_device *adev, const struct amba_id *id) > ch->id = i; > ch->base = pl08x->base + PL080_Cx_BASE(i); > spin_lock_init(&ch->lock); > - ch->serving = NULL; > ch->signal = -1; > + > + /* > + * Nomadik variants can have channels that are locked > + * down for the secure world only. Lock up these channels > + * by perpetually serving a dummy virtual channel. > + */ > + if (vd->nomadik) { > + u32 val; > + > + val = readl(ch->base + PL080_CH_CONFIG); > + if (val & (PL080N_CONFIG_ITPROT | PL080N_CONFIG_SECPROT)) { > + dev_info(&adev->dev, "physical channel %d reserved for secure access only\n", i); > + ch->locked = true; > + } > + } > + > dev_dbg(&adev->dev, "physical channel %d is %s\n", > i, pl08x_phy_channel_busy(ch) ? "BUSY" : "FREE"); > } > @@ -2016,6 +2040,12 @@ static struct vendor_data vendor_pl080 = { > .dualmaster = true, > }; > > +static struct vendor_data vendor_nomadik = { > + .channels = 8, > + .dualmaster = true, > + .nomadik = true, > +}; > + > static struct vendor_data vendor_pl081 = { > .channels = 2, > .dualmaster = false, > @@ -2036,9 +2066,9 @@ static struct amba_id pl08x_ids[] = { > }, > /* Nomadik 8815 PL080 variant */ > { > - .id = 0x00280880, > + .id = 0x00280080, > .mask = 0x00ffffff, > - .data = &vendor_pl080, > + .data = &vendor_nomadik, > }, > { 0, 0 }, > }; > diff --git a/include/linux/amba/pl08x.h b/include/linux/amba/pl08x.h > index e64ce2c..0254901 100644 > --- a/include/linux/amba/pl08x.h > +++ b/include/linux/amba/pl08x.h > @@ -92,6 +92,8 @@ struct pl08x_bus_data { > * right now > * @serving: the virtual channel currently being served by this physical > * channel > + * @locked: channel unavailable for the system, e.g. dedicated to secure > + * world > */ > struct pl08x_phy_chan { > unsigned int id; > @@ -99,6 +101,7 @@ struct pl08x_phy_chan { > spinlock_t lock; > int signal; > struct pl08x_dma_chan *serving; > + bool locked; > }; > > /** -- ~Vinod From mboxrd@z Thu Jan 1 00:00:00 1970 From: vinod.koul@linux.intel.com (Vinod Koul) Date: Mon, 23 Apr 2012 18:24:58 +0530 Subject: [PATCH 2/2 v3] dma/amba-pl08x: add support for the Nomadik variant In-Reply-To: <1334214109-15644-1-git-send-email-linus.walleij@linaro.org> References: <1334214109-15644-1-git-send-email-linus.walleij@linaro.org> Message-ID: <1335185698.31825.116.camel@vkoul-udesk3> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Thu, 2012-04-12 at 09:01 +0200, Linus Walleij wrote: > The Nomadik PL080 variant has some extra protection bits that > may be set, so we need to check these bits to see if the > channels are actually available for the DMAengine to use. > > Cc: Russell King > Cc: Alim Akhtar > Cc: Alessandro Rubini > Reviewed-by: Viresh Kumar > Signed-off-by: Linus Walleij Applied both thanks > --- > ChangeLog v1->v2: > - Fixed these review comments from Viresh Kumar > - Use kzalloc to allocate the physical channel structs so > we don't have to assign values to everything and can assume > booleans are default false > - unbreak long error message. > ChangeLog v2->v3: > - Remove an extraneous NULL assignment now that we have > kzalloc for the physical channel info. > --- > arch/arm/include/asm/hardware/pl080.h | 2 + > drivers/dma/amba-pl08x.c | 44 +++++++++++++++++++++++++++----- > include/linux/amba/pl08x.h | 3 ++ > 3 files changed, 42 insertions(+), 7 deletions(-) > > diff --git a/arch/arm/include/asm/hardware/pl080.h b/arch/arm/include/asm/hardware/pl080.h > index 33c78d7..4eea210 100644 > --- a/arch/arm/include/asm/hardware/pl080.h > +++ b/arch/arm/include/asm/hardware/pl080.h > @@ -102,6 +102,8 @@ > #define PL080_WIDTH_16BIT (0x1) > #define PL080_WIDTH_32BIT (0x2) > > +#define PL080N_CONFIG_ITPROT (1 << 20) > +#define PL080N_CONFIG_SECPROT (1 << 19) > #define PL080_CONFIG_HALT (1 << 18) > #define PL080_CONFIG_ACTIVE (1 << 17) /* RO */ > #define PL080_CONFIG_LOCK (1 << 16) > diff --git a/drivers/dma/amba-pl08x.c b/drivers/dma/amba-pl08x.c > index 08589c6..629250e 100644 > --- a/drivers/dma/amba-pl08x.c > +++ b/drivers/dma/amba-pl08x.c > @@ -95,10 +95,14 @@ static struct amba_driver pl08x_amba_driver; > * struct vendor_data - vendor-specific config parameters for PL08x derivatives > * @channels: the number of channels available in this variant > * @dualmaster: whether this version supports dual AHB masters or not. > + * @nomadik: whether the channels have Nomadik security extension bits > + * that need to be checked for permission before use and some registers are > + * missing > */ > struct vendor_data { > u8 channels; > bool dualmaster; > + bool nomadik; > }; > > /* > @@ -385,7 +389,7 @@ pl08x_get_phy_channel(struct pl08x_driver_data *pl08x, > > spin_lock_irqsave(&ch->lock, flags); > > - if (!ch->serving) { > + if (!ch->locked && !ch->serving) { > ch->serving = virt_chan; > ch->signal = -1; > spin_unlock_irqrestore(&ch->lock, flags); > @@ -1483,6 +1487,9 @@ bool pl08x_filter_id(struct dma_chan *chan, void *chan_id) > */ > static void pl08x_ensure_on(struct pl08x_driver_data *pl08x) > { > + /* The Nomadik variant does not have the config register */ > + if (pl08x->vd->nomadik) > + return; > writel(PL080_CONFIG_ENABLE, pl08x->base + PL080_CONFIG); > } > > @@ -1772,8 +1779,10 @@ static int pl08x_debugfs_show(struct seq_file *s, void *data) > spin_lock_irqsave(&ch->lock, flags); > virt_chan = ch->serving; > > - seq_printf(s, "%d\t\t%s\n", > - ch->id, virt_chan ? virt_chan->name : "(none)"); > + seq_printf(s, "%d\t\t%s%s\n", > + ch->id, > + virt_chan ? virt_chan->name : "(none)", > + ch->locked ? " LOCKED" : ""); > > spin_unlock_irqrestore(&ch->lock, flags); > } > @@ -1917,7 +1926,7 @@ static int pl08x_probe(struct amba_device *adev, const struct amba_id *id) > } > > /* Initialize physical channels */ > - pl08x->phy_chans = kmalloc((vd->channels * sizeof(*pl08x->phy_chans)), > + pl08x->phy_chans = kzalloc((vd->channels * sizeof(*pl08x->phy_chans)), > GFP_KERNEL); > if (!pl08x->phy_chans) { > dev_err(&adev->dev, "%s failed to allocate " > @@ -1932,8 +1941,23 @@ static int pl08x_probe(struct amba_device *adev, const struct amba_id *id) > ch->id = i; > ch->base = pl08x->base + PL080_Cx_BASE(i); > spin_lock_init(&ch->lock); > - ch->serving = NULL; > ch->signal = -1; > + > + /* > + * Nomadik variants can have channels that are locked > + * down for the secure world only. Lock up these channels > + * by perpetually serving a dummy virtual channel. > + */ > + if (vd->nomadik) { > + u32 val; > + > + val = readl(ch->base + PL080_CH_CONFIG); > + if (val & (PL080N_CONFIG_ITPROT | PL080N_CONFIG_SECPROT)) { > + dev_info(&adev->dev, "physical channel %d reserved for secure access only\n", i); > + ch->locked = true; > + } > + } > + > dev_dbg(&adev->dev, "physical channel %d is %s\n", > i, pl08x_phy_channel_busy(ch) ? "BUSY" : "FREE"); > } > @@ -2016,6 +2040,12 @@ static struct vendor_data vendor_pl080 = { > .dualmaster = true, > }; > > +static struct vendor_data vendor_nomadik = { > + .channels = 8, > + .dualmaster = true, > + .nomadik = true, > +}; > + > static struct vendor_data vendor_pl081 = { > .channels = 2, > .dualmaster = false, > @@ -2036,9 +2066,9 @@ static struct amba_id pl08x_ids[] = { > }, > /* Nomadik 8815 PL080 variant */ > { > - .id = 0x00280880, > + .id = 0x00280080, > .mask = 0x00ffffff, > - .data = &vendor_pl080, > + .data = &vendor_nomadik, > }, > { 0, 0 }, > }; > diff --git a/include/linux/amba/pl08x.h b/include/linux/amba/pl08x.h > index e64ce2c..0254901 100644 > --- a/include/linux/amba/pl08x.h > +++ b/include/linux/amba/pl08x.h > @@ -92,6 +92,8 @@ struct pl08x_bus_data { > * right now > * @serving: the virtual channel currently being served by this physical > * channel > + * @locked: channel unavailable for the system, e.g. dedicated to secure > + * world > */ > struct pl08x_phy_chan { > unsigned int id; > @@ -99,6 +101,7 @@ struct pl08x_phy_chan { > spinlock_t lock; > int signal; > struct pl08x_dma_chan *serving; > + bool locked; > }; > > /** -- ~Vinod