* Re: [PATCH 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
From: Marek Vasut @ 2017-03-29 13:09 UTC (permalink / raw)
To: Ludovic BARRE, Cyrille Pitchen
Cc: David Woodhouse, Brian Norris, Boris Brezillon,
Richard Weinberger, Alexandre Torgue, Rob Herring,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <2c364b99-512c-8eb6-7044-7989ba21d53b-qxv4g6HH51o@public.gmane.org>
On 03/29/2017 02:24 PM, Ludovic BARRE wrote:
> hi Marek
Hi!
> thanks for review
> comment below
>
> On 03/29/2017 12:54 PM, Marek Vasut wrote:
>> On 03/27/2017 02:54 PM, Ludovic Barre wrote:
>>> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>>
>>> The quadspi is a specialized communication interface targeting single,
>>> dual or quad SPI Flash memories.
>>>
>>> It can operate in any of the following modes:
>>> -indirect mode: all the operations are performed using the quadspi
>>> registers
>>> -read memory-mapped mode: the external Flash memory is mapped to the
>>> microcontroller address space and is seen by the system as if it was
>>> an internal memory
>>>
>>> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>> Hi! I presume this is not compatible with the Cadence QSPI or any other
>> QSPI controller, huh ?
> it's not a cadence QSPI, it's specific for stm32 platform
OK, got it. Didn't ST use Cadence IP somewhere too though ?
That's probably what confused me, oh well ...
>> Mostly minor nits below ...
>>
>>> ---
>>> drivers/mtd/spi-nor/Kconfig | 7 +
>>> drivers/mtd/spi-nor/Makefile | 1 +
>>> drivers/mtd/spi-nor/stm32-quadspi.c | 679
>>> ++++++++++++++++++++++++++++++++++++
>>> 3 files changed, 687 insertions(+)
>>> create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
>> [...]
>>
>>> +struct stm32_qspi_cmd {
>>> + struct {
>>> + u32 addr_width:8;
>>> + u32 dummy:8;
>>> + u32 data:1;
>>> + } conf;
>> This could all be u8 ? Why this awful construct ?
> yes I could replace each u32 by u8
Yeah, please do .
>>> + u8 opcode;
>>> + u32 framemode;
>>> + u32 qspimode;
>>> + u32 addr;
>>> + size_t len;
>>> + void *buf;
>>> +};
>>> +
>>> +static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi)
>>> +{
>>> + u32 cr;
>>> + int err = 0;
>> Can readl_poll_timeout() or somesuch replace this function ?
> in fact stm32_qspi_wait_cmd test if the transfer has been complete (TCF
> flag)
> else initialize an interrupt completion.
> like the time may be long I prefer keep the interrupt wait, if you agree.
I don't really mind if it fits the hardware better and I agree with you
it does here.
>>> + if (readl_relaxed(qspi->io_base + QUADSPI_SR) & SR_TCF)
>>> + return 0;
>>> +
>>> + reinit_completion(&qspi->cmd_completion);
>>> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>> + writel_relaxed(cr | CR_TCIE, qspi->io_base + QUADSPI_CR);
>>> +
>>> + if
>>> (!wait_for_completion_interruptible_timeout(&qspi->cmd_completion,
>>> + msecs_to_jiffies(1000)))
>>> + err = -ETIMEDOUT;
>>> +
>>> + writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
>>> + return err;
>>> +}
>>> +
>>> +static int stm32_qspi_wait_nobusy(struct stm32_qspi *qspi)
>>> +{
>>> + u32 sr;
>>> +
>>> + return readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR, sr,
>>> + !(sr & SR_BUSY), 10, 100000);
>>> +}
>>> +
>>> +static void stm32_qspi_set_framemode(struct spi_nor *nor,
>>> + struct stm32_qspi_cmd *cmd, bool read)
>>> +{
>>> + u32 dmode = CCR_DMODE_1;
>>> +
>>> + cmd->framemode = CCR_IMODE_1;
>>> +
>>> + if (read) {
>>> + switch (nor->flash_read) {
>>> + case SPI_NOR_NORMAL:
>>> + case SPI_NOR_FAST:
>>> + dmode = CCR_DMODE_1;
>>> + break;
>>> + case SPI_NOR_DUAL:
>>> + dmode = CCR_DMODE_2;
>>> + break;
>>> + case SPI_NOR_QUAD:
>>> + dmode = CCR_DMODE_4;
>>> + break;
>>> + }
>>> + }
>>> +
>>> + cmd->framemode |= cmd->conf.data ? dmode : 0;
>>> + cmd->framemode |= cmd->conf.addr_width ? CCR_ADMODE_1 : 0;
>>> +}
>>> +
>>> +static void stm32_qspi_read_fifo(u8 *val, void __iomem *addr)
>>> +{
>>> + *val = readb_relaxed(addr);
>>> +}
>>> +
>>> +static void stm32_qspi_write_fifo(u8 *val, void __iomem *addr)
>>> +{
>>> + writeb_relaxed(*val, addr);
>>> +}
>>> +
>>> +static int stm32_qspi_tx_poll(struct stm32_qspi *qspi,
>>> + const struct stm32_qspi_cmd *cmd)
>>> +{
>>> + void (*tx_fifo)(u8 *, void __iomem *);
>>> + u32 len = cmd->len, sr;
>>> + u8 *buf = cmd->buf;
>>> + int ret;
>>> +
>>> + if (cmd->qspimode == CCR_FMODE_INDW)
>>> + tx_fifo = stm32_qspi_write_fifo;
>>> + else
>>> + tx_fifo = stm32_qspi_read_fifo;
>>> +
>>> + while (len--) {
>>> + ret = readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR,
>>> + sr, (sr & SR_FTF),
>>> + 10, 30000);
>> Add macros for those ad-hoc timeouts.
> I will add 2 defines (for stm32_qspi_wait_nobusy, stm32_qspi_tx_poll)
> #define STM32_QSPI_FIFO_TIMEOUT_US 30000
> #define STM32_QSPI_BUSY_TIMEOUT_US 100000
Super
>>> + if (ret) {
>>> + dev_err(qspi->dev, "fifo timeout (stat:%#x)\n", sr);
>>> + break;
>>> + }
>>> + tx_fifo(buf++, qspi->io_base + QUADSPI_DR);
>>> + }
>>> +
>>> + return ret;
>>> +}
>>
>> [...]
>>
>>> +static int stm32_qspi_send(struct stm32_qspi_flash *flash,
>>> + const struct stm32_qspi_cmd *cmd)
>>> +{
>>> + struct stm32_qspi *qspi = flash->qspi;
>>> + u32 ccr, dcr, cr;
>>> + int err;
>>> +
>>> + err = stm32_qspi_wait_nobusy(qspi);
>>> + if (err)
>>> + goto abort;
>>> +
>>> + dcr = readl_relaxed(qspi->io_base + QUADSPI_DCR) & ~DCR_FSIZE_MASK;
>>> + dcr |= DCR_FSIZE(flash->fsize);
>>> + writel_relaxed(dcr, qspi->io_base + QUADSPI_DCR);
>>> +
>>> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>>> + cr &= ~CR_PRESC_MASK & ~CR_FSEL;
>>> + cr |= CR_PRESC(flash->presc);
>>> + cr |= flash->cs ? CR_FSEL : 0;
>>> + writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
>>> +
>>> + if (cmd->conf.data)
>>> + writel_relaxed(cmd->len - 1, qspi->io_base + QUADSPI_DLR);
>>> +
>>> + ccr = cmd->framemode | cmd->qspimode;
>>> +
>>> + if (cmd->conf.dummy)
>>> + ccr |= CCR_DCYC(cmd->conf.dummy);
>>> +
>>> + if (cmd->conf.addr_width)
>>> + ccr |= CCR_ADSIZE(cmd->conf.addr_width - 1);
>>> +
>>> + ccr |= CCR_INST(cmd->opcode);
>>> + writel_relaxed(ccr, qspi->io_base + QUADSPI_CCR);
>>> +
>>> + if (cmd->conf.addr_width && cmd->qspimode != CCR_FMODE_MM)
>>> + writel_relaxed(cmd->addr, qspi->io_base + QUADSPI_AR);
>>> +
>>> + err = stm32_qspi_tx(qspi, cmd);
>>> + if (err)
>>> + goto abort;
>>> +
>>> + if (cmd->qspimode != CCR_FMODE_MM) {
>>> + err = stm32_qspi_wait_cmd(qspi);
>>> + if (err)
>>> + goto abort;
>>> + writel_relaxed(FCR_CTCF, qspi->io_base + QUADSPI_FCR);
>>> + }
>>> +
>>> + return err;
>>> +
>>> +abort:
>>> + writel_relaxed(readl_relaxed(qspi->io_base + QUADSPI_CR)
>>> + | CR_ABORT, qspi->io_base + QUADSPI_CR);
>> Use a helper variable here too.
> ok
>>> + dev_err(qspi->dev, "%s abort err:%d\n", __func__, err);
>>> + return err;
>>> +}
>> [...]
>>
>>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>>> + struct device_node *np)
>>> +{
>>> + u32 width, flash_read, presc, cs_num, max_rate = 0;
>>> + struct stm32_qspi_flash *flash;
>>> + struct mtd_info *mtd;
>>> + int ret;
>>> +
>>> + of_property_read_u32(np, "reg", &cs_num);
>>> + if (cs_num >= STM32_MAX_NORCHIP)
>>> + return -EINVAL;
>>> +
>>> + of_property_read_u32(np, "spi-max-frequency", &max_rate);
>>> + if (!max_rate)
>>> + return -EINVAL;
>>> +
>>> + presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>>> +
>>> + if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>>> + width = 1;
>>> +
>>> + if (width == 4)
>>> + flash_read = SPI_NOR_QUAD;
>>> + else if (width == 2)
>>> + flash_read = SPI_NOR_DUAL;
>>> + else if (width == 1)
>>> + flash_read = SPI_NOR_NORMAL;
>>> + else
>>> + return -EINVAL;
>>> +
>>> + flash = &qspi->flash[cs_num];
>>> + flash->qspi = qspi;
>>> + flash->cs = cs_num;
>>> + flash->presc = presc;
>>> +
>>> + flash->nor.dev = qspi->dev;
>>> + spi_nor_set_flash_node(&flash->nor, np);
>>> + flash->nor.priv = flash;
>>> + mtd = &flash->nor.mtd;
>>> + mtd->priv = &flash->nor;
>>> +
>>> + flash->nor.read = stm32_qspi_read;
>>> + flash->nor.write = stm32_qspi_write;
>>> + flash->nor.erase = stm32_qspi_erase;
>>> + flash->nor.read_reg = stm32_qspi_read_reg;
>>> + flash->nor.write_reg = stm32_qspi_write_reg;
>>> + flash->nor.prepare = stm32_qspi_prep;
>>> + flash->nor.unprepare = stm32_qspi_unprep;
>>> +
>>> + writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>>> +
>>> + writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
>>> + | CR_EN, qspi->io_base + QUADSPI_CR);
>>> +
>>> + /* a minimum fsize must be set to sent the command id */
>>> + flash->fsize = 25;
>> I don't understand why this is needed and the comment doesn't make
>> sense. Please fix.
> fsize field defines the size of external memory.
What external memory ? Unclear
> Normaly, this field is used only for memory map mode,
> but in fact is check in indirect mode.
> So while flash scan "spi_nor_scan":
> -I can't let 0.
> -I not know yet the size of flash.
> So I fix a temporary value
>
> I will update my comment
Please do, also please consider that I'm reading the comment and I
barely have any clue about this hardware , so make sure I can understand it.
>>> + ret = spi_nor_scan(&flash->nor, NULL, flash_read);
>>> + if (ret) {
>>> + dev_err(qspi->dev, "device scan failed\n");
>>> + return ret;
>>> + }
>>> +
>>> + flash->fsize = __fls(mtd->size) - 1;
>>> +
>>> + writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
>>> +
>>> + ret = mtd_device_register(mtd, NULL, 0);
>>> + if (ret) {
>>> + dev_err(qspi->dev, "mtd device parse failed\n");
>>> + return ret;
>>> + }
>>> +
>>> + dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
>>> + qspi->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
>>> +
>>> + return 0;
>>> +}
>> [...]
>>
>
--
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc
From: Linus Walleij @ 2017-03-29 13:04 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: jacopo, Jacopo Mondi, Geert Uytterhoeven, Laurent Pinchart,
Chris Brandt, Rob Herring, Mark Rutland, Russell King,
Linux-Renesas, linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdX0OdZXYDOZvFutjfBFVd65C567_G+++kHygre4KK+jLg@mail.gmail.com>
On Wed, Mar 29, 2017 at 1:20 PM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
>> I do not understand the notion of "flags" here. I hope that is not referring
>
> Flags refers to BI_DIR, SWIO_IN, and SWIO_OUT, from
> https://patchwork.kernel.org/patch/9643047/
Aha I will go in and review that closer because it doesn't seem right.
Sorry for missing it.
>> i2c1_pins_a: i2c1@0 {
>> pins {
>> pinmux = <MT8135_PIN_195_SDA1__FUNC_SDA1>,
>> <MT8135_PIN_196_SCL1__FUNC_SCL1>;
>
> If we follow this example, then we can list all combinations in
> include/dt-bindings/pinctrl/r7s72100-pinctrl.h, instead of creating the value
> by combining the bits using a macro where we need it in the DTS.
>
> It's gonna be a long list, though...
Size is not the issue, readability is the issue. I don't see why you would
need to list "all combinations" since the trees go through the C preprocessor
so you can use macros and bit | OR to build them?
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH v7 1/7] clocksource/drivers/clksrc-evt-probe: Describe with the DT both the clocksource and the clockevent
From: Mark Rutland @ 2017-03-29 12:57 UTC (permalink / raw)
To: Daniel Lezcano
Cc: Rob Herring, Alexander Kochetkov, Heiko Stuebner, linux-kernel,
devicetree, linux-arm-kernel, linux-rockchip, Thomas Gleixner,
Russell King, Caesar Wang, Huang Tao
In-Reply-To: <20170329123638.GI2123@mai>
On Wed, Mar 29, 2017 at 02:36:38PM +0200, Daniel Lezcano wrote:
> On Wed, Mar 29, 2017 at 11:49:11AM +0100, Mark Rutland wrote:
> > On Wed, Mar 29, 2017 at 11:22:10AM +0200, Daniel Lezcano wrote:
> > > On Tue, Mar 28, 2017 at 08:51:46PM -0500, Rob Herring wrote:
> > > > On Wed, Mar 22, 2017 at 06:48:28PM +0300, Alexander Kochetkov wrote:
>
> You can have several timers on the system and may want to use the clockevents
> from one IP block and the clocksource from another IP block. For example, in
> the case of a bogus clocksource.
I understand this. However, what I was trying to say is that *how* we
use a particular device should be a software decision. I have a more
concrete suggestion on that below.
> Moreover there are some drivers with a node for a clocksource and
> another one for the clockevent, and the driver is assuming the clockevent is
> defined first and then the clocksource.
>
> eg.
>
> arch/arc/boot/dts/abilis_tb10x.dtsi:
>
> /* TIMER0 with interrupt for clockevent */
> timer0 {
> compatible = "snps,arc-timer";
> interrupts = <3>;
> interrupt-parent = <&intc>;
> clocks = <&cpu_clk>;
> };
>
> /* TIMER1 for free running clocksource */
> timer1 {
> compatible = "snps,arc-timer";
> clocks = <&cpu_clk>;
> };
>
> drivers/clocksource/arc_timer.c:
>
> static int __init arc_of_timer_init(struct device_node *np)
> {
> static int init_count = 0;
> int ret;
>
> if (!init_count) {
> init_count = 1;
> ret = arc_clockevent_setup(np);
> } else {
> ret = arc_cs_setup_timer1(np);
> }
>
> return ret;
> }
>
> Even if that works, it is a fragile mechanism.
>
> So the purpose of these changes is to provide a stronger timer declaration in
> order to clearly split in the kernel a clocksource and a clockevent
> initialization.
I agree that this pattern is not nice. However, I think that splitting
devices as this level makes the problem *worse*.
Users care that they have a clocksource and a clockevent device. They
do not care *which* particular device is used as either. The comments in
the DT above are at best misleading.
What we need is for the kernel to understand that devices can be both
clockevent and clocksource (perhaps mutually exclusively), such that the
kernel can decide how to make use of devices.
That way, for the above the kernel can figure out that timer0 could be
used as clocksource or clockevent, while timer1 can only be used as a
clocksource due to the lack of an interrupt. Thus, it can choose to use
timer0 as a clockevent, and timer1 and a clocksource.
> > > > > With this approach, we allow a mechanism to clearly define a clocksource or a
> > > > > clockevent without aerobatics we can find around in some drivers:
> > > > > timer-sp804.c, arc-timer.c, dw_apb_timer_of.c, mps2-timer.c,
> > > > > renesas-ostm.c, time-efm32.c, time-lpc32xx.c.
> > > >
> > > > These all already have bindings and work. What problem are you trying to
> > > > solve other than restructuring Linux?
> > >
> > > Yes, there is already the bindings, but that force to do some hackish
> > > initialization.
> >
> > Here, you are forcing hackish DT changes that do not truly describe HW.
> > How is that better?
>
> So if this is hackish DT changes, then the existing DTs should be fixed, no?
Yes.
For the above snippet, the only thing that needs to change is the
comment.
> > > I would like to give the opportunity to declare separately a clocksource and a
> > > clockevent, in order to have full control of how this is initialized.
> >
> > To me it sounds like what we need is Linux infrastructure that allows
> > one to register a device as having both clockevent/clocksource
> > functionality.
>
> That was the idea. Create a macro CLOCKEVENT_OF and CLOCKSOURCE_OF both of them
> calling their respective init routines. And in addition a TIMER_OF doing both
> CLOCKEVENT_OF and CLOCKSOURCE_OF.
>
> It is the DT which does not allow to do this separation.
>
> Would be the following approach more acceptable ?
>
> 1. Replace all CLOCKSOURCE_OF by TIMER_OF (just renaming)
I am fine with this renaming.
> 2. A node can have a clockevent and|or a clocksource attributes
As above, this should not be in the DT given it's describing a
(Linux-specific) SW policy and not a HW detail.
So I must disagree with this.
> 3. The timer_probe pass a flag to the driver's init function, so this one knows
> if it should invoke the clockevent/clocksource init functions.
> No attribute defaults to clocksource|clockevent.
>
> That would be backward compatible and will let to create drivers with clutch
> activated device via DT. Also, it will give the opportunity to the existing
> drivers to change consolidate their initialization routines.
I think that if anything, we need a combined clocksource+clockevent
device that we register to the core code. That means all
clocksource/clockevent drivers have a consolidated routine.
Subsequently, core code should determine how specifically to use the
device (e.g. based on what other devices are registered, and their
capabilities).
Thanks,
Mark.
^ permalink raw reply
* Re: [PATCH v3 2/2] PCI: Add tango PCIe host bridge support
From: Mason @ 2017-03-29 12:53 UTC (permalink / raw)
To: Robin Murphy
Cc: Marc Gonzalez, Bjorn Helgaas, Marc Zyngier, Thomas Gleixner,
Lorenzo Pieralisi, Liviu Dudau, David Laight, linux-pci,
Linux ARM, Thibaud Cornic, Phuong Nguyen, LKML, DT
In-Reply-To: <01516ad9-e187-4bac-7c65-a7a90c576ce2@arm.com>
On 29/03/2017 14:19, Robin Murphy wrote:
> On 29/03/17 12:34, Marc Gonzalez wrote:
>
>> + /*
>> + * QUIRK #3
>> + * Unfortunately, config and mem spaces are muxed.
>> + * Linux does not support such a setting, since drivers are free
>> + * to access mem space directly, at any time.
>> + * Therefore, we can only PRAY that config and mem space accesses
>> + * NEVER occur concurrently.
>> + */
>
> What about David's suggestion of using an IPI for safe mutual exclusion?
I was left with the impression that this wouldn't solve the problem.
If a mem space access is "in flight" on core0 when core1 starts a
config space access, an IPI will not prevent breakage.
Did I misunderstand?
For my education, what is the API to send an IPI?
And the API to handle an IPI?
>> + if (of_device_is_compatible(dev->of_node, "sigma,smp8759-pcie"))
>> + smp8759_init(pcie, base);
>
> ...then retrieve it with of_device_get_match_data() here. No need to
> reinvent the wheel (or have to worry about the ordering of multiple
> compatibles once rev. n+1 comes around).
I actually asked about this on IRC. The consensus was "use what
best fits your use case". I need to do some processing based on
the revision, so I thought
if (chip_x)
do_chip_x_init()
was a good way to express my intent. Did I misunderstand?
For example, the init function for rev2 currently looks like this:
static void rev2_init(struct tango_pcie *pcie, void __iomem *base)
{
void __iomem *misc_irq = base + 0x40;
void __iomem *doorbell = base + 0x8c;
pcie->mux = base + 0x2c;
pcie->msi_status = base + 0x4c;
pcie->msi_mask = base + 0x6c;
pcie->msi_doorbell = 0x80000000;
writel(lower_32_bits(pcie->msi_doorbell), doorbell + 0);
writel(upper_32_bits(pcie->msi_doorbell), doorbell + 4);
/* Enable legacy PCI interrupts */
writel(BIT(15), misc_irq);
writel(0xf << 4, misc_irq + 4);
}
>> +#define VENDOR_SIGMA 0x1105
>
> Should this not be in include/linux/pci_ids.h?
Doh! Very likely. Thanks.
Regards.
^ permalink raw reply
* [PATCH 4/4] ARM: dts: rockchip: enable ARM Mali GPU on rk3288-firefly
From: Guillaume Tucker @ 2017-03-29 12:45 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Sjoerd Simons,
Enric Balletbo i Serra, John Reitan, Guillaume Tucker
In-Reply-To: <cover.1490789802.git.guillaume.tucker-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Add reference to the Mali GPU device tree node on rk3288-firefly.
Tested on Firefly board.
Signed-off-by: Guillaume Tucker <guillaume.tucker-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
arch/arm/boot/dts/rk3288-firefly.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/rk3288-firefly.dtsi b/arch/arm/boot/dts/rk3288-firefly.dtsi
index 10793ac18599..f520589493b4 100644
--- a/arch/arm/boot/dts/rk3288-firefly.dtsi
+++ b/arch/arm/boot/dts/rk3288-firefly.dtsi
@@ -594,3 +594,8 @@
&wdt {
status = "okay";
};
+
+&gpu {
+ mali-supply = <&vdd_gpu>;
+ status = "okay";
+};
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 3/4] ARM: dts: rockchip: enable ARM Mali GPU on rk3288-rock2-som
From: Guillaume Tucker @ 2017-03-29 12:45 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Sjoerd Simons,
Enric Balletbo i Serra, John Reitan, Guillaume Tucker
In-Reply-To: <cover.1490789802.git.guillaume.tucker-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Add reference to the Mali GPU device tree node on the
rk3288-rock2-som platform. Tested on a Radxa Rock2 Square board.
Signed-off-by: Guillaume Tucker <guillaume.tucker-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
arch/arm/boot/dts/rk3288-rock2-som.dtsi | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/arch/arm/boot/dts/rk3288-rock2-som.dtsi b/arch/arm/boot/dts/rk3288-rock2-som.dtsi
index 1c0bbc9b928b..f694867fa46a 100644
--- a/arch/arm/boot/dts/rk3288-rock2-som.dtsi
+++ b/arch/arm/boot/dts/rk3288-rock2-som.dtsi
@@ -301,3 +301,8 @@
&wdt {
status = "okay";
};
+
+&gpu {
+ mali-supply = <&vdd_gpu>;
+ status = "okay";
+};
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 2/4] ARM: dts: rockchip: add ARM Mali GPU node for rk3288
From: Guillaume Tucker @ 2017-03-29 12:44 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Sjoerd Simons,
Enric Balletbo i Serra, John Reitan, Guillaume Tucker
In-Reply-To: <cover.1490789802.git.guillaume.tucker-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Add Mali GPU device tree node for the rk3288 SoC, with devfreq
opp table.
Signed-off-by: Guillaume Tucker <guillaume.tucker-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
arch/arm/boot/dts/rk3288.dtsi | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/arch/arm/boot/dts/rk3288.dtsi b/arch/arm/boot/dts/rk3288.dtsi
index df8a0dbe9d91..213224ea309e 100644
--- a/arch/arm/boot/dts/rk3288.dtsi
+++ b/arch/arm/boot/dts/rk3288.dtsi
@@ -43,6 +43,7 @@
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/pinctrl/rockchip.h>
#include <dt-bindings/clock/rk3288-cru.h>
+#include <dt-bindings/power/rk3288-power.h>
#include <dt-bindings/thermal/thermal.h>
#include <dt-bindings/power/rk3288-power.h>
#include <dt-bindings/soc/rockchip,boot-mode.h>
@@ -227,6 +228,28 @@
ports = <&vopl_out>, <&vopb_out>;
};
+ gpu: mali@ffa30000 {
+ compatible = "arm,mali-midgard";
+ reg = <0xffa30000 0x10000>;
+ interrupts = <GIC_SPI 6 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 7 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 8 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "JOB", "MMU", "GPU";
+ clocks = <&cru ACLK_GPU>;
+ clock-names = "clk_mali";
+ operating-points = <
+ /* KHz uV */
+ 100000 950000
+ 200000 950000
+ 300000 1000000
+ 400000 1100000
+ 500000 1200000
+ 600000 1250000
+ >;
+ power-domains = <&power RK3288_PD_GPU>;
+ status = "disabled";
+ };
+
sdmmc: dwmmc@ff0c0000 {
compatible = "rockchip,rk3288-dw-mshc";
max-frequency = <150000000>;
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 1/4] dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU
From: Guillaume Tucker @ 2017-03-29 12:44 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Sjoerd Simons,
Enric Balletbo i Serra, John Reitan, Guillaume Tucker
In-Reply-To: <cover.1490789802.git.guillaume.tucker-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
The ARM Mali Midgard GPU family is present in a number of SoCs
from many different vendors such as Samsung Exynos and Rockchip.
Import the device tree bindings documentation from the r16p0
release of the Mali Midgard GPU kernel driver:
https://developer.arm.com/-/media/Files/downloads/mali-drivers/kernel/mali-midgard-gpu/TX011-SW-99002-r16p0-00rel0.tgz
The following optional bindings have been omitted in this initial
version as they are only used in very specific cases:
* snoop_enable_smc
* snoop_disable_smc
* jm_config
* power_model
* system-coherency
* ipa-model
The example has been simplified accordingly.
The compatible string definition has been limited to
"arm,mali-midgard" to avoid checkpatch.pl warnings and to match
what the driver actually expects (as of r16p0 out-of-tree).
CC: John Reitan <john.reitan-5wv7dgnIgG8@public.gmane.org>
Signed-off-by: Guillaume Tucker <guillaume.tucker-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
---
.../devicetree/bindings/gpu/arm,mali-midgard.txt | 53 ++++++++++++++++++++++
1 file changed, 53 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
diff --git a/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
new file mode 100644
index 000000000000..da8fc6d21bbf
--- /dev/null
+++ b/Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
@@ -0,0 +1,53 @@
+#
+# (C) COPYRIGHT 2013-2016 ARM Limited.
+# Copyright (C) 2017 Collabora Ltd
+#
+# This program is free software and is provided to you under the terms of the
+# GNU General Public License version 2 as published by the Free Software
+# Foundation, and any use by you of this program is subject to the terms
+# of such GNU licence.
+#
+
+
+ARM Mali Midgard GPU
+====================
+
+Required properties:
+
+- compatible : Should be "arm,mali-midgard".
+- reg : Physical base address of the device and length of the register area.
+- interrupts : Contains the three IRQ lines required by Mali Midgard devices.
+- interrupt-names : Contains the names of IRQ resources in the order they were
+ provided in the interrupts property. Must contain: "JOB, "MMU", "GPU".
+
+Optional:
+
+- clocks : Phandle to clock for the Mali Midgard device.
+- clock-names : Shall be "clk_mali".
+- mali-supply : Phandle to regulator for the Mali device. Refer to
+ Documentation/devicetree/bindings/regulator/regulator.txt for details.
+- operating-points : Refer to Documentation/devicetree/bindings/power/opp.txt
+ for details.
+
+Example for a Mali-T602:
+
+gpu@0xfc010000 {
+ compatible = "arm,mali-midgard";
+ reg = <0xfc010000 0x4000>;
+ interrupts = <0 36 4>, <0 37 4>, <0 38 4>;
+ interrupt-names = "JOB", "MMU", "GPU";
+
+ clocks = <&pclk_mali>;
+ clock-names = "clk_mali";
+ mali-supply = <&vdd_mali>;
+ operating-points = <
+ /* KHz uV */
+ 533000 1250000,
+ 450000 1150000,
+ 400000 1125000,
+ 350000 1075000,
+ 266000 1025000,
+ 160000 925000,
+ 100000 912500,
+ >;
+};
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH 0/4] Add ARM Mali Midgard device tree bindings and gpu node for rk3288
From: Guillaume Tucker @ 2017-03-29 12:44 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Sjoerd Simons,
Enric Balletbo i Serra, John Reitan, Guillaume Tucker
The ARM Mali Midgard GPU kernel driver is only available
out-of-tree and is not going to be merged in its current form.
However, it would be useful to have its device tree bindings
merged. In particular, this would enable distributions to create
working driver packages (dkms...) without having to patch the
kernel.
The bindings for the earlier Mali Utgard GPU family have already
been merged, so this is essentially the same scenario but for
newer GPUs (Mali-T604 ~ Mali-T880).
This series of patches first imports the bindings from the latest
driver release with some clean-up then adds a gpu node for the
rk3288 SoC. This was successfully tested on a Radxa Rock2 Square
board using Mali kernel driver r16p0 and r12p0 user-space binary.
Guillaume Tucker (4):
dt-bindings: gpu: add bindings for the ARM Mali Midgard GPU
ARM: dts: rockchip: add ARM Mali GPU node for rk3288
ARM: dts: rockchip: enable ARM Mali GPU on rk3288-rock2-som
ARM: dts: rockchip: enable ARM Mali GPU on rk3288-firefly
.../devicetree/bindings/gpu/arm,mali-midgard.txt | 53 ++++++++++++++++++++++
arch/arm/boot/dts/rk3288-firefly.dtsi | 5 ++
arch/arm/boot/dts/rk3288-rock2-som.dtsi | 5 ++
arch/arm/boot/dts/rk3288.dtsi | 23 ++++++++++
4 files changed, 86 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gpu/arm,mali-midgard.txt
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 10/11] ASoC: add bindings for stm32 DFSDM filter
From: Arnaud Pouliquen @ 2017-03-29 12:42 UTC (permalink / raw)
To: Rob Herring
Cc: Mark Rutland, devicetree@vger.kernel.org,
alsa-devel@alsa-project.org, Lars-Peter Clausen, Olivier MOYSAN,
kernel@stlinux.com, Liam Girdwood, linux-iio@vger.kernel.org,
Takashi Iwai, Maxime Coquelin, Mark Brown,
linux-arm-kernel@lists.infradead.org, Peter Meerwald-Stadler,
Hartmut Knaack, Jonathan Cameron, Alexandre TORGUE
In-Reply-To: <20170324145208.6tj5swol5du5lkyc@rob-hp-laptop>
Hello Rob,
Sorry for this dirty/crappy patch...
Please find answers below
Thanks
Arnaud
On 03/24/2017 03:52 PM, Rob Herring wrote:
> On Fri, Mar 17, 2017 at 03:08:23PM +0100, Arnaud Pouliquen wrote:
>> Add bindings that describes audio settings to support
>> Digital Filter for pulse density modulation(PDM) microphone.
>>
>> Signed-off-by: Arnaud Pouliquen <arnaud.pouliquen@st.com>
>> ---
>> V2->V3:
>> Fixes based on V2 comments
>>
>> .../devicetree/bindings/sound/st,stm32-adfsdm.txt | 41 ++++++++++++++++++++++
>> 1 file changed, 41 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/sound/st,stm32-adfsdm.txt
>>
>> diff --git a/Documentation/devicetree/bindings/sound/st,stm32-adfsdm.txt b/Documentation/devicetree/bindings/sound/st,stm32-adfsdm.txt
>> new file mode 100644
>> index 0000000..ab610bc
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/sound/st,stm32-adfsdm.txt
>> @@ -0,0 +1,40 @@
>> +STMicroelectronics audio DFSDM DT bindings
>> +
>> +This driver supports audio PDM microphone capture through Digital Filter format
>> +Sigma Delta modulators (DFSDM).
>> +
>> +Required properties:
>> + - compatible: "st,stm32h7-adfsdm".
>> +
>> + - #sound-dai-cells : Must be equal to 0
>> +
>> + - io-channels : phandle to iio dfsdm instance node.
>> +
>> +
>> +Example of a simple sound card using audio DFSDM node.
>> +
>> + dmic0: dmic_@0 {
>
> Drop the '_' and unit address.
>
>> + compatible = "dmic-codec";
>> + #sound-dai-cells = <0>;
>> + };
>> +
>> + asoc-pdm@0 {
>
> asoc is a Linux term. Drop the unit address.
>
>> + compatible = "st,stm32h7-adfsdm";
>
> Is this a separate block from the ADC? A drawing of the h/w blocks and
> connections would help.
I don't know how to explain it...So don't hesitate is still not clear.
On DFSDM we can connect 2 ADC types:
- Generic Sigma delta(SD) ADC for analog to digital conversion
- Digital microphones for audio capture.
In both cases, an SPI or Manchester bus is used to transfer 1-bit stream
to DFSDM.
1) In case of SD ADC, The link is described in IIO declaration using
"io-channels" property ( [04/11] IIO: add DT bindings for stm32 DFSDM
filter)
2) For audio purpose, the link is done by the simple or the graph card
So this device exposes the Digital audio interface (DAI) associated to
the DFSDM ADC.
Link between the DFSDM DAI and the DFSDM IIO device is done through the
"io-channels" property.
>
>> + #sound-dai-cells = <0>;
>> + io-channels = <&dfsdm_adc0 0>;
>> + };
>> +
>> + sound_dfsdm_pdm {
>
> sound-card {
>
>> + compatible = "simple-audio-card";
>> + simple-audio-card,name = "dfsdm_pdm";
>> +
>> + dfsdm0_mic0: simple-audio-card,dai-link@0 {
>
> I'd suggest moving to the graph card.
yes should be in V4
>
>> + format = "pdm";
>> + cpu {
>> + sound-dai = <&asoc_pdm1>;
>
> This phandle doesn't point to anything.
>
>> + };
>> + dmic0_codec: codec {
>> + sound-dai = <&dmic0>;
>> + };
>> + };
>> + };
>> \ No newline at end of file
>
> ^^^
>
>> --
>> 1.9.1
>>
^ permalink raw reply
* RE: [PATCH v2 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc
From: Chris Brandt @ 2017-03-29 12:38 UTC (permalink / raw)
To: jacopo, Geert Uytterhoeven, Linus Walleij
Cc: Jacopo Mondi, Geert Uytterhoeven, Laurent Pinchart, Rob Herring,
Mark Rutland, Russell King, Linux-Renesas,
linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <20170329120530.GB6247@w540>
Adding my 2 cents here:
On Wednesday, March 29, 2017, jacopo wrote:
> > > If you really do we may need to go for u64 but ... really? Is there
> > > a rational reason for that other than "we did it like this first"?
> > >
> > > I do not understand the notion of "flags" here. I hope that is not
> > > referring
> >
> > Flags refers to BI_DIR, SWIO_IN, and SWIO_OUT, from
> > https://patchwork.kernel.org/patch/9643047/
> >
> > 32-bit should be enough to cover pins, function, and flags.
> >
>
> Geert already replied, but to avoid any confusion I'll try to remove from
> driver the use of "pin config" when referring to this three flags, which
> are just additional informations the pin controller needs to perform pin
> muxing properly. They're not related the standard pin config properties
> (pull-up/down, bias etc.. actually our hardware does not even support
> these natively)
>
> > > to pin config, because I expect that to use the standard pin config
> > > bindings outside of the pinmux value which should just define the
> > > pin+function combo:
> > >
> > > node {
> > > pinmux = <PIN_NUMBER_PINMUX>;
> > > GENERIC_PINCONFIG;
> > > };
> > >
> > > Example from Mediatek:
> > >
> > > i2c1_pins_a: i2c1@0 {
> > > pins {
> > > pinmux = <MT8135_PIN_195_SDA1__FUNC_SDA1>,
> > > <MT8135_PIN_196_SCL1__FUNC_SCL1>;
> >
> > If we follow this example, then we can list all combinations in
> > include/dt-bindings/pinctrl/r7s72100-pinctrl.h, instead of creating
> > the value by combining the bits using a macro where we need it in the
> DTS.
> >
> > It's gonna be a long list, though...
> >
>
> I'm strongly in favour of something like
> pinmux = <PIN(1, 4) | FUNC# | FLAGS>, .... ;
>
> opposed to
> pinmux = <PIN1_4_FUNC#_FLAGS>, ... ;
I agree. I like "<PIN(1, 4) | FUNC# | FLAGS>".
> Not only because it will save use from having a loong list(*) of macros
> that has to be kept up to date when/if new RZ hardware will arrive, but
> also because of readability and simplicity for down-stream and BSP users.
> Speaking of which, I would like to know what does Chris think of this.
The list of macros would be very long, especially against the different
packaging version of the RZ/A1 series. 11 ports, 16-pins for each port, 8 different
function options for each pin....2 different package/pin variations.
And at the end of the day....there is no benefit for the user over just using
a macro.
A little about "this controller" for the RZ/A1: In my opinion it's a one-shot usage.
I don't foresee future RZ/A SoCs using this exact pin controller.
The reason for the "FLAGS" is to work around a quirky hardware design (in my opinion).
However, future RZ/A SoCs will use a 'similar' type controller where each pin has 8 function
options (but the FLAGs won't be needed anymore...as far as I can see...).
So I foresee the DT interface staying more or less the same, but the underlying register
setup in the driver Will change (become more simple).
Now...if we can only convince the R-Car guys to move to a more simple pin-mux controller...
Chris
^ permalink raw reply
* Re: [PATCH v7 1/7] clocksource/drivers/clksrc-evt-probe: Describe with the DT both the clocksource and the clockevent
From: Daniel Lezcano @ 2017-03-29 12:36 UTC (permalink / raw)
To: Mark Rutland
Cc: Rob Herring, Alexander Kochetkov, Heiko Stuebner,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-rockchip-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Thomas Gleixner,
Russell King, Caesar Wang, Huang Tao
In-Reply-To: <20170329104911.GC23442@leverpostej>
On Wed, Mar 29, 2017 at 11:49:11AM +0100, Mark Rutland wrote:
> Hi,
>
> On Wed, Mar 29, 2017 at 11:22:10AM +0200, Daniel Lezcano wrote:
> > On Tue, Mar 28, 2017 at 08:51:46PM -0500, Rob Herring wrote:
> > > On Wed, Mar 22, 2017 at 06:48:28PM +0300, Alexander Kochetkov wrote:
> > > > From: Daniel Lezcano <daniel.lezcano-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> > > >
> > > > The CLOCKSOURCE_OF_DECLARE() was introduced before the
> > > > CLOCKEVENT_OF_DECLARE() and that resulted in some abuse where the
> > > > clockevent and the clocksource are both initialized in the same init
> > > > routine.
> > > >
> > > > With the introduction of the CLOCKEVENT_OF_DECLARE(), the driver can
> > > > now split the clocksource and the clockevent init code. However, the
> > > > device tree may specify a single node, so the same node will be passed
> > > > to the clockevent/clocksource's init function, with the same base
> > > > address.
> > > >
> > > > with this patch it is possible to specify an attribute to the timer's node to
> > > > specify if it is a clocksource or a clockevent and define two timers node.
> > >
> > > Daniel and I discussed and agreed against this a while back. What
> > > changed?
> >
> > Hi Rob,
> >
> > > > For example:
> > > >
> > > > timer: timer@98400000 {
> > > > compatible = "moxa,moxart-timer";
> > > > reg = <0x98400000 0x42>;
> > >
> > > This overlaps the next node. You can change this to 0x10, but are these
> > > really 2 independent h/w blocks? Don't design the nodes around the
> > > current needs of Linux.
> >
> > Mmh, thanks for raising this. Conceptually speaking there are two (or more)
> > different entities, the clocksource and the clockevents but they share the same
> > IP block.
>
> From the DT's PoV, this is one entity, which is the IP block.
>
> The clocksource/clockevent concept is a Linux implementation detail. The
> DT cannot and should not be aware of that.
>
> [...]
>
> > > > interrupts = <19 1>;
> > > > clocks = <&coreclk>;
> > > > clockevent;
> > >
> > > This is not needed. The presence of "interrupts" is enough to say use
> > > this timer for clockevent.
> >
> > Yes, that is true if no drivers was already taking CLOCKSOURCE_OF and
> > initializing the clockevent. The driver will pass through the clocksource_probe
> > function, check the interrupt and bail out because there is an interrupt
> > declared and assume it is a clockevent, so no initialization for the driver.
> > IOW, it is not backward compatible.
> >
> > We need this attribute somehow in order to separate clearly a clocksource or a
> > clockevent with a new implementation.
>
> Why? A single IP block can provide the functionality of both (though
> there are reasons that functionality may be mutually exclusive). What is
> the benefit of this split?
Hi Mark,
You can have several timers on the system and may want to use the clockevents
from one IP block and the clocksource from another IP block. For example, in
the case of a bogus clocksource.
Moreover there are some drivers with a node for a clocksource and
another one for the clockevent, and the driver is assuming the clockevent is
defined first and then the clocksource.
eg.
arch/arc/boot/dts/abilis_tb10x.dtsi:
/* TIMER0 with interrupt for clockevent */
timer0 {
compatible = "snps,arc-timer";
interrupts = <3>;
interrupt-parent = <&intc>;
clocks = <&cpu_clk>;
};
/* TIMER1 for free running clocksource */
timer1 {
compatible = "snps,arc-timer";
clocks = <&cpu_clk>;
};
drivers/clocksource/arc_timer.c:
static int __init arc_of_timer_init(struct device_node *np)
{
static int init_count = 0;
int ret;
if (!init_count) {
init_count = 1;
ret = arc_clockevent_setup(np);
} else {
ret = arc_cs_setup_timer1(np);
}
return ret;
}
Even if that works, it is a fragile mechanism.
So the purpose of these changes is to provide a stronger timer declaration in
order to clearly split in the kernel a clocksource and a clockevent
initialization.
> > > > With this approach, we allow a mechanism to clearly define a clocksource or a
> > > > clockevent without aerobatics we can find around in some drivers:
> > > > timer-sp804.c, arc-timer.c, dw_apb_timer_of.c, mps2-timer.c,
> > > > renesas-ostm.c, time-efm32.c, time-lpc32xx.c.
> > >
> > > These all already have bindings and work. What problem are you trying to
> > > solve other than restructuring Linux?
> >
> > Yes, there is already the bindings, but that force to do some hackish
> > initialization.
>
> Here, you are forcing hackish DT changes that do not truly describe HW.
> How is that better?
So if this is hackish DT changes, then the existing DTs should be fixed, no?
> > I would like to give the opportunity to declare separately a clocksource and a
> > clockevent, in order to have full control of how this is initialized.
>
> To me it sounds like what we need is Linux infrastructure that allows
> one to register a device as having both clockevent/clocksource
> functionality.
That was the idea. Create a macro CLOCKEVENT_OF and CLOCKSOURCE_OF both of them
calling their respective init routines. And in addition a TIMER_OF doing both
CLOCKEVENT_OF and CLOCKSOURCE_OF.
It is the DT which does not allow to do this separation.
Would be the following approach more acceptable ?
1. Replace all CLOCKSOURCE_OF by TIMER_OF (just renaming)
2. A node can have a clockevent and|or a clocksource attributes
3. The timer_probe pass a flag to the driver's init function, so this one knows
if it should invoke the clockevent/clocksource init functions.
No attribute defaults to clocksource|clockevent.
That would be backward compatible and will let to create drivers with clutch
activated device via DT. Also, it will give the opportunity to the existing
drivers to change consolidate their initialization routines.
> That way, we can choose to do something sane at boot time, and if the
> user really wants specific devices used in specific ways, they can
> request that.
--
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs
Follow Linaro: <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [GIT PULL] PCI: Support for configurable PCI endpoint
From: Kishon Vijay Abraham I @ 2017-03-29 12:36 UTC (permalink / raw)
To: Niklas Cassel, Bjorn Helgaas, Joao Pinto, linux-pci, linux-doc,
linux-kernel, devicetree, linux-omap, linux-arm-kernel
Cc: hch, nsekhar
In-Reply-To: <99ae043b-35fd-0723-c8a7-e2625a862c6d@ti.com>
Hi,
On Wednesday 29 March 2017 05:40 PM, Kishon Vijay Abraham I wrote:
> Hi Niklas,
>
> On Wednesday 29 March 2017 05:12 PM, Niklas Cassel wrote:
>> On 03/27/2017 11:44 AM, Kishon Vijay Abraham I wrote:
>>> Hi Bjorn,
>>>
>>> Please find the pull request for PCI endpoint support below. I've
>>> also included all the history here.
>>>
>>> Changes from v4:
>>> *) add #syscon-cells property and used of_parse_phandle_with_args
>>> to perform a configuration in syscon module (as suggested by
>>> Rob Herring)
>>> *) Remove unnecessary white space.
>>>
>>> Changes from v3:
>>> *) fixed a typo and adapted to https://lkml.org/lkml/2017/3/13/562.
>>>
>>> Changes from v2:
>>> *) changed the configfs structure as suggested by Christoph Hellwig. With
>>> this change the framework creates configfs entry for EP function driver
>>> and EP controller. Previously these entries have to be created by the
>>> the user. (Haven't changed the epc core or epf core except for invoking
>>> configfs APIs to create entries for EP function driver and EP controller.
>>> That's mostly because the EP function device can still be created by
>>> directly invoking the epf core API without using configfs).
>>> *) Now the user has to use configfs entry 'start' to start the link.
>>> This was previously done by the function driver. However in the case of
>>> multi function EP, the function driver shouldn't start the link.
>>>
>>> Changes from v1:
>>> *) The preparation patches for adding EP support is removed and is sent
>>> separately
>>> *) Added device ID for DRA74x/DRA72x and used it instead of
>>> using "PCI_ANY_ID"
>>> *) Added userguide for PCI endpoint test function
>>>
>>> Major Improvements from RFC:
>>> *) support multi-function devices (hw supported not virtual)
>>> *) Access host side buffers
>>> *) Raise MSI interrupts
>>> *) Add user space program to use the host side PCI driver
>>> *) Adapt all other users of designware to use the new design (only
>>> compile tested. Since I have only dra7xx boards, the new design
>>> has only been tested in dra7xx. I'd require the help of others
>>> to test the platforms they have access to).
>>>
>>> This series has been developed over 4.11-rc1 + [1]
>>> [1] -> https://lkml.org/lkml/2017/3/13/562
>>>
>>> Let me know if this has to be re-based to some of your branch.
>>>
>>> Thanks
>>> Kishon
>>>
>>> The following changes since commit 623e87fec8ab7867fb51b3079196bd10718a60ce:
>>>
>>> PCI: dwc: dra7xx: Push request_irq call to the bottom of probe (2017-03-22 20:35:30 +0530)
>>>
>>> are available in the git repository at:
>>>
>>> git://git.kernel.org/pub/scm/linux/kernel/git/kishon/pci-endpoint.git tags/pci-endpoint-for-4.12
>>>
>>> for you to fetch changes up to e98bf80074be4654faae42fe0f5a622a776b6fdd:
>>>
>>> ARM: DRA7: clockdomain: Change the CLKTRCTRL of CM_PCIE_CLKSTCTRL to SW_WKUP (2017-03-27 15:08:22 +0530)
>>>
>>> ----------------------------------------------------------------
>>
>> FWIW:
>> I've tested Kishon's tag pci-endpoint-for-4.12
>> and PCIe on artpec6 SoC is still working fine.
>
> Thanks for testing it.
>>
>> I also included the DRA7xx PCIe driver in my
>> kernel so that pcie-designware-ep.c gets built.
>>
>> My only worry is that the code in pcie-designware-ep.c
>> is not compile tested if DRA7xx is not selected
>> (as it is the only driver using PCIE_DW_EP at
>> the moment).
>
> yeah, we should plan to include COMPILE_TEST in all pci drivers but I guess
> there is some problem with non-ARM builds [1]. As Bjorn mentioned in the
> thread, we could add #ifdef ARM and then include COMPILE_TEST.
I think I misunderstood your concern. yeah, there is no direct way to compile
pcie-designware-ep.c without selecting DRA7xx.
Thanks
Kishon
^ permalink raw reply
* Re: [PATCH 7/15] dt-bindings: display: sun4i: Add allwinner,tcon-channel property
From: Maxime Ripard @ 2017-03-29 12:31 UTC (permalink / raw)
To: Icenowy Zheng
Cc: linux-arm-kernel, Mike Turquette, David Airlie, Mark Rutland,
Daniel Vetter, linux-sunxi, linux-kernel, Stephen Boyd,
devicetree, linux-clk, dri-devel, Chen-Yu Tsai, Rob Herring
In-Reply-To: <20170328100519.145DE203419-Y9/x5g2N/Tt0ykcd9G8QkxTxI0vvWBSX@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2982 bytes --]
On Tue, Mar 28, 2017 at 06:05:08PM +0800, Icenowy Zheng wrote:
>
> 2017年3月27日 上午5:11于 Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>写道：
> >
> > On Fri, Mar 17, 2017 at 11:34:45AM +0800, Chen-Yu Tsai wrote:
> > > On Thu, Mar 16, 2017 at 1:37 AM, Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > > > On Tue, Mar 07, 2017 at 09:56:26AM +0100, Maxime Ripard wrote:
> > > >> The Allwinner Timings Controller has two, mutually exclusive, channels.
> > > >> When the binding has been introduced, it was assumed that there would be
> > > >> only a single user per channel in the system.
> > > >>
> > > >> While this is likely for the channel 0 which only connects to LCD displays,
> > > >> it turns out that the channel 1 can be connected to multiple controllers in
> > > >> the SoC (HDMI and TV encoders for example). And while the simultaneous use
> > > >> of HDMI and TV outputs cannot be achieved, switching from one to the other
> > > >> at runtime definitely sounds plausible.
> > > >>
> > > >> Add an extra property, allwinner,tcon-channel, to specify for a given
> > > >> endpoint which TCON channel it is connected to, while falling back to the
> > > >> previous mechanism if that property is missing.
> > > >
> > > > I think perhaps TCON channels should have been ports rather than
> > > > endpoints. The fact that the channels are mutually exclusive can be
> > > > handled in the driver and doesn't really matter in the binding. How
> > > > painful would it be to rework things to move TCON channel 1 from port 0,
> > > > endpoint 1 to port 1?
> > >
> > > Having a separate output port for channel 1 was one option we discussed.
> > > However it wouldn't work well with the kernel's of_graph based crtc
> > > detection (drm_of_find_possible_crtcs / drm_of_find_possible_crtcs),
> > > which has the crtcs bound via the output port. As the logic is used
> > > by multiple drivers, I'm not sure it's easy to rework or test.
> >
> > Can't we use a different logic than drm_of_find_possible_crtcs to fill
> > the same role?
> >
> > > Also, we still have to support old device trees using channel 1 from
> > > output port 0 endpoint 1. This is the TV encoder on sun5i (A10s/A13/R8).
>
> And from A83T on we will face channel-1 only TCONs., which do not
> have channel 0 at all in hardware.
Yes, but those patches have not been merged yet, so we don't have to
care about the backward compatibility.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to the Google Groups "linux-sunxi" group.
To unsubscribe from this group and stop receiving emails from it, send an email to linux-sunxi+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH v3 1/2] PCI: Add tango MSI controller support
From: Marc Zyngier @ 2017-03-29 12:29 UTC (permalink / raw)
To: Marc Gonzalez, Bjorn Helgaas, Thomas Gleixner
Cc: Robin Murphy, Lorenzo Pieralisi, Liviu Dudau, David Laight,
linux-pci, Linux ARM, Thibaud Cornic, Phuong Nguyen, LKML, DT,
Mason
In-Reply-To: <f37d7b55-e065-72aa-e690-13dd4718a9ce@sigmadesigns.com>
On 29/03/17 12:29, Marc Gonzalez wrote:
> The MSI controller in Tango supports 256 message-signaled interrupts,
> and a single doorbell address.
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
> ---
> Changes since v0.2
> - Support 256 MSIs instead of only 32
> - Use spinlock_t instead of struct mutex
> - Add MSI_FLAG_PCI_MSIX flag
>
> IRQs are acked in tango_msi_isr because handle_simple_irq leaves
> ack, clear, mask and unmask up to the driver. For the same reason,
> interrupt enable mask is updated from tango_irq_domain_alloc/free.
I've asked you to move this to individual methods. You've decided not
to, and that's your call. But I now wonder why I'm even bothering to
review this, as you've so far just wasted my time.
Anyway...
> ---
> drivers/pci/host/pcie-tango.c | 194 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 194 insertions(+)
>
> diff --git a/drivers/pci/host/pcie-tango.c b/drivers/pci/host/pcie-tango.c
> new file mode 100644
> index 000000000000..e88850983a1d
> --- /dev/null
> +++ b/drivers/pci/host/pcie-tango.c
> @@ -0,0 +1,194 @@
> +#include <linux/module.h>
> +#include <linux/irqchip/chained_irq.h>
> +#include <linux/irqdomain.h>
> +#include <linux/pci-ecam.h>
How is that include relevant to this patch?
> +#include <linux/msi.h>
> +
> +#define MSI_MAX 256
> +
> +struct tango_pcie {
> + DECLARE_BITMAP(bitmap, MSI_MAX);
> + spinlock_t lock;
> + void __iomem *mux;
> + void __iomem *msi_status;
> + void __iomem *msi_mask;
> + phys_addr_t msi_doorbell;
> + struct irq_domain *irq_domain;
> + struct irq_domain *msi_domain;
> + int irq;
> +};
> +
> +/*** MSI CONTROLLER SUPPORT ***/
> +
> +static void dispatch(struct tango_pcie *pcie, unsigned long status, int base)
> +{
> + unsigned int pos, virq;
> +
> + for_each_set_bit(pos, &status, 32) {
> + virq = irq_find_mapping(pcie->irq_domain, base + pos);
> + generic_handle_irq(virq);
> + }
> +}
> +
> +static void tango_msi_isr(struct irq_desc *desc)
> +{
> + u32 status;
> + struct irq_chip *chip = irq_desc_get_chip(desc);
> + struct tango_pcie *pcie = irq_desc_get_handler_data(desc);
> + unsigned int base, offset, pos = 0;
> +
> + chained_irq_enter(chip, desc);
> +
> + while ((pos = find_next_bit(pcie->bitmap, MSI_MAX, pos)) < MSI_MAX) {
> + base = round_down(pos, 32);
> + offset = (pos / 32) * 4;
> + status = readl_relaxed(pcie->msi_status + offset);
> + writel_relaxed(status, pcie->msi_status + offset);
> + dispatch(pcie, status, base);
> + pos = base + 32;
> + }
> +
> + chained_irq_exit(chip, desc);
> +}
> +
> +static struct irq_chip tango_msi_irq_chip = {
> + .name = "MSI",
> + .irq_mask = pci_msi_mask_irq,
> + .irq_unmask = pci_msi_unmask_irq,
How do you make that work if the PCI device doesn't support per-MSI masking?
> +};
> +
> +#define USE_DEF_OPS (MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS)
How is that useful?
> +
> +static struct msi_domain_info msi_domain_info = {
> + .flags = USE_DEF_OPS | MSI_FLAG_PCI_MSIX,
> + .chip = &tango_msi_irq_chip,
> +};
> +
> +static void tango_compose_msi_msg(struct irq_data *data, struct msi_msg *msg)
> +{
> + struct tango_pcie *pcie = irq_data_get_irq_chip_data(data);
> +
> + msg->address_lo = lower_32_bits(pcie->msi_doorbell);
> + msg->address_hi = upper_32_bits(pcie->msi_doorbell);
> + msg->data = data->hwirq;
> +}
> +
> +static int tango_set_affinity(struct irq_data *irq_data,
> + const struct cpumask *mask, bool force)
> +{
> + return -EINVAL;
> +}
> +
> +static struct irq_chip tango_msi_chip = {
> + .name = "MSI",
> + .irq_compose_msi_msg = tango_compose_msi_msg,
> + .irq_set_affinity = tango_set_affinity,
> +};
> +
> +static int find_free_msi(struct irq_domain *dom, unsigned int virq)
> +{
> + u32 val;
> + struct tango_pcie *pcie = dom->host_data;
> + unsigned int offset, pos;
> +
> + pos = find_first_zero_bit(pcie->bitmap, MSI_MAX);
> + if (pos >= MSI_MAX)
> + return -ENOSPC;
> +
> + offset = (pos / 32) * 4;
> + val = readl_relaxed(pcie->msi_mask + offset);
> + writel_relaxed(val | BIT(pos % 32), pcie->msi_mask + offset);
Great. I'm now in a position where I can take an interrupt (because of
the broken locking that doesn't disable interrupts), but the bitmap
doesn't indicate it yet. With a bit of luck, I'll never make any forward
progress.
> + __set_bit(pos, pcie->bitmap);
> +
> + irq_domain_set_info(dom, virq, pos, &tango_msi_chip,
> + dom->host_data, handle_simple_irq, NULL, NULL);
I've told you a number of times that PCI MSIs are edge triggered...
> +
> + return 0;
> +}
> +
> +static int tango_irq_domain_alloc(struct irq_domain *dom,
> + unsigned int virq, unsigned int nr_irqs, void *args)
> +{
> + int err;
> + struct tango_pcie *pcie = dom->host_data;
> +
> + spin_lock(&pcie->lock);
> + err = find_free_msi(dom, virq);
> + spin_unlock(&pcie->lock);
> +
> + return err;
> +}
> +
> +static void tango_irq_domain_free(struct irq_domain *dom,
> + unsigned int virq, unsigned int nr_irqs)
> +{
> + u32 val;
> + struct irq_data *d = irq_domain_get_irq_data(dom, virq);
> + struct tango_pcie *pcie = irq_data_get_irq_chip_data(d);
> + unsigned int offset, pos = d->hwirq;
> +
> + spin_lock(&pcie->lock);
> +
> + offset = (pos / 32) * 4;
> + val = readl_relaxed(pcie->msi_mask + offset);
> + writel_relaxed(val & ~BIT(pos % 32), pcie->msi_mask + offset);
> + __clear_bit(pos, pcie->bitmap);
> +
> + spin_unlock(&pcie->lock);
> +}
> +
> +static const struct irq_domain_ops msi_dom_ops = {
> + .alloc = tango_irq_domain_alloc,
> + .free = tango_irq_domain_free,
> +};
> +
> +static int tango_msi_remove(struct platform_device *pdev)
> +{
> + struct tango_pcie *msi = platform_get_drvdata(pdev);
Be consistent in your naming. It's called pcie everywhere else.
> +
> + irq_set_chained_handler_and_data(msi->irq, NULL, NULL);
> + irq_domain_remove(msi->msi_domain);
> + irq_domain_remove(msi->irq_domain);
> +
> + return 0;
> +}
> +
> +static int tango_msi_probe(struct platform_device *pdev, struct tango_pcie *pcie)
> +{
> + int i, virq;
> + struct fwnode_handle *fwnode = of_node_to_fwnode(pdev->dev.of_node);
> + struct irq_domain *msi_dom, *irq_dom;
> +
> + spin_lock_init(&pcie->lock);
> +
> + for (i = 0; i < MSI_MAX / 32; ++i)
> + writel_relaxed(0, pcie->msi_mask + i * 4);
> +
> + irq_dom = irq_domain_create_linear(fwnode, MSI_MAX, &msi_dom_ops, pcie);
> + if (!irq_dom) {
> + pr_err("Failed to create IRQ domain\n");
> + return -ENOMEM;
> + }
> +
> + msi_dom = pci_msi_create_irq_domain(fwnode, &msi_domain_info, irq_dom);
> + if (!msi_dom) {
> + pr_err("Failed to create MSI domain\n");
> + irq_domain_remove(irq_dom);
> + return -ENOMEM;
> + }
> +
> + virq = platform_get_irq(pdev, 1);
> + if (virq <= 0) {
> + pr_err("Failed to map IRQ\n");
> + irq_domain_remove(msi_dom);
> + irq_domain_remove(irq_dom);
> + return -ENXIO;
> + }
> +
> + pcie->irq_domain = irq_dom;
> + pcie->msi_domain = msi_dom;
> + pcie->irq = virq;
> + irq_set_chained_handler_and_data(virq, tango_msi_isr, pcie);
> +
> + return 0;
> +}
>
So there is not much progress from the previous version. It is just
broken in a different ways, and ignores most of the work that is already
done in the irqchip core. I can only repeat what I've said in my
previous review.
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply
* Re: [RFC PATCH 2/3] iio: adc: sun4i-gpadc-iio: add support for H3 thermal sensor
From: Maxime Ripard @ 2017-03-29 12:28 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Quentin Schulz, Zhang Rui, linux-pm-u79uwXL29TY76Z2rM5mHXA,
Rob Herring, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-iio-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Lee Jones, Jonathan Cameron,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Chen-Yu Tsai
In-Reply-To: <20170329065717.D0AF37C1EFF-Y9/x5g2N/Tt0ykcd9G8QkxTxI0vvWBSX@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1374 bytes --]
On Wed, Mar 29, 2017 at 02:57:02PM +0800, Icenowy Zheng wrote:
> > > @@ -691,6 +777,12 @@ static int sun4i_gpadc_remove(struct platform_device *pdev)
> > > if (!info->no_irq && IS_ENABLED(CONFIG_THERMAL_OF))
> > > iio_map_array_unregister(indio_dev);
> > >
> > > + if (info->data->gen2_ths) {
> > > + clk_disable_unprepare(info->ths_clk);
> > > + clk_disable_unprepare(info->ths_bus_clk);
> > > + reset_control_deassert(info->reset);
> > > + }
> > > +
> >
> > I'm not really fond of using this boolean as I don't see it being
> > possibly reused for any other SoCs that has a GPADC or THS.
>
> Because you didn't care new SoCs :-)
>
> All SoCs after H3 (A64, H5, R40) uses the same THS architecture with
> H3.
That's not really Quentin's point. His point is that having things
like flags and/or variables to identify various behaviours that might
differ from one SoC to the other usually works better when you want to
support several of them.
For example, replacing the gen2_ths by one variable with the number of
clocks, one with the number of channels, a bool to say it has a reset,
etc. definitely works better for us when Allwinner does some mix and
match between each SoC. And this happen most of the time.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [PATCH 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
From: Ludovic BARRE @ 2017-03-29 12:24 UTC (permalink / raw)
To: Marek Vasut, Cyrille Pitchen
Cc: David Woodhouse, Brian Norris, Boris Brezillon,
Richard Weinberger, Alexandre Torgue, Rob Herring,
linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1be39452-83b0-5c32-39fe-d6dd5134d1ef-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
hi Marek
thanks for review
comment below
On 03/29/2017 12:54 PM, Marek Vasut wrote:
> On 03/27/2017 02:54 PM, Ludovic Barre wrote:
>> From: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
>>
>> The quadspi is a specialized communication interface targeting single,
>> dual or quad SPI Flash memories.
>>
>> It can operate in any of the following modes:
>> -indirect mode: all the operations are performed using the quadspi
>> registers
>> -read memory-mapped mode: the external Flash memory is mapped to the
>> microcontroller address space and is seen by the system as if it was
>> an internal memory
>>
>> Signed-off-by: Ludovic Barre <ludovic.barre-qxv4g6HH51o@public.gmane.org>
> Hi! I presume this is not compatible with the Cadence QSPI or any other
> QSPI controller, huh ?
it's not a cadence QSPI, it's specific for stm32 platform
>
> Mostly minor nits below ...
>
>> ---
>> drivers/mtd/spi-nor/Kconfig | 7 +
>> drivers/mtd/spi-nor/Makefile | 1 +
>> drivers/mtd/spi-nor/stm32-quadspi.c | 679 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 687 insertions(+)
>> create mode 100644 drivers/mtd/spi-nor/stm32-quadspi.c
> [...]
>
>> +struct stm32_qspi_cmd {
>> + struct {
>> + u32 addr_width:8;
>> + u32 dummy:8;
>> + u32 data:1;
>> + } conf;
> This could all be u8 ? Why this awful construct ?
yes I could replace each u32 by u8
>
>> + u8 opcode;
>> + u32 framemode;
>> + u32 qspimode;
>> + u32 addr;
>> + size_t len;
>> + void *buf;
>> +};
>> +
>> +static int stm32_qspi_wait_cmd(struct stm32_qspi *qspi)
>> +{
>> + u32 cr;
>> + int err = 0;
> Can readl_poll_timeout() or somesuch replace this function ?
in fact stm32_qspi_wait_cmd test if the transfer has been complete (TCF
flag)
else initialize an interrupt completion.
like the time may be long I prefer keep the interrupt wait, if you agree.
>> + if (readl_relaxed(qspi->io_base + QUADSPI_SR) & SR_TCF)
>> + return 0;
>> +
>> + reinit_completion(&qspi->cmd_completion);
>> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>> + writel_relaxed(cr | CR_TCIE, qspi->io_base + QUADSPI_CR);
>> +
>> + if (!wait_for_completion_interruptible_timeout(&qspi->cmd_completion,
>> + msecs_to_jiffies(1000)))
>> + err = -ETIMEDOUT;
>> +
>> + writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
>> + return err;
>> +}
>> +
>> +static int stm32_qspi_wait_nobusy(struct stm32_qspi *qspi)
>> +{
>> + u32 sr;
>> +
>> + return readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR, sr,
>> + !(sr & SR_BUSY), 10, 100000);
>> +}
>> +
>> +static void stm32_qspi_set_framemode(struct spi_nor *nor,
>> + struct stm32_qspi_cmd *cmd, bool read)
>> +{
>> + u32 dmode = CCR_DMODE_1;
>> +
>> + cmd->framemode = CCR_IMODE_1;
>> +
>> + if (read) {
>> + switch (nor->flash_read) {
>> + case SPI_NOR_NORMAL:
>> + case SPI_NOR_FAST:
>> + dmode = CCR_DMODE_1;
>> + break;
>> + case SPI_NOR_DUAL:
>> + dmode = CCR_DMODE_2;
>> + break;
>> + case SPI_NOR_QUAD:
>> + dmode = CCR_DMODE_4;
>> + break;
>> + }
>> + }
>> +
>> + cmd->framemode |= cmd->conf.data ? dmode : 0;
>> + cmd->framemode |= cmd->conf.addr_width ? CCR_ADMODE_1 : 0;
>> +}
>> +
>> +static void stm32_qspi_read_fifo(u8 *val, void __iomem *addr)
>> +{
>> + *val = readb_relaxed(addr);
>> +}
>> +
>> +static void stm32_qspi_write_fifo(u8 *val, void __iomem *addr)
>> +{
>> + writeb_relaxed(*val, addr);
>> +}
>> +
>> +static int stm32_qspi_tx_poll(struct stm32_qspi *qspi,
>> + const struct stm32_qspi_cmd *cmd)
>> +{
>> + void (*tx_fifo)(u8 *, void __iomem *);
>> + u32 len = cmd->len, sr;
>> + u8 *buf = cmd->buf;
>> + int ret;
>> +
>> + if (cmd->qspimode == CCR_FMODE_INDW)
>> + tx_fifo = stm32_qspi_write_fifo;
>> + else
>> + tx_fifo = stm32_qspi_read_fifo;
>> +
>> + while (len--) {
>> + ret = readl_relaxed_poll_timeout(qspi->io_base + QUADSPI_SR,
>> + sr, (sr & SR_FTF),
>> + 10, 30000);
> Add macros for those ad-hoc timeouts.
I will add 2 defines (for stm32_qspi_wait_nobusy, stm32_qspi_tx_poll)
#define STM32_QSPI_FIFO_TIMEOUT_US 30000
#define STM32_QSPI_BUSY_TIMEOUT_US 100000
>
>> + if (ret) {
>> + dev_err(qspi->dev, "fifo timeout (stat:%#x)\n", sr);
>> + break;
>> + }
>> + tx_fifo(buf++, qspi->io_base + QUADSPI_DR);
>> + }
>> +
>> + return ret;
>> +}
>
> [...]
>
>> +static int stm32_qspi_send(struct stm32_qspi_flash *flash,
>> + const struct stm32_qspi_cmd *cmd)
>> +{
>> + struct stm32_qspi *qspi = flash->qspi;
>> + u32 ccr, dcr, cr;
>> + int err;
>> +
>> + err = stm32_qspi_wait_nobusy(qspi);
>> + if (err)
>> + goto abort;
>> +
>> + dcr = readl_relaxed(qspi->io_base + QUADSPI_DCR) & ~DCR_FSIZE_MASK;
>> + dcr |= DCR_FSIZE(flash->fsize);
>> + writel_relaxed(dcr, qspi->io_base + QUADSPI_DCR);
>> +
>> + cr = readl_relaxed(qspi->io_base + QUADSPI_CR);
>> + cr &= ~CR_PRESC_MASK & ~CR_FSEL;
>> + cr |= CR_PRESC(flash->presc);
>> + cr |= flash->cs ? CR_FSEL : 0;
>> + writel_relaxed(cr, qspi->io_base + QUADSPI_CR);
>> +
>> + if (cmd->conf.data)
>> + writel_relaxed(cmd->len - 1, qspi->io_base + QUADSPI_DLR);
>> +
>> + ccr = cmd->framemode | cmd->qspimode;
>> +
>> + if (cmd->conf.dummy)
>> + ccr |= CCR_DCYC(cmd->conf.dummy);
>> +
>> + if (cmd->conf.addr_width)
>> + ccr |= CCR_ADSIZE(cmd->conf.addr_width - 1);
>> +
>> + ccr |= CCR_INST(cmd->opcode);
>> + writel_relaxed(ccr, qspi->io_base + QUADSPI_CCR);
>> +
>> + if (cmd->conf.addr_width && cmd->qspimode != CCR_FMODE_MM)
>> + writel_relaxed(cmd->addr, qspi->io_base + QUADSPI_AR);
>> +
>> + err = stm32_qspi_tx(qspi, cmd);
>> + if (err)
>> + goto abort;
>> +
>> + if (cmd->qspimode != CCR_FMODE_MM) {
>> + err = stm32_qspi_wait_cmd(qspi);
>> + if (err)
>> + goto abort;
>> + writel_relaxed(FCR_CTCF, qspi->io_base + QUADSPI_FCR);
>> + }
>> +
>> + return err;
>> +
>> +abort:
>> + writel_relaxed(readl_relaxed(qspi->io_base + QUADSPI_CR)
>> + | CR_ABORT, qspi->io_base + QUADSPI_CR);
> Use a helper variable here too.
ok
>> + dev_err(qspi->dev, "%s abort err:%d\n", __func__, err);
>> + return err;
>> +}
> [...]
>
>> +static int stm32_qspi_flash_setup(struct stm32_qspi *qspi,
>> + struct device_node *np)
>> +{
>> + u32 width, flash_read, presc, cs_num, max_rate = 0;
>> + struct stm32_qspi_flash *flash;
>> + struct mtd_info *mtd;
>> + int ret;
>> +
>> + of_property_read_u32(np, "reg", &cs_num);
>> + if (cs_num >= STM32_MAX_NORCHIP)
>> + return -EINVAL;
>> +
>> + of_property_read_u32(np, "spi-max-frequency", &max_rate);
>> + if (!max_rate)
>> + return -EINVAL;
>> +
>> + presc = DIV_ROUND_UP(qspi->clk_rate, max_rate) - 1;
>> +
>> + if (of_property_read_u32(np, "spi-rx-bus-width", &width))
>> + width = 1;
>> +
>> + if (width == 4)
>> + flash_read = SPI_NOR_QUAD;
>> + else if (width == 2)
>> + flash_read = SPI_NOR_DUAL;
>> + else if (width == 1)
>> + flash_read = SPI_NOR_NORMAL;
>> + else
>> + return -EINVAL;
>> +
>> + flash = &qspi->flash[cs_num];
>> + flash->qspi = qspi;
>> + flash->cs = cs_num;
>> + flash->presc = presc;
>> +
>> + flash->nor.dev = qspi->dev;
>> + spi_nor_set_flash_node(&flash->nor, np);
>> + flash->nor.priv = flash;
>> + mtd = &flash->nor.mtd;
>> + mtd->priv = &flash->nor;
>> +
>> + flash->nor.read = stm32_qspi_read;
>> + flash->nor.write = stm32_qspi_write;
>> + flash->nor.erase = stm32_qspi_erase;
>> + flash->nor.read_reg = stm32_qspi_read_reg;
>> + flash->nor.write_reg = stm32_qspi_write_reg;
>> + flash->nor.prepare = stm32_qspi_prep;
>> + flash->nor.unprepare = stm32_qspi_unprep;
>> +
>> + writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
>> +
>> + writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
>> + | CR_EN, qspi->io_base + QUADSPI_CR);
>> +
>> + /* a minimum fsize must be set to sent the command id */
>> + flash->fsize = 25;
> I don't understand why this is needed and the comment doesn't make
> sense. Please fix.
fsize field defines the size of external memory.
Normaly, this field is used only for memory map mode,
but in fact is check in indirect mode.
So while flash scan "spi_nor_scan":
-I can't let 0.
-I not know yet the size of flash.
So I fix a temporary value
I will update my comment
>
>> + ret = spi_nor_scan(&flash->nor, NULL, flash_read);
>> + if (ret) {
>> + dev_err(qspi->dev, "device scan failed\n");
>> + return ret;
>> + }
>> +
>> + flash->fsize = __fls(mtd->size) - 1;
>> +
>> + writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
>> +
>> + ret = mtd_device_register(mtd, NULL, 0);
>> + if (ret) {
>> + dev_err(qspi->dev, "mtd device parse failed\n");
>> + return ret;
>> + }
>> +
>> + dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
>> + qspi->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
>> +
>> + return 0;
>> +}
> [...]
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 2/2] PCI: Add tango PCIe host bridge support
From: Robin Murphy @ 2017-03-29 12:19 UTC (permalink / raw)
To: Marc Gonzalez, Bjorn Helgaas, Marc Zyngier, Thomas Gleixner
Cc: Lorenzo Pieralisi, Liviu Dudau, David Laight, linux-pci,
Linux ARM, Thibaud Cornic, Phuong Nguyen, LKML, DT, Mason
In-Reply-To: <65114e62-7458-b6f7-327c-f07a5096a452-y1yR0Z3OICC7zZZRDBGcUA@public.gmane.org>
On 29/03/17 12:34, Marc Gonzalez wrote:
> This driver is used to work around HW bugs in the controller.
>
> Note: the controller does NOT support the following features.
>
> Legacy PCI interrupts
> IO space
>
> Signed-off-by: Marc Gonzalez <marc_gonzalez-y1yR0Z3OICC7zZZRDBGcUA@public.gmane.org>
> ---
> Documentation/devicetree/bindings/pci/tango-pcie.txt | 33 +++++++++
> drivers/pci/host/Kconfig | 7 ++
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pcie-tango.c | 150 +++++++++++++++++++++++++++++++++++++++
> 4 files changed, 191 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/pci/tango-pcie.txt b/Documentation/devicetree/bindings/pci/tango-pcie.txt
> new file mode 100644
> index 000000000000..f8e150ec41d8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/tango-pcie.txt
> @@ -0,0 +1,33 @@
> +Sigma Designs Tango PCIe controller
> +
> +Required properties:
> +
> +- compatible: "sigma,smp8759-pcie"
> +- reg: address/size of PCI configuration space, and pcie_reg
> +- bus-range: defined by size of PCI configuration space
> +- device_type: "pci"
> +- #size-cells: <2>
> +- #address-cells: <3>
> +- #interrupt-cells: <1>
> +- ranges: translation from system to bus addresses
> +- interrupts: spec for misc interrupts and MSI
> +- msi-controller
> +
> +Example:
> +
> + pcie@2e000 {
> + compatible = "sigma,smp8759-pcie";
> + reg = <0x50000000 SZ_64M>, <0x2e000 0x100>;
> + bus-range = <0 63>;
> + device_type = "pci";
> + #size-cells = <2>;
> + #address-cells = <3>;
> + #interrupt-cells = <1>;
> + /* http://elinux.org/Device_Tree_Usage#PCI_Address_Translation */
> + /* BUS_ADDRESS(3) CPU_PHYSICAL(1) SIZE(2) */
> + ranges = <0x02000000 0x0 0x04000000 0x54000000 0x0 SZ_192M>;
> + interrupts =
> + <54 IRQ_TYPE_LEVEL_HIGH>, /* misc interrupts */
> + <55 IRQ_TYPE_LEVEL_HIGH>; /* MSI */
> + msi-controller;
> + };
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index d7e7c0a827c3..8a622578a760 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -285,6 +285,13 @@ config PCIE_ROCKCHIP
> There is 1 internal PCIe port available to support GEN2 with
> 4 slots.
>
> +config PCIE_TANGO
> + tristate "Tango PCIe controller"
> + depends on ARCH_TANGO && PCI_MSI && OF
> + select PCI_HOST_COMMON
> + help
> + Say Y here to enable PCIe controller support on Tango SoC.
Nit: in line with the other help texts, that could probably benefit from
a wee bit more context (i.e. "Sigma Designs Tango SoCs")
> +
> config VMD
> depends on PCI_MSI && X86_64
> tristate "Intel Volume Management Device Driver"
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 084cb4983645..fc7ea90196f3 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -32,4 +32,5 @@ obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
> obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
> obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
> obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
> +obj-$(CONFIG_PCIE_TANGO) += pcie-tango.o
> obj-$(CONFIG_VMD) += vmd.o
> diff --git a/drivers/pci/host/pcie-tango.c b/drivers/pci/host/pcie-tango.c
> index e88850983a1d..dbc2663486b5 100644
> --- a/drivers/pci/host/pcie-tango.c
> +++ b/drivers/pci/host/pcie-tango.c
> @@ -192,3 +192,153 @@ static int tango_msi_probe(struct platform_device *pdev, struct tango_pcie *pcie
>
> return 0;
> }
> +
> +/*** HOST BRIDGE SUPPORT ***/
> +
> +static int smp8759_config_read(struct pci_bus *bus,
> + unsigned int devfn, int where, int size, u32 *val)
> +{
> + int ret;
> + struct pci_config_window *cfg = bus->sysdata;
> + struct tango_pcie *pcie = dev_get_drvdata(cfg->parent);
> +
> + /*
> + * QUIRK #1
> + * Reads in configuration space outside devfn 0 return garbage.
> + */
> + if (devfn != 0) {
> + *val = ~0; /* Is this required even if we return an error? */
> + return PCIBIOS_FUNC_NOT_SUPPORTED; /* Error seems appropriate */
> + }
> +
> + /*
> + * QUIRK #2
> + * The root complex advertizes a fake BAR, which is used to filter
> + * bus-to-system requests. Hide it from Linux.
> + */
> + if (where == PCI_BASE_ADDRESS_0 && bus->number == 0) {
> + *val = 0; /* 0 or ~0 to hide the BAR from Linux? */
> + return PCIBIOS_SUCCESSFUL; /* Should we return error or success? */
> + }
> +
> + /*
> + * QUIRK #3
> + * Unfortunately, config and mem spaces are muxed.
> + * Linux does not support such a setting, since drivers are free
> + * to access mem space directly, at any time.
> + * Therefore, we can only PRAY that config and mem space accesses
> + * NEVER occur concurrently.
> + */
What about David's suggestion of using an IPI for safe mutual exclusion?
> + writel_relaxed(1, pcie->mux);
> + ret = pci_generic_config_read(bus, devfn, where, size, val);
> + writel_relaxed(0, pcie->mux);
> +
> + return ret;
> +}
> +
> +static int smp8759_config_write(struct pci_bus *bus,
> + unsigned int devfn, int where, int size, u32 val)
> +{
> + int ret;
> + struct pci_config_window *cfg = bus->sysdata;
> + struct tango_pcie *pcie = dev_get_drvdata(cfg->parent);
> +
> + writel_relaxed(1, pcie->mux);
> + ret = pci_generic_config_write(bus, devfn, where, size, val);
> + writel_relaxed(0, pcie->mux);
> +
> + return ret;
> +}
> +
> +static struct pci_ecam_ops smp8759_ecam_ops = {
> + .bus_shift = 20,
> + .pci_ops = {
> + .map_bus = pci_ecam_map_bus,
> + .read = smp8759_config_read,
> + .write = smp8759_config_write,
> + }
> +};
> +
> +static const struct of_device_id tango_pcie_ids[] = {
> + { .compatible = "sigma,smp8759-pcie" },
General tip: use your .data member here...
> + { /* sentinel */ },
> +};
> +
> +static void smp8759_init(struct tango_pcie *pcie, void __iomem *base)
> +{
> + pcie->mux = base + 0x48;
> + pcie->msi_status = base + 0x80;
> + pcie->msi_mask = base + 0xa0;
> + pcie->msi_doorbell = 0xa0000000 + 0x2e07c;
> +}
> +
> +static int tango_pcie_probe(struct platform_device *pdev)
> +{
> + int ret;
> + void __iomem *base;
> + struct resource *res;
> + struct tango_pcie *pcie;
> + struct device *dev = &pdev->dev;
> +
> + pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
> + if (!pcie)
> + return -ENOMEM;
> +
> + platform_set_drvdata(pdev, pcie);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
> + base = devm_ioremap_resource(&pdev->dev, res);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + if (of_device_is_compatible(dev->of_node, "sigma,smp8759-pcie"))
> + smp8759_init(pcie, base);
...then retrieve it with of_device_get_match_data() here. No need to
reinvent the wheel (or have to worry about the ordering of multiple
compatibles once rev. n+1 comes around).
> +
> + ret = tango_msi_probe(pdev, pcie);
> + if (ret)
> + return ret;
> +
> + return pci_host_common_probe(pdev, &smp8759_ecam_ops);
> +}
> +
> +static int tango_pcie_remove(struct platform_device *pdev)
> +{
> + return tango_msi_remove(pdev);
> +}
> +
> +static struct platform_driver tango_pcie_driver = {
> + .probe = tango_pcie_probe,
> + .remove = tango_pcie_remove,
> + .driver = {
> + .name = KBUILD_MODNAME,
> + .of_match_table = tango_pcie_ids,
> + },
> +};
> +
> +module_platform_driver(tango_pcie_driver);
> +
> +#define VENDOR_SIGMA 0x1105
Should this not be in include/linux/pci_ids.h?
Robin.
> +
> +/*
> + * QUIRK #4
> + * The root complex advertizes the wrong device class.
> + * Header Type 1 is for PCI-to-PCI bridges.
> + */
> +static void tango_fixup_class(struct pci_dev *dev)
> +{
> + dev->class = PCI_CLASS_BRIDGE_PCI << 8;
> +}
> +DECLARE_PCI_FIXUP_EARLY(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_class);
> +
> +/*
> + * QUIRK #5
> + * Only transfers within the root complex BAR are forwarded to the host.
> + * By default, the DMA framework expects that
> + * PCI address 0x8000_0000 maps to system address 0x8000_0000
> + * which is where DRAM0 is mapped.
> + */
> +static void tango_fixup_bar(struct pci_dev *dev)
> +{
> + pci_write_config_dword(dev, PCI_BASE_ADDRESS_0, 0x80000000);
> +}
> +DECLARE_PCI_FIXUP_FINAL(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_bar);
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCHv5] mfd: cpcap: implement irq sense helper
From: Sebastian Reichel @ 2017-03-29 12:18 UTC (permalink / raw)
To: Sebastian Reichel, Lee Jones
Cc: Tony Lindgren, Dmitry Torokhov, Rob Herring, Mark Rutland,
linux-input, devicetree, linux-kernel
In-Reply-To: <20170329080423.ijbl4seczla2s4nm@dell>
CPCAP can sense if IRQ is currently set or not. This
functionality is required for a few subdevices, such
as the power button and usb phy modules.
Acked-by: Tony Lindgren <tony@atomide.com>
Signed-off-by: Sebastian Reichel <sre@kernel.org>
---
Changes since PATCHv3:
- add extern to function definition
- use BIT macro for mask variable
- avoid magic numbers
Changes since PATCHv4:
- rename base to irq_base
---
drivers/mfd/motorola-cpcap.c | 28 ++++++++++++++++++++++++++++
include/linux/mfd/motorola-cpcap.h | 2 ++
2 files changed, 30 insertions(+)
diff --git a/drivers/mfd/motorola-cpcap.c b/drivers/mfd/motorola-cpcap.c
index 6aeada7d7ce5..a9097efcefa5 100644
--- a/drivers/mfd/motorola-cpcap.c
+++ b/drivers/mfd/motorola-cpcap.c
@@ -23,6 +23,8 @@
#define CPCAP_NR_IRQ_REG_BANKS 6
#define CPCAP_NR_IRQ_CHIPS 3
+#define CPCAP_REGISTER_SIZE 4
+#define CPCAP_REGISTER_BITS 16
struct cpcap_ddata {
struct spi_device *spi;
@@ -32,6 +34,32 @@ struct cpcap_ddata {
struct regmap *regmap;
};
+static int cpcap_sense_irq(struct regmap *regmap, int irq)
+{
+ int regnum = irq / CPCAP_REGISTER_BITS;
+ int mask = BIT(irq % CPCAP_REGISTER_BITS);
+ int reg = CPCAP_REG_INTS1 + (regnum * CPCAP_REGISTER_SIZE);
+ int err, val;
+
+ if (reg < CPCAP_REG_INTS1 || reg > CPCAP_REG_INTS4)
+ return -EINVAL;
+
+ err = regmap_read(regmap, reg, &val);
+ if (err)
+ return err;
+
+ return !!(val & mask);
+}
+
+int cpcap_sense_virq(struct regmap *regmap, int virq)
+{
+ struct regmap_irq_chip_data *d = irq_get_chip_data(virq);
+ int irq_base = regmap_irq_chip_get_base(d);
+
+ return cpcap_sense_irq(regmap, virq - irq_base);
+}
+EXPORT_SYMBOL_GPL(cpcap_sense_virq);
+
static int cpcap_check_revision(struct cpcap_ddata *cpcap)
{
u16 vendor, rev;
diff --git a/include/linux/mfd/motorola-cpcap.h b/include/linux/mfd/motorola-cpcap.h
index b4031c2b2214..793aa695faa0 100644
--- a/include/linux/mfd/motorola-cpcap.h
+++ b/include/linux/mfd/motorola-cpcap.h
@@ -290,3 +290,5 @@ static inline int cpcap_get_vendor(struct device *dev,
return 0;
}
+
+extern int cpcap_sense_virq(struct regmap *regmap, int virq);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v6 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller
From: Benjamin Herrenschmidt @ 2017-03-29 12:11 UTC (permalink / raw)
To: Brendan Higgins
Cc: Wolfram Sang, robh+dt, mark.rutland, tglx, jason, Marc Zyngier,
Joel Stanley, Vladimir Zapolskiy, Kachalov Anton,
Cédric Le Goater, linux-i2c, devicetree, linux-kernel,
OpenBMC Maillist
In-Reply-To: <CAFd5g47Pij6TxkmS6akWoAL0FZ2WLh+FWUrr099RiabWn+pneQ@mail.gmail.com>
On Wed, 2017-03-29 at 03:34 -0700, Brendan Higgins wrote:
> I think I addressed this on the other email with the actual driver.
> Anyway, I thought that this is pretty much the dummy irqchip code is
> for; I have seen some other drivers do the same thing. It is true
> that
> this is a really basic "interrupt controller;" it cannot mask on its
> own, etc; nevertheless, I think you will pretty much end up with the
> same code for an "I2C controller;" it just won't use an irq_domain.
Don't worry too much about this. As I think I mention it's not a huge
deal at this stage, I just wanted to make sure you were aware of the
compromise(s) involved.
Regarding the other comment about the "fast mode", my main worry here
is that somebody might come up with a 2Mhz capable device, we'll hit
your 1Mhz test, enable fast mode, and shoot it with 3.4Mhz which it
might not be happy at all about...
I think the cut-off for switching to the "fast" mode should basically
be the fast speed mode frequency (which isn't clear from the spec but
seems to be 3.4Mhz). Otherwise people will end up with higher speeds
than what they asked for and that's bad.
Cheers,
Ben.
^ permalink raw reply
* Re: [GIT PULL] PCI: Support for configurable PCI endpoint
From: Kishon Vijay Abraham I @ 2017-03-29 12:10 UTC (permalink / raw)
To: Niklas Cassel, Bjorn Helgaas, Joao Pinto, linux-pci, linux-doc,
linux-kernel, devicetree, linux-omap, linux-arm-kernel
Cc: hch, nsekhar
In-Reply-To: <0cfe5acf-332c-00c9-e5d5-1403c4e80ebe@axis.com>
Hi Niklas,
On Wednesday 29 March 2017 05:12 PM, Niklas Cassel wrote:
> On 03/27/2017 11:44 AM, Kishon Vijay Abraham I wrote:
>> Hi Bjorn,
>>
>> Please find the pull request for PCI endpoint support below. I've
>> also included all the history here.
>>
>> Changes from v4:
>> *) add #syscon-cells property and used of_parse_phandle_with_args
>> to perform a configuration in syscon module (as suggested by
>> Rob Herring)
>> *) Remove unnecessary white space.
>>
>> Changes from v3:
>> *) fixed a typo and adapted to https://lkml.org/lkml/2017/3/13/562.
>>
>> Changes from v2:
>> *) changed the configfs structure as suggested by Christoph Hellwig. With
>> this change the framework creates configfs entry for EP function driver
>> and EP controller. Previously these entries have to be created by the
>> the user. (Haven't changed the epc core or epf core except for invoking
>> configfs APIs to create entries for EP function driver and EP controller.
>> That's mostly because the EP function device can still be created by
>> directly invoking the epf core API without using configfs).
>> *) Now the user has to use configfs entry 'start' to start the link.
>> This was previously done by the function driver. However in the case of
>> multi function EP, the function driver shouldn't start the link.
>>
>> Changes from v1:
>> *) The preparation patches for adding EP support is removed and is sent
>> separately
>> *) Added device ID for DRA74x/DRA72x and used it instead of
>> using "PCI_ANY_ID"
>> *) Added userguide for PCI endpoint test function
>>
>> Major Improvements from RFC:
>> *) support multi-function devices (hw supported not virtual)
>> *) Access host side buffers
>> *) Raise MSI interrupts
>> *) Add user space program to use the host side PCI driver
>> *) Adapt all other users of designware to use the new design (only
>> compile tested. Since I have only dra7xx boards, the new design
>> has only been tested in dra7xx. I'd require the help of others
>> to test the platforms they have access to).
>>
>> This series has been developed over 4.11-rc1 + [1]
>> [1] -> https://lkml.org/lkml/2017/3/13/562
>>
>> Let me know if this has to be re-based to some of your branch.
>>
>> Thanks
>> Kishon
>>
>> The following changes since commit 623e87fec8ab7867fb51b3079196bd10718a60ce:
>>
>> PCI: dwc: dra7xx: Push request_irq call to the bottom of probe (2017-03-22 20:35:30 +0530)
>>
>> are available in the git repository at:
>>
>> git://git.kernel.org/pub/scm/linux/kernel/git/kishon/pci-endpoint.git tags/pci-endpoint-for-4.12
>>
>> for you to fetch changes up to e98bf80074be4654faae42fe0f5a622a776b6fdd:
>>
>> ARM: DRA7: clockdomain: Change the CLKTRCTRL of CM_PCIE_CLKSTCTRL to SW_WKUP (2017-03-27 15:08:22 +0530)
>>
>> ----------------------------------------------------------------
>
> FWIW:
> I've tested Kishon's tag pci-endpoint-for-4.12
> and PCIe on artpec6 SoC is still working fine.
Thanks for testing it.
>
> I also included the DRA7xx PCIe driver in my
> kernel so that pcie-designware-ep.c gets built.
>
> My only worry is that the code in pcie-designware-ep.c
> is not compile tested if DRA7xx is not selected
> (as it is the only driver using PCIE_DW_EP at
> the moment).
yeah, we should plan to include COMPILE_TEST in all pci drivers but I guess
there is some problem with non-ARM builds [1]. As Bjorn mentioned in the
thread, we could add #ifdef ARM and then include COMPILE_TEST.
Thanks
Kishon
[1] -> http://www.spinics.net/lists/linux-pci/msg58134.html
Thanks
Kishon
>
^ permalink raw reply
* Re: [PATCH v2 2/7] dt-bindings: pinctrl: Add RZ/A1 bindings doc
From: jacopo @ 2017-03-29 12:05 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Linus Walleij, Jacopo Mondi, Geert Uytterhoeven, Laurent Pinchart,
Chris Brandt, Rob Herring, Mark Rutland, Russell King,
Linux-Renesas, linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
In-Reply-To: <CAMuHMdX0OdZXYDOZvFutjfBFVd65C567_G+++kHygre4KK+jLg@mail.gmail.com>
Hi Geert, Linus
On Wed, Mar 29, 2017 at 01:20:39PM +0200, Geert Uytterhoeven wrote:
> Hi Linus,
>
> On Wed, Mar 29, 2017 at 12:15 PM, Linus Walleij
> <linus.walleij@linaro.org> wrote:
> > On Wed, Mar 29, 2017 at 9:35 AM, Geert Uytterhoeven
> > <geert@linux-m68k.org> wrote:
> >>> See for example:
> >>> include/dt-bindings/pinctrl/mt65xx.h
> >>>
> >>> And how that is used in:
> >>> arch/arm/boot/dts/mt2701-pinfunc.h
> >>> arch/arm/boot/dts/mt2701-evb.dts
> >>>
> >>> The docs are here:
> >>> Documentation/devicetree/bindings/pinctrl/pinctrl-mt65xx.txt
> >>
> >> All of the above pack the information for a pin into a single 32-bit integer.
> >> Is that what you want us to use, too?
> >> Currently we use two integers: 1) pin index, and 2) function/flags combo.
> >
> > I don't really know what you need, sorry. But some kind of figure, yes.
> > I would say whatever makes sense. 16+16 bits makes sense in most
> > combinatorial spaces does it not? If you split 32 bits in 16 bits for
> > pin and 16 bits for function, do you have more than 2^16 pins or 2^16
> > functions?
> >
> > If you really do we may need to go for u64 but ... really? Is there
> > a rational reason for that other than "we did it like this first"?
> >
> > I do not understand the notion of "flags" here. I hope that is not referring
>
> Flags refers to BI_DIR, SWIO_IN, and SWIO_OUT, from
> https://patchwork.kernel.org/patch/9643047/
>
> 32-bit should be enough to cover pins, function, and flags.
>
Geert already replied, but to avoid any confusion I'll try to remove
from driver the use of "pin config" when referring to this three
flags, which are just additional informations the pin controller needs
to perform pin muxing properly. They're not related the standard pin
config properties (pull-up/down, bias etc.. actually our hardware does
not even support these natively)
> > to pin config, because I expect that to use the standard pin config
> > bindings outside of the pinmux value which should just define the
> > pin+function combo:
> >
> > node {
> > pinmux = <PIN_NUMBER_PINMUX>;
> > GENERIC_PINCONFIG;
> > };
> >
> > Example from Mediatek:
> >
> > i2c1_pins_a: i2c1@0 {
> > pins {
> > pinmux = <MT8135_PIN_195_SDA1__FUNC_SDA1>,
> > <MT8135_PIN_196_SCL1__FUNC_SCL1>;
>
> If we follow this example, then we can list all combinations in
> include/dt-bindings/pinctrl/r7s72100-pinctrl.h, instead of creating the value
> by combining the bits using a macro where we need it in the DTS.
>
> It's gonna be a long list, though...
>
I'm strongly in favour of something like
pinmux = <PIN(1, 4) | FUNC# | FLAGS>, .... ;
opposed to
pinmux = <PIN1_4_FUNC#_FLAGS>, ... ;
Not only because it will save use from having a loong list(*) of macros that has
to be kept up to date when/if new RZ hardware will arrive, but also
because of readability and simplicity for down-stream and BSP users.
Speaking of which, I would like to know what does Chris think of this.
Is there any strong preference between the two for you Linus?
Thanks
j
(*) 12 ports, 16 pins, 8 functions, 3 flags: you can do the math
yourselves and see this is going to be hard to maintain.
> > bias-pull-up = <55>;
> > };
> > };
> >
> > So this allows for a combine pin+function number but pull ups,
> > bias etc are not baked into the thing, they have to be added on
> > separately with the generic bindings, which is nice and very readable.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
^ permalink raw reply
* Re: [GIT PULL] PCI: Support for configurable PCI endpoint
From: Niklas Cassel @ 2017-03-29 11:42 UTC (permalink / raw)
To: Kishon Vijay Abraham I, Bjorn Helgaas, Joao Pinto, linux-pci,
linux-doc, linux-kernel, devicetree, linux-omap, linux-arm-kernel
Cc: hch, nsekhar
In-Reply-To: <20170327094520.3129-1-kishon@ti.com>
On 03/27/2017 11:44 AM, Kishon Vijay Abraham I wrote:
> Hi Bjorn,
>
> Please find the pull request for PCI endpoint support below. I've
> also included all the history here.
>
> Changes from v4:
> *) add #syscon-cells property and used of_parse_phandle_with_args
> to perform a configuration in syscon module (as suggested by
> Rob Herring)
> *) Remove unnecessary white space.
>
> Changes from v3:
> *) fixed a typo and adapted to https://lkml.org/lkml/2017/3/13/562.
>
> Changes from v2:
> *) changed the configfs structure as suggested by Christoph Hellwig. With
> this change the framework creates configfs entry for EP function driver
> and EP controller. Previously these entries have to be created by the
> the user. (Haven't changed the epc core or epf core except for invoking
> configfs APIs to create entries for EP function driver and EP controller.
> That's mostly because the EP function device can still be created by
> directly invoking the epf core API without using configfs).
> *) Now the user has to use configfs entry 'start' to start the link.
> This was previously done by the function driver. However in the case of
> multi function EP, the function driver shouldn't start the link.
>
> Changes from v1:
> *) The preparation patches for adding EP support is removed and is sent
> separately
> *) Added device ID for DRA74x/DRA72x and used it instead of
> using "PCI_ANY_ID"
> *) Added userguide for PCI endpoint test function
>
> Major Improvements from RFC:
> *) support multi-function devices (hw supported not virtual)
> *) Access host side buffers
> *) Raise MSI interrupts
> *) Add user space program to use the host side PCI driver
> *) Adapt all other users of designware to use the new design (only
> compile tested. Since I have only dra7xx boards, the new design
> has only been tested in dra7xx. I'd require the help of others
> to test the platforms they have access to).
>
> This series has been developed over 4.11-rc1 + [1]
> [1] -> https://lkml.org/lkml/2017/3/13/562
>
> Let me know if this has to be re-based to some of your branch.
>
> Thanks
> Kishon
>
> The following changes since commit 623e87fec8ab7867fb51b3079196bd10718a60ce:
>
> PCI: dwc: dra7xx: Push request_irq call to the bottom of probe (2017-03-22 20:35:30 +0530)
>
> are available in the git repository at:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/kishon/pci-endpoint.git tags/pci-endpoint-for-4.12
>
> for you to fetch changes up to e98bf80074be4654faae42fe0f5a622a776b6fdd:
>
> ARM: DRA7: clockdomain: Change the CLKTRCTRL of CM_PCIE_CLKSTCTRL to SW_WKUP (2017-03-27 15:08:22 +0530)
>
> ----------------------------------------------------------------
FWIW:
I've tested Kishon's tag pci-endpoint-for-4.12
and PCIe on artpec6 SoC is still working fine.
I also included the DRA7xx PCIe driver in my
kernel so that pcie-designware-ep.c gets built.
My only worry is that the code in pcie-designware-ep.c
is not compile tested if DRA7xx is not selected
(as it is the only driver using PCIE_DW_EP at
the moment).
^ permalink raw reply
* [PATCH v3 2/2] PCI: Add tango PCIe host bridge support
From: Marc Gonzalez @ 2017-03-29 11:34 UTC (permalink / raw)
To: Bjorn Helgaas, Marc Zyngier, Thomas Gleixner
Cc: DT, Lorenzo Pieralisi, Mason, linux-pci, Thibaud Cornic,
Liviu Dudau, LKML, David Laight, Phuong Nguyen, Robin Murphy,
Linux ARM
In-Reply-To: <5309e718-5813-5b79-db57-9d702b50d0f9@sigmadesigns.com>
This driver is used to work around HW bugs in the controller.
Note: the controller does NOT support the following features.
Legacy PCI interrupts
IO space
Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
---
Documentation/devicetree/bindings/pci/tango-pcie.txt | 33 +++++++++
drivers/pci/host/Kconfig | 7 ++
drivers/pci/host/Makefile | 1 +
drivers/pci/host/pcie-tango.c | 150 +++++++++++++++++++++++++++++++++++++++
4 files changed, 191 insertions(+)
diff --git a/Documentation/devicetree/bindings/pci/tango-pcie.txt b/Documentation/devicetree/bindings/pci/tango-pcie.txt
new file mode 100644
index 000000000000..f8e150ec41d8
--- /dev/null
+++ b/Documentation/devicetree/bindings/pci/tango-pcie.txt
@@ -0,0 +1,33 @@
+Sigma Designs Tango PCIe controller
+
+Required properties:
+
+- compatible: "sigma,smp8759-pcie"
+- reg: address/size of PCI configuration space, and pcie_reg
+- bus-range: defined by size of PCI configuration space
+- device_type: "pci"
+- #size-cells: <2>
+- #address-cells: <3>
+- #interrupt-cells: <1>
+- ranges: translation from system to bus addresses
+- interrupts: spec for misc interrupts and MSI
+- msi-controller
+
+Example:
+
+ pcie@2e000 {
+ compatible = "sigma,smp8759-pcie";
+ reg = <0x50000000 SZ_64M>, <0x2e000 0x100>;
+ bus-range = <0 63>;
+ device_type = "pci";
+ #size-cells = <2>;
+ #address-cells = <3>;
+ #interrupt-cells = <1>;
+ /* http://elinux.org/Device_Tree_Usage#PCI_Address_Translation */
+ /* BUS_ADDRESS(3) CPU_PHYSICAL(1) SIZE(2) */
+ ranges = <0x02000000 0x0 0x04000000 0x54000000 0x0 SZ_192M>;
+ interrupts =
+ <54 IRQ_TYPE_LEVEL_HIGH>, /* misc interrupts */
+ <55 IRQ_TYPE_LEVEL_HIGH>; /* MSI */
+ msi-controller;
+ };
diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
index d7e7c0a827c3..8a622578a760 100644
--- a/drivers/pci/host/Kconfig
+++ b/drivers/pci/host/Kconfig
@@ -285,6 +285,13 @@ config PCIE_ROCKCHIP
There is 1 internal PCIe port available to support GEN2 with
4 slots.
+config PCIE_TANGO
+ tristate "Tango PCIe controller"
+ depends on ARCH_TANGO && PCI_MSI && OF
+ select PCI_HOST_COMMON
+ help
+ Say Y here to enable PCIe controller support on Tango SoC.
+
config VMD
depends on PCI_MSI && X86_64
tristate "Intel Volume Management Device Driver"
diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
index 084cb4983645..fc7ea90196f3 100644
--- a/drivers/pci/host/Makefile
+++ b/drivers/pci/host/Makefile
@@ -32,4 +32,5 @@ obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
obj-$(CONFIG_PCIE_ARMADA_8K) += pcie-armada8k.o
obj-$(CONFIG_PCIE_ARTPEC6) += pcie-artpec6.o
obj-$(CONFIG_PCIE_ROCKCHIP) += pcie-rockchip.o
+obj-$(CONFIG_PCIE_TANGO) += pcie-tango.o
obj-$(CONFIG_VMD) += vmd.o
diff --git a/drivers/pci/host/pcie-tango.c b/drivers/pci/host/pcie-tango.c
index e88850983a1d..dbc2663486b5 100644
--- a/drivers/pci/host/pcie-tango.c
+++ b/drivers/pci/host/pcie-tango.c
@@ -192,3 +192,153 @@ static int tango_msi_probe(struct platform_device *pdev, struct tango_pcie *pcie
return 0;
}
+
+/*** HOST BRIDGE SUPPORT ***/
+
+static int smp8759_config_read(struct pci_bus *bus,
+ unsigned int devfn, int where, int size, u32 *val)
+{
+ int ret;
+ struct pci_config_window *cfg = bus->sysdata;
+ struct tango_pcie *pcie = dev_get_drvdata(cfg->parent);
+
+ /*
+ * QUIRK #1
+ * Reads in configuration space outside devfn 0 return garbage.
+ */
+ if (devfn != 0) {
+ *val = ~0; /* Is this required even if we return an error? */
+ return PCIBIOS_FUNC_NOT_SUPPORTED; /* Error seems appropriate */
+ }
+
+ /*
+ * QUIRK #2
+ * The root complex advertizes a fake BAR, which is used to filter
+ * bus-to-system requests. Hide it from Linux.
+ */
+ if (where == PCI_BASE_ADDRESS_0 && bus->number == 0) {
+ *val = 0; /* 0 or ~0 to hide the BAR from Linux? */
+ return PCIBIOS_SUCCESSFUL; /* Should we return error or success? */
+ }
+
+ /*
+ * QUIRK #3
+ * Unfortunately, config and mem spaces are muxed.
+ * Linux does not support such a setting, since drivers are free
+ * to access mem space directly, at any time.
+ * Therefore, we can only PRAY that config and mem space accesses
+ * NEVER occur concurrently.
+ */
+ writel_relaxed(1, pcie->mux);
+ ret = pci_generic_config_read(bus, devfn, where, size, val);
+ writel_relaxed(0, pcie->mux);
+
+ return ret;
+}
+
+static int smp8759_config_write(struct pci_bus *bus,
+ unsigned int devfn, int where, int size, u32 val)
+{
+ int ret;
+ struct pci_config_window *cfg = bus->sysdata;
+ struct tango_pcie *pcie = dev_get_drvdata(cfg->parent);
+
+ writel_relaxed(1, pcie->mux);
+ ret = pci_generic_config_write(bus, devfn, where, size, val);
+ writel_relaxed(0, pcie->mux);
+
+ return ret;
+}
+
+static struct pci_ecam_ops smp8759_ecam_ops = {
+ .bus_shift = 20,
+ .pci_ops = {
+ .map_bus = pci_ecam_map_bus,
+ .read = smp8759_config_read,
+ .write = smp8759_config_write,
+ }
+};
+
+static const struct of_device_id tango_pcie_ids[] = {
+ { .compatible = "sigma,smp8759-pcie" },
+ { /* sentinel */ },
+};
+
+static void smp8759_init(struct tango_pcie *pcie, void __iomem *base)
+{
+ pcie->mux = base + 0x48;
+ pcie->msi_status = base + 0x80;
+ pcie->msi_mask = base + 0xa0;
+ pcie->msi_doorbell = 0xa0000000 + 0x2e07c;
+}
+
+static int tango_pcie_probe(struct platform_device *pdev)
+{
+ int ret;
+ void __iomem *base;
+ struct resource *res;
+ struct tango_pcie *pcie;
+ struct device *dev = &pdev->dev;
+
+ pcie = devm_kzalloc(dev, sizeof(*pcie), GFP_KERNEL);
+ if (!pcie)
+ return -ENOMEM;
+
+ platform_set_drvdata(pdev, pcie);
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
+ base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ if (of_device_is_compatible(dev->of_node, "sigma,smp8759-pcie"))
+ smp8759_init(pcie, base);
+
+ ret = tango_msi_probe(pdev, pcie);
+ if (ret)
+ return ret;
+
+ return pci_host_common_probe(pdev, &smp8759_ecam_ops);
+}
+
+static int tango_pcie_remove(struct platform_device *pdev)
+{
+ return tango_msi_remove(pdev);
+}
+
+static struct platform_driver tango_pcie_driver = {
+ .probe = tango_pcie_probe,
+ .remove = tango_pcie_remove,
+ .driver = {
+ .name = KBUILD_MODNAME,
+ .of_match_table = tango_pcie_ids,
+ },
+};
+
+module_platform_driver(tango_pcie_driver);
+
+#define VENDOR_SIGMA 0x1105
+
+/*
+ * QUIRK #4
+ * The root complex advertizes the wrong device class.
+ * Header Type 1 is for PCI-to-PCI bridges.
+ */
+static void tango_fixup_class(struct pci_dev *dev)
+{
+ dev->class = PCI_CLASS_BRIDGE_PCI << 8;
+}
+DECLARE_PCI_FIXUP_EARLY(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_class);
+
+/*
+ * QUIRK #5
+ * Only transfers within the root complex BAR are forwarded to the host.
+ * By default, the DMA framework expects that
+ * PCI address 0x8000_0000 maps to system address 0x8000_0000
+ * which is where DRAM0 is mapped.
+ */
+static void tango_fixup_bar(struct pci_dev *dev)
+{
+ pci_write_config_dword(dev, PCI_BASE_ADDRESS_0, 0x80000000);
+}
+DECLARE_PCI_FIXUP_FINAL(VENDOR_SIGMA, PCI_ANY_ID, tango_fixup_bar);
--
2.11.0
^ permalink raw reply related
* Re: [PATCH v2 1/8] arm64: mm: Allow installation of memory abort handlers
From: Mark Rutland @ 2017-03-29 11:32 UTC (permalink / raw)
To: Doug Berger, catalin.marinas, will.deacon
Cc: robh+dt, computersforpeace, gregory.0xf0, f.fainelli,
bcm-kernel-feedback-list, wangkefeng.wang, james.morse, mingo,
sandeepa.s.prabhu, shijie.huang, linus.walleij, treding,
jonathanh, olof, mirza.krak, suzuki.poulose, bgolaszewski,
devicetree, linux-kernel, linux-arm-kernel
In-Reply-To: <20170328213431.10904-2-opendmb@gmail.com>
Hi,
On Tue, Mar 28, 2017 at 02:34:24PM -0700, Doug Berger wrote:
> From: Florian Fainelli <f.fainelli@gmail.com>
>
> Similarly to what the ARM/Linux kernel provides, add a hook_fault_code()
> function which allows drivers or other parts of the kernel to install
> custom memory abort handlers. This is useful when a given SoC's busing
> does not propagate the exact faulting physical address, but there is a
> way to read it through e.g: a special arbiter driver.
>
> Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
Personally, I do not think that it makes sense to allow arbitrary code
to hook such low-level fault handling.
IMO, if it is truly necessary to allow drivers to handle particular
faults, that should be driven by data associated with the relevant
mapping (e.g. the VMA), rather than allowing code to hook *all* faults.
>From my PoV, NAK to this interface to take over low-level fault
handling.
Catalin and Will have the final say here, as the arm64 maintainers.
Thanks,
Mark.
> ---
> arch/arm64/include/asm/system_misc.h | 3 +++
> arch/arm64/mm/fault.c | 15 ++++++++++++++-
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm64/include/asm/system_misc.h b/arch/arm64/include/asm/system_misc.h
> index bc812435bc76..e05f5b8c7c1c 100644
> --- a/arch/arm64/include/asm/system_misc.h
> +++ b/arch/arm64/include/asm/system_misc.h
> @@ -38,6 +38,9 @@ void arm64_notify_die(const char *str, struct pt_regs *regs,
> void hook_debug_fault_code(int nr, int (*fn)(unsigned long, unsigned int,
> struct pt_regs *),
> int sig, int code, const char *name);
> +void hook_fault_code(int nr, int (*fn)(unsigned long, unsigned int,
> + struct pt_regs *),
> + int sig, int code, const char *name);
>
> struct mm_struct;
> extern void show_pte(struct mm_struct *mm, unsigned long addr);
> diff --git a/arch/arm64/mm/fault.c b/arch/arm64/mm/fault.c
> index 4bf899fb451b..cdf1260f1005 100644
> --- a/arch/arm64/mm/fault.c
> +++ b/arch/arm64/mm/fault.c
> @@ -488,7 +488,7 @@ static int do_bad(unsigned long addr, unsigned int esr, struct pt_regs *regs)
> return 1;
> }
>
> -static const struct fault_info {
> +static struct fault_info {
> int (*fn)(unsigned long addr, unsigned int esr, struct pt_regs *regs);
> int sig;
> int code;
> @@ -560,6 +560,19 @@ static const struct fault_info {
> { do_bad, SIGBUS, 0, "unknown 63" },
> };
>
> +void __init hook_fault_code(int nr,
> + int (*fn)(unsigned long, unsigned int, struct pt_regs *),
> + int sig, int code, const char *name)
> +{
> + BUG_ON(nr < 0 || nr >= ARRAY_SIZE(fault_info));
> +
> + fault_info[nr].fn = fn;
> + fault_info[nr].sig = sig;
> + fault_info[nr].code = code;
> + fault_info[nr].name = name;
> +}
> +
> +
> static const char *fault_name(unsigned int esr)
> {
> const struct fault_info *inf = fault_info + (esr & 63);
> --
> 2.12.0
>
^ 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