* [PATCH v5 3/4] soc: zte: pm_domains: Add support for zx296718 board
From: Shawn Guo @ 2017-01-04 5:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1483489157-10782-3-git-send-email-baoyou.xie@linaro.org>
On Wed, Jan 04, 2017 at 08:19:16AM +0800, Baoyou Xie wrote:
> This patch introduces the power domain driver of zx296718
> which belongs to zte's zx2967 family.
>
> Signed-off-by: Baoyou Xie <baoyou.xie@linaro.org>
> Reviewed-by: Shawn Guo <shawnguo@kernel.org>
> Reviewed-by: Jun Nie <jun.nie@linaro.org>
Jun did give his Reviewed-by tag on v2 of this patch, but I did not.
Once again, I put quite a few comments on v3 of this patch [1]. But
neither you responded to nor address any of them in reposting.
Shawn
[1] http://www.spinics.net/lists/arm-kernel/msg547691.html
^ permalink raw reply
* [PATCH 1/2] arm64: dma_mapping: allow PCI host driver to limit DMA mask
From: Nikita Yushchenko @ 2017-01-04 6:24 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5224989.KFLmAz9Gqk@wuerfel>
> commit 9a57d58d116800a535510053136c6dd7a9c26e25
> Author: Arnd Bergmann <arnd@arndb.de>
> Date: Tue Nov 17 14:06:55 2015 +0100
>
> [EXPERIMENTAL] ARM64: check implement dma_set_mask
>
> Needs work for coherent mask
>
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Unfortunately this is far incomplete
> @@ -957,6 +983,18 @@ void arch_setup_dma_ops(struct device *dev, u64 dma_base, u64 size,
> if (!dev->archdata.dma_ops)
> dev->archdata.dma_ops = &swiotlb_dma_ops;
>
> + /*
> + * we don't yet support buses that have a non-zero mapping.
> + * Let's hope we won't need it
> + */
> + WARN_ON(dma_base != 0);
> +
> + /*
> + * Whatever the parent bus can set. A device must not set
> + * a DMA mask larger than this.
> + */
> + dev->archdata.parent_dma_mask = size;
> +
... because size/mask passed here for PCI devices are meaningless.
For OF platforms, this is called via of_dma_configure(), that checks
dma-ranges of node that is *parent* for host bridge. Host bridge
currently does not control this at all.
In current device trees no dma-ranges is defined for nodes that are
parents to pci host bridges. This will make of_dma_configure() to fall
back to 32-bit size for all devices on all current platforms. Thus
applying this patch will immediately break 64-bit dma masks on all
hardware that supports it.
Also related: dma-ranges property used by several pci host bridges is
*not* compatible with "legacy" dma-ranges parsed by of_get_dma_range() -
former uses additional flags word at beginning.
^ permalink raw reply
* [PATCH 1/2] pwm: sunxi: allow the pwm to finish its pulse before disable
From: Thierry Reding @ 2017-01-04 6:36 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170103165554.6vq7fyhtpx7olqyf@piout.net>
On Tue, Jan 03, 2017 at 05:55:54PM +0100, Alexandre Belloni wrote:
> On 03/01/2017 at 16:59:57 +0100, Olliver Schinagl wrote :
> > On 12-12-16 13:24, Maxime Ripard wrote:
> > > On Thu, Dec 08, 2016 at 02:23:39PM +0100, Olliver Schinagl wrote:
> > > > Hey Maxime,
> > > >
> > > > first off, also sorry for the slow delay :) (pun not intended)
> > > >
> > > > On 27-08-16 00:19, Maxime Ripard wrote:
> > > > > On Thu, Aug 25, 2016 at 07:50:10PM +0200, Olliver Schinagl wrote:
> > > > > > When we inform the PWM block to stop toggeling the output, we may end up
> > > > > > in a state where the output is not what we would expect (e.g. not the
> > > > > > low-pulse) but whatever the output was at when the clock got disabled.
> > > > > >
> > > > > > To counter this we have to wait for maximally the time of one whole
> > > > > > period to ensure the pwm hardware was able to finish. Since we already
> > > > > > told the PWM hardware to disable it self, it will not continue toggling
> > > > > > but merly finish its current pulse.
> > > > > >
> > > > > > If a whole period is considered to much, it may be contemplated to use a
> > > > > > half period + a little bit to ensure we get passed the transition.
> > > > > >
> > > > > > Signed-off-by: Olliver Schinagl<oliver@schinagl.nl>
> > > > > > ---
> > > > > > drivers/pwm/pwm-sun4i.c | 11 +++++++++++
> > > > > > 1 file changed, 11 insertions(+)
> > > > > >
> > > > > > diff --git a/drivers/pwm/pwm-sun4i.c b/drivers/pwm/pwm-sun4i.c
> > > > > > index 03a99a5..5e97c8a 100644
> > > > > > --- a/drivers/pwm/pwm-sun4i.c
> > > > > > +++ b/drivers/pwm/pwm-sun4i.c
> > > > > > @@ -8,6 +8,7 @@
> > > > > > #include <linux/bitops.h>
> > > > > > #include <linux/clk.h>
> > > > > > +#include <linux/delay.h>
> > > > > > #include <linux/err.h>
> > > > > > #include <linux/io.h>
> > > > > > #include <linux/module.h>
> > > > > > @@ -245,6 +246,16 @@ static void sun4i_pwm_disable(struct pwm_chip *chip, struct pwm_device *pwm)
> > > > > > spin_lock(&sun4i_pwm->ctrl_lock);
> > > > > > val = sun4i_pwm_readl(sun4i_pwm, PWM_CTRL_REG);
> > > > > > val &= ~BIT_CH(PWM_EN, pwm->hwpwm);
> > > > > > + sun4i_pwm_writel(sun4i_pwm, val, PWM_CTRL_REG);
> > > > > > + spin_unlock(&sun4i_pwm->ctrl_lock);
> > > > > > +
> > > > > > + /* Allow for the PWM hardware to finish its last toggle. The pulse
> > > > > > + * may have just started and thus we should wait a full period.
> > > > > > + */
> > > > > > + ndelay(pwm_get_period(pwm));
> > > > > Can't that use the ready bit as well?
> > > >
> > > > I started to implement our earlier discussed suggestions, but I do not think
> > > > they will work. The read bit is not to let the user know it is ready with
> > > > all of its commands, but only if the period registers are ready. I think it
> > > > is some write lock while it copies the data into its internal control loop.
> > > > From the manual:
> > > > PWM0 period register ready.
> > > > 0: PWM0 period register is ready to write,
> > > > 1: PWM0 period register is busy.
> > > >
> > > >
> > > > So no, I don't think i can use the ready bit here at all. The only thing we
> > > > can do here, but I doubt it's worth it, is to read the period register,
> > > > caluclate a time from it, and then ndelay(pwm_get_period(pwm) - ran_time)
> > > >
> > > > The only 'win' then is that we could are potentially not waiting the full
> > > > pwm period, but only a fraction of it. Since we are disabling the hardware
> > > > (for power reasons) anyway, I don't think this is any significant win,
> > > > except for extreme situations. E.g. we have a pwm period of 10 seconds, we
> > > > disable it after 9.9 second, and now we have to wait for 10 seconds before
> > > > the pwm_disable is finally done. So this could in that case be reduced to
> > > > then only wait for 0.2 seconds since it is 'done' sooner.
> > > >
> > > > However that optimization is also not 'free'. We have to read the period
> > > > register and calculate back the time. I suggest to do that when reworking
> > > > this driver to work with atomic mode, and merge this patch 'as is' to
> > > > atleast fix te bug where simply not finish properly.
> > >
> > > That whole discussion made me realise something that is really
> > > bad. AFAIK, pwm_get_period returns a 32 bits register, which means a
> > > theorical period of 4s. Busy looping during 4 seconds is already very
> > > bad, as you basically kill one CPU during that time, but doing so in a
> > > (potentially) atomic context is even worse.
> > Well technically, isn't it a 16 bit register? (half for the period, other
> > half for the duty cycle?) Anyway, I think the delay can be far exceeding 4
> > seconds (though I haven't checked what the PWM delay max option is).
> >
>
> That's 196.8s.
>
> But you can rely on the RDY bit because what you want to
> achieve actually relies on the fact that the duty cycle will be set to 0
> before disabling the channel.
>
>
> > Anyway, you are right, we should absolutely not do this!
> >
> > >
> > > NACK.
> > Absolutely! But what do you suggest? Would usleep (or msleep) instead of the
> > ndelay work properly?
> >
>
> We can probably set up a timer and disable the channel when it expires.
> Obviously it will be necessary to cancel the timer if the channel is
> reenabled in the mean time.
>
> Or maybe we can make all the functions blocking and forget about
> synchronizing both channels.
Yes, I think blocking is fine. All drivers report "might sleep" since
v4.5 anyway, so by now all users must have gained support for blocking
drivers.
I'm also not aware of any users that couldn't cope with blocking PWM
devices. The only case that I'm aware of is the LED class that uses it
for blink support and it needs to support the blocking case anyway for
devices that sit on a slow bus, for example.
I'll post a patch to remove pwm_can_sleep() altogether, it's long been
redundant. Because of that I also think it's safe for every driver to
block during any of the operations, which is true both for legacy ones
as well as atomic ones.
Thierry
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170104/61650aae/attachment-0001.sig>
^ permalink raw reply
* [PATCH v3 0/3] dmaengine: xilinx_dma: Bug fixes
From: Kedareswara rao Appana @ 2017-01-04 6:54 UTC (permalink / raw)
To: linux-arm-kernel
This patch series fixes below bugs in DMA and VDMA IP's
---> Do not start VDMA until frame buffer is processed by the h/w Fix
---> bug in Multi frame sotres handling in VDMA Fix issues w.r.to multi
---> frame descriptors submit with AXI DMA S2MM(recv) Side.
Kedareswara rao Appana (3):
dmaengine: xilinx_dma: Check for channel idle state before submitting
dma descriptor
dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in
vdma
dmaengine: xilinx_dma: Fix race condition in the driver for multiple
descriptor scenario
.../devicetree/bindings/dma/xilinx/xilinx_dma.txt | 2 +
drivers/dma/xilinx/xilinx_dma.c | 265 ++++++++++++---------
2 files changed, 156 insertions(+), 111 deletions(-)
--
2.1.2
^ permalink raw reply
* [PATCH v3 1/3] dmaengine: xilinx_dma: Check for channel idle state before submitting dma descriptor
From: Kedareswara rao Appana @ 2017-01-04 6:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1483512847-25710-1-git-send-email-appanad@xilinx.com>
Add channel idle state to ensure that dma descriptor is not
submitted when VDMA engine is in progress.
Reviewed-by: Jose Abreu <joabreu@synopsys.com>
Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v3:
---> None.
Changes for v2:
---> Add idle check in the reset as suggested by Jose Abreu
---> Removed xilinx_dma_is_running/xilinx_dma_is_idle checks
in the driver and used common idle checks across the driver
as suggested by Laurent Pinchart.
drivers/dma/xilinx/xilinx_dma.c | 56 +++++++++++++----------------------------
1 file changed, 17 insertions(+), 39 deletions(-)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 8288fe4..be7eb41 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -321,6 +321,7 @@ struct xilinx_dma_tx_descriptor {
* @cyclic: Check for cyclic transfers.
* @genlock: Support genlock mode
* @err: Channel has errors
+ * @idle: Check for channel idle
* @tasklet: Cleanup work after irq
* @config: Device configuration info
* @flush_on_fsync: Flush on Frame sync
@@ -351,6 +352,7 @@ struct xilinx_dma_chan {
bool cyclic;
bool genlock;
bool err;
+ bool idle;
struct tasklet_struct tasklet;
struct xilinx_vdma_config config;
bool flush_on_fsync;
@@ -920,32 +922,6 @@ static enum dma_status xilinx_dma_tx_status(struct dma_chan *dchan,
}
/**
- * xilinx_dma_is_running - Check if DMA channel is running
- * @chan: Driver specific DMA channel
- *
- * Return: '1' if running, '0' if not.
- */
-static bool xilinx_dma_is_running(struct xilinx_dma_chan *chan)
-{
- return !(dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
- XILINX_DMA_DMASR_HALTED) &&
- (dma_ctrl_read(chan, XILINX_DMA_REG_DMACR) &
- XILINX_DMA_DMACR_RUNSTOP);
-}
-
-/**
- * xilinx_dma_is_idle - Check if DMA channel is idle
- * @chan: Driver specific DMA channel
- *
- * Return: '1' if idle, '0' if not.
- */
-static bool xilinx_dma_is_idle(struct xilinx_dma_chan *chan)
-{
- return dma_ctrl_read(chan, XILINX_DMA_REG_DMASR) &
- XILINX_DMA_DMASR_IDLE;
-}
-
-/**
* xilinx_dma_halt - Halt DMA channel
* @chan: Driver specific DMA channel
*/
@@ -966,6 +942,7 @@ static void xilinx_dma_halt(struct xilinx_dma_chan *chan)
chan, dma_ctrl_read(chan, XILINX_DMA_REG_DMASR));
chan->err = true;
}
+ chan->idle = true;
}
/**
@@ -1007,6 +984,9 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
if (chan->err)
return;
+ if (!chan->idle)
+ return;
+
if (list_empty(&chan->pending_list))
return;
@@ -1018,13 +998,6 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
tail_segment = list_last_entry(&tail_desc->segments,
struct xilinx_vdma_tx_segment, node);
- /* If it is SG mode and hardware is busy, cannot submit */
- if (chan->has_sg && xilinx_dma_is_running(chan) &&
- !xilinx_dma_is_idle(chan)) {
- dev_dbg(chan->dev, "DMA controller still busy\n");
- return;
- }
-
/*
* If hardware is idle, then all descriptors on the running lists are
* done, start new transfers
@@ -1110,6 +1083,7 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, last->hw.vsize);
}
+ chan->idle = false;
if (!chan->has_sg) {
list_del(&desc->node);
list_add_tail(&desc->node, &chan->active_list);
@@ -1136,6 +1110,9 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
if (chan->err)
return;
+ if (!chan->idle)
+ return;
+
if (list_empty(&chan->pending_list))
return;
@@ -1181,6 +1158,7 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
list_splice_tail_init(&chan->pending_list, &chan->active_list);
chan->desc_pendingcount = 0;
+ chan->idle = false;
}
/**
@@ -1196,15 +1174,11 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
if (chan->err)
return;
- if (list_empty(&chan->pending_list))
+ if (!chan->idle)
return;
- /* If it is SG mode and hardware is busy, cannot submit */
- if (chan->has_sg && xilinx_dma_is_running(chan) &&
- !xilinx_dma_is_idle(chan)) {
- dev_dbg(chan->dev, "DMA controller still busy\n");
+ if (list_empty(&chan->pending_list))
return;
- }
head_desc = list_first_entry(&chan->pending_list,
struct xilinx_dma_tx_descriptor, node);
@@ -1302,6 +1276,7 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
list_splice_tail_init(&chan->pending_list, &chan->active_list);
chan->desc_pendingcount = 0;
+ chan->idle = false;
}
/**
@@ -1366,6 +1341,7 @@ static int xilinx_dma_reset(struct xilinx_dma_chan *chan)
}
chan->err = false;
+ chan->idle = true;
return err;
}
@@ -1447,6 +1423,7 @@ static irqreturn_t xilinx_dma_irq_handler(int irq, void *data)
if (status & XILINX_DMA_DMASR_FRM_CNT_IRQ) {
spin_lock(&chan->lock);
xilinx_dma_complete_descriptor(chan);
+ chan->idle = true;
chan->start_transfer(chan);
spin_unlock(&chan->lock);
}
@@ -2327,6 +2304,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
chan->has_sg = xdev->has_sg;
chan->desc_pendingcount = 0x0;
chan->ext_addr = xdev->ext_addr;
+ chan->idle = true;
spin_lock_init(&chan->lock);
INIT_LIST_HEAD(&chan->pending_list);
--
2.1.2
^ permalink raw reply related
* [PATCH v3 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma
From: Kedareswara rao Appana @ 2017-01-04 6:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1483512847-25710-1-git-send-email-appanad@xilinx.com>
When VDMA is configured for more than one frame in the h/w
for example h/w is configured for n number of frames and user
Submits n number of frames and triggered the DMA using issue_pending API.
In the current driver flow we are submitting one frame at a time
but we should submit all the n number of frames at one time as the h/w
Is configured for n number of frames.
This patch fixes this issue.
Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v3:
---> Added Checks for frame store configuration. If frame store
Configuration is not present at the h/w level and user
Submits less frames added debug prints in the driver as relevant.
Changes for v2:
---> Fixed race conditions in the driver as suggested by Jose Abreu
---> Fixed unnecessray if else checks in the vdma_start_transfer
as suggested by Laurent Pinchart.
.../devicetree/bindings/dma/xilinx/xilinx_dma.txt | 2 +
drivers/dma/xilinx/xilinx_dma.c | 78 +++++++++++++++-------
2 files changed, 57 insertions(+), 23 deletions(-)
diff --git a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
index a2b8bfa..1f65e09 100644
--- a/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
+++ b/Documentation/devicetree/bindings/dma/xilinx/xilinx_dma.txt
@@ -66,6 +66,8 @@ Optional child node properties:
Optional child node properties for VDMA:
- xlnx,genlock-mode: Tells Genlock synchronization is
enabled/disabled in hardware.
+- xlnx,fstore-config: Tells Whether Frame Store Configuration is
+ enabled/disabled in hardware.
Optional child node properties for AXI DMA:
-dma-channels: Number of dma channels in child node.
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index be7eb41..7cd8e08 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -322,6 +322,7 @@ struct xilinx_dma_tx_descriptor {
* @genlock: Support genlock mode
* @err: Channel has errors
* @idle: Check for channel idle
+ * @has_fstoreconfig: Check for frame store configuration
* @tasklet: Cleanup work after irq
* @config: Device configuration info
* @flush_on_fsync: Flush on Frame sync
@@ -353,6 +354,7 @@ struct xilinx_dma_chan {
bool genlock;
bool err;
bool idle;
+ bool has_fstoreconfig;
struct tasklet_struct tasklet;
struct xilinx_vdma_config config;
bool flush_on_fsync;
@@ -990,6 +992,26 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
if (list_empty(&chan->pending_list))
return;
+ /*
+ * Note: When VDMA is built with default h/w configuration
+ * On the S2MM(recv) side user should submit frames upto
+ * H/W configured. If users submits less than h/w configured
+ * VDMA engine tries to write to a invalid location
+ * Results undefined behaviour/memory corruption.
+ *
+ * If user would like to submit frames less than h/w capable
+ * On S2MM side please enable debug info 13 at the h/w level
+ * It will allows the frame buffers numbers to be modified at runtime.
+ */
+ if (!chan->has_fstoreconfig && chan->direction == DMA_DEV_TO_MEM &&
+ chan->desc_pendingcount < chan->num_frms) {
+ dev_dbg(chan->dev, "Frame Store Configuration is not enabled at the");
+ dev_dbg(chan->dev, " H/w level enable Debug info 13 at the h/w level");
+ dev_dbg(chan->dev, " OR Submit the frames upto h/w Capable\n\r");
+
+ return;
+ }
+
desc = list_first_entry(&chan->pending_list,
struct xilinx_dma_tx_descriptor, node);
tail_desc = list_last_entry(&chan->pending_list,
@@ -1052,25 +1074,38 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
if (chan->has_sg) {
dma_ctrl_write(chan, XILINX_DMA_REG_TAILDESC,
tail_segment->phys);
+ list_splice_tail_init(&chan->pending_list, &chan->active_list);
+ chan->desc_pendingcount = 0;
} else {
struct xilinx_vdma_tx_segment *segment, *last = NULL;
- int i = 0;
+ int i = 0, j = 0;
if (chan->desc_submitcount < chan->num_frms)
i = chan->desc_submitcount;
- list_for_each_entry(segment, &desc->segments, node) {
- if (chan->ext_addr)
- vdma_desc_write_64(chan,
- XILINX_VDMA_REG_START_ADDRESS_64(i++),
- segment->hw.buf_addr,
- segment->hw.buf_addr_msb);
- else
- vdma_desc_write(chan,
- XILINX_VDMA_REG_START_ADDRESS(i++),
- segment->hw.buf_addr);
-
- last = segment;
+ for (j = 0; j < chan->num_frms; ) {
+ list_for_each_entry(segment, &desc->segments, node) {
+ if (chan->ext_addr)
+ vdma_desc_write_64(chan,
+ XILINX_VDMA_REG_START_ADDRESS_64(i++),
+ segment->hw.buf_addr,
+ segment->hw.buf_addr_msb);
+ else
+ vdma_desc_write(chan,
+ XILINX_VDMA_REG_START_ADDRESS(i++),
+ segment->hw.buf_addr);
+
+ last = segment;
+ }
+ list_del(&desc->node);
+ list_add_tail(&desc->node, &chan->active_list);
+ j++;
+ if (list_empty(&chan->pending_list) ||
+ (i == chan->num_frms))
+ break;
+ desc = list_first_entry(&chan->pending_list,
+ struct xilinx_dma_tx_descriptor,
+ node);
}
if (!last)
@@ -1081,20 +1116,14 @@ static void xilinx_vdma_start_transfer(struct xilinx_dma_chan *chan)
vdma_desc_write(chan, XILINX_DMA_REG_FRMDLY_STRIDE,
last->hw.stride);
vdma_desc_write(chan, XILINX_DMA_REG_VSIZE, last->hw.vsize);
- }
- chan->idle = false;
- if (!chan->has_sg) {
- list_del(&desc->node);
- list_add_tail(&desc->node, &chan->active_list);
- chan->desc_submitcount++;
- chan->desc_pendingcount--;
+ chan->desc_submitcount += j;
+ chan->desc_pendingcount -= j;
if (chan->desc_submitcount == chan->num_frms)
chan->desc_submitcount = 0;
- } else {
- list_splice_tail_init(&chan->pending_list, &chan->active_list);
- chan->desc_pendingcount = 0;
}
+
+ chan->idle = false;
}
/**
@@ -1342,6 +1371,7 @@ static int xilinx_dma_reset(struct xilinx_dma_chan *chan)
chan->err = false;
chan->idle = true;
+ chan->desc_submitcount = 0;
return err;
}
@@ -2315,6 +2345,8 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
has_dre = of_property_read_bool(node, "xlnx,include-dre");
chan->genlock = of_property_read_bool(node, "xlnx,genlock-mode");
+ chan->has_fstoreconfig = of_property_read_bool(node,
+ "xlnx,fstore-config");
err = of_property_read_u32(node, "xlnx,datawidth", &value);
if (err) {
--
2.1.2
^ permalink raw reply related
* [PATCH v3 3/3] dmaengine: xilinx_dma: Fix race condition in the driver for multiple descriptor scenario
From: Kedareswara rao Appana @ 2017-01-04 6:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1483512847-25710-1-git-send-email-appanad@xilinx.com>
When driver is handling AXI DMA SoftIP
When user submits multiple descriptors back to back on the S2MM(recv)
side with the current driver flow the last buffer descriptor next bd
points to a invalid location resulting the invalid data or errors in the
DMA engine.
This patch fixes this issue by creating a BD Chain during
channel allocation itself and use those BD's.
Signed-off-by: Kedareswara rao Appana <appanad@xilinx.com>
---
Changes for v3:
---> None.
Changes for v2:
---> None.
drivers/dma/xilinx/xilinx_dma.c | 133 +++++++++++++++++++++++++---------------
1 file changed, 83 insertions(+), 50 deletions(-)
diff --git a/drivers/dma/xilinx/xilinx_dma.c b/drivers/dma/xilinx/xilinx_dma.c
index 7cd8e08..822ccf00 100644
--- a/drivers/dma/xilinx/xilinx_dma.c
+++ b/drivers/dma/xilinx/xilinx_dma.c
@@ -163,6 +163,7 @@
#define XILINX_DMA_BD_SOP BIT(27)
#define XILINX_DMA_BD_EOP BIT(26)
#define XILINX_DMA_COALESCE_MAX 255
+#define XILINX_DMA_NUM_DESCS 255
#define XILINX_DMA_NUM_APP_WORDS 5
/* Multi-Channel DMA Descriptor offsets*/
@@ -310,6 +311,7 @@ struct xilinx_dma_tx_descriptor {
* @pending_list: Descriptors waiting
* @active_list: Descriptors ready to submit
* @done_list: Complete descriptors
+ * @free_seg_list: Free descriptors
* @common: DMA common channel
* @desc_pool: Descriptors pool
* @dev: The dma device
@@ -331,7 +333,9 @@ struct xilinx_dma_tx_descriptor {
* @desc_submitcount: Descriptor h/w submitted count
* @residue: Residue for AXI DMA
* @seg_v: Statically allocated segments base
+ * @seg_p: Physical allocated segments base
* @cyclic_seg_v: Statically allocated segment base for cyclic transfers
+ * @cyclic_seg_p: Physical allocated segments base for cyclic dma
* @start_transfer: Differentiate b/w DMA IP's transfer
*/
struct xilinx_dma_chan {
@@ -342,6 +346,7 @@ struct xilinx_dma_chan {
struct list_head pending_list;
struct list_head active_list;
struct list_head done_list;
+ struct list_head free_seg_list;
struct dma_chan common;
struct dma_pool *desc_pool;
struct device *dev;
@@ -363,7 +368,9 @@ struct xilinx_dma_chan {
u32 desc_submitcount;
u32 residue;
struct xilinx_axidma_tx_segment *seg_v;
+ dma_addr_t seg_p;
struct xilinx_axidma_tx_segment *cyclic_seg_v;
+ dma_addr_t cyclic_seg_p;
void (*start_transfer)(struct xilinx_dma_chan *chan);
u16 tdest;
};
@@ -569,17 +576,31 @@ static struct xilinx_axidma_tx_segment *
xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan)
{
struct xilinx_axidma_tx_segment *segment;
- dma_addr_t phys;
-
- segment = dma_pool_zalloc(chan->desc_pool, GFP_ATOMIC, &phys);
- if (!segment)
- return NULL;
+ unsigned long flags;
- segment->phys = phys;
+ spin_lock_irqsave(&chan->lock, flags);
+ if (!list_empty(&chan->free_seg_list)) {
+ segment = list_first_entry(&chan->free_seg_list,
+ struct xilinx_axidma_tx_segment,
+ node);
+ list_del(&segment->node);
+ }
+ spin_unlock_irqrestore(&chan->lock, flags);
return segment;
}
+static void xilinx_dma_clean_hw_desc(struct xilinx_axidma_desc_hw *hw)
+{
+ u32 next_desc = hw->next_desc;
+ u32 next_desc_msb = hw->next_desc_msb;
+
+ memset(hw, 0, sizeof(struct xilinx_axidma_desc_hw));
+
+ hw->next_desc = next_desc;
+ hw->next_desc_msb = next_desc_msb;
+}
+
/**
* xilinx_dma_free_tx_segment - Free transaction segment
* @chan: Driver specific DMA channel
@@ -588,7 +609,9 @@ xilinx_axidma_alloc_tx_segment(struct xilinx_dma_chan *chan)
static void xilinx_dma_free_tx_segment(struct xilinx_dma_chan *chan,
struct xilinx_axidma_tx_segment *segment)
{
- dma_pool_free(chan->desc_pool, segment, segment->phys);
+ xilinx_dma_clean_hw_desc(&segment->hw);
+
+ list_add_tail(&segment->node, &chan->free_seg_list);
}
/**
@@ -713,16 +736,26 @@ static void xilinx_dma_free_descriptors(struct xilinx_dma_chan *chan)
static void xilinx_dma_free_chan_resources(struct dma_chan *dchan)
{
struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
+ unsigned long flags;
dev_dbg(chan->dev, "Free all channel resources.\n");
xilinx_dma_free_descriptors(chan);
+
if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
- xilinx_dma_free_tx_segment(chan, chan->cyclic_seg_v);
- xilinx_dma_free_tx_segment(chan, chan->seg_v);
+ spin_lock_irqsave(&chan->lock, flags);
+ INIT_LIST_HEAD(&chan->free_seg_list);
+ spin_unlock_irqrestore(&chan->lock, flags);
+
+ /* Free Memory that is allocated for cyclic DMA Mode */
+ dma_free_coherent(chan->dev, sizeof(*chan->cyclic_seg_v),
+ chan->cyclic_seg_v, chan->cyclic_seg_p);
+ }
+
+ if (chan->xdev->dma_config->dmatype != XDMA_TYPE_AXIDMA) {
+ dma_pool_destroy(chan->desc_pool);
+ chan->desc_pool = NULL;
}
- dma_pool_destroy(chan->desc_pool);
- chan->desc_pool = NULL;
}
/**
@@ -805,6 +838,7 @@ static void xilinx_dma_do_tasklet(unsigned long data)
static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
{
struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
+ int i;
/* Has this channel already been allocated? */
if (chan->desc_pool)
@@ -815,11 +849,30 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
* for meeting Xilinx VDMA specification requirement.
*/
if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
- chan->desc_pool = dma_pool_create("xilinx_dma_desc_pool",
- chan->dev,
- sizeof(struct xilinx_axidma_tx_segment),
- __alignof__(struct xilinx_axidma_tx_segment),
- 0);
+ /* Allocate the buffer descriptors. */
+ chan->seg_v = dma_zalloc_coherent(chan->dev,
+ sizeof(*chan->seg_v) *
+ XILINX_DMA_NUM_DESCS,
+ &chan->seg_p, GFP_KERNEL);
+ if (!chan->seg_v) {
+ dev_err(chan->dev,
+ "unable to allocate channel %d descriptors\n",
+ chan->id);
+ return -ENOMEM;
+ }
+
+ for (i = 0; i < XILINX_DMA_NUM_DESCS; i++) {
+ chan->seg_v[i].hw.next_desc =
+ lower_32_bits(chan->seg_p + sizeof(*chan->seg_v) *
+ ((i + 1) % XILINX_DMA_NUM_DESCS));
+ chan->seg_v[i].hw.next_desc_msb =
+ upper_32_bits(chan->seg_p + sizeof(*chan->seg_v) *
+ ((i + 1) % XILINX_DMA_NUM_DESCS));
+ chan->seg_v[i].phys = chan->seg_p +
+ sizeof(*chan->seg_v) * i;
+ list_add_tail(&chan->seg_v[i].node,
+ &chan->free_seg_list);
+ }
} else if (chan->xdev->dma_config->dmatype == XDMA_TYPE_CDMA) {
chan->desc_pool = dma_pool_create("xilinx_cdma_desc_pool",
chan->dev,
@@ -834,7 +887,8 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
0);
}
- if (!chan->desc_pool) {
+ if (!chan->desc_pool &&
+ (chan->xdev->dma_config->dmatype != XDMA_TYPE_AXIDMA)) {
dev_err(chan->dev,
"unable to allocate channel %d descriptor pool\n",
chan->id);
@@ -843,22 +897,20 @@ static int xilinx_dma_alloc_chan_resources(struct dma_chan *dchan)
if (chan->xdev->dma_config->dmatype == XDMA_TYPE_AXIDMA) {
/*
- * For AXI DMA case after submitting a pending_list, keep
- * an extra segment allocated so that the "next descriptor"
- * pointer on the tail descriptor always points to a
- * valid descriptor, even when paused after reaching taildesc.
- * This way, it is possible to issue additional
- * transfers without halting and restarting the channel.
- */
- chan->seg_v = xilinx_axidma_alloc_tx_segment(chan);
-
- /*
* For cyclic DMA mode we need to program the tail Descriptor
* register with a value which is not a part of the BD chain
* so allocating a desc segment during channel allocation for
* programming tail descriptor.
*/
- chan->cyclic_seg_v = xilinx_axidma_alloc_tx_segment(chan);
+ chan->cyclic_seg_v = dma_zalloc_coherent(chan->dev,
+ sizeof(*chan->cyclic_seg_v),
+ &chan->cyclic_seg_p, GFP_KERNEL);
+ if (!chan->cyclic_seg_v) {
+ dev_err(chan->dev,
+ "unable to allocate desc segment for cyclic DMA\n");
+ return -ENOMEM;
+ }
+ chan->cyclic_seg_v->phys = chan->cyclic_seg_p;
}
dma_cookie_init(dchan);
@@ -1197,7 +1249,7 @@ static void xilinx_cdma_start_transfer(struct xilinx_dma_chan *chan)
static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
{
struct xilinx_dma_tx_descriptor *head_desc, *tail_desc;
- struct xilinx_axidma_tx_segment *tail_segment, *old_head, *new_head;
+ struct xilinx_axidma_tx_segment *tail_segment;
u32 reg;
if (chan->err)
@@ -1216,21 +1268,6 @@ static void xilinx_dma_start_transfer(struct xilinx_dma_chan *chan)
tail_segment = list_last_entry(&tail_desc->segments,
struct xilinx_axidma_tx_segment, node);
- if (chan->has_sg && !chan->xdev->mcdma) {
- old_head = list_first_entry(&head_desc->segments,
- struct xilinx_axidma_tx_segment, node);
- new_head = chan->seg_v;
- /* Copy Buffer Descriptor fields. */
- new_head->hw = old_head->hw;
-
- /* Swap and save new reserve */
- list_replace_init(&old_head->node, &new_head->node);
- chan->seg_v = old_head;
-
- tail_segment->hw.next_desc = chan->seg_v->phys;
- head_desc->async_tx.phys = new_head->phys;
- }
-
reg = dma_ctrl_read(chan, XILINX_DMA_REG_DMACR);
if (chan->desc_pendingcount <= XILINX_DMA_COALESCE_MAX) {
@@ -1728,7 +1765,7 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
{
struct xilinx_dma_chan *chan = to_xilinx_chan(dchan);
struct xilinx_dma_tx_descriptor *desc;
- struct xilinx_axidma_tx_segment *segment = NULL, *prev = NULL;
+ struct xilinx_axidma_tx_segment *segment = NULL;
u32 *app_w = (u32 *)context;
struct scatterlist *sg;
size_t copy;
@@ -1779,10 +1816,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
XILINX_DMA_NUM_APP_WORDS);
}
- if (prev)
- prev->hw.next_desc = segment->phys;
-
- prev = segment;
sg_used += copy;
/*
@@ -1796,7 +1829,6 @@ static struct dma_async_tx_descriptor *xilinx_dma_prep_slave_sg(
segment = list_first_entry(&desc->segments,
struct xilinx_axidma_tx_segment, node);
desc->async_tx.phys = segment->phys;
- prev->hw.next_desc = segment->phys;
/* For the last DMA_MEM_TO_DEV transfer, set EOP */
if (chan->direction == DMA_MEM_TO_DEV) {
@@ -2340,6 +2372,7 @@ static int xilinx_dma_chan_probe(struct xilinx_dma_device *xdev,
INIT_LIST_HEAD(&chan->pending_list);
INIT_LIST_HEAD(&chan->done_list);
INIT_LIST_HEAD(&chan->active_list);
+ INIT_LIST_HEAD(&chan->free_seg_list);
/* Retrieve the channel properties from the device tree */
has_dre = of_property_read_bool(node, "xlnx,include-dre");
--
2.1.2
^ permalink raw reply related
* [PATCH v2 2/3] dmaeninge: xilinx_dma: Fix bug in multiple frame stores scenario in vdma
From: Appana Durga Kedareswara Rao @ 2017-01-04 6:57 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <a8f5c216-cac0-daf9-a880-3820ae9b5e13@synopsys.com>
Hi Jose Miguel Abreu,
Thanks for the review...
> >> If so then there is no race condition, but the HW image that I have
> >> does not have this register enabled so I was getting this result
> >> (memory corruption because not all framebuffers had addresses set).
> > Thanks for the explanation.
> > Agree the issue that you mentioned won't come when
> > XILINX_DMA_REG_FRMSTORE
> > (C_ENABLE_DEBUG_INFO_5 and C_ENABLE_DEBUG_INFO_13) Register is
> enabled in the IP.
> > But this register won't get enabled with the default IP configuration
> (C_ENABLE_DEBUG_INFO_5 and C_ENABLE_DEBUG_INFO_13).
> >
> > When user is not enabled XILINX_DMA_REG_FRMSTORE in the h/w and
> submits frames less than h/w capable.
> > The solution that I am thinking is to throw an error in the driver
> > saying that either enable the num frame store feature in the IP or submit the
> frames up to h/w capable what do you think???
>
> Sounds fine by me.
Thanks posted the v3 series please review when you have some time...
Regards,
Kedar.
>
> Best regards,
> Jose Miguel Abreu
>
> >
> > Regards,
> > Kedar.
> >
> >> Best regards,
> >> Jose Miguel Abreu
> >>
> >>> Regards,
> >>> Kedar.
> >>>
> >>>> Best regards,
> >>>> Jose Miguel Abreu
> >>>>
^ permalink raw reply
* [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI
From: Hanjun Guo @ 2017-01-04 7:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <8cdc4bfa-18a3-b9f6-aaba-0efe1f75fb40@semihalf.com>
Hi Tomasz,
On 2017/1/3 15:41, Tomasz Nowicki wrote:
> Hi,
>
> Can we merge patch 4 & 6 into one patch so that we keep refactoring part
> as one piece ? I do not see a reason to keep them separate or have patch
> 5 in between. You can refactor what needs to be refactored, add
> necessary functions to iort.c and then support ACPI for
> irq-gic-v3-its-platform-msi.c
There are two functions here,
- retrieve the dev id from IORT which was DT based only;
- init the platform msi domain from MADT;
For each of them split it into two steps,
- refactor the code for ACPI later and it's easy for review
because wen can easily to figure out it has functional
change or not
- add ACPI functionality
Does it make sense?
Thanks
Hanjun
^ permalink raw reply
* [PATCH resend v4 2/3] ARM: dts: imx6: Support Savageboard dual
From: Milo Kim @ 2017-01-04 7:04 UTC (permalink / raw)
To: linux-arm-kernel
Common savageboard DT file is used for board support.
Add the vendor name and specify the dtb file for i.MX6Q build.
Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
---
.../devicetree/bindings/vendor-prefixes.txt | 1 +
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/imx6dl-savageboard.dts | 51 ++++++++++++++++++++++
3 files changed, 53 insertions(+)
create mode 100644 arch/arm/boot/dts/imx6dl-savageboard.dts
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 16d3b5e7f5d1..552b63651ab9 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -227,6 +227,7 @@ pine64 Pine64
pixcir PIXCIR MICROELECTRONICS Co., Ltd
plathome Plat'Home Co., Ltd.
plda PLDA
+poslab Poslab Technology Co., Ltd.
powervr PowerVR (deprecated, use img)
pulsedlight PulsedLight, Inc
qca Qualcomm Atheros, Inc.
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index cccdbcb557b6..fb7f904a5235 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -357,6 +357,7 @@ dtb-$(CONFIG_SOC_IMX6Q) += \
imx6dl-sabreauto.dtb \
imx6dl-sabrelite.dtb \
imx6dl-sabresd.dtb \
+ imx6dl-savageboard.dtb \
imx6dl-ts4900.dtb \
imx6dl-tx6dl-comtft.dtb \
imx6dl-tx6s-8034.dtb \
diff --git a/arch/arm/boot/dts/imx6dl-savageboard.dts b/arch/arm/boot/dts/imx6dl-savageboard.dts
new file mode 100644
index 000000000000..b95469c520a4
--- /dev/null
+++ b/arch/arm/boot/dts/imx6dl-savageboard.dts
@@ -0,0 +1,51 @@
+/*
+ * Copyright (C) 2017 Milo Kim <woogyom.kim@gmail.com>
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ * a) This file is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This file is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * Or, alternatively,
+ *
+ * b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+
+#include "imx6dl.dtsi"
+#include "imx6qdl-savageboard.dtsi"
+
+/ {
+ model = "Poslab SavageBoard Dual";
+ compatible = "poslab,imx6dl-savageboard", "fsl,imx6dl";
+};
--
2.11.0
^ permalink raw reply related
* [PATCH v4 2/3] ARM: dts: imx6: Support Savageboard dual
From: Milo Kim @ 2017-01-04 7:12 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170104045553.26576-3-woogyom.kim@gmail.com>
On 01/04/2017 01:55 PM, Milo Kim wrote:
> Common savageboard DT file is used for board support.
> Add the vendor name and specify the dtb file for i.MX6Q build.
>
> Reviewed-by: Fabio Estevam <fabio.estevam@nxp.com>
> Signed-off-by: Milo Kim <woogyom.kim@gmail.com>
> ---
> .../devicetree/bindings/vendor-prefixes.txt | 1 +
> arch/arm/boot/dts/Makefile | 1 +
> arch/arm/boot/dts/imx6dl-savageboard.dts | 51 ++++++++++++++++++++++
> 3 files changed, 53 insertions(+)
> create mode 100644 arch/arm/boot/dts/imx6dl-savageboard.dts
>
> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt b/Documentation/devicetree/bindings/vendor-prefixes.txt
> index 16d3b5e7f5d1..88c33d827e51 100644
> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
> @@ -227,6 +227,7 @@ pine64 Pine64
> pixcir PIXCIR MICROELECTRONICS Co., Ltd
> plathome Plat'Home Co., Ltd.
> plda PLDA
> +poslab Poslab Technology Co., Ltd.
I should input tab here instead of spaces, so updated single patch was
just sent. Please refer to the patch named "[PATCH resend v4 2/3] ARM:
dts: imx6: Support Savageboard dual".
Sorry for the inconvenience.
Best regards,
Milo
^ permalink raw reply
* [PATCH v4 07/12] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Adrian Hunter @ 2017-01-04 7:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <f48940626343ca9cf68b32b7324b243e76a11960.1481651244.git-series.gregory.clement@free-electrons.com>
On 13/12/16 19:48, Gregory CLEMENT wrote:
> From: Hu Ziji <huziji@marvell.com>
>
> Add Xenon eMMC/SD/SDIO host controller core functionality.
> Add Xenon specific intialization process.
> Add Xenon specific mmc_host_ops APIs.
> Add Xenon specific register definitions.
>
> Add CONFIG_MMC_SDHCI_XENON support in drivers/mmc/host/Kconfig.
>
> Marvell Xenon SDHC conforms to SD Physical Layer Specification
> Version 3.01 and is designed according to the guidelines provided
> in the SD Host Controller Standard Specification Version 3.00.
>
> Signed-off-by: Hu Ziji <huziji@marvell.com>
> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
> ---
> MAINTAINERS | 1 +-
> drivers/mmc/host/Kconfig | 9 +-
> drivers/mmc/host/Makefile | 3 +-
> drivers/mmc/host/sdhci-xenon.c | 612 ++++++++++++++++++++++++++++++++++-
> drivers/mmc/host/sdhci-xenon.h | 70 ++++-
> 5 files changed, 695 insertions(+)
> create mode 100644 drivers/mmc/host/sdhci-xenon.c
> create mode 100644 drivers/mmc/host/sdhci-xenon.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 850a0afb0c8d..bb33286aeb48 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -7608,6 +7608,7 @@ MARVELL XENON MMC/SD/SDIO HOST CONTROLLER DRIVER
> M: Ziji Hu <huziji@marvell.com>
> L: linux-mmc at vger.kernel.org
> S: Supported
> +F: drivers/mmc/host/sdhci-xenon*
> F: Documentation/devicetree/bindings/mmc/marvell,xenon-sdhci.txt
>
> MATROX FRAMEBUFFER DRIVER
> diff --git a/drivers/mmc/host/Kconfig b/drivers/mmc/host/Kconfig
> index 5274f503a39a..85a53623526a 100644
> --- a/drivers/mmc/host/Kconfig
> +++ b/drivers/mmc/host/Kconfig
> @@ -798,3 +798,12 @@ config MMC_SDHCI_BRCMSTB
> Broadcom STB SoCs.
>
> If unsure, say Y.
> +
> +config MMC_SDHCI_XENON
> + tristate "Marvell Xenon eMMC/SD/SDIO SDHCI driver"
> + depends on MMC_SDHCI && MMC_SDHCI_PLTFM
> + help
> + This selects Marvell Xenon eMMC/SD/SDIO SDHCI.
> + If you have a machine with integrated Marvell Xenon SDHC IP,
> + say Y or M here.
> + If unsure, say N.
> diff --git a/drivers/mmc/host/Makefile b/drivers/mmc/host/Makefile
> index e2bdaaf43184..75eaf743486c 100644
> --- a/drivers/mmc/host/Makefile
> +++ b/drivers/mmc/host/Makefile
> @@ -80,3 +80,6 @@ obj-$(CONFIG_MMC_SDHCI_BRCMSTB) += sdhci-brcmstb.o
> ifeq ($(CONFIG_CB710_DEBUG),y)
> CFLAGS-cb710-mmc += -DDEBUG
> endif
> +
> +obj-$(CONFIG_MMC_SDHCI_XENON) += sdhci-xenon-driver.o
> +sdhci-xenon-driver-y += sdhci-xenon.o
> diff --git a/drivers/mmc/host/sdhci-xenon.c b/drivers/mmc/host/sdhci-xenon.c
> new file mode 100644
> index 000000000000..c71439fbc308
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-xenon.c
> @@ -0,0 +1,612 @@
> +/*
> + * Driver for Marvell Xenon SDHC as a platform device
> + *
> + * Copyright (C) 2016 Marvell, All Rights Reserved.
> + *
> + * Author: Hu Ziji <huziji@marvell.com>
> + * Date: 2016-8-24
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + *
> + * Inspired by Jisheng Zhang <jszhang@marvell.com>
> + * Special thanks to Video BG4 project team.
> + */
> +
> +#include <linux/delay.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +
> +#include "sdhci-pltfm.h"
> +#include "sdhci-xenon.h"
> +
> +static int enable_xenon_internal_clk(struct sdhci_host *host)
> +{
> + u32 reg;
> + u8 timeout;
> +
> + reg = sdhci_readl(host, SDHCI_CLOCK_CONTROL);
> + reg |= SDHCI_CLOCK_INT_EN;
> + sdhci_writel(host, reg, SDHCI_CLOCK_CONTROL);
> + /* Wait max 20 ms */
> + timeout = 20;
> + while (!((reg = sdhci_readw(host, SDHCI_CLOCK_CONTROL))
> + & SDHCI_CLOCK_INT_STABLE)) {
> + if (timeout == 0) {
> + pr_err("%s: Internal clock never stabilised.\n",
> + mmc_hostname(host->mmc));
> + return -ETIMEDOUT;
> + }
> + timeout--;
> + mdelay(1);
> + }
> +
> + return 0;
> +}
> +
> +/* Set SDCLK-off-while-idle */
> +static void xenon_set_sdclk_off_idle(struct sdhci_host *host,
> + unsigned char sdhc_id, bool enable)
> +{
> + u32 reg;
> + u32 mask;
> +
> + reg = sdhci_readl(host, SDHCI_SYS_OP_CTRL);
> + /* Get the bit shift basing on the SDHC index */
> + mask = (0x1 << (SDHCI_SDCLK_IDLEOFF_ENABLE_SHIFT + sdhc_id));
> + if (enable)
> + reg |= mask;
> + else
> + reg &= ~mask;
> +
> + sdhci_writel(host, reg, SDHCI_SYS_OP_CTRL);
> +}
> +
> +/* Enable/Disable the Auto Clock Gating function */
> +static void xenon_set_acg(struct sdhci_host *host, bool enable)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHCI_SYS_OP_CTRL);
> + if (enable)
> + reg &= ~SDHCI_AUTO_CLKGATE_DISABLE_MASK;
> + else
> + reg |= SDHCI_AUTO_CLKGATE_DISABLE_MASK;
> + sdhci_writel(host, reg, SDHCI_SYS_OP_CTRL);
> +}
> +
> +/* Enable this SDHC */
> +static void xenon_enable_sdhc(struct sdhci_host *host,
> + unsigned char sdhc_id)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHCI_SYS_OP_CTRL);
> + reg |= (BIT(sdhc_id) << SDHCI_SLOT_ENABLE_SHIFT);
> + sdhci_writel(host, reg, SDHCI_SYS_OP_CTRL);
> +
> + /*
> + * Manually set the flag which all the card types require,
> + * including SD, eMMC, SDIO
> + */
> + host->mmc->caps |= MMC_CAP_WAIT_WHILE_BUSY;
> +}
> +
> +/* Disable this SDHC */
> +static void xenon_disable_sdhc(struct sdhci_host *host,
> + unsigned char sdhc_id)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHCI_SYS_OP_CTRL);
> + reg &= ~(BIT(sdhc_id) << SDHCI_SLOT_ENABLE_SHIFT);
> + sdhci_writel(host, reg, SDHCI_SYS_OP_CTRL);
> +}
> +
> +/* Enable Parallel Transfer Mode */
> +static void xenon_enable_sdhc_parallel_tran(struct sdhci_host *host,
> + unsigned char sdhc_id)
> +{
> + u32 reg;
> +
> + reg = sdhci_readl(host, SDHCI_SYS_EXT_OP_CTRL);
> + reg |= BIT(sdhc_id);
> + sdhci_writel(host, reg, SDHCI_SYS_EXT_OP_CTRL);
> +}
> +
> +static void xenon_sdhc_tuning_setup(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u32 reg;
> +
> + /* Disable the Re-Tuning Request functionality */
> + reg = sdhci_readl(host, SDHCI_SLOT_RETUNING_REQ_CTRL);
> + reg &= ~SDHCI_RETUNING_COMPATIBLE;
> + sdhci_writel(host, reg, SDHCI_SLOT_RETUNING_REQ_CTRL);
> +
> + /* Disable the Re-tuning Event Signal Enable */
> + reg = sdhci_readl(host, SDHCI_SIGNAL_ENABLE);
> + reg &= ~SDHCI_INT_RETUNE;
> + sdhci_writel(host, reg, SDHCI_SIGNAL_ENABLE);
> +
> + /* Force to use Tuning Mode 1 */
> + host->tuning_mode = SDHCI_TUNING_MODE_1;
> + /* Set re-tuning period */
> + host->tuning_count = 1 << (priv->tuning_count - 1);
host->tuning_mode and host->tuning_count get overwritten in
sdhci_setup_host() called by sdhci_add_host()
> +}
> +
> +/*
> + * Operations inside struct sdhci_ops
> + */
> +/* Recover the Register Setting cleared during SOFTWARE_RESET_ALL */
> +static void sdhci_xenon_reset_exit(struct sdhci_host *host,
> + unsigned char sdhc_id, u8 mask)
> +{
> + /* Only SOFTWARE RESET ALL will clear the register setting */
> + if (!(mask & SDHCI_RESET_ALL))
> + return;
> +
> + /* Disable tuning request and auto-retuning again */
> + xenon_sdhc_tuning_setup(host);
> +
> + xenon_set_acg(host, true);
> +
> + xenon_set_sdclk_off_idle(host, sdhc_id, false);
> +}
> +
> +static void sdhci_xenon_reset(struct sdhci_host *host, u8 mask)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + sdhci_reset(host, mask);
> + sdhci_xenon_reset_exit(host, priv->sdhc_id, mask);
> +}
> +
> +/*
> + * Xenon defines different values for HS200 and HS400
> + * in Host_Control_2
> + */
> +static void xenon_set_uhs_signaling(struct sdhci_host *host,
> + unsigned int timing)
> +{
> + u16 ctrl_2;
> +
> + ctrl_2 = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + /* Select Bus Speed Mode for host */
> + ctrl_2 &= ~SDHCI_CTRL_UHS_MASK;
> + if (timing == MMC_TIMING_MMC_HS200)
> + ctrl_2 |= SDHCI_XENON_CTRL_HS200;
> + else if (timing == MMC_TIMING_UHS_SDR104)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR104;
> + else if (timing == MMC_TIMING_UHS_SDR12)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR12;
> + else if (timing == MMC_TIMING_UHS_SDR25)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR25;
> + else if (timing == MMC_TIMING_UHS_SDR50)
> + ctrl_2 |= SDHCI_CTRL_UHS_SDR50;
> + else if ((timing == MMC_TIMING_UHS_DDR50) ||
> + (timing == MMC_TIMING_MMC_DDR52))
> + ctrl_2 |= SDHCI_CTRL_UHS_DDR50;
> + else if (timing == MMC_TIMING_MMC_HS400)
> + ctrl_2 |= SDHCI_XENON_CTRL_HS400;
> + sdhci_writew(host, ctrl_2, SDHCI_HOST_CONTROL2);
> +}
> +
> +static const struct sdhci_ops sdhci_xenon_ops = {
> + .set_clock = sdhci_set_clock,
> + .set_bus_width = sdhci_set_bus_width,
> + .reset = sdhci_xenon_reset,
> + .set_uhs_signaling = xenon_set_uhs_signaling,
> + .get_max_clock = sdhci_pltfm_clk_get_max_clock,
> +};
> +
> +static const struct sdhci_pltfm_data sdhci_xenon_pdata = {
> + .ops = &sdhci_xenon_ops,
> + .quirks = SDHCI_QUIRK_NO_ENDATTR_IN_NOPDESC |
> + SDHCI_QUIRK_NO_SIMULT_VDD_AND_POWER |
> + SDHCI_QUIRK_CAP_CLOCK_BASE_BROKEN,
> +};
> +
> +/*
> + * Xenon Specific Operations in mmc_host_ops
> + */
> +static void xenon_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + unsigned long flags;
> + u32 reg;
> +
> + /*
> + * HS400/HS200/eMMC HS doesn't have Preset Value register.
> + * However, sdhci_set_ios will read HS400/HS200 Preset register.
> + * Disable Preset Value register for HS400/HS200.
> + * eMMC HS with preset_enabled set will trigger a bug in
> + * get_preset_value().
> + */
> + spin_lock_irqsave(&host->lock, flags);
> + if ((ios->timing == MMC_TIMING_MMC_HS400) ||
> + (ios->timing == MMC_TIMING_MMC_HS200) ||
> + (ios->timing == MMC_TIMING_MMC_HS)) {
> + host->preset_enabled = false;
> + host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> +
> + reg = sdhci_readw(host, SDHCI_HOST_CONTROL2);
> + reg &= ~SDHCI_CTRL_PRESET_VAL_ENABLE;
> + sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
> + } else {
> + host->quirks2 &= ~SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
> + }
> + spin_unlock_irqrestore(&host->lock, flags);
At some point we will have to get rid of SDHCI_QUIRK2_PRESET_VALUE_BROKEN
and add a callback instead.
> +
> + sdhci_set_ios(mmc, ios);
> +
> + if (host->clock > SDHCI_DEFAULT_SDCLK_FREQ) {
> + spin_lock_irqsave(&host->lock, flags);
> + xenon_set_sdclk_off_idle(host, priv->sdhc_id, true);
> + spin_unlock_irqrestore(&host->lock, flags);
> + }
> +}
> +
> +static int xenon_emmc_signal_voltage_switch(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + unsigned char voltage = ios->signal_voltage;
> + struct sdhci_host *host = mmc_priv(mmc);
> + unsigned char voltage_code;
> + u32 ctrl;
> +
> + if ((voltage == MMC_SIGNAL_VOLTAGE_330) ||
> + (voltage == MMC_SIGNAL_VOLTAGE_180)) {
> + if (voltage == MMC_SIGNAL_VOLTAGE_330)
> + voltage_code = SDHCI_EMMC_VCCQ_3_3V;
> + else if (voltage == MMC_SIGNAL_VOLTAGE_180)
> + voltage_code = SDHCI_EMMC_VCCQ_1_8V;
> +
> + /*
> + * This host is for eMMC, XENON self-defined
> + * eMMC control register should be accessed
> + * instead of Host Control 2
> + */
> + ctrl = sdhci_readl(host, SDHCI_SLOT_EMMC_CTRL);
> + ctrl &= ~SDHCI_EMMC_VCCQ_MASK;
> + ctrl |= voltage_code;
> + sdhci_writel(host, ctrl, SDHCI_SLOT_EMMC_CTRL);
> +
> + /* There is no standard to determine this waiting period */
> + usleep_range(1000, 2000);
> +
> + /* Check whether io voltage switch is done */
> + ctrl = sdhci_readl(host, SDHCI_SLOT_EMMC_CTRL);
> + ctrl &= SDHCI_EMMC_VCCQ_MASK;
> + /*
> + * This bit is set only when regulator feeds back
> + * the voltage switch results to Xenon SDHC.
> + * However, in actaul implementation, regulator might not
> + * provide this feedback.
> + * Thus we shall not rely on this bit to determine
> + * if switch failed.
> + * If the bit is not set, just throw a message.
> + * Besides, error code should not be returned.
> + */
> + if (ctrl != voltage_code)
> + dev_info(mmc_dev(mmc), "fail to detect eMMC signal voltage stable\n");
> + return 0;
> + }
> +
> + dev_err(mmc_dev(mmc), "Unsupported signal voltage: %d\n", voltage);
> + return -EINVAL;
> +}
> +
> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
> + struct mmc_ios *ios)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + /*
> + * Before SD/SDIO set signal voltage, SD bus clock should be
> + * disabled. However, sdhci_set_clock will also disable the Internal
> + * clock in mmc_set_signal_voltage().
> + * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
> + * Thus here manually enable internal clock.
> + *
> + * After switch completes, it is unnecessary to disable internal clock,
> + * since keeping internal clock active obeys SD spec.
> + */
> + enable_xenon_internal_clk(host);
We could try the attached patch.
> +
> + if (priv->init_card_type == MMC_TYPE_MMC)
> + return xenon_emmc_signal_voltage_switch(mmc, ios);
> +
> + return sdhci_start_signal_voltage_switch(mmc, ios);
> +}
> +
> +/*
> + * Update card type.
> + * priv->init_card_type will be used in PHY timing adjustment.
> + */
> +static void xenon_init_card(struct mmc_host *mmc, struct mmc_card *card)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> +
> + /* Update card type*/
> + priv->init_card_type = card->type;
> +}
> +
> +static int xenon_execute_tuning(struct mmc_host *mmc, u32 opcode)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> +
> + if (host->timing == MMC_TIMING_UHS_DDR50)
> + return 0;
> +
> + return sdhci_execute_tuning(mmc, opcode);
> +}
> +
> +static void xenon_enable_sdio_irq(struct mmc_host *mmc, int enable)
> +{
> + struct sdhci_host *host = mmc_priv(mmc);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u32 reg;
> + u8 sdhc_id = priv->sdhc_id;
> +
> + sdhci_enable_sdio_irq(mmc, enable);
> +
> + if (enable) {
> + /*
> + * Set SDIO Card Inserted indication
> + * to enable detecting SDIO async irq.
> + */
> + reg = sdhci_readl(host, SDHCI_SYS_CFG_INFO);
> + reg |= (1 << (sdhc_id + SDHCI_SLOT_TYPE_SDIO_SHIFT));
> + sdhci_writel(host, reg, SDHCI_SYS_CFG_INFO);
> + } else {
> + /* Clear SDIO Card Inserted indication */
> + reg = sdhci_readl(host, SDHCI_SYS_CFG_INFO);
> + reg &= ~(1 << (sdhc_id + SDHCI_SLOT_TYPE_SDIO_SHIFT));
> + sdhci_writel(host, reg, SDHCI_SYS_CFG_INFO);
> + }
> +}
> +
> +static void xenon_replace_mmc_host_ops(struct sdhci_host *host)
> +{
> + host->mmc_host_ops.set_ios = xenon_set_ios;
> + host->mmc_host_ops.start_signal_voltage_switch =
> + xenon_start_signal_voltage_switch;
> + host->mmc_host_ops.init_card = xenon_init_card;
> + host->mmc_host_ops.execute_tuning = xenon_execute_tuning;
> + host->mmc_host_ops.enable_sdio_irq = xenon_enable_sdio_irq;
> +}
> +
> +/*
> + * Parse child node in Xenon DT.
> + * Search for the following item(s):
> + * - eMMC card type
> + */
> +static int xenon_child_node_of_parse(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + struct mmc_host *mmc = host->mmc;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + struct device_node *child;
> + int nr_child;
> +
> + priv->init_card_type = SDHCI_CARD_TYPE_UNKNOWN;
> +
> + nr_child = of_get_child_count(np);
> + if (!nr_child)
> + return 0;
> +
> + for_each_child_of_node(np, child) {
> + if (of_device_is_compatible(child, "mmc-card")) {
> + priv->init_card_type = MMC_TYPE_MMC;
> + mmc->caps |= MMC_CAP_NONREMOVABLE;
> +
> + /*
> + * Force to clear BUS_TEST to
> + * skip bus_test_pre and bus_test_post
> + */
> + mmc->caps &= ~MMC_CAP_BUS_WIDTH_TEST;
> + mmc->caps2 |= MMC_CAP2_HC_ERASE_SZ |
> + MMC_CAP2_PACKED_CMD |
> + MMC_CAP2_NO_SD |
> + MMC_CAP2_NO_SDIO;
> + }
> + }
> +
> + return 0;
> +}
> +
> +static int xenon_probe_dt(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + struct mmc_host *mmc = host->mmc;
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + int err;
> + u32 sdhc_id, nr_sdhc;
> + u32 tuning_count;
> +
> + /* Standard MMC property */
> + err = mmc_of_parse(mmc);
> + if (err)
> + return err;
> +
> + /* Standard SDHCI property */
> + sdhci_get_of_property(pdev);
> +
> + /*
> + * Xenon Specific property:
> + * init_card_type: check whether this SDHC is for eMMC
> + * sdhc-id: the index of current SDHC.
> + * Refer to SDHCI_SYS_CFG_INFO register
> + * tun-count: the interval between re-tuning
> + */
> + /* Parse child node, including checking emmc type */
> + err = xenon_child_node_of_parse(pdev);
> + if (err)
> + return err;
> +
> + priv->sdhc_id = 0x0;
> + if (!of_property_read_u32(np, "marvell,xenon-sdhc-id", &sdhc_id)) {
> + nr_sdhc = sdhci_readl(host, SDHCI_SYS_CFG_INFO);
> + nr_sdhc &= SDHCI_NR_SUPPORTED_SLOT_MASK;
> + if (unlikely(sdhc_id > nr_sdhc)) {
> + dev_err(mmc_dev(mmc), "SDHC Index %d exceeds Number of SDHCs %d\n",
> + sdhc_id, nr_sdhc);
> + return -EINVAL;
> + }
> + }
> +
> + tuning_count = SDHCI_DEF_TUNING_COUNT;
> + if (!of_property_read_u32(np, "marvell,xenon-tun-count",
> + &tuning_count)) {
> + if (unlikely(tuning_count >= SDHCI_TMR_RETUN_NO_PRESENT)) {
> + dev_err(mmc_dev(mmc), "Wrong Re-tuning Count. Set default value %d\n",
> + SDHCI_DEF_TUNING_COUNT);
> + tuning_count = SDHCI_DEF_TUNING_COUNT;
> + }
> + }
> + priv->tuning_count = tuning_count;
> +
> + return err;
> +}
> +
> +static int xenon_sdhc_probe(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u8 sdhc_id = priv->sdhc_id;
> +
> + /* Enable SDHC */
> + xenon_enable_sdhc(host, sdhc_id);
> +
> + /* Enable ACG */
> + xenon_set_acg(host, true);
> +
> + /* Enable Parallel Transfer Mode */
> + xenon_enable_sdhc_parallel_tran(host, sdhc_id);
> +
> + /* Set tuning functionality of this SDHC */
> + xenon_sdhc_tuning_setup(host);
> +
> + return 0;
> +}
> +
> +static void xenon_sdhc_remove(struct sdhci_host *host)
> +{
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
> + u8 sdhc_id = priv->sdhc_id;
> +
> + /* disable SDHC */
> + xenon_disable_sdhc(host, sdhc_id);
> +}
> +
> +static int sdhci_xenon_probe(struct platform_device *pdev)
> +{
> + struct sdhci_pltfm_host *pltfm_host;
> + struct sdhci_host *host;
> + struct sdhci_xenon_priv *priv;
> + int err;
> +
> + host = sdhci_pltfm_init(pdev, &sdhci_xenon_pdata,
> + sizeof(struct sdhci_xenon_priv));
> + if (IS_ERR(host))
> + return PTR_ERR(host);
> +
> + pltfm_host = sdhci_priv(host);
> + priv = sdhci_pltfm_priv(pltfm_host);
> +
> + xenon_set_acg(host, false);
> +
> + /*
> + * Link Xenon specific mmc_host_ops function,
> + * to replace standard ones in sdhci_ops.
> + */
> + xenon_replace_mmc_host_ops(host);
> +
> + pltfm_host->clk = devm_clk_get(&pdev->dev, "core");
> + if (IS_ERR(pltfm_host->clk)) {
> + err = PTR_ERR(pltfm_host->clk);
> + dev_err(&pdev->dev, "Failed to setup input clk: %d\n", err);
> + goto free_pltfm;
> + }
> + err = clk_prepare_enable(pltfm_host->clk);
> + if (err)
> + goto free_pltfm;
> +
> + err = xenon_probe_dt(pdev);
> + if (err)
> + goto err_clk;
> +
> + err = xenon_sdhc_probe(host);
> + if (err)
> + goto err_clk;
> +
> + err = sdhci_add_host(host);
> + if (err)
> + goto remove_sdhc;
> +
> + return 0;
> +
> +remove_sdhc:
> + xenon_sdhc_remove(host);
> +err_clk:
> + clk_disable_unprepare(pltfm_host->clk);
> +free_pltfm:
> + sdhci_pltfm_free(pdev);
> + return err;
> +}
> +
> +static int sdhci_xenon_remove(struct platform_device *pdev)
> +{
> + struct sdhci_host *host = platform_get_drvdata(pdev);
> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
> + int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xFFFFFFFF);
This 'dead' check was originally for PCI I think. Unless you know it makes
sense for your device, I would leave it out. i.e. just do
sdhci_remove_host(host, 0);
> +
> + xenon_sdhc_remove(host);
> +
> + sdhci_remove_host(host, dead);
> +
> + clk_disable_unprepare(pltfm_host->clk);
> +
> + sdhci_pltfm_free(pdev);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id sdhci_xenon_dt_ids[] = {
> + { .compatible = "marvell,armada-7000-sdhci",},
> + { .compatible = "marvell,armada-3700-sdhci",},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);
> +
> +static struct platform_driver sdhci_xenon_driver = {
> + .driver = {
> + .name = "xenon-sdhci",
> + .of_match_table = sdhci_xenon_dt_ids,
> + .pm = &sdhci_pltfm_pmops,
> + },
> + .probe = sdhci_xenon_probe,
> + .remove = sdhci_xenon_remove,
> +};
> +
> +module_platform_driver(sdhci_xenon_driver);
> +
> +MODULE_DESCRIPTION("SDHCI platform driver for Marvell Xenon SDHC");
> +MODULE_AUTHOR("Hu Ziji <huziji@marvell.com>");
> +MODULE_LICENSE("GPL v2");
> diff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h
> new file mode 100644
> index 000000000000..d50cd663a265
> --- /dev/null
> +++ b/drivers/mmc/host/sdhci-xenon.h
> @@ -0,0 +1,70 @@
> +/*
> + * Copyright (C) 2016 Marvell, All Rights Reserved.
> + *
> + * Author: Hu Ziji <huziji@marvell.com>
> + * Date: 2016-8-24
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation version 2.
> + */
> +#ifndef SDHCI_XENON_H_
> +#define SDHCI_XENON_H_
> +
> +
Double blank line
> +/* Register Offset of Xenon SDHC self-defined register */
> +#define SDHCI_SYS_CFG_INFO 0x0104
A lot of these defines look like they could be just in sdhci-xenon.c or
sdhci-xenon-phy.c. It is also a little odd that they are prefixed by
"SDHCI" because they are not standard. "XENON" would be better.
> +#define SDHCI_SLOT_TYPE_SDIO_SHIFT 24
> +#define SDHCI_NR_SUPPORTED_SLOT_MASK 0x7
> +
> +#define SDHCI_SYS_OP_CTRL 0x0108
> +#define SDHCI_AUTO_CLKGATE_DISABLE_MASK BIT(20)
> +#define SDHCI_SDCLK_IDLEOFF_ENABLE_SHIFT 8
> +#define SDHCI_SLOT_ENABLE_SHIFT 0
> +
> +#define SDHCI_SYS_EXT_OP_CTRL 0x010C
> +
> +#define SDHCI_SLOT_EMMC_CTRL 0x0130
> +#define SDHCI_EMMC_VCCQ_MASK 0x3
> +#define SDHCI_EMMC_VCCQ_1_8V 0x1
> +#define SDHCI_EMMC_VCCQ_3_3V 0x3
> +
> +#define SDHCI_SLOT_RETUNING_REQ_CTRL 0x0144
> +/* retuning compatible */
> +#define SDHCI_RETUNING_COMPATIBLE 0x1
> +
> +/* Tuning Parameter */
> +#define SDHCI_TMR_RETUN_NO_PRESENT 0xF
> +#define SDHCI_DEF_TUNING_COUNT 0x9
> +
> +#define SDHCI_DEFAULT_SDCLK_FREQ (400000)
Unnecessary ()
> +
> +/* Xenon specific Mode Select value */
> +#define SDHCI_XENON_CTRL_HS200 0x5
> +#define SDHCI_XENON_CTRL_HS400 0x6
> +
> +/* Indicate Card Type is not clear yet */
> +#define SDHCI_CARD_TYPE_UNKNOWN 0xF
> +
> +struct sdhci_xenon_priv {
> + unsigned char tuning_count;
> + /* idx of SDHC */
> + u8 sdhc_id;
> +
> + /*
> + * eMMC/SD/SDIO require different PHY settings or
> + * voltage control. It's necessary for Xenon driver to
> + * recognize card type during, or even before initialization.
> + * However, mmc_host->card is not available yet at that time.
> + * This field records the card type during init.
> + * For eMMC, it is updated in dt parse. For SD/SDIO, it is
> + * updated in xenon_init_card().
> + *
> + * It is only valid during initialization after it is updated.
> + * Do not access this variable in normal transfers after
> + * initialization completes.
> + */
> + unsigned int init_card_type;
> +};
> +
> +#endif
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-mmc-sdhci-Leave-internal-clock-on-when-bus-power-is-.patch
Type: text/x-patch
Size: 1423 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20170104/f4ab8f52/attachment-0001.bin>
^ permalink raw reply
* [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI
From: Tomasz Nowicki @ 2017-01-04 7:29 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <601cbdf2-823d-8bde-bbd9-fcc6a1c67f2c@linaro.org>
On 04.01.2017 08:02, Hanjun Guo wrote:
> Hi Tomasz,
>
> On 2017/1/3 15:41, Tomasz Nowicki wrote:
>> Hi,
>>
>> Can we merge patch 4 & 6 into one patch so that we keep refactoring part
>> as one piece ? I do not see a reason to keep them separate or have patch
>> 5 in between. You can refactor what needs to be refactored, add
>> necessary functions to iort.c and then support ACPI for
>> irq-gic-v3-its-platform-msi.c
>
> There are two functions here,
> - retrieve the dev id from IORT which was DT based only;
>
> - init the platform msi domain from MADT;
>
> For each of them split it into two steps,
> - refactor the code for ACPI later and it's easy for review
> because wen can easily to figure out it has functional
> change or not
>
> - add ACPI functionality
>
> Does it make sense?
It is up to Marc, but personally I prefer:
1. Refactor dev id retrieving and init function in one patch and
highlight no functional changes in changelog
2. Crate necessary infrastructure in iort.c
3. Then add ACPI support to irq-gic-v3-its-platform-msi.c
Thanks,
Tomasz
^ permalink raw reply
* [PATCH v6 01/14] ACPI: ARM64: IORT: minor cleanup for iort_match_node_callback()
From: Hanjun Guo @ 2017-01-04 7:56 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170103140839.GC7478@red-moon>
Hi Lorenzo,
On 2017/1/3 22:08, Lorenzo Pieralisi wrote:
> On Mon, Jan 02, 2017 at 09:31:32PM +0800, Hanjun Guo wrote:
>> Cleanup iort_match_node_callback() a little bit to reduce
>> some lines of code, aslo fix the indentation in iort_scan_node().
>
> s/aslo/also
>
> "Also" in a commit log is a sign a patch should be split and that's what
> you should do even though I know it is tempting to merge all trivial
> changes into one single patch.
>
> Make it two patches please.
Will do, thanks!
Do you have more comments regarding this patch set? I will
incorporate all the comments and send out a new version.
Thanks
Hanjun
^ permalink raw reply
* [PATCH] iommu: Drop the of_iommu_{set/get}_ops() interface
From: Sricharan @ 2017-01-04 8:00 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20170103173456.18154-1-lorenzo.pieralisi@arm.com>
Hi Lorenzo,
>With the introduction of the new iommu_{register/get}_instance()
>interface in commit e4f10ffe4c9b ("iommu: Make of_iommu_set/get_ops() DT
>agnostic") (based on struct fwnode_handle as look-up token, so firmware
>agnostic) to register IOMMU instances with the core IOMMU layer there is
>no reason to keep the old OF based interface around any longer.
>
>Convert all the IOMMU drivers (and OF IOMMU core code) that rely on the
>of_iommu_{set/get}_ops() to the new kernel interface to register/retrieve
>IOMMU instances and remove the of_iommu_{set/get}_ops() remaining glue
>code in order to complete the interface rework.
>
>Signed-off-by: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
>Cc: Matthias Brugger <matthias.bgg@gmail.com>
>Cc: Will Deacon <will.deacon@arm.com>
>Cc: Robin Murphy <robin.murphy@arm.com>
>Cc: Joerg Roedel <joro@8bytes.org>
>Cc: Marek Szyprowski <m.szyprowski@samsung.com>
>---
>Exynos, msm and mtk code compile tested only owing to lack of
>test platforms, I would appreciate some help in testing this
>patch on those platforms before merging it even if it is just
>a simple interface conversion.
>
For msm,
Tested-by: Sricharan R <sricharan@codeaurora.org>
Regards,
Sricharan
^ permalink raw reply
* [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI
From: Hanjun Guo @ 2017-01-04 8:25 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <b51b7be9-f8ee-57f9-6d53-6e8bd73dd431@semihalf.com>
On 2017/1/4 15:29, Tomasz Nowicki wrote:
> On 04.01.2017 08:02, Hanjun Guo wrote:
>> Hi Tomasz,
>>
>> On 2017/1/3 15:41, Tomasz Nowicki wrote:
>>> Hi,
>>>
>>> Can we merge patch 4 & 6 into one patch so that we keep refactoring part
>>> as one piece ? I do not see a reason to keep them separate or have patch
>>> 5 in between. You can refactor what needs to be refactored, add
>>> necessary functions to iort.c and then support ACPI for
>>> irq-gic-v3-its-platform-msi.c
>>
>> There are two functions here,
>> - retrieve the dev id from IORT which was DT based only;
>>
>> - init the platform msi domain from MADT;
>>
>> For each of them split it into two steps,
>> - refactor the code for ACPI later and it's easy for review
>> because wen can easily to figure out it has functional
>> change or not
>>
>> - add ACPI functionality
>>
>> Does it make sense?
>
> It is up to Marc, but personally I prefer:
> 1. Refactor dev id retrieving and init function in one patch and
> highlight no functional changes in changelog
> 2. Crate necessary infrastructure in iort.c
> 3. Then add ACPI support to irq-gic-v3-its-platform-msi.c
I have no strong preferences, and it's easy to do so as just
need to squash/reorder the patches.
Marc, Lorenzo, could you give some suggestions here?
Thanks
Hanjun
^ permalink raw reply
* [PATCH v4 07/12] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Ziji Hu @ 2017-01-04 8:51 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <aedd36af-8bf6-815f-34ee-9a3166e4fe5b@intel.com>
Hi Adrian,
On 2017/1/4 15:26, Adrian Hunter wrote:
> On 13/12/16 19:48, Gregory CLEMENT wrote:
>> From: Hu Ziji <huziji@marvell.com>
>>
>> Add Xenon eMMC/SD/SDIO host controller core functionality.
>> Add Xenon specific intialization process.
>> Add Xenon specific mmc_host_ops APIs.
>> Add Xenon specific register definitions.
>>
>> Add CONFIG_MMC_SDHCI_XENON support in drivers/mmc/host/Kconfig.
>>
>> Marvell Xenon SDHC conforms to SD Physical Layer Specification
>> Version 3.01 and is designed according to the guidelines provided
>> in the SD Host Controller Standard Specification Version 3.00.
>>
>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
<snip>
>> +static void xenon_sdhc_tuning_setup(struct sdhci_host *host)
>> +{
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> + u32 reg;
>> +
>> + /* Disable the Re-Tuning Request functionality */
>> + reg = sdhci_readl(host, SDHCI_SLOT_RETUNING_REQ_CTRL);
>> + reg &= ~SDHCI_RETUNING_COMPATIBLE;
>> + sdhci_writel(host, reg, SDHCI_SLOT_RETUNING_REQ_CTRL);
>> +
>> + /* Disable the Re-tuning Event Signal Enable */
>> + reg = sdhci_readl(host, SDHCI_SIGNAL_ENABLE);
>> + reg &= ~SDHCI_INT_RETUNE;
>> + sdhci_writel(host, reg, SDHCI_SIGNAL_ENABLE);
>> +
>> + /* Force to use Tuning Mode 1 */
>> + host->tuning_mode = SDHCI_TUNING_MODE_1;
>> + /* Set re-tuning period */
>> + host->tuning_count = 1 << (priv->tuning_count - 1);
>
> host->tuning_mode and host->tuning_count get overwritten in
> sdhci_setup_host() called by sdhci_add_host()
>
You are correct.
I will move it after sdhci_add_host().
>> +}
>> +
<snip>
>> +/*
>> + * Xenon Specific Operations in mmc_host_ops
>> + */
>> +static void xenon_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>> +{
>> + struct sdhci_host *host = mmc_priv(mmc);
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> + unsigned long flags;
>> + u32 reg;
>> +
>> + /*
>> + * HS400/HS200/eMMC HS doesn't have Preset Value register.
>> + * However, sdhci_set_ios will read HS400/HS200 Preset register.
>> + * Disable Preset Value register for HS400/HS200.
>> + * eMMC HS with preset_enabled set will trigger a bug in
>> + * get_preset_value().
>> + */
>> + spin_lock_irqsave(&host->lock, flags);
>> + if ((ios->timing == MMC_TIMING_MMC_HS400) ||
>> + (ios->timing == MMC_TIMING_MMC_HS200) ||
>> + (ios->timing == MMC_TIMING_MMC_HS)) {
>> + host->preset_enabled = false;
>> + host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
>> +
>> + reg = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>> + reg &= ~SDHCI_CTRL_PRESET_VAL_ENABLE;
>> + sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
>> + } else {
>> + host->quirks2 &= ~SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
>> + }
>> + spin_unlock_irqrestore(&host->lock, flags);
>
> At some point we will have to get rid of SDHCI_QUIRK2_PRESET_VALUE_BROKEN
> and add a callback instead.
>
Thanks for the information.
I would like to keep this workaround here, before the detailed patch is brought up.
Is it OK to you?
>> +
<snip>
>> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
>> + struct mmc_ios *ios)
>> +{
>> + struct sdhci_host *host = mmc_priv(mmc);
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>> +
>> + /*
>> + * Before SD/SDIO set signal voltage, SD bus clock should be
>> + * disabled. However, sdhci_set_clock will also disable the Internal
>> + * clock in mmc_set_signal_voltage().
>> + * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
>> + * Thus here manually enable internal clock.
>> + *
>> + * After switch completes, it is unnecessary to disable internal clock,
>> + * since keeping internal clock active obeys SD spec.
>> + */
>> + enable_xenon_internal_clk(host);
>
> We could try the attached patch.
>
I test your patch. It can work on my platforms.
Thanks a lot.
May I keep this workaround now?
I would like to remove this workaround after your attached patch is applied.
>> +
<snip>
>> +static int sdhci_xenon_remove(struct platform_device *pdev)
>> +{
>> + struct sdhci_host *host = platform_get_drvdata(pdev);
>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>> + int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xFFFFFFFF);
>
> This 'dead' check was originally for PCI I think. Unless you know it makes
> sense for your device, I would leave it out. i.e. just do
> sdhci_remove_host(host, 0);
>
Got it. I will remove it.
>> +
>> + xenon_sdhc_remove(host);
>> +
>> + sdhci_remove_host(host, dead);
>> +
>> + clk_disable_unprepare(pltfm_host->clk);
>> +
>> + sdhci_pltfm_free(pdev);
>> +
>> + return 0;
>> +}
>> +
>> +static const struct of_device_id sdhci_xenon_dt_ids[] = {
>> + { .compatible = "marvell,armada-7000-sdhci",},
>> + { .compatible = "marvell,armada-3700-sdhci",},
>> + {}
>> +};
>> +MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);
>> +
>> +static struct platform_driver sdhci_xenon_driver = {
>> + .driver = {
>> + .name = "xenon-sdhci",
>> + .of_match_table = sdhci_xenon_dt_ids,
>> + .pm = &sdhci_pltfm_pmops,
>> + },
>> + .probe = sdhci_xenon_probe,
>> + .remove = sdhci_xenon_remove,
>> +};
>> +
>> +module_platform_driver(sdhci_xenon_driver);
>> +
>> +MODULE_DESCRIPTION("SDHCI platform driver for Marvell Xenon SDHC");
>> +MODULE_AUTHOR("Hu Ziji <huziji@marvell.com>");
>> +MODULE_LICENSE("GPL v2");
>> diff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h
>> new file mode 100644
>> index 000000000000..d50cd663a265
>> --- /dev/null
>> +++ b/drivers/mmc/host/sdhci-xenon.h
>> @@ -0,0 +1,70 @@
>> +/*
>> + * Copyright (C) 2016 Marvell, All Rights Reserved.
>> + *
>> + * Author: Hu Ziji <huziji@marvell.com>
>> + * Date: 2016-8-24
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public License as
>> + * published by the Free Software Foundation version 2.
>> + */
>> +#ifndef SDHCI_XENON_H_
>> +#define SDHCI_XENON_H_
>> +
>> +
>
> Double blank line
>
Will remove it.
>> +/* Register Offset of Xenon SDHC self-defined register */
>> +#define SDHCI_SYS_CFG_INFO 0x0104
>
> A lot of these defines look like they could be just in sdhci-xenon.c or
> sdhci-xenon-phy.c. It is also a little odd that they are prefixed by
> "SDHCI" because they are not standard. "XENON" would be better.
>
Some registers are accessed by bother sdhci-xenon.c and sdhci-xenon-phy.c.
As a result, I list all the registers here in sdhci-xenon.h, for convenience.
Previously, Ulf asked me to prefix them all with "SDHCI".
I would like to know which prefix sounds more reasonable, "XENON_" or "SDHCI_XENON_"?
>> +#define SDHCI_SLOT_TYPE_SDIO_SHIFT 24
>> +#define SDHCI_NR_SUPPORTED_SLOT_MASK 0x7
>> +
>> +#define SDHCI_SYS_OP_CTRL 0x0108
>> +#define SDHCI_AUTO_CLKGATE_DISABLE_MASK BIT(20)
>> +#define SDHCI_SDCLK_IDLEOFF_ENABLE_SHIFT 8
>> +#define SDHCI_SLOT_ENABLE_SHIFT 0
>> +
>> +#define SDHCI_SYS_EXT_OP_CTRL 0x010C
>> +
>> +#define SDHCI_SLOT_EMMC_CTRL 0x0130
>> +#define SDHCI_EMMC_VCCQ_MASK 0x3
>> +#define SDHCI_EMMC_VCCQ_1_8V 0x1
>> +#define SDHCI_EMMC_VCCQ_3_3V 0x3
>> +
>> +#define SDHCI_SLOT_RETUNING_REQ_CTRL 0x0144
>> +/* retuning compatible */
>> +#define SDHCI_RETUNING_COMPATIBLE 0x1
>> +
>> +/* Tuning Parameter */
>> +#define SDHCI_TMR_RETUN_NO_PRESENT 0xF
>> +#define SDHCI_DEF_TUNING_COUNT 0x9
>> +
>> +#define SDHCI_DEFAULT_SDCLK_FREQ (400000)
>
> Unnecessary ()
>
Will fix it.
Thanks a lot for the review.
Best regards,
Hu Ziji
>> +
>> +/* Xenon specific Mode Select value */
>> +#define SDHCI_XENON_CTRL_HS200 0x5
>> +#define SDHCI_XENON_CTRL_HS400 0x6
>> +
>> +/* Indicate Card Type is not clear yet */
>> +#define SDHCI_CARD_TYPE_UNKNOWN 0xF
>> +
>> +struct sdhci_xenon_priv {
>> + unsigned char tuning_count;
>> + /* idx of SDHC */
>> + u8 sdhc_id;
>> +
>> + /*
>> + * eMMC/SD/SDIO require different PHY settings or
>> + * voltage control. It's necessary for Xenon driver to
>> + * recognize card type during, or even before initialization.
>> + * However, mmc_host->card is not available yet at that time.
>> + * This field records the card type during init.
>> + * For eMMC, it is updated in dt parse. For SD/SDIO, it is
>> + * updated in xenon_init_card().
>> + *
>> + * It is only valid during initialization after it is updated.
>> + * Do not access this variable in normal transfers after
>> + * initialization completes.
>> + */
>> + unsigned int init_card_type;
>> +};
>> +
>> +#endif
>>
>
^ permalink raw reply
* [PATCH 0/3] linux/const.h: cleanups of macros such as UL(), _BITUL(), BIT() etc.
From: Masahiro Yamada @ 2017-01-04 8:55 UTC (permalink / raw)
To: linux-arm-kernel
ARM, arm64, unicore32 define UL() as a shorthand of _AC(..., UL).
In the future, more architectures may introduce it, so move
the definition to include/linux/const.h.
The _AC() is used for either (likely) UL or (unlikely) ULL.
Having UL(L) in a common place can avoid direct use of _AC().
The _AC() is defined under uapi directory, so it compels
underscore-prefixed macros even for unexported headers.
I see similar situation for _BITUL(). This is available in
both C and assembly. However, it is defined in an uapi header,
so direct use of the underscore macro is needed even for unexported
headers. The 3/3 makes BIT() available in assembly too, which will
be more suitable for use in unexported headers.
Masahiro Yamada (3):
linux/const.h: move UL() macro to include/linux/const.h
linux/const.h: refactor _BITUL and _BITULL a bit
linux/const.h: move BIT(_ULL) to linux/const.h for use in assembly
arch/arm/include/asm/memory.h | 6 ------
arch/arm64/include/asm/memory.h | 6 ------
arch/m68k/mm/init.c | 6 +++---
arch/unicore32/include/asm/memory.h | 6 ------
include/linux/bitops.h | 3 +--
include/linux/const.h | 12 ++++++++++++
include/uapi/linux/const.h | 13 ++++++++-----
7 files changed, 24 insertions(+), 28 deletions(-)
create mode 100644 include/linux/const.h
--
2.7.4
^ permalink raw reply
* [PATCH 1/3] linux/const.h: move UL() macro to include/linux/const.h
From: Masahiro Yamada @ 2017-01-04 8:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1483520127-29316-1-git-send-email-yamada.masahiro@socionext.com>
Some architectures are duplicating the definition of UL():
#define UL(x) _AC(x, UL)
This is not actually arch-specific, so it will be useful to move it
to a common header. Currently, we only have the uapi variant for
linux/const.h, so I am creating include/linux/const.h.
I am also adding _UL(x), _ULL(x), ULL(x) because _AC() is used for
UL in most places (and ULL in some places). I expect _AC(..., UL)
will be replaced with _UL(...) or UL(...). The underscore-prefixed
one should be used for exported headers.
Note:
I renamed UL(x) in arch/m68k/mm/init.c, where it is used for a
different meaning.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
arch/arm/include/asm/memory.h | 6 ------
arch/arm64/include/asm/memory.h | 6 ------
arch/m68k/mm/init.c | 6 +++---
arch/unicore32/include/asm/memory.h | 6 ------
include/linux/const.h | 9 +++++++++
include/uapi/linux/const.h | 9 ++++++---
6 files changed, 18 insertions(+), 24 deletions(-)
create mode 100644 include/linux/const.h
diff --git a/arch/arm/include/asm/memory.h b/arch/arm/include/asm/memory.h
index 76cbd9c..7558247 100644
--- a/arch/arm/include/asm/memory.h
+++ b/arch/arm/include/asm/memory.h
@@ -22,12 +22,6 @@
#include <mach/memory.h>
#endif
-/*
- * Allow for constants defined here to be used from assembly code
- * by prepending the UL suffix only with actual C code compilation.
- */
-#define UL(x) _AC(x, UL)
-
/* PAGE_OFFSET - the virtual address of the start of the kernel image */
#define PAGE_OFFSET UL(CONFIG_PAGE_OFFSET)
diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h
index bfe6328..4310bcc 100644
--- a/arch/arm64/include/asm/memory.h
+++ b/arch/arm64/include/asm/memory.h
@@ -28,12 +28,6 @@
#include <asm/sizes.h>
/*
- * Allow for constants defined here to be used from assembly code
- * by prepending the UL suffix only with actual C code compilation.
- */
-#define UL(x) _AC(x, UL)
-
-/*
* Size of the PCI I/O space. This must remain a power of two so that
* IO_SPACE_LIMIT acts as a mask for the low bits of I/O addresses.
*/
diff --git a/arch/m68k/mm/init.c b/arch/m68k/mm/init.c
index 9c1e656..a625144 100644
--- a/arch/m68k/mm/init.c
+++ b/arch/m68k/mm/init.c
@@ -121,9 +121,9 @@ void free_initmem(void)
void __init print_memmap(void)
{
-#define UL(x) ((unsigned long) (x))
-#define MLK(b, t) UL(b), UL(t), (UL(t) - UL(b)) >> 10
-#define MLM(b, t) UL(b), UL(t), (UL(t) - UL(b)) >> 20
+#define TO_UL(x) ((unsigned long) (x))
+#define MLK(b, t) TO_UL(b), TO_UL(t), (TO_UL(t) - TO_UL(b)) >> 10
+#define MLM(b, t) TO_UL(b), TO_UL(t), (TO_UL(t) - TO_UL(b)) >> 20
#define MLK_ROUNDUP(b, t) b, t, DIV_ROUND_UP(((t) - (b)), 1024)
pr_notice("Virtual kernel memory layout:\n"
diff --git a/arch/unicore32/include/asm/memory.h b/arch/unicore32/include/asm/memory.h
index 3bb0a29..66bb9f6 100644
--- a/arch/unicore32/include/asm/memory.h
+++ b/arch/unicore32/include/asm/memory.h
@@ -20,12 +20,6 @@
#include <mach/memory.h>
/*
- * Allow for constants defined here to be used from assembly code
- * by prepending the UL suffix only with actual C code compilation.
- */
-#define UL(x) _AC(x, UL)
-
-/*
* PAGE_OFFSET - the virtual address of the start of the kernel image
* TASK_SIZE - the maximum size of a user space task.
* TASK_UNMAPPED_BASE - the lower boundary of the mmap VM area
diff --git a/include/linux/const.h b/include/linux/const.h
new file mode 100644
index 0000000..7b55a55
--- /dev/null
+++ b/include/linux/const.h
@@ -0,0 +1,9 @@
+#ifndef _LINUX_CONST_H
+#define _LINUX_CONST_H
+
+#include <uapi/linux/const.h>
+
+#define UL(x) (_UL(x))
+#define ULL(x) (_ULL(x))
+
+#endif /* _LINUX_CONST_H */
diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h
index c872bfd..76fb0f9 100644
--- a/include/uapi/linux/const.h
+++ b/include/uapi/linux/const.h
@@ -1,7 +1,7 @@
/* const.h: Macros for dealing with constants. */
-#ifndef _LINUX_CONST_H
-#define _LINUX_CONST_H
+#ifndef _UAPI_LINUX_CONST_H
+#define _UAPI_LINUX_CONST_H
/* Some constant macros are used in both assembler and
* C code. Therefore we cannot annotate them always with
@@ -21,7 +21,10 @@
#define _AT(T,X) ((T)(X))
#endif
+#define _UL(x) (_AC(x, UL))
+#define _ULL(x) (_AC(x, ULL))
+
#define _BITUL(x) (_AC(1,UL) << (x))
#define _BITULL(x) (_AC(1,ULL) << (x))
-#endif /* !(_LINUX_CONST_H) */
+#endif /* _UAPI_LINUX_CONST_H */
--
2.7.4
^ permalink raw reply related
* [PATCH 2/3] linux/const.h: refactor _BITUL and _BITULL a bit
From: Masahiro Yamada @ 2017-01-04 8:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1483520127-29316-1-git-send-email-yamada.masahiro@socionext.com>
Minor cleanups available by _UL and _ULL.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
include/uapi/linux/const.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/uapi/linux/const.h b/include/uapi/linux/const.h
index 76fb0f9..13e5165 100644
--- a/include/uapi/linux/const.h
+++ b/include/uapi/linux/const.h
@@ -24,7 +24,7 @@
#define _UL(x) (_AC(x, UL))
#define _ULL(x) (_AC(x, ULL))
-#define _BITUL(x) (_AC(1,UL) << (x))
-#define _BITULL(x) (_AC(1,ULL) << (x))
+#define _BITUL(x) (_UL(1) << (x))
+#define _BITULL(x) (_ULL(1) << (x))
#endif /* _UAPI_LINUX_CONST_H */
--
2.7.4
^ permalink raw reply related
* [PATCH 3/3] linux/const.h: move BIT(_ULL) to linux/const.h for use in assembly
From: Masahiro Yamada @ 2017-01-04 8:55 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1483520127-29316-1-git-send-email-yamada.masahiro@socionext.com>
Commit 2fc016c5bd8a ("linux/const.h: Add _BITUL() and _BITULL()")
introduced _BITUL() and _BITULL(). Its git-log says the difference
from the already existing BIT() are:
1. The namespace is such that they can be used in uapi definitions.
2. The type is set with the _AC() macro to allow it to be used in
assembly.
3. The type is explicitly specified to be UL or ULL.
However, I found _BITUL() is often used for "2. use in assembly",
while "1. use in uapi" is unneeded. If we address only "2.", we can
improve the existing BIT() for that. It will allow us to replace
many _BITUL() instances with BIT(), i.e. avoid needless use of
underscore-prefixed macros, in the end, for better de-couple of
userspace/kernel headers.
Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---
include/linux/bitops.h | 3 +--
include/linux/const.h | 3 +++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/include/linux/bitops.h b/include/linux/bitops.h
index a83c822..5f45fa5 100644
--- a/include/linux/bitops.h
+++ b/include/linux/bitops.h
@@ -1,10 +1,9 @@
#ifndef _LINUX_BITOPS_H
#define _LINUX_BITOPS_H
+#include <linux/const.h>
#include <asm/types.h>
#ifdef __KERNEL__
-#define BIT(nr) (1UL << (nr))
-#define BIT_ULL(nr) (1ULL << (nr))
#define BIT_MASK(nr) (1UL << ((nr) % BITS_PER_LONG))
#define BIT_WORD(nr) ((nr) / BITS_PER_LONG)
#define BIT_ULL_MASK(nr) (1ULL << ((nr) % BITS_PER_LONG_LONG))
diff --git a/include/linux/const.h b/include/linux/const.h
index 7b55a55..200892d 100644
--- a/include/linux/const.h
+++ b/include/linux/const.h
@@ -6,4 +6,7 @@
#define UL(x) (_UL(x))
#define ULL(x) (_ULL(x))
+#define BIT(x) (_BITUL(x))
+#define BIT_ULL(x) (_BITULL(x))
+
#endif /* _LINUX_CONST_H */
--
2.7.4
^ permalink raw reply related
* [PATCH v6 06/14] irqchip: gicv3-its: platform-msi: refactor its_pmsi_init() to prepare for ACPI
From: Marc Zyngier @ 2017-01-04 9:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <254387f0-1f63-4c83-ef90-570ef072cddf@linaro.org>
On 04/01/17 08:25, Hanjun Guo wrote:
> On 2017/1/4 15:29, Tomasz Nowicki wrote:
>> On 04.01.2017 08:02, Hanjun Guo wrote:
>>> Hi Tomasz,
>>>
>>> On 2017/1/3 15:41, Tomasz Nowicki wrote:
>>>> Hi,
>>>>
>>>> Can we merge patch 4 & 6 into one patch so that we keep refactoring part
>>>> as one piece ? I do not see a reason to keep them separate or have patch
>>>> 5 in between. You can refactor what needs to be refactored, add
>>>> necessary functions to iort.c and then support ACPI for
>>>> irq-gic-v3-its-platform-msi.c
>>>
>>> There are two functions here,
>>> - retrieve the dev id from IORT which was DT based only;
>>>
>>> - init the platform msi domain from MADT;
>>>
>>> For each of them split it into two steps,
>>> - refactor the code for ACPI later and it's easy for review
>>> because wen can easily to figure out it has functional
>>> change or not
>>>
>>> - add ACPI functionality
>>>
>>> Does it make sense?
>>
>> It is up to Marc, but personally I prefer:
>> 1. Refactor dev id retrieving and init function in one patch and
>> highlight no functional changes in changelog
>> 2. Crate necessary infrastructure in iort.c
>> 3. Then add ACPI support to irq-gic-v3-its-platform-msi.c
>
> I have no strong preferences, and it's easy to do so as just
> need to squash/reorder the patches.
>
> Marc, Lorenzo, could you give some suggestions here?
I think it'd make the reviewing easier to have patches that are
semantically grouped together (all the ACPI IORT together, for example).
It would help understanding where you're aiming at instead of jumping
from irqchip to ACPI and back every other patch...
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* [PATCH v4 07/12] mmc: sdhci-xenon: Add Marvell Xenon SDHC core functionality
From: Adrian Hunter @ 2017-01-04 9:08 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <3dc96305-d711-5357-8128-2e8939b154ff@marvell.com>
On 04/01/17 10:51, Ziji Hu wrote:
> Hi Adrian,
>
> On 2017/1/4 15:26, Adrian Hunter wrote:
>> On 13/12/16 19:48, Gregory CLEMENT wrote:
>>> From: Hu Ziji <huziji@marvell.com>
>>>
>>> Add Xenon eMMC/SD/SDIO host controller core functionality.
>>> Add Xenon specific intialization process.
>>> Add Xenon specific mmc_host_ops APIs.
>>> Add Xenon specific register definitions.
>>>
>>> Add CONFIG_MMC_SDHCI_XENON support in drivers/mmc/host/Kconfig.
>>>
>>> Marvell Xenon SDHC conforms to SD Physical Layer Specification
>>> Version 3.01 and is designed according to the guidelines provided
>>> in the SD Host Controller Standard Specification Version 3.00.
>>>
>>> Signed-off-by: Hu Ziji <huziji@marvell.com>
>>> Signed-off-by: Gregory CLEMENT <gregory.clement@free-electrons.com>
>
> <snip>
>
>>> +static void xenon_sdhc_tuning_setup(struct sdhci_host *host)
>>> +{
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>> + u32 reg;
>>> +
>>> + /* Disable the Re-Tuning Request functionality */
>>> + reg = sdhci_readl(host, SDHCI_SLOT_RETUNING_REQ_CTRL);
>>> + reg &= ~SDHCI_RETUNING_COMPATIBLE;
>>> + sdhci_writel(host, reg, SDHCI_SLOT_RETUNING_REQ_CTRL);
>>> +
>>> + /* Disable the Re-tuning Event Signal Enable */
>>> + reg = sdhci_readl(host, SDHCI_SIGNAL_ENABLE);
>>> + reg &= ~SDHCI_INT_RETUNE;
>>> + sdhci_writel(host, reg, SDHCI_SIGNAL_ENABLE);
>>> +
>>> + /* Force to use Tuning Mode 1 */
>>> + host->tuning_mode = SDHCI_TUNING_MODE_1;
>>> + /* Set re-tuning period */
>>> + host->tuning_count = 1 << (priv->tuning_count - 1);
>>
>> host->tuning_mode and host->tuning_count get overwritten in
>> sdhci_setup_host() called by sdhci_add_host()
>>
>
> You are correct.
> I will move it after sdhci_add_host().
>
>>> +}
>>> +
> <snip>
>
>>> +/*
>>> + * Xenon Specific Operations in mmc_host_ops
>>> + */
>>> +static void xenon_set_ios(struct mmc_host *mmc, struct mmc_ios *ios)
>>> +{
>>> + struct sdhci_host *host = mmc_priv(mmc);
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>> + unsigned long flags;
>>> + u32 reg;
>>> +
>>> + /*
>>> + * HS400/HS200/eMMC HS doesn't have Preset Value register.
>>> + * However, sdhci_set_ios will read HS400/HS200 Preset register.
>>> + * Disable Preset Value register for HS400/HS200.
>>> + * eMMC HS with preset_enabled set will trigger a bug in
>>> + * get_preset_value().
>>> + */
>>> + spin_lock_irqsave(&host->lock, flags);
>>> + if ((ios->timing == MMC_TIMING_MMC_HS400) ||
>>> + (ios->timing == MMC_TIMING_MMC_HS200) ||
>>> + (ios->timing == MMC_TIMING_MMC_HS)) {
>>> + host->preset_enabled = false;
>>> + host->quirks2 |= SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
>>> +
>>> + reg = sdhci_readw(host, SDHCI_HOST_CONTROL2);
>>> + reg &= ~SDHCI_CTRL_PRESET_VAL_ENABLE;
>>> + sdhci_writew(host, reg, SDHCI_HOST_CONTROL2);
>>> + } else {
>>> + host->quirks2 &= ~SDHCI_QUIRK2_PRESET_VALUE_BROKEN;
>>> + }
>>> + spin_unlock_irqrestore(&host->lock, flags);
>>
>> At some point we will have to get rid of SDHCI_QUIRK2_PRESET_VALUE_BROKEN
>> and add a callback instead.
>>
>
> Thanks for the information.
> I would like to keep this workaround here, before the detailed patch is brought up.
> Is it OK to you?
Sure
>
>>> +
> <snip>
>>> +static int xenon_start_signal_voltage_switch(struct mmc_host *mmc,
>>> + struct mmc_ios *ios)
>>> +{
>>> + struct sdhci_host *host = mmc_priv(mmc);
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + struct sdhci_xenon_priv *priv = sdhci_pltfm_priv(pltfm_host);
>>> +
>>> + /*
>>> + * Before SD/SDIO set signal voltage, SD bus clock should be
>>> + * disabled. However, sdhci_set_clock will also disable the Internal
>>> + * clock in mmc_set_signal_voltage().
>>> + * If Internal clock is disabled, the 3.3V/1.8V bit can not be updated.
>>> + * Thus here manually enable internal clock.
>>> + *
>>> + * After switch completes, it is unnecessary to disable internal clock,
>>> + * since keeping internal clock active obeys SD spec.
>>> + */
>>> + enable_xenon_internal_clk(host);
>>
>> We could try the attached patch.
>>
>
> I test your patch. It can work on my platforms.
> Thanks a lot.
>
> May I keep this workaround now?
> I would like to remove this workaround after your attached patch is applied.
Ok
>
>>> +
> <snip>
>>> +static int sdhci_xenon_remove(struct platform_device *pdev)
>>> +{
>>> + struct sdhci_host *host = platform_get_drvdata(pdev);
>>> + struct sdhci_pltfm_host *pltfm_host = sdhci_priv(host);
>>> + int dead = (readl(host->ioaddr + SDHCI_INT_STATUS) == 0xFFFFFFFF);
>>
>> This 'dead' check was originally for PCI I think. Unless you know it makes
>> sense for your device, I would leave it out. i.e. just do
>> sdhci_remove_host(host, 0);
>>
>
> Got it. I will remove it.
>
>>> +
>>> + xenon_sdhc_remove(host);
>>> +
>>> + sdhci_remove_host(host, dead);
>>> +
>>> + clk_disable_unprepare(pltfm_host->clk);
>>> +
>>> + sdhci_pltfm_free(pdev);
>>> +
>>> + return 0;
>>> +}
>>> +
>>> +static const struct of_device_id sdhci_xenon_dt_ids[] = {
>>> + { .compatible = "marvell,armada-7000-sdhci",},
>>> + { .compatible = "marvell,armada-3700-sdhci",},
>>> + {}
>>> +};
>>> +MODULE_DEVICE_TABLE(of, sdhci_xenon_dt_ids);
>>> +
>>> +static struct platform_driver sdhci_xenon_driver = {
>>> + .driver = {
>>> + .name = "xenon-sdhci",
>>> + .of_match_table = sdhci_xenon_dt_ids,
>>> + .pm = &sdhci_pltfm_pmops,
>>> + },
>>> + .probe = sdhci_xenon_probe,
>>> + .remove = sdhci_xenon_remove,
>>> +};
>>> +
>>> +module_platform_driver(sdhci_xenon_driver);
>>> +
>>> +MODULE_DESCRIPTION("SDHCI platform driver for Marvell Xenon SDHC");
>>> +MODULE_AUTHOR("Hu Ziji <huziji@marvell.com>");
>>> +MODULE_LICENSE("GPL v2");
>>> diff --git a/drivers/mmc/host/sdhci-xenon.h b/drivers/mmc/host/sdhci-xenon.h
>>> new file mode 100644
>>> index 000000000000..d50cd663a265
>>> --- /dev/null
>>> +++ b/drivers/mmc/host/sdhci-xenon.h
>>> @@ -0,0 +1,70 @@
>>> +/*
>>> + * Copyright (C) 2016 Marvell, All Rights Reserved.
>>> + *
>>> + * Author: Hu Ziji <huziji@marvell.com>
>>> + * Date: 2016-8-24
>>> + *
>>> + * This program is free software; you can redistribute it and/or
>>> + * modify it under the terms of the GNU General Public License as
>>> + * published by the Free Software Foundation version 2.
>>> + */
>>> +#ifndef SDHCI_XENON_H_
>>> +#define SDHCI_XENON_H_
>>> +
>>> +
>>
>> Double blank line
>>
> Will remove it.
>
>>> +/* Register Offset of Xenon SDHC self-defined register */
>>> +#define SDHCI_SYS_CFG_INFO 0x0104
>>
>> A lot of these defines look like they could be just in sdhci-xenon.c or
>> sdhci-xenon-phy.c. It is also a little odd that they are prefixed by
>> "SDHCI" because they are not standard. "XENON" would be better.
>>
>
> Some registers are accessed by bother sdhci-xenon.c and sdhci-xenon-phy.c.
> As a result, I list all the registers here in sdhci-xenon.h, for convenience.
>
> Previously, Ulf asked me to prefix them all with "SDHCI".
> I would like to know which prefix sounds more reasonable, "XENON_" or "SDHCI_XENON_"?
I would use "XENON_"
>
>>> +#define SDHCI_SLOT_TYPE_SDIO_SHIFT 24
>>> +#define SDHCI_NR_SUPPORTED_SLOT_MASK 0x7
>>> +
>>> +#define SDHCI_SYS_OP_CTRL 0x0108
>>> +#define SDHCI_AUTO_CLKGATE_DISABLE_MASK BIT(20)
>>> +#define SDHCI_SDCLK_IDLEOFF_ENABLE_SHIFT 8
>>> +#define SDHCI_SLOT_ENABLE_SHIFT 0
>>> +
>>> +#define SDHCI_SYS_EXT_OP_CTRL 0x010C
>>> +
>>> +#define SDHCI_SLOT_EMMC_CTRL 0x0130
>>> +#define SDHCI_EMMC_VCCQ_MASK 0x3
>>> +#define SDHCI_EMMC_VCCQ_1_8V 0x1
>>> +#define SDHCI_EMMC_VCCQ_3_3V 0x3
>>> +
>>> +#define SDHCI_SLOT_RETUNING_REQ_CTRL 0x0144
>>> +/* retuning compatible */
>>> +#define SDHCI_RETUNING_COMPATIBLE 0x1
>>> +
>>> +/* Tuning Parameter */
>>> +#define SDHCI_TMR_RETUN_NO_PRESENT 0xF
>>> +#define SDHCI_DEF_TUNING_COUNT 0x9
>>> +
>>> +#define SDHCI_DEFAULT_SDCLK_FREQ (400000)
>>
>> Unnecessary ()
>>
> Will fix it.
>
> Thanks a lot for the review.
>
> Best regards,
> Hu Ziji
>
>>> +
>>> +/* Xenon specific Mode Select value */
>>> +#define SDHCI_XENON_CTRL_HS200 0x5
>>> +#define SDHCI_XENON_CTRL_HS400 0x6
>>> +
>>> +/* Indicate Card Type is not clear yet */
>>> +#define SDHCI_CARD_TYPE_UNKNOWN 0xF
>>> +
>>> +struct sdhci_xenon_priv {
>>> + unsigned char tuning_count;
>>> + /* idx of SDHC */
>>> + u8 sdhc_id;
>>> +
>>> + /*
>>> + * eMMC/SD/SDIO require different PHY settings or
>>> + * voltage control. It's necessary for Xenon driver to
>>> + * recognize card type during, or even before initialization.
>>> + * However, mmc_host->card is not available yet at that time.
>>> + * This field records the card type during init.
>>> + * For eMMC, it is updated in dt parse. For SD/SDIO, it is
>>> + * updated in xenon_init_card().
>>> + *
>>> + * It is only valid during initialization after it is updated.
>>> + * Do not access this variable in normal transfers after
>>> + * initialization completes.
>>> + */
>>> + unsigned int init_card_type;
>>> +};
>>> +
>>> +#endif
>>>
>>
>
^ permalink raw reply
* [PATCH] drm/meson: Fix plane atomic check when no crtc for the plane
From: Daniel Vetter @ 2017-01-04 9:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1483435254-27955-1-git-send-email-narmstrong@baylibre.com>
On Tue, Jan 03, 2017 at 10:20:54AM +0100, Neil Armstrong wrote:
> When no CRTC is associated with the plane, the meson_plane_atomic_check()
> call breaks the kernel with an Oops.
>
> Fixes: bbbe775ec5b5 ("drm: Add support for Amlogic Meson Graphic Controller")
> Signed-off-by: Neil Armstrong <narmstrong@baylibre.com>
Makes sense, pls send a pull request with this to Dave.
-Daniel
> ---
> drivers/gpu/drm/meson/meson_plane.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/gpu/drm/meson/meson_plane.c b/drivers/gpu/drm/meson/meson_plane.c
> index 4942ca0..7890e30 100644
> --- a/drivers/gpu/drm/meson/meson_plane.c
> +++ b/drivers/gpu/drm/meson/meson_plane.c
> @@ -51,6 +51,9 @@ static int meson_plane_atomic_check(struct drm_plane *plane,
> struct drm_crtc_state *crtc_state;
> struct drm_rect clip = { 0, };
>
> + if (!state->crtc)
> + return 0;
> +
> crtc_state = drm_atomic_get_crtc_state(state->state, state->crtc);
> if (IS_ERR(crtc_state))
> return PTR_ERR(crtc_state);
> --
> 1.9.1
>
> _______________________________________________
> dri-devel mailing list
> dri-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
^ permalink raw reply
* [PATCH v2 2/2] arm: perf: Mark as non-removable
From: Alexander Stein @ 2017-01-04 9:19 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161222224547.GA30170@remoulade>
Hi,
On Thursday 22 December 2016 22:48:32, Mark Rutland wrote:
> On Wed, Dec 21, 2016 at 04:03:40PM +0100, Alexander Stein wrote:
> > This driver can only built into the kernel. So disallow driver bind/unbind
> > and also prevent a kernel error in case DEBUG_TEST_DRIVER_REMOVE is
> > enabled.
> >
> > Signed-off-by: Alexander Stein <alexander.stein@systec-electronic.com>
> > ---
> >
> > arch/arm/kernel/perf_event_v7.c | 1 +
> > 1 file changed, 1 insertion(+)
> >
> > diff --git a/arch/arm/kernel/perf_event_v7.c
> > b/arch/arm/kernel/perf_event_v7.c index b942349..795e373 100644
> > --- a/arch/arm/kernel/perf_event_v7.c
> > +++ b/arch/arm/kernel/perf_event_v7.c
> > @@ -2029,6 +2029,7 @@ static int armv7_pmu_device_probe(struct
> > platform_device *pdev)>
> > static struct platform_driver armv7_pmu_driver = {
> >
> > .driver = {
> >
> > .name = "armv7-pmu",
> >
> > + .suppress_bind_attrs = true,
> >
> > .of_match_table = armv7_pmu_of_device_ids,
> >
> > },
>
> While this patch looks correct, the other perf_event_* drivers (e.g. those
> under arch/arm/) will need similar treatment.
Yep, but this is the only one I can actually test.
> More generally, updating each and every driver in this manner seems like a
> scattergun approach that is tiresome and error prone.
>
> IMO, it would be vastly better for a higher layer to enforce that we don't
> attempt to unbind drivers where the driver does not have a remove callback,
> as is the case here (and I suspect most over cases where
> DEBUG_TEST_DRIVER_REMOVE is blowing up).
You mean something like this?
> diff --git a/drivers/base/driver.c b/drivers/base/driver.c
> index 4eabfe2..3b6c1a2d 100644
> --- a/drivers/base/driver.c
> +++ b/drivers/base/driver.c
> @@ -158,6 +158,9 @@ int driver_register(struct device_driver *drv)
>
> printk(KERN_WARNING "Driver '%s' needs updating - please use
> "
>
> "bus_type methods\n", drv->name);
>
> + if (!drv->remove)
> + drv->suppress_bind_attrs = true;
> +
>
> other = driver_find(drv->name, drv->bus);
> if (other) {
>
> printk(KERN_ERR "Error: Driver '%s' is already registered, "
> Is there any reason that can't be enforced at the bus layer, say?
I'm not sure if the change above works with remove functions set in struct
bus_type too.
But on the other hand this would hide errors in drivers which are actually
removable but do not cleanup properly which DEBUG_TEST_DRIVER_REMOVE tries to
detect. By setting .suppress_bind_attrs = true explicitely you state "This
driver cannot be removed!", so the remove callback is not missing by accident.
Best regards,
Alexander
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox