* Re: [PATCH 2/2] gpio: DT bindings, move tca9554 from pcf857x to pca953x
From: Linus Walleij @ 2017-04-24 16:31 UTC (permalink / raw)
To: Anders Darander
Cc: Alexandre Courbot, Rob Herring, linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org, linux-kernel@vger.kernel.org,
Mark Rutland
In-Reply-To: <20170421124631.19269-2-anders@chargestorm.se>
On Fri, Apr 21, 2017 at 2:46 PM, Anders Darander <anders@chargestorm.se> wrote:
> The TCA9554 is similar to the PCA9554. Update the DT binding docs.
>
> Signed-off-by: Anders Darander <anders@chargestorm.se>
Patch applied. Seems like this component was misqualified at some point,
let's repair it.
yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH] ARM: at91/at91-pinctrl documentation: fix spelling mistake: "contoller" -> "controller"
From: Linus Walleij @ 2017-04-24 16:33 UTC (permalink / raw)
To: Colin King
Cc: Rob Herring, Mark Rutland, linux-gpio@vger.kernel.org,
devicetree@vger.kernel.org, kernel-janitors,
linux-kernel@vger.kernel.org
In-Reply-To: <20170421120713.4939-1-colin.king@canonical.com>
On Fri, Apr 21, 2017 at 2:07 PM, Colin King <colin.king@canonical.com> wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> trivial fix to spelling mistake in documentation
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Patch applied with Nico's ACK.
Yours,
Linus Walleij
^ permalink raw reply
* Re: [PATCH 1/4] pinctrl: stm32: set pin to gpio input when used as interrupt
From: Alexandre Torgue @ 2017-04-24 16:40 UTC (permalink / raw)
To: Linus Walleij
Cc: devicetree@vger.kernel.org, linux-gpio@vger.kernel.org,
Patrice Chotard, linux-kernel@vger.kernel.org, Paul Gortmaker,
Rob Herring, Maxime Coquelin,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CACRpkdYZixkJroS-gQxYL6xW47-J9UiNv4hOO9F-1b2eYLr8XQ@mail.gmail.com>
Hi Linus,
On 04/24/2017 02:36 PM, Linus Walleij wrote:
> On Fri, Apr 7, 2017 at 5:10 PM, Alexandre TORGUE
> <alexandre.torgue@st.com> wrote:
>
>> This patch ensures that pin is correctly set as gpio input when it is used
>> as an interrupt.
>>
>> Signed-off-by: Alexandre TORGUE <alexandre.torgue@st.com>
> (...)
>
>> +static int stm32_gpio_irq_request_resources(struct irq_data *irq_data)
>> +{
>> + struct stm32_gpio_bank *bank = irq_data->domain->host_data;
>> + u32 ret;
>> +
>> + if (!gpiochip_is_requested(&bank->gpio_chip, irq_data->hwirq)) {
>> + ret = stm32_gpio_request(&bank->gpio_chip, irq_data->hwirq);
>> + if (ret)
>> + return ret;
>> + }
>
> This is wrong. You should only use gpiochip_lock_as_irq(), because of the
> following in Documentation/gpio/driver.txt:
>
> ---------------
> It is legal for any IRQ consumer to request an IRQ from any irqchip no matter
> if that is a combined GPIO+IRQ driver. The basic premise is that gpio_chip and
> irq_chip are orthogonal, and offering their services independent of each
> other.
> (...)
> So always prepare the hardware and make it ready for action in respective
> callbacks from the GPIO and irqchip APIs. Do not rely on gpiod_to_irq() having
> been called first.
Ok, so actually the action to set pin in input mode is necessary in
stm32_gpio_irq_request_resources.
I'll fix it in V2.
>
> This orthogonality leads to ambiguities that we need to solve: if there is
> competition inside the subsystem which side is using the resource (a certain
> GPIO line and register for example) it needs to deny certain operations and
> keep track of usage inside of the gpiolib subsystem. This is why the API
> below exists.
>
> Locking IRQ usage
> -----------------
> Input GPIOs can be used as IRQ signals. When this happens, a driver is requested
> to mark the GPIO as being used as an IRQ:
>
> int gpiochip_lock_as_irq(struct gpio_chip *chip, unsigned int offset)
>
> This will prevent the use of non-irq related GPIO APIs until the GPIO IRQ lock
> is released:
>
> void gpiochip_unlock_as_irq(struct gpio_chip *chip, unsigned int offset)
>
> When implementing an irqchip inside a GPIO driver, these two functions should
> typically be called in the .startup() and .shutdown() callbacks from the
> irqchip.
>
> When using the gpiolib irqchip helpers, these callback are automatically
> assigned.
> --------------
>
> It is because of easy to make errors like this that I prefer that people try
> to use GPIOLIB_IRQCHIP helpers insteaf of rolling their own irqchip code.
I understand. It was difficult in our case due to design.
Thanks for review.
Alex
>
> Yours,
> Linus Walleij
>
^ permalink raw reply
* Re: [PATCH 1/5 v3] usb: host: add DT bindings for faraday fotg2
From: Hans Ulli Kroll @ 2017-04-24 16:53 UTC (permalink / raw)
To: Linus Walleij
Cc: openwrt-devel, devicetree, Paulius Zaleckas, Greg Kroah-Hartman,
linux-usb, Janos Laube, linux-arm-kernel
In-Reply-To: <20170421204058.6206-1-linus.walleij@linaro.org>
Hi Linus
On Fri, 21 Apr 2017, Linus Walleij wrote:
> From: Hans Ulli Kroll <ulli.kroll@googlemail.com>
>
> This adds device tree bindings for the Faraday FOTG2
> dual-mode host controller.
>
> Cc: devicetree@vger.kernel.org
> Signed-off-by: Hans Ulli Kroll <ulli.kroll@googlemail.com>
> Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
> ---
> ChangeLog v1->v3:
> - Change compatible to "faraday,fotg210" as the name of the
> hardware block.
> - Add an elaborate SoC-specific compatible string for the
> Cortina Systems Gemini so that SoC-specific features can
> be enabled.
> - Add cortina,gemini-mini-b to indicate a Gemini PHY with
> a Mini-B adapter connected.
> - Indicated that the Gemini version can handle "wakeup-source".
> - Add optional IP block clock.
> ---
> .../devicetree/bindings/usb/faraday,fotg210.txt | 35 ++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/usb/faraday,fotg210.txt
>
Got NAK'ed from Rob on some ealier round due missing "device mode" on this
IP. I've blatantly overrided this to a host only driver.
These are the needed changes in DT to support both modes
Note the -dr at the end of fotg210, to reflect this in an dual role device
diff --git a/Documentation/devicetree/bindings/usb/faraday,fotg210.txt
b/Documentation/devicetree/bindings/usb/faraday,fotg210.txt
index cf06808303e2..862cda19e9d3 100644
--- a/Documentation/devicetree/bindings/usb/faraday,fotg210.txt
+++ b/Documentation/devicetree/bindings/usb/faraday,fotg210.txt
@@ -13,6 +13,9 @@ Required properties:
Optional properties:
- clocks: should contain the IP block clock
- clock-names: should be "PCLK" for the IP block clock
+- dr_mode : indicates the working mode for "fotg210-dr" compatible
+ controllers. Can be "host", "peripheral". Default to
+ "host" if not defined for backward compatibility.
Required properties for "cortina,gemini-usb" compatible:
- syscon: a phandle to the system controller to access PHY registers
@@ -25,7 +28,7 @@ Optional properties for "cortina,gemini-usb" compatible:
Example for Gemini:
usb@68000000 {
- compatible = "cortina,gemini-usb", "faraday,fotg210";
+ compatible = "cortina,gemini-usb", "faraday,fotg210-dr";
reg = <0x68000000 0x1000>;
interrupts = <10 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&cc 12>;
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
^ permalink raw reply related
* Re: [PATCH 1/3] of: overlay_adjust_phandles() - do not modify const field
From: Rob Herring @ 2017-04-24 16:56 UTC (permalink / raw)
To: Frank Rowand
Cc: Stephen Boyd, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1493011204-27635-2-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Mon, Apr 24, 2017 at 12:20 AM, <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>
> When adjusting overlay phandles to apply to the live device tree, can
> not modify the property value because it is type const.
>
> This is to resolve the issue found by Stephen Boyd [1] when he changed
> the type of struct property.value from void * to const void *. As
> a result of the type change, the overlay code had compile errors
> where the resolver updates phandle values.
Conceptually, I prefer your first version. phandles are special and
there's little reason to expose them except to generate a dts or dtb
from /proc/device-tree. We could still generate the phandle file in
that case, but I don't know if special casing phandle is worth it.
>
> [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
>
> Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
> ---
> drivers/of/base.c | 4 ++--
> drivers/of/dynamic.c | 28 +++++++++++++++++++++------
> drivers/of/of_private.h | 3 +++
> drivers/of/resolver.c | 51 ++++++++++++++++++++++++++++++-------------------
> 4 files changed, 58 insertions(+), 28 deletions(-)
>
> diff --git a/drivers/of/base.c b/drivers/of/base.c
> index d7c4629a3a2d..b41650fd0fcf 100644
> --- a/drivers/of/base.c
> +++ b/drivers/of/base.c
> @@ -220,8 +220,8 @@ void __init of_core_init(void)
> proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
> }
>
> -static struct property *__of_find_property(const struct device_node *np,
> - const char *name, int *lenp)
> +struct property *__of_find_property(const struct device_node *np,
> + const char *name, int *lenp)
> {
> struct property *pp;
>
> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
> index 888fdbc09992..44963b4e7235 100644
> --- a/drivers/of/dynamic.c
> +++ b/drivers/of/dynamic.c
> @@ -354,17 +354,17 @@ void of_node_release(struct kobject *kobj)
> }
>
> /**
> - * __of_prop_dup - Copy a property dynamically.
> + * __of_prop_alloc - Create a property dynamically.
> * @prop: Property to copy
> * @allocflags: Allocation flags (typically pass GFP_KERNEL)
> *
> - * Copy a property by dynamically allocating the memory of both the
> + * Create a property by dynamically allocating the memory of both the
> * property structure and the property name & contents. The property's
> * flags have the OF_DYNAMIC bit set so that we can differentiate between
> * dynamically allocated properties and not.
> * Returns the newly allocated property or NULL on out of memory error.
> */
> -struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
> +struct property *__of_prop_alloc(char *name, void *value, int len, gfp_t allocflags)
> {
> struct property *new;
>
> @@ -378,9 +378,9 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
> * of zero bytes. We do this to work around the use
> * of of_get_property() calls on boolean values.
> */
> - new->name = kstrdup(prop->name, allocflags);
> - new->value = kmemdup(prop->value, prop->length, allocflags);
> - new->length = prop->length;
> + new->name = kstrdup(name, allocflags);
> + new->value = kmemdup(value, len, allocflags);
> + new->length = len;
> if (!new->name || !new->value)
> goto err_free;
>
> @@ -397,6 +397,22 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
> }
>
> /**
> + * __of_prop_dup - Copy a property dynamically.
> + * @prop: Property to copy
> + * @allocflags: Allocation flags (typically pass GFP_KERNEL)
> + *
> + * Copy a property by dynamically allocating the memory of both the
> + * property structure and the property name & contents. The property's
> + * flags have the OF_DYNAMIC bit set so that we can differentiate between
> + * dynamically allocated properties and not.
> + * Returns the newly allocated property or NULL on out of memory error.
> + */
> +struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
> +{
> + return __of_prop_alloc(prop->name, prop->value, prop->length, allocflags);
> +}
> +
> +/**
> * __of_node_dup() - Duplicate or create an empty device node dynamically.
> * @fmt: Format string (plus vargs) for new full name of the device node
> *
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index 18bbb4517e25..554394c96569 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -62,6 +62,7 @@ static inline int of_property_notify(int action, struct device_node *np,
> * without taking node references, so you either have to
> * own the devtree lock or work on detached trees only.
> */
> +struct property *__of_prop_alloc(char *name, void *value, int len, gfp_t allocflags);
> struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags);
> __printf(2, 3) struct device_node *__of_node_dup(const struct device_node *np, const char *fmt, ...);
>
> @@ -70,6 +71,8 @@ extern const void *__of_get_property(const struct device_node *np,
> extern int __of_add_property(struct device_node *np, struct property *prop);
> extern int __of_add_property_sysfs(struct device_node *np,
> struct property *prop);
> +extern struct property *__of_find_property(const struct device_node *np,
> + const char *name, int *lenp);
> extern int __of_remove_property(struct device_node *np, struct property *prop);
> extern void __of_remove_property_sysfs(struct device_node *np,
> struct property *prop);
> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
> index 7ae9863cb0a4..a2d5b8f0b7bf 100644
> --- a/drivers/of/resolver.c
> +++ b/drivers/of/resolver.c
> @@ -20,6 +20,8 @@
> #include <linux/errno.h>
> #include <linux/slab.h>
>
> +#include "of_private.h"
> +
> /* illegal phandle value (set when unresolved) */
> #define OF_PHANDLE_ILLEGAL 0xdeadbeef
>
> @@ -67,36 +69,43 @@ static phandle live_tree_max_phandle(void)
> return phandle;
> }
>
> -static void adjust_overlay_phandles(struct device_node *overlay,
> +static int adjust_overlay_phandles(struct device_node *overlay,
> int phandle_delta)
> {
> struct device_node *child;
> + struct property *newprop;
> struct property *prop;
> phandle phandle;
Some of these can move into the if statement. That will save some
stack space on recursion (or maybe the compiler is smart enough).
> + int ret;
>
> - /* adjust node's phandle in node */
> - if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL)
> - overlay->phandle += phandle_delta;
> -
> - /* copy adjusted phandle into *phandle properties */
> - for_each_property_of_node(overlay, prop) {
> + if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL) {
>
> - if (of_prop_cmp(prop->name, "phandle") &&
> - of_prop_cmp(prop->name, "linux,phandle"))
> - continue;
> -
> - if (prop->length < 4)
> - continue;
> + overlay->phandle += phandle_delta;
>
> - phandle = be32_to_cpup(prop->value);
> - if (phandle == OF_PHANDLE_ILLEGAL)
> - continue;
> + phandle = cpu_to_be32(overlay->phandle);
> +
> + prop = __of_find_property(overlay, "phandle", NULL);
> + newprop = __of_prop_alloc(prop->name, &phandle, sizeof(phandle),
> + GFP_KERNEL);
> + if (!newprop)
> + return -ENOMEM;
> + __of_update_property(overlay, newprop, &prop);
> +
> + prop = __of_find_property(overlay, "linux,phandle", NULL);
> + newprop = __of_prop_alloc(prop->name, &phandle, sizeof(phandle),
> + GFP_KERNEL);
There is no reason to support "linux,phandle" for overlays. That is
legacy (pre ePAPR) which predates any overlays by a long time.
Also, dtc still defaults to generating both phandle and linux,phandle
properties which maybe we should switch off now. If anything, we're
wasting a bit of memory storing both. I think we should only store
"phandle" and convert any cases of only a "linux,phandle" property to
"phandle".
Rob
--
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/2] dmaengine: Add DW AXI DMAC driver
From: Andy Shevchenko @ 2017-04-24 16:56 UTC (permalink / raw)
To: Eugeniy Paltsev
Cc: vinod.koul@intel.com, linux-kernel@vger.kernel.org,
robh+dt@kernel.org, Alexey.Brodkin@synopsys.com,
devicetree@vger.kernel.org, linux-snps-arc@lists.infradead.org,
dan.j.williams@intel.com, dmaengine@vger.kernel.org
In-Reply-To: <1493049305.25985.4.camel@synopsys.com>
On Mon, 2017-04-24 at 15:55 +0000, Eugeniy Paltsev wrote:
> Hi,
> On Fri, 2017-04-21 at 18:13 +0300, Andy Shevchenko wrote:
> > On Fri, 2017-04-21 at 14:29 +0000, Eugeniy Paltsev wrote:
> > > On Tue, 2017-04-18 at 15:31 +0300, Andy Shevchenko wrote:
> > > > On Fri, 2017-04-07 at 17:04 +0300, Eugeniy Paltsev wrote:
> > > > > This patch adds support for the DW AXI DMAC controller.
> > > > > +#define AXI_DMA_BUSWIDTHS \
> > > > > + (DMA_SLAVE_BUSWIDTH_1_BYTE | \
> > > > > + DMA_SLAVE_BUSWIDTH_2_BYTES | \
> > > > > + DMA_SLAVE_BUSWIDTH_4_BYTES | \
> > > > > + DMA_SLAVE_BUSWIDTH_8_BYTES | \
> > > > > + DMA_SLAVE_BUSWIDTH_16_BYTES | \
> > > > > + DMA_SLAVE_BUSWIDTH_32_BYTES | \
> > > > > + DMA_SLAVE_BUSWIDTH_64_BYTES)
> > > > > +/* TODO: check: do we need to use BIT() macro here? */
> > > >
> > > > Still TODO? I remember I answered to this on the first round.
> > >
> > > Yes, I remember it.
> > > I left this TODO as a reminder because src_addr_widths and
> > > dst_addr_widths are
> > > not used anywhere and they are set differently in different
> > > drivers
> > > (with or without BIT macro).
> >
> > Strange. AFAIK they are representing bits (which is not the best
> > idea) in the resulting u64 field. So, anything bigger than 63
> > doesn't
> > make sense.
>
> They are u32 fields!
Even "better"!
> From dmaengine.h :
> struct dma_device {
> ...
> u32 src_addr_widths;
> u32 dst_addr_widths;
> };
>
> > In drivers where they are not bits quite likely a bug is hidden.
>
> For example (from pxa_dma.c):
> const enum dma_slave_buswidth widths = DMA_SLAVE_BUSWIDTH_1_BYTE |
> DMA_SLAVE_BUSWIDTH_2_BYTES | DMA_SLAVE_BUSWIDTH_4_BYTES;
>
> And there are a lot of drivers, which don't use BIT for this fields.
> sh/shdmac.c
> sh/rcar-dmac.c
> qcom/bam_dma.c
> mmp_pdma.c
> ste_dma40.c
> And many others...
Definitely the concept of that interface never thought enough and broken
by design.
> > > > > +static inline void
> > > > > +axi_dma_iowrite32(struct axi_dma_chip *chip, u32 reg, u32
> > > > > val)
> > > > > +{
> > > > > + iowrite32(val, chip->regs + reg);
> > > >
> > > > Are you going to use IO ports for this IP? I don't think so.
> > > > Wouldn't be better to call readl()/writel() instead?
> > >
> > > As I understand, it's better to use ioread/iowrite as more
> > > universal
> > > IO
> > > access way. Am I wrong?
> >
> > As I said above the ioreadX/iowriteX makes only sense when your IP
> > would be accessed via IO region or MMIO. I'm pretty sure IO is not
> > the case at all for this IP.
>
> MMIO? This IP works exactly via memory-mapped I/O.
Yes, and why do you need to check this on each IO read/write?
Please, switch to plain readX()/writeX() instead.
> > > > > + val = axi_chan_ioread32(chan,
> > > > > CH_INTSTATUS_ENA);
> > > > > + val &= ~irq_mask;
> > > > > + axi_chan_iowrite32(chan, CH_INTSTATUS_ENA,
> > > > > val);
> > > > > + }
> > > > > +
> > > > > + return min_t(size_t, __ffs(sdl), max_width);
> > > > > +}
> > > > > +static void axi_desc_put(struct axi_dma_desc *desc)
> > > > > +{
> > > > > + struct axi_dma_chan *chan = desc->chan;
> > > > > + struct dw_axi_dma *dw = chan->chip->dw;
> > > > > + struct axi_dma_desc *child, *_next;
> > > > > + unsigned int descs_put = 0;
> > > > > + list_for_each_entry_safe(child, _next, &desc-
> > > > > > xfer_list,
> > > > >
> > > > > xfer_list) {
> > > >
> > > > xfer_list looks redundant.
> > > > Can you elaborate why virtual channel management is not working
> > > > for
> > > > you?
> > >
> > > Each virtual descriptor encapsulates several hardware descriptors,
> > > which belong to same transfer.
> > > This list (xfer_list) is used only for allocating/freeing these
> > > descriptors and it doesn't affect on virtual dma work logic.
> > > I can see this approach in several drivers with VirtDMA (but they
> > > mostly use array instead of list)
> >
> > You described how most of the DMA drivers are implemented, though
> > they
> > are using just sg_list directly. I would recommend to do the same
> > and
> > get rid of this list.
>
> This IP can be (ans is) configured with small block size.
> (note, that I am not saying about runtime HW configuration)
>
> And there is opportunity what we can't use sg_list directly and need
> to
> split sg_list to a smaller chunks.
That's what I have referred quite ago. The driver should provide an
interface to tell potential caller what maximum block (number of items
with given bus width) it supports.
We have struct dma_parms in struct device, but what we actually need is
to support similar on per channel basis in DMAengine framework.
So, instead of working around this I recommend either to implement it
properly or rely on the fact that in the future someone eventually does
that for you.
Each driver which has this re-splitting mechanism should be cleaned up
and refactored.
> > > > Btw, are you planning to use priority at all? For now on I
> > > > didn't
> > > > see
> > > > a single driver (from the set I have checked, like 4-5 of them)
> > > > that
> > > > uses priority anyhow. It makes driver more complex for nothing.
> > >
> > > Only for dma slave operations.
> >
> > So, in other words you *have* an actual two or more users that
> > *need*
> > prioritization?
>
> As I remember there was an idea to give higher priority to audio dma
> chanels.
I don't see cyclic transfers support in the driver. So, I would suggest
just drop entire prioritization for now. When it would be actual user
one may start thinking of it.
Just a rule of common sense: do not implement something which will have
no user or solve non-existing problem.
> > > > As I said earlier dw_dmac is *bad* example of the (virtual
> > > > channel
> > > > based) DMA driver.
> > > >
> > > > I guess you may just fail the descriptor and don't pretend it
> > > > has
> > > > been processed successfully.
> > >
> > > What do you mean by saying "fail the descriptor"?
> > > After I get error I cancel current transfer and free all
> > > descriptors
> > > from it (by calling vchan_cookie_complete).
> > > I can't store error status in descriptor structure because it will
> > > be
> > > freed by vchan_cookie_complete.
> > > I can't store error status in channel structure because it will be
> > > overwritten by next transfer.
> >
> > Better not to pretend that it has been processed successfully. Don't
> > call callback on it and set its status to DMA_ERROR (that's why
> > descriptors in many drivers have dma_status field). When user asks
> > for
> > status (using cookie) the saved value would be returned until
> > descriptor
> > is active.
> >
> > Do you have some other workflow in mind?
>
> Hmm...
> Do you mean I should left error descriptors in desc_issued list
> or I should create another list (like desc_error) in my driver and
> move
> error descriptors to desc_error list?
>
> And when exactly should I free error descriptors?
See below.
> I checked hsu/hsu.c dma driver implementation:
> vdma descriptor is deleted from desc_issued list when transfer
> starts. When descriptor marked as error descriptor
> vchan_cookie_complete isn't called for this descriptor. And this
> descriptor isn't placed in any list. So error descriptors *never*
> will be freed.
> I don't actually like this approach.
Descriptor is active until terminate_all() is called or new descriptor
is supplied. So, the caller has a quite time to check on it.
So, what's wrong on it by your opinion?
Of course, if you want to keep by some reason (should be stated what the
reason in comment) erred descriptors, you can do that.
> > > > > +static const struct dev_pm_ops dw_axi_dma_pm_ops = {
> > > > > + SET_RUNTIME_PM_OPS(axi_dma_runtime_suspend,
> > > > > axi_dma_runtime_resume, NULL)
> > > > > +};
> > > >
> > > > Have you tried to build with CONFIG_PM disabled?
> > >
> > > Yes.
> > >
> > > > I'm pretty sure you need __maybe_unused applied to your PM ops.
> > >
> > > I call axi_dma_runtime_suspend / axi_dma_runtime_resume even I
> > > dont't
> > > use PM.
> > > (I call them in probe / remove function.)
> >
> > Hmm... I didn't check your ->probe() and ->remove(). Do you mean you
> > call them explicitly by those names?
> >
> > If so, please don't do that. Use pm_runtime_*() instead. And...
> >
> > > So I don't need to declare them with __maybe_unused.
> >
> > ...in that case it's possible you have them defined but not used.
> >
>
> From my ->probe() function:
>
> pm_runtime_get_noresume(chip->dev);
> ret = axi_dma_runtime_resume(chip->dev);
>
> Firstly I only incrememt counter.
> Secondly explicitly call my resume function.
>
> I call them explicitly because I need driver to work also without
> Runtime PM. So I can't just call pm_runtime_get here instead of
> pm_runtime_get_noresume + axi_dma_runtime_resume.
>
> Of course I can copy *all* code from axi_dma_runtime_resume
> to ->probe() function, but I don't really like this idea.
It looks like you need more time to investigate how runtime PM works
from driver point of view, but you shouldn't call your PM callbacks
directly without a really good reason (weird silicon bugs, architectural
impediments). I don't think that is the case here.
> > > > > +
> > > > > + /* these other elements are all protected by vc.lock
> > > > > */
> > > > > + bool is_paused;
> > > >
> > > > I still didn't get (already forgot) why you can't use dma_status
> > > > instead for the active descriptor?
> > >
> > > As I said before, I checked several driver, which have status
> > > variable
> > > in their channel structure - it is used *only* for determinating
> > > is
> > > channel paused or not. So there is no much sense in replacing
> > > "is_paused" to "status" and I left "is_paused" variable untouched.
> >
> > Not only (see above), the errored descriptor keeps that status.
> >
> > > (I described above why we can't use status in channel structure
> > > for
> > > error handling)
> >
> > Ah, I'm talking about descriptor.
>
> Again - PAUSED is per-channel flag. So, even if we have status field
> in
> each descriptor, it is simpler to use one per-channel flag instead of
> plenty per-descriptor flags.
> When we pausing/resuming dma channel it is simpler to set only one
> flag
> instead of writing DMA_PAUSED to *each* descriptor status field.
What do you mean by "each"? I don't recall the driver which can handle
more than one *active* descriptor per channel. Do you?
In that case status of active descriptor == status of channel. That
trick (I also already referred to earlier) is used in some drivers.
> > I mean, who are the users of them? If it's only one module, there is
> > no need to put them in header.
>
> Yes, only one module.
> Should I move all this definitions to axi_dma_platform.c file and rid
> of both axi_dma_platform_reg.h and axi_dma_platform.h headers?
Depends on your design.
If it would be just one C module it might make sense to use driver.c and
driver.h or just driver.c.
I see several drivers in current linux-next that are using latter and
some that are using former, and number of plain driver.c variant is
bigger.
--
Andy Shevchenko <andriy.shevchenko@linux.intel.com>
Intel Finland Oy
^ permalink raw reply
* Re: [PATCH] rtc: ds1374: Add trickle charger device tree binding
From: Alexandre Belloni @ 2017-04-24 17:16 UTC (permalink / raw)
To: Moritz Fischer
Cc: Rob Herring, rtc-linux, Devicetree List, Alessandro Zummo,
Mark Rutland
In-Reply-To: <CAAtXAHfNuRXninLWySAOdtV99ih4PBt=JPszn3UkNa08OvXLFQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 22/04/2017 at 10:54:23 -0700, Moritz Fischer wrote:
> On Thu, Apr 20, 2017 at 10:25 AM, Moritz Fischer <mdf-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > On Thu, Apr 20, 2017 at 10:56:34AM -0500, Rob Herring wrote:
> >> On Mon, Apr 17, 2017 at 03:40:10PM -0700, Moritz Fischer wrote:
> >> > Introduce a device tree binding for specifying the trickle charger
> >> > configuration for ds1374. This is based on the code for ds13390.
> >> >
> >> > Signed-off-by: Moritz Fischer <mdf-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> >> > ---
> >> > .../devicetree/bindings/rtc/dallas,ds1374.txt | 18 ++++++++
> >> > drivers/rtc/rtc-ds1374.c | 54 ++++++++++++++++++++++
> >> > 2 files changed, 72 insertions(+)
> >> > create mode 100644 Documentation/devicetree/bindings/rtc/dallas,ds1374.txt
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/rtc/dallas,ds1374.txt b/Documentation/devicetree/bindings/rtc/dallas,ds1374.txt
> >> > new file mode 100644
> >> > index 0000000..4cf5bd7
> >> > --- /dev/null
> >> > +++ b/Documentation/devicetree/bindings/rtc/dallas,ds1374.txt
> >> > @@ -0,0 +1,18 @@
> >> > +* Dallas DS1374 I2C Real-Time Clock / WDT
> >>
> >> Please remove from trivial-devices.txt, too. (which is moving in 4.12
> >> BTW)
> >
> > Ok, I'll redo this on top of b7e252fcddfa573bb1ee275b53bba6cef85671d4
> > (Documentation: devicetree: move trivial-devices out of I2C realm) then.
>
> Follow up question, right now one selects between WDT and ALARM mode with
> a CONFIG_RTC_DRV_DS1374_WDT=y statically at compile time.
>
> I'd like to add a 'dallas,mode = <DS1374_WDT>;' or
> dallas,enable-watchdog property
> to the binding, same goes for the ability to remap the WDT reset output to the
> interrupt pin (which is currently not supported, but my hardware needs this).
>
> Would be the right way to add the remapping something like
> 'dallas,remap-reset-to-int' ?
>
> Ideas? This change would obviously break people's setups where they
> select one or
> the other behavior via the build time option. Is that acceptable seen
> that relying on
> build time CONFIG_FOO seems like a bad assumption to begin with?
>
> A bit of background: I currently started cleaning up a bunch of issues
> in this driver,
> like refactoring it to use the watchdog framework instead of open
> coding everything,
> make setting the timeout actually work (right now it the timeout to
> tick conversion is
> hosed).
>
I think I would do something with MFD as you were suggesting on IRC. You
can have a look at the flexcom driver (atmel-flexcom.c) which is simply
selecting a mode and then using whatever IP driver already existed
(UART, I2C or SPI).
I didn't have a close look but maybe that fits. Then, as you are
concerned wtih backward compatibility, you could enforce the mode from
the MFD driver, depending on the CONFIG_RTC_DRV_DS1374_WDT value.
--
Alexandre Belloni, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
You received this message because you are subscribed to "rtc-linux".
Membership options at http://groups.google.com/group/rtc-linux .
Please read http://groups.google.com/group/rtc-linux/web/checklist
before submitting a driver.
---
You received this message because you are subscribed to the Google Groups "rtc-linux" group.
To unsubscribe from this group and stop receiving emails from it, send an email to rtc-linux+unsubscribe-/JYPxA39Uh5TLH3MbocFF+G/Ez6ZCGd0@public.gmane.org
For more options, visit https://groups.google.com/d/optout.
^ permalink raw reply
* Re: [PATCH 2/2] of: Add unit tests for applying overlays.
From: Rob Herring @ 2017-04-24 17:16 UTC (permalink / raw)
To: Frank Rowand
Cc: Stephen Boyd, Michal Marek,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Kbuild mailing list
In-Reply-To: <1493012595-696-3-git-send-email-frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On Mon, Apr 24, 2017 at 12:43 AM, <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>
> Existing overlay unit tests examine individual pieces of the overlay
> code. The new tests target the entire process of applying an overlay.
>
> Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
> ---
>
> There are checkpatch warnings. I have reviewed them and feel they
> can be ignored.
>
> drivers/of/fdt.c | 45 +++++
> drivers/of/of_private.h | 2 +
> drivers/of/unittest-data/Makefile | 17 +-
> drivers/of/unittest-data/ot.dts | 53 ++++++
> drivers/of/unittest-data/ot_bad_phandle.dts | 20 +++
> drivers/of/unittest-data/ot_base.dts | 71 ++++++++
> drivers/of/unittest.c | 264 ++++++++++++++++++++++++++++
> 7 files changed, 469 insertions(+), 3 deletions(-)
> create mode 100644 drivers/of/unittest-data/ot.dts
> create mode 100644 drivers/of/unittest-data/ot_bad_phandle.dts
> create mode 100644 drivers/of/unittest-data/ot_base.dts
ot? Overlay Test? That's not very obvious if so.
> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
> index e5ce4b59e162..54120ab8f322 100644
> --- a/drivers/of/fdt.c
> +++ b/drivers/of/fdt.c
> @@ -31,6 +31,8 @@
> #include <asm/setup.h> /* for COMMAND_LINE_SIZE */
> #include <asm/page.h>
>
> +#include "of_private.h"
> +
> /*
> * of_fdt_limit_memory - limit the number of regions in the /memory node
> * @limit: maximum entries
> @@ -1256,11 +1258,54 @@ bool __init early_init_dt_scan(void *params)
> */
> void __init unflatten_device_tree(void)
> {
> +#ifdef CONFIG_OF_UNITTEST
> + extern uint8_t __dtb_ot_base_begin[];
> + extern uint8_t __dtb_ot_base_end[];
> + struct device_node *ot_base_root;
> + void *ot_base;
> + u32 data_size;
> + u32 size;
> +#endif
> +
> __unflatten_device_tree(initial_boot_params, NULL, &of_root,
> early_init_dt_alloc_memory_arch, false);
>
> /* Get pointer to "/chosen" and "/aliases" nodes for use everywhere */
> of_alias_scan(early_init_dt_alloc_memory_arch);
Just make __unflatten_device_tree accessible to the unit test code and
move all this to it. Then you don't need the ifdefery.
Does this need to be immediately after unflattening the base tree?
> +
> +#ifdef CONFIG_OF_UNITTEST
> + /*
> + * Base device tree for the overlay unittest.
> + * Do as much as possible the same way as done for the normal FDT.
> + * Have to stop before resolving phandles, because that uses kmalloc.
> + */
> +
> + data_size = __dtb_ot_base_end - __dtb_ot_base_begin;
> + if (!data_size) {
> + pr_err("No __dtb_ot_base_begin to attach\n");
> + return;
> + }
> +
> + size = fdt_totalsize(__dtb_ot_base_begin);
> + if (size != data_size) {
> + pr_err("__dtb_ot_base_begin header totalsize != actual size");
> + return;
> + }
> +
> + ot_base = early_init_dt_alloc_memory_arch(size,
> + roundup_pow_of_two(FDT_V17_SIZE));
> + if (!ot_base) {
> + pr_err("alloc of ot_base failed");
> + return;
> + }
> +
> + memcpy(ot_base, __dtb_ot_base_begin, size);
> +
> + __unflatten_device_tree(ot_base, NULL, &ot_base_root,
> + early_init_dt_alloc_memory_arch, true);
> +
> + unittest_set_ot_base_root(ot_base_root);
> +#endif
> }
>
> /**
> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
> index f4f6793d2f04..02d54da078ac 100644
> --- a/drivers/of/of_private.h
> +++ b/drivers/of/of_private.h
> @@ -55,6 +55,8 @@ static inline int of_property_notify(int action, struct device_node *np,
> }
> #endif /* CONFIG_OF_DYNAMIC */
>
> +extern void unittest_set_ot_base_root(struct device_node *dn);
> +
> /**
> * General utilities for working with live trees.
> *
> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
> index 1ac5cc01d627..6879befe29d2 100644
> --- a/drivers/of/unittest-data/Makefile
> +++ b/drivers/of/unittest-data/Makefile
> @@ -1,7 +1,18 @@
> obj-y += testcases.dtb.o
> +obj-y += ot.dtb.o
> +obj-y += ot_bad_phandle.dtb.o
> +obj-y += ot_base.dtb.o
>
> targets += testcases.dtb testcases.dtb.S
> +targets += ot.dtb ot.dtb.S
> +targets += ot_bad_phandle.dtb ot_bad_phandle.dtb.S
> +targets += ot_base.dtb ot_base.dtb.S
>
> -.SECONDARY: \
> - $(obj)/testcases.dtb.S \
> - $(obj)/testcases.dtb
> +.PRECIOUS: \
> + $(obj)/%.dtb.S \
> + $(obj)/%.dtb
> +
> +# enable creation of __symbols__ node
> +DTC_FLAGS_ot := -@
> +DTC_FLAGS_ot_base := -@
> +DTC_FLAGS_ot_bad_phandle := -@
> diff --git a/drivers/of/unittest-data/ot.dts b/drivers/of/unittest-data/ot.dts
> new file mode 100644
> index 000000000000..37e105622b7a
> --- /dev/null
> +++ b/drivers/of/unittest-data/ot.dts
> @@ -0,0 +1,53 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +
> + fragment@0 {
> + target = <&electric_1>;
> +
> + __overlay__ {
> + status = "ok";
> +
> + hvac_2: hvac_large_1 {
We should follow best practice here and not use '_' for node names. I
wonder what warnings dtc throws with W=2 for the unittests. Don't
think I tried that when adding them.
> + compatible = "ot,hvac-large";
> + heat-range = < 40 75 >;
> + cool-range = < 65 80 >;
> + };
> + };
> + };
> +
> + fragment@1 {
> + target = <&rides_1>;
> +
> + __overlay__ {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + status = "ok";
> +
> + ride@200 {
> + compatible = "ot,ferris-wheel";
> + reg = < 0x00000200 0x100 >;
> + hvac-provider = < &hvac_2 >;
> + hvac-thermostat = < 27 32 > ;
> + hvac-zones = < 12 5 >;
> + hvac-zone-names = "operator", "snack-bar";
> + spin-controller = < &spin_ctrl_1 3 >;
> + spin-rph = < 30 >;
> + gondolas = < 16 >;
> + gondola-capacity = < 6 >;
> + };
> + };
> + };
> +
> + fragment@2 {
> + target = <&lights_2>;
> +
> + __overlay__ {
> + status = "ok";
> + color = "purple", "white", "red", "green";
> + rate = < 3 256 >;
> + };
> + };
> +
> +};
> diff --git a/drivers/of/unittest-data/ot_bad_phandle.dts b/drivers/of/unittest-data/ot_bad_phandle.dts
> new file mode 100644
> index 000000000000..234d5f1dcfe4
> --- /dev/null
> +++ b/drivers/of/unittest-data/ot_bad_phandle.dts
> @@ -0,0 +1,20 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> +
> + fragment@0 {
> + target = <&electric_1>;
> +
> + __overlay__ {
> +
> + // This label should cause an error when the overlay
> + // is applied. There is already a phandle value
> + // in the base tree for motor_1.
> + spin_ctrl_1_conflict: motor_1 {
> + accelerate = < 3 >;
> + decelerate = < 5 >;
> + };
> + };
> + };
> +};
> diff --git a/drivers/of/unittest-data/ot_base.dts b/drivers/of/unittest-data/ot_base.dts
> new file mode 100644
> index 000000000000..0a4fbe598b02
> --- /dev/null
> +++ b/drivers/of/unittest-data/ot_base.dts
> @@ -0,0 +1,71 @@
> +/dts-v1/;
> +/plugin/;
> +
> +/ {
> + testcase-data-2 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> +
> + electric_1: substation@100 {
> + compatible = "ot,big-volts-control";
> + reg = < 0x00000100 0x100 >;
> + status = "disabled";
> +
> + hvac_1: hvac_medium_1 {
> + compatible = "ot,hvac-medium";
> + heat-range = < 50 75 >;
> + cool-range = < 60 80 >;
> + };
> +
> + spin_ctrl_1: motor_1 {
> + compatible = "ot,ferris-wheel-motor";
> + spin = "clockwise";
> + };
> +
> + spin_ctrl_2: motor_8 {
> + compatible = "ot,roller-coaster-motor";
> + };
> + };
> +
> + rides_1: fairway_1 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "ot,rides";
> + status = "disabled";
> + orientation = < 127 >;
> +
> + ride@100 {
> + compatible = "ot,roller-coaster";
> + reg = < 0x00000100 0x100 >;
> + hvac-provider = < &hvac_1 >;
> + hvac-thermostat = < 29 > ;
> + hvac-zones = < 14 >;
> + hvac-zone-names = "operator";
> + spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
> + spin-controller-names = "track_1", "track_2";
> + queues = < 2 >;
> + };
> + };
> +
> + lights_1: lights@30000 {
> + compatible = "ot,work-lights";
> + reg = < 0x00030000 0x1000 >;
> + status = "disabled";
> + };
> +
> + lights_2: lights@40000 {
> + compatible = "ot,show-lights";
> + reg = < 0x00040000 0x1000 >;
> + status = "disabled";
> + rate = < 13 138 >;
> + };
> +
> + retail_1: vending@50000 {
> + reg = < 0x00050000 0x1000 >;
> + compatible = "ot,tickets";
> + status = "disabled";
> + };
> +
> + };
> +};
> +
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index 62db55b97c10..599eb10e9b40 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -8,6 +8,7 @@
> #include <linux/err.h>
> #include <linux/errno.h>
> #include <linux/hashtable.h>
> +#include <linux/libfdt.h>
> #include <linux/of.h>
> #include <linux/of_fdt.h>
> #include <linux/of_irq.h>
> @@ -42,6 +43,13 @@
> failed; \
> })
>
> +static struct device_node *ot_base_root;
> +
> +void unittest_set_ot_base_root(struct device_node *dn)
> +{
> + ot_base_root = dn;
> +}
> +
> static void __init of_unittest_find_node_by_name(void)
> {
> struct device_node *np;
> @@ -1925,6 +1933,259 @@ static void __init of_unittest_overlay(void)
> static inline void __init of_unittest_overlay(void) { }
> #endif
>
> +#define OVERLAY_INFO_EXTERN(name) \
> + extern uint8_t __dtb_##name##_begin[]; \
> + extern uint8_t __dtb_##name##_end[]
> +
> +#define OVERLAY_INFO(name, expected) \
> +{ .dtb_begin = __dtb_##name##_begin, \
> + .dtb_end = __dtb_##name##_end, \
> + .expected_result = expected, \
> +}
> +
> +struct overlay_info {
> + uint8_t *dtb_begin;
> + uint8_t *dtb_end;
> + void *data;
> + struct device_node *np_overlay;
> + int expected_result;
> + int overlay_id;
> +};
> +
> +OVERLAY_INFO_EXTERN(ot);
> +OVERLAY_INFO_EXTERN(ot_bad_phandle);
> +
> +struct overlay_info overlays[] = {
> + OVERLAY_INFO(ot, 0),
> + OVERLAY_INFO(ot_bad_phandle, -EINVAL),
> + {}
> +};
> +
> +#ifdef CONFIG_OF_OVERLAY
> +/*
> + * The purpose of of_unittest_overlay_test_data_add is to add an
> + * overlay in the normal fashion. This is a test of the whole
> + * picture, instead of testing individual elements.
> + *
> + * A secondary purpose is to be able to verify that the contents of
> + * /proc/device-tree/ contains the updated structure and values from
> + * the overlay. That must be verified separately in user space.
> + *
> + * Return 0 on unexpected error.
> + */
> +static int __init overlay_test_data_add(int onum)
There's a need for a general function to apply built-in overlays
beyond just unittests. See
drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c. It's pretty close to the
same set of calls.
Rob
--
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] arm64: dts: freescale: ls2081ardb: Add DTS support for NXP LS2081ARDB
From: Yang Li @ 2017-04-24 17:20 UTC (permalink / raw)
To: Priyanka Jain
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Mark Rutland,
Shawn Guo,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Li Yang
In-Reply-To: <1492663067-2364-1-git-send-email-priyanka.jain-3arQi8VN3Tc@public.gmane.org>
We probably should rename the freescale folder to nxp with a separate
patch, but it is better to stop using the name in the patch title
right now.
On Wed, Apr 19, 2017 at 11:37 PM, Priyanka Jain <priyanka.jain-3arQi8VN3Tc@public.gmane.org> wrote:
> This patch add support for NXP LS2081ARDB board which has
> LS2081A SoC.
>
> LS2081A SoC is 40-pin derivative of LS2088A SoC
> So, from functional perspective both are same.
> Hence,ls2088a SoC dtsi files are included from ls2081ARDB dts
>
> Signed-off-by: Priyanka Jain <priyanka.jain-3arQi8VN3Tc@public.gmane.org>
> ---
> arch/arm64/boot/dts/freescale/Makefile | 1 +
> arch/arm64/boot/dts/freescale/fsl-ls2081a-rdb.dts | 139 +++++++++++++++++++++
> 2 files changed, 140 insertions(+), 0 deletions(-)
> create mode 100755 arch/arm64/boot/dts/freescale/fsl-ls2081a-rdb.dts
>
> diff --git a/arch/arm64/boot/dts/freescale/Makefile b/arch/arm64/boot/dts/freescale/Makefile
> index 72c4b52..58b80de 100644
> --- a/arch/arm64/boot/dts/freescale/Makefile
> +++ b/arch/arm64/boot/dts/freescale/Makefile
> @@ -9,6 +9,7 @@ dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1088a-qds.dtb
> dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls1088a-rdb.dtb
> dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2080a-qds.dtb
> dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2080a-rdb.dtb
> +dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2081a-rdb.dtb
> dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2080a-simu.dtb
> dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2088a-qds.dtb
> dtb-$(CONFIG_ARCH_LAYERSCAPE) += fsl-ls2088a-rdb.dtb
> diff --git a/arch/arm64/boot/dts/freescale/fsl-ls2081a-rdb.dts b/arch/arm64/boot/dts/freescale/fsl-ls2081a-rdb.dts
> new file mode 100444
> index 0000000..7ae408e
> --- /dev/null
> +++ b/arch/arm64/boot/dts/freescale/fsl-ls2081a-rdb.dts
> @@ -0,0 +1,139 @@
> +/*
> + * Device Tree file for NXP LS2081A RDB Board.
> + *
> + * Copyright (C) 2017, NXP Semiconductors
The formal copyright claim formal should be
Copyright 2017 NXP
> + *
> + * Priyanka Jain <priyanka.jain-3arQi8VN3Tc@public.gmane.org>
> + *
> + * This file is dual-licensed: you can use it either under the terms
> + * of the GPLv2 or the X11 license, at your option. Note that this dual
> + * licensing only applies to this file, and not this project as a
> + * whole.
> + *
> + * a) This library is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License as
> + * published by the Free Software Foundation; either version 2 of the
> + * License, or (at your option) any later version.
> + *
> + * This library is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * Or, alternatively,
> + *
> + * b) Permission is hereby granted, free of charge, to any person
> + * obtaining a copy of this software and associated documentation
> + * files (the "Software"), to deal in the Software without
> + * restriction, including without limitation the rights to use,
> + * copy, modify, merge, publish, distribute, sublicense, and/or
> + * sell copies of the Software, and to permit persons to whom the
> + * Software is furnished to do so, subject to the following
> + * conditions:
> + *
> + * The above copyright notice and this permission notice shall be
> + * included in all copies or substantial portions of the Software.
> + *
> + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
> + * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
> + * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
> + * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
> + * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
> + * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
> + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
> + * OTHER DEALINGS IN THE SOFTWARE.
> + */
> +
> +/dts-v1/;
> +
> +#include "fsl-ls2088a.dtsi"
> +
> +/ {
> + model = "NXP Layerscape 2081A RDB Board";
> + compatible = "fsl,ls2081a-rdb", "fsl,ls2081a";
> +
> + aliases {
> + serial0 = &serial0;
> + serial1 = &serial1;
> + };
> +
> + chosen {
> + stdout-path = "serial1:115200n8";
> + };
> +};
> +
> +&esdhc {
> + status = "okay";
> +};
> +
> +&ifc {
> + status = "disabled";
> +};
> +
> +&i2c0 {
> + status = "okay";
> + pca9547@75 {
> + compatible = "nxp,pca9547";
> + reg = <0x75>;
> + #address-cells = <1>;
> + #size-cells = <0>;
> + i2c@1 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x01>;
> + rtc@51 {
> + compatible = "nxp,pcf2129";
> + reg = <0x51>;
> + };
> + };
> +
> + i2c@3 {
> + #address-cells = <1>;
> + #size-cells = <0>;
> + reg = <0x3>;
> +
> + adt7481@4c {
> + compatible = "adi,adt7461";
> + reg = <0x4c>;
> + };
> + };
> + };
> +};
> +
> +&dspi {
> + status = "okay";
> + dflash0: n25q512a {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "st,m25p80";
> + spi-max-frequency = <3000000>;
> + reg = <0>;
> + };
> +};
> +
> +
> +&qspi {
> + status = "okay";
> + flash0: n25q512a@0 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "st,m25p80";
> + spi-max-frequency = <20000000>;
> + reg = <0>;
> + };
> + flash1: n25q512a@1 {
> + #address-cells = <1>;
> + #size-cells = <1>;
> + compatible = "st,m25p80";
> + spi-max-frequency = <20000000>;
> + reg = <0>;
> + };
> +};
> +
> +&usb0 {
> + status = "okay";
> +};
> +
> +&usb1 {
> + status = "okay";
> +};
> --
> 1.7.4.1
>
>
> --
> 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
--
- Leo
--
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: [RFC 2/3] of: reserved_mem: Accessor for acquiring reserved_mem
From: Frank Rowand @ 2017-04-24 17:27 UTC (permalink / raw)
To: Bjorn Andersson, Andy Gross, David Brown, Rob Herring,
Mark Rutland
Cc: linux-arm-msm, linux-soc, devicetree, linux-kernel
In-Reply-To: <20170422173519.5782-2-bjorn.andersson@linaro.org>
On 04/22/17 10:35, Bjorn Andersson wrote:
> In some cases drivers referencing a reserved-memory region might want to
> remap the entire region, but when defining the reserved-memory by "size"
> the client driver has no means to know the associated base address of
> the reserved memory region.
>
> This patch adds an accessor for such drivers to acquire a handle to
> their associated reserved-memory for this purpose.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>
> I would have preferred if we could provide a mechanism for drivers to find the
> reserved_mem of their own device_node, but without a phandle I have not been
> able to figure out a sane way to make the match.
>
> Suggestions are very welcome.
>
> drivers/of/of_reserved_mem.c | 26 ++++++++++++++++++++++++++
> include/linux/of_reserved_mem.h | 8 ++++++++
> 2 files changed, 34 insertions(+)
>
> diff --git a/drivers/of/of_reserved_mem.c b/drivers/of/of_reserved_mem.c
> index d507c3569a88..aa69c9590a5c 100644
> --- a/drivers/of/of_reserved_mem.c
> +++ b/drivers/of/of_reserved_mem.c
> @@ -397,3 +397,29 @@ void of_reserved_mem_device_release(struct device *dev)
> rmem->ops->device_release(rmem, dev);
> }
> EXPORT_SYMBOL_GPL(of_reserved_mem_device_release);
> +
> +/**
> + * of_get_reserved_mem_by_idx() - acquire reserved_mem from memory-region
> + * @np: node pointer containing the "memory-region"
> + * @idx: index within memory-region
> + *
> + * This function allows drivers to acquire a reference to the reserved_mem
> + * struct which is referenced by their memory-region.
> + *
> + * Returns a reserved_mem reference, or NULL on error.
> + */
> +struct reserved_mem *of_get_reserved_mem_by_idx(struct device_node *np, int idx)
> +{
> + struct device_node *target;
> + struct reserved_mem *rmem;
> +
> + target = of_parse_phandle(np, "memory-region", idx);
> + if (!target)
> + return NULL;
> +
> + rmem = __find_rmem(target);
> + of_node_put(target);
> +
> + return rmem;
> +}
> +EXPORT_SYMBOL_GPL(of_get_reserved_mem_by_idx);
> diff --git a/include/linux/of_reserved_mem.h b/include/linux/of_reserved_mem.h
> index f8e1992d6423..a9abbe7dd3de 100644
> --- a/include/linux/of_reserved_mem.h
> +++ b/include/linux/of_reserved_mem.h
> @@ -34,6 +34,8 @@ int of_reserved_mem_device_init_by_idx(struct device *dev,
> struct device_node *np, int idx);
> void of_reserved_mem_device_release(struct device *dev);
>
> +struct reserved_mem *of_get_reserved_mem_by_idx(struct device_node *np, int idx);
> +
> int early_init_dt_alloc_reserved_memory_arch(phys_addr_t size,
> phys_addr_t align,
> phys_addr_t start,
> @@ -52,6 +54,12 @@ static inline int of_reserved_mem_device_init_by_idx(struct device *dev,
> }
> static inline void of_reserved_mem_device_release(struct device *pdev) { }
>
> +static inline struct reserved_mem *of_get_reserved_mem_by_idx(struct device_node *np,
> + int idx);
> +{
> + return NULL;
> +}
> +
> static inline void fdt_init_reserved_mem(void) { }
> static inline void fdt_reserved_mem_save_node(unsigned long node,
> const char *uname, phys_addr_t base, phys_addr_t size) { }
>
Reviewed-by: Frank Rowand <frank.rowand@sony.com>
^ permalink raw reply
* Re: [RESEND PATCH 1/2] arc: axs10x: Add DT bindings for I2S audio playback
From: Vineet Gupta @ 2017-04-24 17:36 UTC (permalink / raw)
To: Jose Abreu,
linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
Cc: Carlos Palminha, Alexey Brodkin, Rob Herring,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <7ed6deaf1ae6d5819364c793835aa30316077a7e.1487787376.git.joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
On 04/21/2017 03:15 AM, Jose Abreu wrote:
> This patch adds the necessary DT bindings to get HDMI audio
> output in ARC AXS10x SDP. The bindings for I2S controller were
> added as well as the bindings for simple audio card.
Are these waiting on Rob or is it OK for me to pick these up for 4.12 ?
-Vineet
>
> Signed-off-by: Jose Abreu <joabreu-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Acked-by: Alexey Brodkin <abrodkin-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Cc: Carlos Palminha <palminha-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Cc: Alexey Brodkin <abrodkin-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Cc: Vineet Gupta <vgupta-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
> Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-snps-arc-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
> arch/arc/boot/dts/axs10x_mb.dtsi | 22 ++++++++++++++++++----
> 1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/arch/arc/boot/dts/axs10x_mb.dtsi b/arch/arc/boot/dts/axs10x_mb.dtsi
> index d6c1bbc..9d882b1 100644
> --- a/arch/arc/boot/dts/axs10x_mb.dtsi
> +++ b/arch/arc/boot/dts/axs10x_mb.dtsi
> @@ -149,12 +149,13 @@
> interrupts = <14>;
> };
>
> - i2c@0x1e000 {
> - compatible = "snps,designware-i2c";
> + i2s: i2s@1e000 {
> + compatible = "snps,designware-i2s";
> reg = <0x1e000 0x100>;
> - clock-frequency = <400000>;
> - clocks = <&i2cclk>;
> + clocks = <&i2sclk 0>;
> + clock-names = "i2sclk";
> interrupts = <15>;
> + #sound-dai-cells = <0>;
> };
>
> i2c@0x1f000 {
> @@ -174,6 +175,7 @@
> adi,input-colorspace = "rgb";
> adi,input-clock = "1x";
> adi,clock-delay = <0x03>;
> + #sound-dai-cells = <0>;
>
> ports {
> #address-cells = <1>;
> @@ -295,5 +297,17 @@
> };
> };
> };
> +
> + sound_playback {
> + compatible = "simple-audio-card";
> + simple-audio-card,name = "AXS10x HDMI Audio";
> + simple-audio-card,format = "i2s";
> + simple-audio-card,cpu {
> + sound-dai = <&i2s>;
> + };
> + simple-audio-card,codec {
> + sound-dai = <&adv7511>;
> + };
> + };
> };
> };
--
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 2/2] of: Add unit tests for applying overlays.
From: Frank Rowand @ 2017-04-24 17:47 UTC (permalink / raw)
To: Rob Herring
Cc: Stephen Boyd, Michal Marek,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Linux Kbuild mailing list
In-Reply-To: <CAL_JsqK3-MgkqBo6mGbs9dmo8Lwwx9dYW4SKyGjUDt_7wSecnA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 04/24/17 10:16, Rob Herring wrote:
> On Mon, Apr 24, 2017 at 12:43 AM, <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>>
>> Existing overlay unit tests examine individual pieces of the overlay
>> code. The new tests target the entire process of applying an overlay.
>>
>> Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>> ---
>>
>> There are checkpatch warnings. I have reviewed them and feel they
>> can be ignored.
>>
>> drivers/of/fdt.c | 45 +++++
>> drivers/of/of_private.h | 2 +
>> drivers/of/unittest-data/Makefile | 17 +-
>> drivers/of/unittest-data/ot.dts | 53 ++++++
>> drivers/of/unittest-data/ot_bad_phandle.dts | 20 +++
>> drivers/of/unittest-data/ot_base.dts | 71 ++++++++
>> drivers/of/unittest.c | 264 ++++++++++++++++++++++++++++
>> 7 files changed, 469 insertions(+), 3 deletions(-)
>> create mode 100644 drivers/of/unittest-data/ot.dts
>> create mode 100644 drivers/of/unittest-data/ot_bad_phandle.dts
>> create mode 100644 drivers/of/unittest-data/ot_base.dts
>
> ot? Overlay Test? That's not very obvious if so.
Yes, and yes. I'll make more explicit.
>
>> diff --git a/drivers/of/fdt.c b/drivers/of/fdt.c
>> index e5ce4b59e162..54120ab8f322 100644
>> --- a/drivers/of/fdt.c
>> +++ b/drivers/of/fdt.c
>> @@ -31,6 +31,8 @@
>> #include <asm/setup.h> /* for COMMAND_LINE_SIZE */
>> #include <asm/page.h>
>>
>> +#include "of_private.h"
>> +
>> /*
>> * of_fdt_limit_memory - limit the number of regions in the /memory node
>> * @limit: maximum entries
>> @@ -1256,11 +1258,54 @@ bool __init early_init_dt_scan(void *params)
>> */
>> void __init unflatten_device_tree(void)
>> {
>> +#ifdef CONFIG_OF_UNITTEST
>> + extern uint8_t __dtb_ot_base_begin[];
>> + extern uint8_t __dtb_ot_base_end[];
>> + struct device_node *ot_base_root;
>> + void *ot_base;
>> + u32 data_size;
>> + u32 size;
>> +#endif
>> +
>> __unflatten_device_tree(initial_boot_params, NULL, &of_root,
>> early_init_dt_alloc_memory_arch, false);
>>
>> /* Get pointer to "/chosen" and "/aliases" nodes for use everywhere */
>> of_alias_scan(early_init_dt_alloc_memory_arch);
>
> Just make __unflatten_device_tree accessible to the unit test code and
> move all this to it. Then you don't need the ifdefery.
Good idea. I'll do that.
> Does this need to be immediately after unflattening the base tree?
My goal is to make the creation of the test data in the tree follow
the normal process as much as possible, so that real code is tested
instead of testing test code.
This flattened device tree contains the base information that the
test overlays are applied against.
>> +
>> +#ifdef CONFIG_OF_UNITTEST
>> + /*
>> + * Base device tree for the overlay unittest.
>> + * Do as much as possible the same way as done for the normal FDT.
>> + * Have to stop before resolving phandles, because that uses kmalloc.
>> + */
>> +
>> + data_size = __dtb_ot_base_end - __dtb_ot_base_begin;
>> + if (!data_size) {
>> + pr_err("No __dtb_ot_base_begin to attach\n");
>> + return;
>> + }
>> +
>> + size = fdt_totalsize(__dtb_ot_base_begin);
>> + if (size != data_size) {
>> + pr_err("__dtb_ot_base_begin header totalsize != actual size");
>> + return;
>> + }
>> +
>> + ot_base = early_init_dt_alloc_memory_arch(size,
>> + roundup_pow_of_two(FDT_V17_SIZE));
>> + if (!ot_base) {
>> + pr_err("alloc of ot_base failed");
>> + return;
>> + }
>> +
>> + memcpy(ot_base, __dtb_ot_base_begin, size);
>> +
>> + __unflatten_device_tree(ot_base, NULL, &ot_base_root,
>> + early_init_dt_alloc_memory_arch, true);
>> +
>> + unittest_set_ot_base_root(ot_base_root);
>> +#endif
>> }
>>
>> /**
>> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
>> index f4f6793d2f04..02d54da078ac 100644
>> --- a/drivers/of/of_private.h
>> +++ b/drivers/of/of_private.h
>> @@ -55,6 +55,8 @@ static inline int of_property_notify(int action, struct device_node *np,
>> }
>> #endif /* CONFIG_OF_DYNAMIC */
>>
>> +extern void unittest_set_ot_base_root(struct device_node *dn);
>> +
>> /**
>> * General utilities for working with live trees.
>> *
>> diff --git a/drivers/of/unittest-data/Makefile b/drivers/of/unittest-data/Makefile
>> index 1ac5cc01d627..6879befe29d2 100644
>> --- a/drivers/of/unittest-data/Makefile
>> +++ b/drivers/of/unittest-data/Makefile
>> @@ -1,7 +1,18 @@
>> obj-y += testcases.dtb.o
>> +obj-y += ot.dtb.o
>> +obj-y += ot_bad_phandle.dtb.o
>> +obj-y += ot_base.dtb.o
>>
>> targets += testcases.dtb testcases.dtb.S
>> +targets += ot.dtb ot.dtb.S
>> +targets += ot_bad_phandle.dtb ot_bad_phandle.dtb.S
>> +targets += ot_base.dtb ot_base.dtb.S
>>
>> -.SECONDARY: \
>> - $(obj)/testcases.dtb.S \
>> - $(obj)/testcases.dtb
>> +.PRECIOUS: \
>> + $(obj)/%.dtb.S \
>> + $(obj)/%.dtb
>> +
>> +# enable creation of __symbols__ node
>> +DTC_FLAGS_ot := -@
>> +DTC_FLAGS_ot_base := -@
>> +DTC_FLAGS_ot_bad_phandle := -@
>> diff --git a/drivers/of/unittest-data/ot.dts b/drivers/of/unittest-data/ot.dts
>> new file mode 100644
>> index 000000000000..37e105622b7a
>> --- /dev/null
>> +++ b/drivers/of/unittest-data/ot.dts
>> @@ -0,0 +1,53 @@
>> +/dts-v1/;
>> +/plugin/;
>> +
>> +/ {
>> +
>> + fragment@0 {
>> + target = <&electric_1>;
>> +
>> + __overlay__ {
>> + status = "ok";
>> +
>> + hvac_2: hvac_large_1 {
>
> We should follow best practice here and not use '_' for node names. I
> wonder what warnings dtc throws with W=2 for the unittests. Don't
> think I tried that when adding them.
Oops. Will fix.
>> + compatible = "ot,hvac-large";
>> + heat-range = < 40 75 >;
>> + cool-range = < 65 80 >;
>> + };
>> + };
>> + };
>> +
>> + fragment@1 {
>> + target = <&rides_1>;
>> +
>> + __overlay__ {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + status = "ok";
>> +
>> + ride@200 {
>> + compatible = "ot,ferris-wheel";
>> + reg = < 0x00000200 0x100 >;
>> + hvac-provider = < &hvac_2 >;
>> + hvac-thermostat = < 27 32 > ;
>> + hvac-zones = < 12 5 >;
>> + hvac-zone-names = "operator", "snack-bar";
>> + spin-controller = < &spin_ctrl_1 3 >;
>> + spin-rph = < 30 >;
>> + gondolas = < 16 >;
>> + gondola-capacity = < 6 >;
>> + };
>> + };
>> + };
>> +
>> + fragment@2 {
>> + target = <&lights_2>;
>> +
>> + __overlay__ {
>> + status = "ok";
>> + color = "purple", "white", "red", "green";
>> + rate = < 3 256 >;
>> + };
>> + };
>> +
>> +};
>> diff --git a/drivers/of/unittest-data/ot_bad_phandle.dts b/drivers/of/unittest-data/ot_bad_phandle.dts
>> new file mode 100644
>> index 000000000000..234d5f1dcfe4
>> --- /dev/null
>> +++ b/drivers/of/unittest-data/ot_bad_phandle.dts
>> @@ -0,0 +1,20 @@
>> +/dts-v1/;
>> +/plugin/;
>> +
>> +/ {
>> +
>> + fragment@0 {
>> + target = <&electric_1>;
>> +
>> + __overlay__ {
>> +
>> + // This label should cause an error when the overlay
>> + // is applied. There is already a phandle value
>> + // in the base tree for motor_1.
>> + spin_ctrl_1_conflict: motor_1 {
>> + accelerate = < 3 >;
>> + decelerate = < 5 >;
>> + };
>> + };
>> + };
>> +};
>> diff --git a/drivers/of/unittest-data/ot_base.dts b/drivers/of/unittest-data/ot_base.dts
>> new file mode 100644
>> index 000000000000..0a4fbe598b02
>> --- /dev/null
>> +++ b/drivers/of/unittest-data/ot_base.dts
>> @@ -0,0 +1,71 @@
>> +/dts-v1/;
>> +/plugin/;
>> +
>> +/ {
>> + testcase-data-2 {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> +
>> + electric_1: substation@100 {
>> + compatible = "ot,big-volts-control";
>> + reg = < 0x00000100 0x100 >;
>> + status = "disabled";
>> +
>> + hvac_1: hvac_medium_1 {
>> + compatible = "ot,hvac-medium";
>> + heat-range = < 50 75 >;
>> + cool-range = < 60 80 >;
>> + };
>> +
>> + spin_ctrl_1: motor_1 {
>> + compatible = "ot,ferris-wheel-motor";
>> + spin = "clockwise";
>> + };
>> +
>> + spin_ctrl_2: motor_8 {
>> + compatible = "ot,roller-coaster-motor";
>> + };
>> + };
>> +
>> + rides_1: fairway_1 {
>> + #address-cells = <1>;
>> + #size-cells = <1>;
>> + compatible = "ot,rides";
>> + status = "disabled";
>> + orientation = < 127 >;
>> +
>> + ride@100 {
>> + compatible = "ot,roller-coaster";
>> + reg = < 0x00000100 0x100 >;
>> + hvac-provider = < &hvac_1 >;
>> + hvac-thermostat = < 29 > ;
>> + hvac-zones = < 14 >;
>> + hvac-zone-names = "operator";
>> + spin-controller = < &spin_ctrl_2 5 &spin_ctrl_2 7 >;
>> + spin-controller-names = "track_1", "track_2";
>> + queues = < 2 >;
>> + };
>> + };
>> +
>> + lights_1: lights@30000 {
>> + compatible = "ot,work-lights";
>> + reg = < 0x00030000 0x1000 >;
>> + status = "disabled";
>> + };
>> +
>> + lights_2: lights@40000 {
>> + compatible = "ot,show-lights";
>> + reg = < 0x00040000 0x1000 >;
>> + status = "disabled";
>> + rate = < 13 138 >;
>> + };
>> +
>> + retail_1: vending@50000 {
>> + reg = < 0x00050000 0x1000 >;
>> + compatible = "ot,tickets";
>> + status = "disabled";
>> + };
>> +
>> + };
>> +};
>> +
>> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
>> index 62db55b97c10..599eb10e9b40 100644
>> --- a/drivers/of/unittest.c
>> +++ b/drivers/of/unittest.c
>> @@ -8,6 +8,7 @@
>> #include <linux/err.h>
>> #include <linux/errno.h>
>> #include <linux/hashtable.h>
>> +#include <linux/libfdt.h>
>> #include <linux/of.h>
>> #include <linux/of_fdt.h>
>> #include <linux/of_irq.h>
>> @@ -42,6 +43,13 @@
>> failed; \
>> })
>>
>> +static struct device_node *ot_base_root;
>> +
>> +void unittest_set_ot_base_root(struct device_node *dn)
>> +{
>> + ot_base_root = dn;
>> +}
>> +
>> static void __init of_unittest_find_node_by_name(void)
>> {
>> struct device_node *np;
>> @@ -1925,6 +1933,259 @@ static void __init of_unittest_overlay(void)
>> static inline void __init of_unittest_overlay(void) { }
>> #endif
>>
>> +#define OVERLAY_INFO_EXTERN(name) \
>> + extern uint8_t __dtb_##name##_begin[]; \
>> + extern uint8_t __dtb_##name##_end[]
>> +
>> +#define OVERLAY_INFO(name, expected) \
>> +{ .dtb_begin = __dtb_##name##_begin, \
>> + .dtb_end = __dtb_##name##_end, \
>> + .expected_result = expected, \
>> +}
>> +
>> +struct overlay_info {
>> + uint8_t *dtb_begin;
>> + uint8_t *dtb_end;
>> + void *data;
>> + struct device_node *np_overlay;
>> + int expected_result;
>> + int overlay_id;
>> +};
>> +
>> +OVERLAY_INFO_EXTERN(ot);
>> +OVERLAY_INFO_EXTERN(ot_bad_phandle);
>> +
>> +struct overlay_info overlays[] = {
>> + OVERLAY_INFO(ot, 0),
>> + OVERLAY_INFO(ot_bad_phandle, -EINVAL),
>> + {}
>> +};
>> +
>> +#ifdef CONFIG_OF_OVERLAY
>> +/*
>> + * The purpose of of_unittest_overlay_test_data_add is to add an
>> + * overlay in the normal fashion. This is a test of the whole
>> + * picture, instead of testing individual elements.
>> + *
>> + * A secondary purpose is to be able to verify that the contents of
>> + * /proc/device-tree/ contains the updated structure and values from
>> + * the overlay. That must be verified separately in user space.
>> + *
>> + * Return 0 on unexpected error.
>> + */
>> +static int __init overlay_test_data_add(int onum)
>
> There's a need for a general function to apply built-in overlays
> beyond just unittests. See
> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c. It's pretty close to the
> same set of calls.
Yes, agreed.
My plan in the next release cycle is to first clean up drivers/of/overlay.c.
No functional changes, just cosmetic such as aligning function names with
what they actually do.
Then make some (hopefully) minor correctness changes, such as locking
correctly around phandle adjustments.
Then create the general function to apply built-in overlays and convert
all (two) separate implementations to use the common function. I did
not want to delay adding the unit tests to wait for this step.
> Rob
>
--
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 v8 1/3] backlight arcxcnn add arc to vendor prefix
From: Olimpiu Dejeu @ 2017-04-24 17:56 UTC (permalink / raw)
To: 'Rob Herring'
Cc: 'Lee Jones', linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, 'Jingoo Han',
'Brian Dodge', 'Joe Perches',
'Matthew D'Asaro', 'Daniel Thompson'
In-Reply-To: <CAL_JsqJ+=fQ2e752iCHs0dbdrkotuDtKp=2Z2_xnyCtMnxWV8A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Mon, April 24, 2017 11:10 AM, Rob Herring < robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Wed, Mar 15, 2017 at 2:45 PM, Olimpiu Dejeu <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org> wrote:
>> backlight: Add arc to vendor prefixes
>> Signed-off-by: Olimpiu Dejeu <olimpiu-eV7fy4qpoLhpLGFMi4vTTA@public.gmane.org>
>> ---
>> v8:
>> - Version to match other patches in set
>>
>> Documentation/devicetree/bindings/vendor-prefixes.txt | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> index 16d3b5e..6f33a4b 100644
>> --- a/Documentation/devicetree/bindings/vendor-prefixes.txt
>> +++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
>> @@ -28,6 +28,7 @@ andestech Andes Technology Corporation
>> apm Applied Micro Circuits Corporation (APM)
>> aptina Aptina Imaging
>> arasan Arasan Chip Systems
>> +arc Arctic Sand
>arc is also a cpu arch. While not a vendor, it could be confusing. How about "arctic" >instead?
Rob, will do, i.e. I will change it to "arctic"
>BTW, some reason your patches are not going to the DT list.
I'm emailing to devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, I think this is the correct list. Please advise.
>Rob
Olimpiu
--
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
* [PATCH v7 0/5] i2c: aspeed: added driver for Aspeed I2C
From: Brendan Higgins @ 2017-04-24 18:18 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, tglx-hfZtesqFncYOwBW4kG4KsQ,
jason-NLaQJdtUoK4Be96aLqz0jA, marc.zyngier-5wv7dgnIgG8,
joel-U3u1mxZcP9KHXe+LvDLADg, vz-ChpfBGZJDbMAvxtiuMwx3w,
mouse-Pma6HLj0uuo, clg-Bxea+6Xhats,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ
Addressed comments from:
- Ben in: http://www.spinics.net/lists/devicetree/msg170491.html
and: http://www.spinics.net/lists/devicetree/msg171232.html
- Rob: http://www.spinics.net/lists/devicetree/msg171593.html
- Joel in: http://www.spinics.net/lists/devicetree/msg171204.html
Changes since previous update:
- Renamed irq domain for consistency
- Changed clock-frequency to bus-frequency in device tree
- Made some fixes to clock divider code
- Added hardware reset function
- Marked functions that need to be called with the lock held as "unlocked"
- Did a bunch of clean up
Looks like there still might be some more work to do with multi-master support
and the clock divider stuff, but I will leave that up for others to decide.
As before, tested on Aspeed 2500 evaluation board and a real platform with an
Aspeed 2520.
--
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
* [PATCH v7 1/5] irqchip/aspeed-i2c-ic: binding docs for Aspeed I2C Interrupt Controller
From: Brendan Higgins @ 2017-04-24 18:18 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, tglx-hfZtesqFncYOwBW4kG4KsQ,
jason-NLaQJdtUoK4Be96aLqz0jA, marc.zyngier-5wv7dgnIgG8,
joel-U3u1mxZcP9KHXe+LvDLADg, vz-ChpfBGZJDbMAvxtiuMwx3w,
mouse-Pma6HLj0uuo, clg-Bxea+6Xhats,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, Brendan Higgins
In-Reply-To: <20170424181818.2754-1-brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Added device tree binding documentation for Aspeed I2C Interrupt
Controller.
Signed-off-by: Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
Added in v6:
- Pulled "aspeed_i2c_controller" out into a interrupt controller since that is
what it actually does.
Changes for v7:
- None
---
.../interrupt-controller/aspeed,ast2400-i2c-ic.txt | 25 ++++++++++++++++++++++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt
diff --git a/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt
new file mode 100644
index 000000000000..033cc82e5684
--- /dev/null
+++ b/Documentation/devicetree/bindings/interrupt-controller/aspeed,ast2400-i2c-ic.txt
@@ -0,0 +1,25 @@
+Device tree configuration for the I2C Interrupt Controller on the AST24XX and
+AST25XX SoCs.
+
+Required Properties:
+- #address-cells : should be 1
+- #size-cells : should be 1
+- #interrupt-cells : should be 1
+- compatible : should be "aspeed,ast2400-i2c-ic"
+ or "aspeed,ast2500-i2c-ic"
+- reg : address start and range of controller
+- interrupts : interrupt number
+- interrupt-controller : denotes that the controller receives and fires
+ new interrupts for child busses
+
+Example:
+
+i2c_ic: interrupt-controller@0 {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ #interrupt-cells = <1>;
+ compatible = "aspeed,ast2400-i2c-ic";
+ reg = <0x0 0x40>;
+ interrupts = <12>;
+ interrupt-controller;
+};
--
2.12.2.816.g2cccc81164-goog
--
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 v7 2/5] irqchip/aspeed-i2c-ic: Add I2C IRQ controller for Aspeed
From: Brendan Higgins @ 2017-04-24 18:18 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, tglx-hfZtesqFncYOwBW4kG4KsQ,
jason-NLaQJdtUoK4Be96aLqz0jA, marc.zyngier-5wv7dgnIgG8,
joel-U3u1mxZcP9KHXe+LvDLADg, vz-ChpfBGZJDbMAvxtiuMwx3w,
mouse-Pma6HLj0uuo, clg-Bxea+6Xhats,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, Brendan Higgins
In-Reply-To: <20170424181818.2754-1-brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
The Aspeed 24XX/25XX chips share a single hardware interrupt across 14
separate I2C busses. This adds a dummy irqchip which maps the single
hardware interrupt to software interrupts for each of the busses.
Signed-off-by: Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
Added in v6:
- Pulled "aspeed_i2c_controller" out into a interrupt controller since that is
what it actually does.
Changes for v7:
- Renamed irq domain for consistency
---
drivers/irqchip/Makefile | 2 +-
drivers/irqchip/irq-aspeed-i2c-ic.c | 102 ++++++++++++++++++++++++++++++++++++
2 files changed, 103 insertions(+), 1 deletion(-)
create mode 100644 drivers/irqchip/irq-aspeed-i2c-ic.c
diff --git a/drivers/irqchip/Makefile b/drivers/irqchip/Makefile
index 152bc40b6762..c136c2bd1761 100644
--- a/drivers/irqchip/Makefile
+++ b/drivers/irqchip/Makefile
@@ -74,6 +74,6 @@ obj-$(CONFIG_MVEBU_ODMI) += irq-mvebu-odmi.o
obj-$(CONFIG_MVEBU_PIC) += irq-mvebu-pic.o
obj-$(CONFIG_LS_SCFG_MSI) += irq-ls-scfg-msi.o
obj-$(CONFIG_EZNPS_GIC) += irq-eznps.o
-obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o
+obj-$(CONFIG_ARCH_ASPEED) += irq-aspeed-vic.o irq-aspeed-i2c-ic.o
obj-$(CONFIG_STM32_EXTI) += irq-stm32-exti.o
obj-$(CONFIG_QCOM_IRQ_COMBINER) += qcom-irq-combiner.o
diff --git a/drivers/irqchip/irq-aspeed-i2c-ic.c b/drivers/irqchip/irq-aspeed-i2c-ic.c
new file mode 100644
index 000000000000..a36fb09c10c2
--- /dev/null
+++ b/drivers/irqchip/irq-aspeed-i2c-ic.c
@@ -0,0 +1,102 @@
+/*
+ * Aspeed 24XX/25XX I2C Interrupt Controller.
+ *
+ * Copyright (C) 2012-2017 ASPEED Technology Inc.
+ * Copyright 2017 IBM Corporation
+ * Copyright 2017 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/irq.h>
+#include <linux/irqchip.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/io.h>
+
+
+#define ASPEED_I2C_IC_NUM_BUS 14
+
+struct aspeed_i2c_ic {
+ void __iomem *base;
+ int parent_irq;
+ struct irq_domain *irq_domain;
+};
+
+/*
+ * The aspeed chip provides a single hardware interrupt for all of the I2C
+ * busses, so we use a dummy interrupt chip to translate this single interrupt
+ * into multiple interrupts, each associated with a single I2C bus.
+ */
+static void aspeed_i2c_ic_irq_handler(struct irq_desc *desc)
+{
+ struct aspeed_i2c_ic *i2c_ic = irq_desc_get_handler_data(desc);
+ struct irq_chip *chip = irq_desc_get_chip(desc);
+ unsigned long bit, status;
+ unsigned int bus_irq;
+
+ chained_irq_enter(chip, desc);
+ status = readl(i2c_ic->base);
+ for_each_set_bit(bit, &status, ASPEED_I2C_IC_NUM_BUS) {
+ bus_irq = irq_find_mapping(i2c_ic->irq_domain, bit);
+ generic_handle_irq(bus_irq);
+ }
+ chained_irq_exit(chip, desc);
+}
+
+/*
+ * Set simple handler and mark IRQ as valid. Nothing interesting to do here
+ * since we are using a dummy interrupt chip.
+ */
+static int aspeed_i2c_ic_map_irq_domain(struct irq_domain *domain,
+ unsigned int irq, irq_hw_number_t hwirq)
+{
+ irq_set_chip_and_handler(irq, &dummy_irq_chip, handle_simple_irq);
+ irq_set_chip_data(irq, domain->host_data);
+
+ return 0;
+}
+
+static const struct irq_domain_ops aspeed_i2c_ic_irq_domain_ops = {
+ .map = aspeed_i2c_ic_map_irq_domain,
+};
+
+static int __init aspeed_i2c_ic_of_init(struct device_node *node,
+ struct device_node *parent)
+{
+ struct aspeed_i2c_ic *i2c_ic;
+
+ i2c_ic = kzalloc(sizeof(*i2c_ic), GFP_KERNEL);
+ if (!i2c_ic)
+ return -ENOMEM;
+
+ i2c_ic->base = of_iomap(node, 0);
+ if (IS_ERR(i2c_ic->base))
+ return PTR_ERR(i2c_ic->base);
+
+ i2c_ic->parent_irq = irq_of_parse_and_map(node, 0);
+ if (i2c_ic->parent_irq < 0)
+ return i2c_ic->parent_irq;
+
+ i2c_ic->irq_domain = irq_domain_add_linear(
+ node, ASPEED_I2C_IC_NUM_BUS,
+ &aspeed_i2c_ic_irq_domain_ops, NULL);
+ if (!i2c_ic->irq_domain)
+ return -ENOMEM;
+
+ i2c_ic->irq_domain->name = "aspeed-i2c-domain";
+
+ irq_set_chained_handler_and_data(i2c_ic->parent_irq,
+ aspeed_i2c_ic_irq_handler, i2c_ic);
+
+ pr_info("i2c controller registered, irq %d\n", i2c_ic->parent_irq);
+
+ return 0;
+}
+
+IRQCHIP_DECLARE(ast2400_i2c_ic, "aspeed,ast2400-i2c-ic", aspeed_i2c_ic_of_init);
+IRQCHIP_DECLARE(ast2500_i2c_ic, "aspeed,ast2500-i2c-ic", aspeed_i2c_ic_of_init);
--
2.12.2.816.g2cccc81164-goog
--
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 v7 3/5] i2c: aspeed: added documentation for Aspeed I2C driver
From: Brendan Higgins @ 2017-04-24 18:18 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, tglx, jason, marc.zyngier, joel, vz,
mouse, clg, benh
Cc: linux-i2c, devicetree, linux-kernel, openbmc, Brendan Higgins
In-Reply-To: <20170424181818.2754-1-brendanhiggins@google.com>
Added device tree binding documentation for Aspeed I2C busses.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Changes for v2:
- None
Changes for v3:
- Removed reference to "bus" device tree param
Changes for v4:
- None
Changes for v5:
- None
Changes for v6:
- Replaced the controller property with and interrupt controller, leaving only
the busses in the I2C documentation.
Changes for v7:
- Changed clock-frequency to bus-frequency in device tree
---
.../devicetree/bindings/i2c/i2c-aspeed.txt | 47 ++++++++++++++++++++++
1 file changed, 47 insertions(+)
create mode 100644 Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
diff --git a/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
new file mode 100644
index 000000000000..08ae65251080
--- /dev/null
+++ b/Documentation/devicetree/bindings/i2c/i2c-aspeed.txt
@@ -0,0 +1,47 @@
+Device tree configuration for the I2C busses on the AST24XX and AST25XX SoCs.
+
+Required Properties:
+- #address-cells : should be 1
+- #size-cells : should be 0
+- reg : address offset and range of bus
+- compatible : should be "aspeed,ast2400-i2c-bus"
+ or "aspeed,ast2500-i2c-bus"
+- clocks : root clock of bus, should reference the APB
+ clock
+- interrupts : interrupt number
+- interrupt-parent : interrupt controller for bus, should reference a
+ aspeed,ast2400-i2c-ic or aspeed,ast2500-i2c-ic
+ interrupt controller
+
+Optional Properties:
+- bus-frequency : frequency of the bus clock in Hz defaults to 100 kHz when not
+ specified
+
+Example:
+
+i2c {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 0x1e78a000 0x1000>;
+
+ i2c_ic: interrupt-controller@0 {
+ #interrupt-cells = <1>;
+ compatible = "aspeed,ast2400-i2c-ic";
+ reg = <0x0 0x40>;
+ interrupts = <12>;
+ interrupt-controller;
+ };
+
+ i2c0: i2c-bus@40 {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #interrupt-cells = <1>;
+ reg = <0x40 0x40>;
+ compatible = "aspeed,ast2400-i2c-bus";
+ clocks = <&clk_apb>;
+ bus-frequency = <100000>;
+ interrupts = <0>;
+ interrupt-parent = <&i2c_ic>;
+ };
+};
--
2.12.2.816.g2cccc81164-goog
^ permalink raw reply related
* [PATCH v7 4/5] i2c: aspeed: added driver for Aspeed I2C
From: Brendan Higgins @ 2017-04-24 18:18 UTC (permalink / raw)
To: wsa, robh+dt, mark.rutland, tglx, jason, marc.zyngier, joel, vz,
mouse, clg, benh
Cc: linux-i2c, devicetree, linux-kernel, openbmc, Brendan Higgins
In-Reply-To: <20170424181818.2754-1-brendanhiggins@google.com>
Added initial master support for Aspeed I2C controller. Supports
fourteen busses present in AST24XX and AST25XX BMC SoCs by Aspeed.
Signed-off-by: Brendan Higgins <brendanhiggins@google.com>
---
Changes for v2:
- Added single module_init (multiple was breaking some builds).
Changes for v3:
- Removed "bus" device tree param; now extracted from bus address offset
Changes for v4:
- I2C adapter number is now generated dynamically unless specified in alias.
Changes for v5:
- Removed irq_chip used to multiplex IRQ and replaced it with dummy_irq_chip
along with some other IRQ cleanup.
- Addressed comments from Cedric, and Vladimir, mostly stylistic things and
using devm managed resources.
- Increased max clock frequency before the bus is put in HighSpeed mode, as
per Kachalov's comment.
Changes for v6:
- No longer arbitrarily restrict bus to be slave xor master.
- Pulled out "struct aspeed_i2c_controller" as a interrupt controller.
- Pulled out slave support into its own commit.
- Rewrote code that sets clock divider register because the original version
set it incorrectly.
- Rewrote the aspeed_i2c_master_irq handler because the old method of
completing a completion in between restarts was too slow causing devices to
misbehave.
- Added support for I2C_M_RECV_LEN which I had incorrectly said was supported
before.
- Addressed other comments from Vladimir.
Changes for v7:
- Changed clock-frequency to bus-frequency
- Made some fixes to clock divider code
- Added hardware reset function
- Marked functions that need to be called with the lock held as "unlocked"
- Did a bunch of clean up
---
drivers/i2c/busses/Kconfig | 10 +
drivers/i2c/busses/Makefile | 1 +
drivers/i2c/busses/i2c-aspeed.c | 689 ++++++++++++++++++++++++++++++++++++++++
3 files changed, 700 insertions(+)
create mode 100644 drivers/i2c/busses/i2c-aspeed.c
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 8adc0f1d7ad0..48fca492ec2f 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -326,6 +326,16 @@ config I2C_POWERMAC
comment "I2C system bus drivers (mostly embedded / system-on-chip)"
+config I2C_ASPEED
+ tristate "Aspeed I2C Controller"
+ depends on ARCH_ASPEED
+ help
+ If you say yes to this option, support will be included for the
+ Aspeed I2C controller.
+
+ This driver can also be built as a module. If so, the module
+ will be called i2c-aspeed.
+
config I2C_AT91
tristate "Atmel AT91 I2C Two-Wire interface (TWI)"
depends on ARCH_AT91
diff --git a/drivers/i2c/busses/Makefile b/drivers/i2c/busses/Makefile
index 30b60855fbcd..e84604b9bf3b 100644
--- a/drivers/i2c/busses/Makefile
+++ b/drivers/i2c/busses/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_I2C_HYDRA) += i2c-hydra.o
obj-$(CONFIG_I2C_POWERMAC) += i2c-powermac.o
# Embedded system I2C/SMBus host controller drivers
+obj-$(CONFIG_I2C_ASPEED) += i2c-aspeed.o
obj-$(CONFIG_I2C_AT91) += i2c-at91.o
obj-$(CONFIG_I2C_AU1550) += i2c-au1550.o
obj-$(CONFIG_I2C_AXXIA) += i2c-axxia.o
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
new file mode 100644
index 000000000000..778bcaa4ccf4
--- /dev/null
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -0,0 +1,689 @@
+/*
+ * Aspeed 24XX/25XX I2C Controller.
+ *
+ * Copyright (C) 2012-2017 ASPEED Technology Inc.
+ * Copyright 2017 IBM Corporation
+ * Copyright 2017 Google, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/err.h>
+#include <linux/errno.h>
+#include <linux/i2c.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/irq.h>
+#include <linux/irqchip/chained_irq.h>
+#include <linux/irqdomain.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/slab.h>
+
+/* I2C Register */
+#define ASPEED_I2C_FUN_CTRL_REG 0x00
+#define ASPEED_I2C_AC_TIMING_REG1 0x04
+#define ASPEED_I2C_AC_TIMING_REG2 0x08
+#define ASPEED_I2C_INTR_CTRL_REG 0x0c
+#define ASPEED_I2C_INTR_STS_REG 0x10
+#define ASPEED_I2C_CMD_REG 0x14
+#define ASPEED_I2C_DEV_ADDR_REG 0x18
+#define ASPEED_I2C_BYTE_BUF_REG 0x20
+
+/* Global Register Definition */
+/* 0x00 : I2C Interrupt Status Register */
+/* 0x08 : I2C Interrupt Target Assignment */
+
+/* Device Register Definition */
+/* 0x00 : I2CD Function Control Register */
+#define ASPEED_I2CD_MULTI_MASTER_DIS BIT(15)
+#define ASPEED_I2CD_SDA_DRIVE_1T_EN BIT(8)
+#define ASPEED_I2CD_M_SDA_DRIVE_1T_EN BIT(7)
+#define ASPEED_I2CD_M_HIGH_SPEED_EN BIT(6)
+#define ASPEED_I2CD_MASTER_EN BIT(0)
+
+/* 0x04 : I2CD Clock and AC Timing Control Register #1 */
+#define ASPEED_I2CD_TIME_SCL_HIGH_SHIFT 16
+#define ASPEED_I2CD_TIME_SCL_HIGH_MASK GENMASK(19, 16)
+#define ASPEED_I2CD_TIME_SCL_LOW_SHIFT 12
+#define ASPEED_I2CD_TIME_SCL_LOW_MASK GENMASK(15, 12)
+#define ASPEED_I2CD_TIME_BASE_DIVISOR_MASK GENMASK(3, 0)
+#define ASPEED_I2CD_TIME_SCL_REG_MAX GENMASK(3, 0)
+/* 0x08 : I2CD Clock and AC Timing Control Register #2 */
+#define ASPEED_NO_TIMEOUT_CTRL 0
+
+/* 0x0c : I2CD Interrupt Control Register &
+ * 0x10 : I2CD Interrupt Status Register
+ *
+ * These share bit definitions, so use the same values for the enable &
+ * status bits.
+ */
+#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT BIT(14)
+#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE BIT(13)
+#define ASPEED_I2CD_INTR_SCL_TIMEOUT BIT(6)
+#define ASPEED_I2CD_INTR_ABNORMAL BIT(5)
+#define ASPEED_I2CD_INTR_NORMAL_STOP BIT(4)
+#define ASPEED_I2CD_INTR_ARBIT_LOSS BIT(3)
+#define ASPEED_I2CD_INTR_RX_DONE BIT(2)
+#define ASPEED_I2CD_INTR_TX_NAK BIT(1)
+#define ASPEED_I2CD_INTR_TX_ACK BIT(0)
+#define ASPEED_I2CD_INTR_ERROR \
+ (ASPEED_I2CD_INTR_ARBIT_LOSS | \
+ ASPEED_I2CD_INTR_ABNORMAL | \
+ ASPEED_I2CD_INTR_SCL_TIMEOUT | \
+ ASPEED_I2CD_INTR_SDA_DL_TIMEOUT)
+#define ASPEED_I2CD_INTR_ALL \
+ (ASPEED_I2CD_INTR_SDA_DL_TIMEOUT | \
+ ASPEED_I2CD_INTR_BUS_RECOVER_DONE | \
+ ASPEED_I2CD_INTR_SCL_TIMEOUT | \
+ ASPEED_I2CD_INTR_ABNORMAL | \
+ ASPEED_I2CD_INTR_NORMAL_STOP | \
+ ASPEED_I2CD_INTR_ARBIT_LOSS | \
+ ASPEED_I2CD_INTR_RX_DONE | \
+ ASPEED_I2CD_INTR_TX_NAK | \
+ ASPEED_I2CD_INTR_TX_ACK)
+
+/* 0x14 : I2CD Command/Status Register */
+#define ASPEED_I2CD_SCL_LINE_STS BIT(18)
+#define ASPEED_I2CD_SDA_LINE_STS BIT(17)
+#define ASPEED_I2CD_BUS_BUSY_STS BIT(16)
+#define ASPEED_I2CD_BUS_RECOVER_CMD BIT(11)
+
+/* Command Bit */
+#define ASPEED_I2CD_M_STOP_CMD BIT(5)
+#define ASPEED_I2CD_M_S_RX_CMD_LAST BIT(4)
+#define ASPEED_I2CD_M_RX_CMD BIT(3)
+#define ASPEED_I2CD_S_TX_CMD BIT(2)
+#define ASPEED_I2CD_M_TX_CMD BIT(1)
+#define ASPEED_I2CD_M_START_CMD BIT(0)
+
+enum aspeed_i2c_master_state {
+ ASPEED_I2C_MASTER_START,
+ ASPEED_I2C_MASTER_TX_FIRST,
+ ASPEED_I2C_MASTER_TX,
+ ASPEED_I2C_MASTER_RX_FIRST,
+ ASPEED_I2C_MASTER_RX,
+ ASPEED_I2C_MASTER_STOP,
+ ASPEED_I2C_MASTER_INACTIVE,
+};
+
+struct aspeed_i2c_bus {
+ struct i2c_adapter adap;
+ struct device *dev;
+ void __iomem *base;
+ /* Synchronizes I/O mem access to base. */
+ spinlock_t lock;
+ struct completion cmd_complete;
+ int irq;
+ /* Transaction state. */
+ enum aspeed_i2c_master_state master_state;
+ struct i2c_msg *msgs;
+ size_t buf_index;
+ size_t msgs_index;
+ size_t msgs_count;
+ bool send_stop;
+ int cmd_err;
+};
+
+static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
+
+static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
+{
+ unsigned long time_left, flags;
+ int ret = 0;
+ u32 command;
+
+ spin_lock_irqsave(&bus->lock, flags);
+ command = readl(bus->base + ASPEED_I2C_CMD_REG);
+
+ if (command & ASPEED_I2CD_SDA_LINE_STS) {
+ /* Bus is idle: no recovery needed. */
+ if (command & ASPEED_I2CD_SCL_LINE_STS)
+ goto out;
+ dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
+ command);
+
+ reinit_completion(&bus->cmd_complete);
+ writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_timeout(
+ &bus->cmd_complete, bus->adap.timeout);
+
+ spin_lock_irqsave(&bus->lock, flags);
+ if (time_left == 0)
+ goto reset_out;
+ else if (bus->cmd_err)
+ goto reset_out;
+ /* Recovery failed. */
+ else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
+ ASPEED_I2CD_SCL_LINE_STS))
+ ret = -EIO;
+ /* Bus error. */
+ } else {
+ dev_dbg(bus->dev, "bus hung (state %x), attempting recovery\n",
+ command);
+
+ reinit_completion(&bus->cmd_complete);
+ writel(ASPEED_I2CD_BUS_RECOVER_CMD,
+ bus->base + ASPEED_I2C_CMD_REG);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_timeout(
+ &bus->cmd_complete, bus->adap.timeout);
+
+ spin_lock_irqsave(&bus->lock, flags);
+ if (time_left == 0)
+ goto reset_out;
+ else if (bus->cmd_err)
+ goto reset_out;
+ /* Recovery failed. */
+ else if (!(readl(bus->base + ASPEED_I2C_CMD_REG) &
+ ASPEED_I2CD_SDA_LINE_STS))
+ ret = -EIO;
+ }
+
+out:
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ return ret;
+
+reset_out:
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ return aspeed_i2c_reset(bus);
+}
+
+static void __aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
+{
+ u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
+ struct i2c_msg *msg = &bus->msgs[bus->msgs_index];
+ u8 slave_addr = msg->addr << 1;
+
+ bus->master_state = ASPEED_I2C_MASTER_START;
+ bus->buf_index = 0;
+
+ if (msg->flags & I2C_M_RD) {
+ slave_addr |= 1;
+ command |= ASPEED_I2CD_M_RX_CMD;
+ /* Need to let the hardware know to NACK after RX. */
+ if (msg->len == 1 && !(msg->flags & I2C_M_RECV_LEN))
+ command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+ }
+
+ writel(slave_addr, bus->base + ASPEED_I2C_BYTE_BUF_REG);
+ writel(command, bus->base + ASPEED_I2C_CMD_REG);
+}
+
+static void __aspeed_i2c_do_stop(struct aspeed_i2c_bus *bus)
+{
+ bus->master_state = ASPEED_I2C_MASTER_STOP;
+ writel(ASPEED_I2CD_M_STOP_CMD, bus->base + ASPEED_I2C_CMD_REG);
+}
+
+static void __aspeed_i2c_next_msg_or_stop(struct aspeed_i2c_bus *bus)
+{
+ if (bus->msgs_index + 1 < bus->msgs_count) {
+ bus->msgs_index++;
+ __aspeed_i2c_do_start(bus);
+ } else {
+ __aspeed_i2c_do_stop(bus);
+ }
+}
+
+static bool aspeed_i2c_master_irq(struct aspeed_i2c_bus *bus)
+{
+ u32 irq_status, status_ack = 0, command = 0;
+ struct i2c_msg *msg;
+ u8 recv_byte;
+
+ spin_lock(&bus->lock);
+ irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
+
+ if (irq_status & ASPEED_I2CD_INTR_BUS_RECOVER_DONE) {
+ bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+ status_ack |= ASPEED_I2CD_INTR_BUS_RECOVER_DONE;
+ goto out_complete;
+ }
+
+ /*
+ * Either we encountered an interrupt that reports an error, or we are
+ * in an invalid state.
+ */
+ if (irq_status & ASPEED_I2CD_INTR_ERROR ||
+ (!bus->msgs && bus->master_state != ASPEED_I2C_MASTER_STOP)) {
+ dev_dbg(bus->dev, "received error interrupt: 0x%08x",
+ irq_status);
+ bus->cmd_err = -EIO;
+ __aspeed_i2c_do_stop(bus);
+ goto out_no_complete;
+ }
+ msg = &bus->msgs[bus->msgs_index];
+
+ /*
+ * START is a special case because we still have to handle a subsequent
+ * TX or RX immediately after we handle it, so we handle it here and
+ * then update the state and handle the new state below.
+ */
+ if (bus->master_state == ASPEED_I2C_MASTER_START) {
+ if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
+ dev_dbg(bus->dev,
+ "no slave present at %02x", msg->addr);
+ status_ack |= ASPEED_I2CD_INTR_TX_NAK;
+ goto error_and_stop;
+ }
+ status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+ if (msg->flags & I2C_M_RD)
+ bus->master_state = ASPEED_I2C_MASTER_RX_FIRST;
+ else
+ bus->master_state = ASPEED_I2C_MASTER_TX_FIRST;
+ }
+
+ switch (bus->master_state) {
+ case ASPEED_I2C_MASTER_TX:
+ if (unlikely(irq_status & ASPEED_I2CD_INTR_TX_NAK)) {
+ dev_dbg(bus->dev, "slave NACKed TX");
+ status_ack |= ASPEED_I2CD_INTR_TX_NAK;
+ goto error_and_stop;
+ } else if (unlikely(!(irq_status & ASPEED_I2CD_INTR_TX_ACK))) {
+ dev_err(bus->dev, "slave failed to ACK TX");
+ goto error_and_stop;
+ }
+ status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+ /* fallthrough intended */
+ case ASPEED_I2C_MASTER_TX_FIRST:
+ if (bus->buf_index < msg->len) {
+ bus->master_state = ASPEED_I2C_MASTER_TX;
+ writel(msg->buf[bus->buf_index++],
+ bus->base + ASPEED_I2C_BYTE_BUF_REG);
+ writel(ASPEED_I2CD_M_TX_CMD,
+ bus->base + ASPEED_I2C_CMD_REG);
+ } else {
+ __aspeed_i2c_next_msg_or_stop(bus);
+ }
+ goto out_no_complete;
+ case ASPEED_I2C_MASTER_RX_FIRST:
+ /* RX may not have completed yet (only address cycle) */
+ if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE))
+ goto out_no_complete;
+ /* fallthrough intended */
+ case ASPEED_I2C_MASTER_RX:
+ if (unlikely(!(irq_status & ASPEED_I2CD_INTR_RX_DONE))) {
+ dev_err(bus->dev, "master failed to RX");
+ goto error_and_stop;
+ }
+ status_ack |= ASPEED_I2CD_INTR_RX_DONE;
+
+ recv_byte = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8;
+ msg->buf[bus->buf_index++] = recv_byte;
+
+ if (msg->flags & I2C_M_RECV_LEN) {
+ if (unlikely(recv_byte > I2C_SMBUS_BLOCK_MAX)) {
+ bus->cmd_err = -EPROTO;
+ __aspeed_i2c_do_stop(bus);
+ goto out_no_complete;
+ }
+ msg->len = recv_byte +
+ ((msg->flags & I2C_CLIENT_PEC) ? 2 : 1);
+ msg->flags &= ~I2C_M_RECV_LEN;
+ }
+
+ if (bus->buf_index < msg->len) {
+ bus->master_state = ASPEED_I2C_MASTER_RX;
+ command = ASPEED_I2CD_M_RX_CMD;
+ if (bus->buf_index + 1 == msg->len)
+ command |= ASPEED_I2CD_M_S_RX_CMD_LAST;
+ writel(command, bus->base + ASPEED_I2C_CMD_REG);
+ } else {
+ __aspeed_i2c_next_msg_or_stop(bus);
+ }
+ goto out_no_complete;
+ case ASPEED_I2C_MASTER_STOP:
+ if (unlikely(!(irq_status & ASPEED_I2CD_INTR_NORMAL_STOP))) {
+ dev_err(bus->dev, "master failed to STOP");
+ bus->cmd_err = -EIO;
+ /* Do not STOP as we have already tried. */
+ } else {
+ status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
+ }
+
+ bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+ goto out_complete;
+ case ASPEED_I2C_MASTER_INACTIVE:
+ dev_err(bus->dev,
+ "master received interrupt 0x%08x, but is inactive",
+ irq_status);
+ bus->cmd_err = -EIO;
+ /* Do not STOP as we should be inactive. */
+ goto out_complete;
+ default:
+ WARN(1, "unknown master state\n");
+ bus->master_state = ASPEED_I2C_MASTER_INACTIVE;
+ bus->cmd_err = -EIO;
+ goto out_complete;
+ }
+error_and_stop:
+ bus->cmd_err = -EIO;
+ __aspeed_i2c_do_stop(bus);
+ goto out_no_complete;
+out_complete:
+ complete(&bus->cmd_complete);
+out_no_complete:
+ if (irq_status != status_ack)
+ dev_err(bus->dev,
+ "irq handled != irq. expected 0x%08x, but was 0x%08x\n",
+ irq_status, status_ack);
+ writel(irq_status, bus->base + ASPEED_I2C_INTR_STS_REG);
+ spin_unlock(&bus->lock);
+ return !!irq_status;
+}
+
+static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
+{
+ struct aspeed_i2c_bus *bus = dev_id;
+
+ if (aspeed_i2c_master_irq(bus))
+ return IRQ_HANDLED;
+ else
+ return IRQ_NONE;
+}
+
+static int aspeed_i2c_master_xfer(struct i2c_adapter *adap,
+ struct i2c_msg *msgs, int num)
+{
+ struct aspeed_i2c_bus *bus = adap->algo_data;
+ unsigned long time_left, flags;
+ int ret = 0;
+
+ spin_lock_irqsave(&bus->lock, flags);
+ bus->cmd_err = 0;
+
+ /* If bus is busy, attempt recovery. We assume a single master
+ * environment.
+ */
+ if (readl(bus->base + ASPEED_I2C_CMD_REG) & ASPEED_I2CD_BUS_BUSY_STS) {
+ spin_unlock_irqrestore(&bus->lock, flags);
+ ret = aspeed_i2c_recover_bus(bus);
+ if (ret)
+ return ret;
+ spin_lock_irqsave(&bus->lock, flags);
+ }
+
+ bus->msgs = msgs;
+ bus->msgs_index = 0;
+ bus->msgs_count = num;
+
+ reinit_completion(&bus->cmd_complete);
+ __aspeed_i2c_do_start(bus);
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ time_left = wait_for_completion_timeout(&bus->cmd_complete,
+ bus->adap.timeout);
+
+ spin_lock_irqsave(&bus->lock, flags);
+ bus->msgs = NULL;
+ if (time_left == 0)
+ ret = -ETIMEDOUT;
+ else
+ ret = bus->cmd_err;
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ /* If nothing went wrong, return number of messages transferred. */
+ if (ret >= 0)
+ return bus->msgs_index + 1;
+ else
+ return ret;
+}
+
+static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
+{
+ return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
+}
+
+static const struct i2c_algorithm aspeed_i2c_algo = {
+ .master_xfer = aspeed_i2c_master_xfer,
+ .functionality = aspeed_i2c_functionality,
+};
+
+static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
+{
+ u32 base_clk, clk_high, clk_low, tmp;
+
+ /*
+ * The actual clock frequency of SCL is:
+ * SCL_freq = APB_freq / (base_freq * (SCL_high + SCL_low))
+ * = APB_freq / divisor
+ * where base_freq is a programmable clock divider; its value is
+ * base_freq = 1 << base_clk
+ * SCL_high is the number of base_freq clock cycles that SCL stays high
+ * and SCL_low is the number of base_freq clock cycles that SCL stays
+ * low for a period of SCL.
+ * The actual register has a minimum SCL_high and SCL_low minimum of 1;
+ * thus, they start counting at zero. So
+ * SCL_high = clk_high + 1
+ * SCL_low = clk_low + 1
+ * Thus,
+ * SCL_freq = APB_freq /
+ * ((1 << base_clk) * (clk_high + 1 + clk_low + 1))
+ * The documentation recommends clk_high >= 8 and clk_low >= 7 when
+ * possible; this last constraint gives us the following solution:
+ */
+ base_clk = divisor > 33 ? ilog2((divisor - 1) / 32) + 1 : 0;
+ tmp = divisor / (1 << base_clk);
+ clk_high = tmp / 2 + tmp % 2;
+ clk_low = tmp - clk_high;
+
+ clk_high -= 1;
+ clk_low -= 1;
+
+ return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
+ & ASPEED_I2CD_TIME_SCL_HIGH_MASK)
+ | ((clk_low << ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
+ & ASPEED_I2CD_TIME_SCL_LOW_MASK)
+ | (base_clk & ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
+}
+
+static int __aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
+ struct platform_device *pdev)
+{
+ u32 clk_freq, divisor, clk_reg_val;
+ struct clk *pclk;
+ int ret;
+
+ pclk = devm_clk_get(&pdev->dev, NULL);
+ if (IS_ERR(pclk)) {
+ dev_err(&pdev->dev, "clk_get failed\n");
+ return PTR_ERR(pclk);
+ }
+ ret = of_property_read_u32(pdev->dev.of_node,
+ "bus-frequency", &clk_freq);
+ if (ret < 0) {
+ dev_err(&pdev->dev,
+ "Could not read bus-frequency property\n");
+ clk_freq = 100000;
+ }
+ divisor = clk_get_rate(pclk) / clk_freq;
+ /* We just need the clock rate, we don't actually use the clk object. */
+ devm_clk_put(&pdev->dev, pclk);
+
+ clk_reg_val = aspeed_i2c_get_clk_reg_val(divisor);
+ writel(clk_reg_val, bus->base + ASPEED_I2C_AC_TIMING_REG1);
+
+ /*
+ * If the base divisor is non-zero then we do not want to enable high
+ * speed mode, otherwise we might as well enable it.
+ * For reference, setting high speed mode will make the base divisor
+ * zero and corresponds to a minimum SCL frequency of about 1.5MHz.
+ */
+ if (clk_reg_val & ASPEED_I2CD_TIME_BASE_DIVISOR_MASK) {
+ writel(ASPEED_NO_TIMEOUT_CTRL,
+ bus->base + ASPEED_I2C_AC_TIMING_REG2);
+ } else {
+ writel(readl(bus->base + ASPEED_I2C_FUN_CTRL_REG) |
+ ASPEED_I2CD_M_HIGH_SPEED_EN |
+ ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
+ ASPEED_I2CD_SDA_DRIVE_1T_EN,
+ bus->base + ASPEED_I2C_FUN_CTRL_REG);
+
+ writel(0x3, bus->base + ASPEED_I2C_AC_TIMING_REG2);
+ }
+
+ return 0;
+}
+
+static int __aspeed_i2c_init(struct aspeed_i2c_bus *bus,
+ struct platform_device *pdev)
+{
+ int ret;
+
+ /* Disable everything. */
+ writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
+
+ ret = __aspeed_i2c_init_clk(bus, pdev);
+ if (ret < 0)
+ return ret;
+
+ /* Enable Master Mode */
+ writel(readl(bus->base + ASPEED_I2C_FUN_CTRL_REG) |
+ ASPEED_I2CD_MASTER_EN |
+ /* TODO: provide device tree option for multi-master mode. */
+ ASPEED_I2CD_MULTI_MASTER_DIS,
+ bus->base + ASPEED_I2C_FUN_CTRL_REG);
+
+ /* Set interrupt generation of I2C controller */
+ writel(ASPEED_I2CD_INTR_ALL, bus->base + ASPEED_I2C_INTR_CTRL_REG);
+
+ return 0;
+}
+
+static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus)
+{
+ struct platform_device *pdev = to_platform_device(bus->dev);
+ unsigned long flags;
+ int ret;
+
+ spin_lock_irqsave(&bus->lock, flags);
+
+ /* Disable and quiesce interrupts. */
+ reinit_completion(&bus->cmd_complete);
+ writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
+
+ spin_unlock_irqrestore(&bus->lock, flags);
+ /*
+ * We need to make sure that there are no interrupts that fired just
+ * before we grabbed the lock; if that did not happen, then we are going
+ * to timeout and that is okay.
+ */
+ wait_for_completion_timeout(&bus->cmd_complete, bus->adap.timeout);
+ spin_lock_irqsave(&bus->lock, flags);
+
+ ret = __aspeed_i2c_init(bus, pdev);
+
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ return ret;
+}
+
+static int aspeed_i2c_probe_bus(struct platform_device *pdev)
+{
+ struct aspeed_i2c_bus *bus;
+ struct resource *res;
+ int ret;
+
+ bus = devm_kzalloc(&pdev->dev, sizeof(*bus), GFP_KERNEL);
+ if (!bus)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ bus->base = devm_ioremap_resource(&pdev->dev, res);
+ if (IS_ERR(bus->base))
+ return PTR_ERR(bus->base);
+
+ /* Initialize the I2C adapter */
+ spin_lock_init(&bus->lock);
+ init_completion(&bus->cmd_complete);
+ bus->adap.owner = THIS_MODULE;
+ bus->adap.retries = 0;
+ bus->adap.timeout = 5 * HZ;
+ bus->adap.algo = &aspeed_i2c_algo;
+ bus->adap.algo_data = bus;
+ bus->adap.dev.parent = &pdev->dev;
+ bus->adap.dev.of_node = pdev->dev.of_node;
+ snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed i2c");
+
+ bus->dev = &pdev->dev;
+
+ /*
+ * No need to quiesce interrupts because there is no interrupt handler
+ * installed.
+ */
+ writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
+ ret = __aspeed_i2c_init(bus, pdev);
+ if (ret < 0)
+ return ret;
+
+ bus->irq = irq_of_parse_and_map(pdev->dev.of_node, 0);
+ ret = devm_request_irq(&pdev->dev, bus->irq, aspeed_i2c_bus_irq,
+ 0, dev_name(&pdev->dev), bus);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_add_adapter(&bus->adap);
+ if (ret < 0)
+ return ret;
+
+ platform_set_drvdata(pdev, bus);
+
+ dev_info(bus->dev, "i2c bus %d registered, irq %d\n",
+ bus->adap.nr, bus->irq);
+
+ return 0;
+}
+
+static int aspeed_i2c_remove_bus(struct platform_device *pdev)
+{
+ struct aspeed_i2c_bus *bus = platform_get_drvdata(pdev);
+ unsigned long flags;
+
+ spin_lock_irqsave(&bus->lock, flags);
+
+ /* Disable everything. */
+ writel(0, bus->base + ASPEED_I2C_FUN_CTRL_REG);
+ writel(0, bus->base + ASPEED_I2C_INTR_CTRL_REG);
+
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ i2c_del_adapter(&bus->adap);
+
+ return 0;
+}
+
+static const struct of_device_id aspeed_i2c_bus_of_table[] = {
+ { .compatible = "aspeed,ast2400-i2c-bus", },
+ { .compatible = "aspeed,ast2500-i2c-bus", },
+ { },
+};
+MODULE_DEVICE_TABLE(of, aspeed_i2c_bus_of_table);
+
+static struct platform_driver aspeed_i2c_bus_driver = {
+ .probe = aspeed_i2c_probe_bus,
+ .remove = aspeed_i2c_remove_bus,
+ .driver = {
+ .name = "aspeed-i2c-bus",
+ .of_match_table = aspeed_i2c_bus_of_table,
+ },
+};
+module_platform_driver(aspeed_i2c_bus_driver);
+
+MODULE_AUTHOR("Brendan Higgins <brendanhiggins@google.com>");
+MODULE_DESCRIPTION("Aspeed I2C Bus Driver");
+MODULE_LICENSE("GPL v2");
--
2.12.2.816.g2cccc81164-goog
^ permalink raw reply related
* [PATCH v7 5/5] i2c: aspeed: added slave support for Aspeed I2C driver
From: Brendan Higgins @ 2017-04-24 18:18 UTC (permalink / raw)
To: wsa-z923LK4zBo2bacvFa/9K2g, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, tglx-hfZtesqFncYOwBW4kG4KsQ,
jason-NLaQJdtUoK4Be96aLqz0jA, marc.zyngier-5wv7dgnIgG8,
joel-U3u1mxZcP9KHXe+LvDLADg, vz-ChpfBGZJDbMAvxtiuMwx3w,
mouse-Pma6HLj0uuo, clg-Bxea+6Xhats,
benh-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r
Cc: linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
openbmc-uLR06cmDAlY/bJ5BZ2RsiQ, Brendan Higgins
In-Reply-To: <20170424181818.2754-1-brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
Added slave support for Aspeed I2C controller. Supports fourteen busses
present in AST24XX and AST25XX BMC SoCs by Aspeed.
Signed-off-by: Brendan Higgins <brendanhiggins-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
Added in v6:
- Pulled slave support out of initial driver commit into its own commit.
- No longer arbitrarily restrict bus to be slave xor master.
Changes for v7:
- Added hardware reset function
- Marked functions that need to be called with the lock held as "unlocked"
- Did some cleanup
---
drivers/i2c/busses/i2c-aspeed.c | 201 ++++++++++++++++++++++++++++++++++++++++
1 file changed, 201 insertions(+)
diff --git a/drivers/i2c/busses/i2c-aspeed.c b/drivers/i2c/busses/i2c-aspeed.c
index 778bcaa4ccf4..7bd2328eb0fb 100644
--- a/drivers/i2c/busses/i2c-aspeed.c
+++ b/drivers/i2c/busses/i2c-aspeed.c
@@ -49,6 +49,7 @@
#define ASPEED_I2CD_SDA_DRIVE_1T_EN BIT(8)
#define ASPEED_I2CD_M_SDA_DRIVE_1T_EN BIT(7)
#define ASPEED_I2CD_M_HIGH_SPEED_EN BIT(6)
+#define ASPEED_I2CD_SLAVE_EN BIT(1)
#define ASPEED_I2CD_MASTER_EN BIT(0)
/* 0x04 : I2CD Clock and AC Timing Control Register #1 */
@@ -69,6 +70,7 @@
*/
#define ASPEED_I2CD_INTR_SDA_DL_TIMEOUT BIT(14)
#define ASPEED_I2CD_INTR_BUS_RECOVER_DONE BIT(13)
+#define ASPEED_I2CD_INTR_SLAVE_MATCH BIT(7)
#define ASPEED_I2CD_INTR_SCL_TIMEOUT BIT(6)
#define ASPEED_I2CD_INTR_ABNORMAL BIT(5)
#define ASPEED_I2CD_INTR_NORMAL_STOP BIT(4)
@@ -106,6 +108,9 @@
#define ASPEED_I2CD_M_TX_CMD BIT(1)
#define ASPEED_I2CD_M_START_CMD BIT(0)
+/* 0x18 : I2CD Slave Device Address Register */
+#define ASPEED_I2CD_DEV_ADDR_MASK GENMASK(6, 0)
+
enum aspeed_i2c_master_state {
ASPEED_I2C_MASTER_START,
ASPEED_I2C_MASTER_TX_FIRST,
@@ -116,6 +121,15 @@ enum aspeed_i2c_master_state {
ASPEED_I2C_MASTER_INACTIVE,
};
+enum aspeed_i2c_slave_state {
+ ASPEED_I2C_SLAVE_START,
+ ASPEED_I2C_SLAVE_READ_REQUESTED,
+ ASPEED_I2C_SLAVE_READ_PROCESSED,
+ ASPEED_I2C_SLAVE_WRITE_REQUESTED,
+ ASPEED_I2C_SLAVE_WRITE_RECEIVED,
+ ASPEED_I2C_SLAVE_STOP,
+};
+
struct aspeed_i2c_bus {
struct i2c_adapter adap;
struct device *dev;
@@ -132,6 +146,10 @@ struct aspeed_i2c_bus {
size_t msgs_count;
bool send_stop;
int cmd_err;
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ struct i2c_client *slave;
+ enum aspeed_i2c_slave_state slave_state;
+#endif /* CONFIG_I2C_SLAVE */
};
static int aspeed_i2c_reset(struct aspeed_i2c_bus *bus);
@@ -203,6 +221,110 @@ static int aspeed_i2c_recover_bus(struct aspeed_i2c_bus *bus)
return aspeed_i2c_reset(bus);
}
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static bool aspeed_i2c_slave_irq(struct aspeed_i2c_bus *bus)
+{
+ u32 command, irq_status, status_ack = 0;
+ struct i2c_client *slave = bus->slave;
+ bool irq_handled = true;
+ u8 value;
+
+ spin_lock(&bus->lock);
+ if (!slave) {
+ irq_handled = false;
+ goto out;
+ }
+
+ command = readl(bus->base + ASPEED_I2C_CMD_REG);
+ irq_status = readl(bus->base + ASPEED_I2C_INTR_STS_REG);
+
+ /* Slave was requested, restart state machine. */
+ if (irq_status & ASPEED_I2CD_INTR_SLAVE_MATCH) {
+ status_ack |= ASPEED_I2CD_INTR_SLAVE_MATCH;
+ bus->slave_state = ASPEED_I2C_SLAVE_START;
+ }
+
+ /* Slave is not currently active, irq was for someone else. */
+ if (bus->slave_state == ASPEED_I2C_SLAVE_STOP) {
+ irq_handled = false;
+ goto out;
+ }
+
+ dev_dbg(bus->dev, "slave irq status 0x%08x, cmd 0x%08x\n",
+ irq_status, command);
+
+ /* Slave was sent something. */
+ if (irq_status & ASPEED_I2CD_INTR_RX_DONE) {
+ value = readl(bus->base + ASPEED_I2C_BYTE_BUF_REG) >> 8;
+ /* Handle address frame. */
+ if (bus->slave_state == ASPEED_I2C_SLAVE_START) {
+ if (value & 0x1)
+ bus->slave_state =
+ ASPEED_I2C_SLAVE_READ_REQUESTED;
+ else
+ bus->slave_state =
+ ASPEED_I2C_SLAVE_WRITE_REQUESTED;
+ }
+ status_ack |= ASPEED_I2CD_INTR_RX_DONE;
+ }
+
+ /* Slave was asked to stop. */
+ if (irq_status & ASPEED_I2CD_INTR_NORMAL_STOP) {
+ status_ack |= ASPEED_I2CD_INTR_NORMAL_STOP;
+ bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+ }
+ if (irq_status & ASPEED_I2CD_INTR_TX_NAK) {
+ status_ack |= ASPEED_I2CD_INTR_TX_NAK;
+ bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+ }
+
+ switch (bus->slave_state) {
+ case ASPEED_I2C_SLAVE_READ_REQUESTED:
+ if (irq_status & ASPEED_I2CD_INTR_TX_ACK)
+ dev_err(bus->dev, "Unexpected ACK on read request.\n");
+ bus->slave_state = ASPEED_I2C_SLAVE_READ_PROCESSED;
+
+ i2c_slave_event(slave, I2C_SLAVE_READ_REQUESTED, &value);
+ writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
+ writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG);
+ break;
+ case ASPEED_I2C_SLAVE_READ_PROCESSED:
+ status_ack |= ASPEED_I2CD_INTR_TX_ACK;
+ if (!(irq_status & ASPEED_I2CD_INTR_TX_ACK))
+ dev_err(bus->dev,
+ "Expected ACK after processed read.\n");
+ i2c_slave_event(slave, I2C_SLAVE_READ_PROCESSED, &value);
+ writel(value, bus->base + ASPEED_I2C_BYTE_BUF_REG);
+ writel(ASPEED_I2CD_S_TX_CMD, bus->base + ASPEED_I2C_CMD_REG);
+ break;
+ case ASPEED_I2C_SLAVE_WRITE_REQUESTED:
+ bus->slave_state = ASPEED_I2C_SLAVE_WRITE_RECEIVED;
+ i2c_slave_event(slave, I2C_SLAVE_WRITE_REQUESTED, &value);
+ break;
+ case ASPEED_I2C_SLAVE_WRITE_RECEIVED:
+ i2c_slave_event(slave, I2C_SLAVE_WRITE_RECEIVED, &value);
+ break;
+ case ASPEED_I2C_SLAVE_STOP:
+ i2c_slave_event(slave, I2C_SLAVE_STOP, &value);
+ break;
+ default:
+ dev_err(bus->dev, "unhandled slave_state: %d\n",
+ bus->slave_state);
+ break;
+ }
+
+ if (status_ack != irq_status)
+ dev_err(bus->dev,
+ "irq handled != irq. expected %x, but was %x\n",
+ irq_status, status_ack);
+ writel(status_ack, bus->base + ASPEED_I2C_INTR_STS_REG);
+
+out:
+ spin_unlock(&bus->lock);
+ return irq_handled;
+}
+#endif /* CONFIG_I2C_SLAVE */
+
static void __aspeed_i2c_do_start(struct aspeed_i2c_bus *bus)
{
u32 command = ASPEED_I2CD_M_START_CMD | ASPEED_I2CD_M_TX_CMD;
@@ -391,6 +513,13 @@ static irqreturn_t aspeed_i2c_bus_irq(int irq, void *dev_id)
{
struct aspeed_i2c_bus *bus = dev_id;
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ if (aspeed_i2c_slave_irq(bus)) {
+ dev_dbg(bus->dev, "irq handled by slave.\n");
+ return IRQ_HANDLED;
+ }
+#endif /* CONFIG_I2C_SLAVE */
+
if (aspeed_i2c_master_irq(bus))
return IRQ_HANDLED;
else
@@ -449,9 +578,75 @@ static u32 aspeed_i2c_functionality(struct i2c_adapter *adap)
return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_SMBUS_BLOCK_DATA;
}
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+static void __aspeed_i2c_reg_slave(struct aspeed_i2c_bus *bus, u16 slave_addr)
+{
+ u32 addr_reg_val, func_ctrl_reg_val;
+
+ /* Set slave addr. */
+ addr_reg_val = readl(bus->base + ASPEED_I2C_DEV_ADDR_REG);
+ addr_reg_val &= ~ASPEED_I2CD_DEV_ADDR_MASK;
+ addr_reg_val |= slave_addr & ASPEED_I2CD_DEV_ADDR_MASK;
+ writel(addr_reg_val, bus->base + ASPEED_I2C_DEV_ADDR_REG);
+
+ /* Turn on slave mode. */
+ func_ctrl_reg_val = readl(bus->base + ASPEED_I2C_FUN_CTRL_REG);
+ func_ctrl_reg_val |= ASPEED_I2CD_SLAVE_EN;
+ writel(func_ctrl_reg_val, bus->base + ASPEED_I2C_FUN_CTRL_REG);
+}
+
+static int aspeed_i2c_reg_slave(struct i2c_client *client)
+{
+ struct aspeed_i2c_bus *bus;
+ unsigned long flags;
+
+ bus = client->adapter->algo_data;
+ spin_lock_irqsave(&bus->lock, flags);
+ if (bus->slave) {
+ spin_unlock_irqrestore(&bus->lock, flags);
+ return -EINVAL;
+ }
+
+ __aspeed_i2c_reg_slave(bus, client->addr);
+
+ bus->slave = client;
+ bus->slave_state = ASPEED_I2C_SLAVE_STOP;
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ return 0;
+}
+
+static int aspeed_i2c_unreg_slave(struct i2c_client *client)
+{
+ struct aspeed_i2c_bus *bus = client->adapter->algo_data;
+ u32 func_ctrl_reg_val;
+ unsigned long flags;
+
+ spin_lock_irqsave(&bus->lock, flags);
+ if (!bus->slave) {
+ spin_unlock_irqrestore(&bus->lock, flags);
+ return -EINVAL;
+ }
+
+ /* Turn off slave mode. */
+ func_ctrl_reg_val = readl(bus->base + ASPEED_I2C_FUN_CTRL_REG);
+ func_ctrl_reg_val &= ~ASPEED_I2CD_SLAVE_EN;
+ writel(func_ctrl_reg_val, bus->base + ASPEED_I2C_FUN_CTRL_REG);
+
+ bus->slave = NULL;
+ spin_unlock_irqrestore(&bus->lock, flags);
+
+ return 0;
+}
+#endif /* CONFIG_I2C_SLAVE */
+
static const struct i2c_algorithm aspeed_i2c_algo = {
.master_xfer = aspeed_i2c_master_xfer,
.functionality = aspeed_i2c_functionality,
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ .reg_slave = aspeed_i2c_reg_slave,
+ .unreg_slave = aspeed_i2c_unreg_slave,
+#endif /* CONFIG_I2C_SLAVE */
};
static u32 aspeed_i2c_get_clk_reg_val(u32 divisor)
@@ -559,6 +754,12 @@ static int __aspeed_i2c_init(struct aspeed_i2c_bus *bus,
ASPEED_I2CD_MULTI_MASTER_DIS,
bus->base + ASPEED_I2C_FUN_CTRL_REG);
+#if IS_ENABLED(CONFIG_I2C_SLAVE)
+ /* If slave has already been registered, re-enable it. */
+ if (bus->slave)
+ __aspeed_i2c_reg_slave(bus, bus->slave->addr);
+#endif /* CONFIG_I2C_SLAVE */
+
/* Set interrupt generation of I2C controller */
writel(ASPEED_I2CD_INTR_ALL, bus->base + ASPEED_I2C_INTR_CTRL_REG);
--
2.12.2.816.g2cccc81164-goog
--
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
* Re: [PATCH] ARM: dts: Add devicetree for the Raspberry Pi 3, for arm32 (v4)
From: Olof Johansson @ 2017-04-24 18:26 UTC (permalink / raw)
To: Eric Anholt
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Lee Jones, Florian Fainelli, Rob Herring, Mark Rutland,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Stephen Warren, Stefan Wahren, Broadcom Kernel Feedback List,
Gerd Hoffmann
In-Reply-To: <20170330002605.15213-1-eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
Hi,
On Wed, Mar 29, 2017 at 5:26 PM, Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org> wrote:
> Raspbian and Fedora have decided to support the Pi3 in 32-bit mode for
> now, so it's useful to be able to test that mode on an upstream
> kernel. It's also been useful for me to use the same board for 32-bit
> and 64-bit development.
>
> Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
> ---
>
> v1: Gerd's patch that put the ../../../arm64/... link in the Makefile
> v2: Michael's patch that #included from ../../../arm64/... in a new
> bcm2837-rpi-3-b.dts.
> v3: Mine, using symlinks to make sure that we don't break the split DT
> tree.
> v4: Rely on the new include/arm64 symlink.
>
> Assuming positive review feedback, I assume it would be acceptable to
> merge the shared/dt-symlinks branch in a PR of my own for the 32-bit
> DT branch?
>
> arch/arm/boot/dts/Makefile | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
> index 011808490fed..27d258cb50f2 100644
> --- a/arch/arm/boot/dts/Makefile
> +++ b/arch/arm/boot/dts/Makefile
> @@ -72,6 +72,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
> bcm2835-rpi-b-plus.dtb \
> bcm2835-rpi-a-plus.dtb \
> bcm2836-rpi-2-b.dtb \
> + include/arm64/broadcom/bcm2837-rpi-3-b.dtb \
Building straight out of (and into) the include dir is a little odd here.
A tiny wrapper *.dtb in this dir, that just includes a shared dts/dtsi
would be a lot nicer.
If you do that, we can still pick it up for 4.12.
-Olof
--
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 1/3] of: overlay_adjust_phandles() - do not modify const field
From: Frank Rowand @ 2017-04-24 18:54 UTC (permalink / raw)
To: Rob Herring
Cc: Stephen Boyd, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_Jsq+DxGJcggE6KKT_n76CpcCJkRB2sa3Lfm2GRTW78K_tUw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 04/24/17 09:56, Rob Herring wrote:
> On Mon, Apr 24, 2017 at 12:20 AM, <frowand.list-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>> From: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>>
>> When adjusting overlay phandles to apply to the live device tree, can
>> not modify the property value because it is type const.
>>
>> This is to resolve the issue found by Stephen Boyd [1] when he changed
>> the type of struct property.value from void * to const void *. As
>> a result of the type change, the overlay code had compile errors
>> where the resolver updates phandle values.
>
> Conceptually, I prefer your first version. phandles are special and
> there's little reason to expose them except to generate a dts or dtb
> from /proc/device-tree. We could still generate the phandle file in
> that case, but I don't know if special casing phandle is worth it.
The biggest thing that makes me wary about my first version is PPC
and Sparc. I can read their code, but do not know what the firmware
is feeding into the kernel, so I felt like there might be some
incorrect assumptions or fundamental misunderstandings that I
may have.
If we do remove the phandle properties from the live tree, I think
that phandle still needs to be exposed in /proc/device-tree
because that is important information for being able to understand
(or debug) code via reading the source. It isn't a lot code.
One factor I was not sure of to help choose between the first version
and this approach is net memory size of the device tree:
first version:
Adds struct bin_attribute (28 bytes on 32 bit arm) to every node
Removes "linux,phandle" and "phandle" properties from nodes that
have a phandle (64 + 72 = 136 bytes)
second version plus subsequent "linux,phandle" removal:
Removes "linux,phandle" properties from nodes
that have a phandle (72 bytes)
I do not have a feel of how many nodes have phandles in the many
different device trees, so I'm not sure of the end result for the
first version.
I do not have a strong preference between my first approach and second
approach. But now that I have done both, a choice can be made. Let me
know which way you want to go and I'll respin the one you prefer.
For this version I'll make the change you suggested. For the first
version, I'll modify of_attach_mode() slightly more to remove any
"phandle", "linux,phandle", and "ibm,phandle" property from the node
before attaching it, and add the call to add the phandle sysfs
file: __of_add_phandle_sysfs(np);
>>
>> [1] http://lkml.iu.edu/hypermail/linux/kernel/1702.1/04160.html
>>
>> Signed-off-by: Frank Rowand <frank.rowand-7U/KSKJipcs@public.gmane.org>
>> ---
>> drivers/of/base.c | 4 ++--
>> drivers/of/dynamic.c | 28 +++++++++++++++++++++------
>> drivers/of/of_private.h | 3 +++
>> drivers/of/resolver.c | 51 ++++++++++++++++++++++++++++++-------------------
>> 4 files changed, 58 insertions(+), 28 deletions(-)
>>
>> diff --git a/drivers/of/base.c b/drivers/of/base.c
>> index d7c4629a3a2d..b41650fd0fcf 100644
>> --- a/drivers/of/base.c
>> +++ b/drivers/of/base.c
>> @@ -220,8 +220,8 @@ void __init of_core_init(void)
>> proc_symlink("device-tree", NULL, "/sys/firmware/devicetree/base");
>> }
>>
>> -static struct property *__of_find_property(const struct device_node *np,
>> - const char *name, int *lenp)
>> +struct property *__of_find_property(const struct device_node *np,
>> + const char *name, int *lenp)
>> {
>> struct property *pp;
>>
>> diff --git a/drivers/of/dynamic.c b/drivers/of/dynamic.c
>> index 888fdbc09992..44963b4e7235 100644
>> --- a/drivers/of/dynamic.c
>> +++ b/drivers/of/dynamic.c
>> @@ -354,17 +354,17 @@ void of_node_release(struct kobject *kobj)
>> }
>>
>> /**
>> - * __of_prop_dup - Copy a property dynamically.
>> + * __of_prop_alloc - Create a property dynamically.
>> * @prop: Property to copy
>> * @allocflags: Allocation flags (typically pass GFP_KERNEL)
>> *
>> - * Copy a property by dynamically allocating the memory of both the
>> + * Create a property by dynamically allocating the memory of both the
>> * property structure and the property name & contents. The property's
>> * flags have the OF_DYNAMIC bit set so that we can differentiate between
>> * dynamically allocated properties and not.
>> * Returns the newly allocated property or NULL on out of memory error.
>> */
>> -struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
>> +struct property *__of_prop_alloc(char *name, void *value, int len, gfp_t allocflags)
>> {
>> struct property *new;
>>
>> @@ -378,9 +378,9 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
>> * of zero bytes. We do this to work around the use
>> * of of_get_property() calls on boolean values.
>> */
>> - new->name = kstrdup(prop->name, allocflags);
>> - new->value = kmemdup(prop->value, prop->length, allocflags);
>> - new->length = prop->length;
>> + new->name = kstrdup(name, allocflags);
>> + new->value = kmemdup(value, len, allocflags);
>> + new->length = len;
>> if (!new->name || !new->value)
>> goto err_free;
>>
>> @@ -397,6 +397,22 @@ struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
>> }
>>
>> /**
>> + * __of_prop_dup - Copy a property dynamically.
>> + * @prop: Property to copy
>> + * @allocflags: Allocation flags (typically pass GFP_KERNEL)
>> + *
>> + * Copy a property by dynamically allocating the memory of both the
>> + * property structure and the property name & contents. The property's
>> + * flags have the OF_DYNAMIC bit set so that we can differentiate between
>> + * dynamically allocated properties and not.
>> + * Returns the newly allocated property or NULL on out of memory error.
>> + */
>> +struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags)
>> +{
>> + return __of_prop_alloc(prop->name, prop->value, prop->length, allocflags);
>> +}
>> +
>> +/**
>> * __of_node_dup() - Duplicate or create an empty device node dynamically.
>> * @fmt: Format string (plus vargs) for new full name of the device node
>> *
>> diff --git a/drivers/of/of_private.h b/drivers/of/of_private.h
>> index 18bbb4517e25..554394c96569 100644
>> --- a/drivers/of/of_private.h
>> +++ b/drivers/of/of_private.h
>> @@ -62,6 +62,7 @@ static inline int of_property_notify(int action, struct device_node *np,
>> * without taking node references, so you either have to
>> * own the devtree lock or work on detached trees only.
>> */
>> +struct property *__of_prop_alloc(char *name, void *value, int len, gfp_t allocflags);
>> struct property *__of_prop_dup(const struct property *prop, gfp_t allocflags);
>> __printf(2, 3) struct device_node *__of_node_dup(const struct device_node *np, const char *fmt, ...);
>>
>> @@ -70,6 +71,8 @@ extern const void *__of_get_property(const struct device_node *np,
>> extern int __of_add_property(struct device_node *np, struct property *prop);
>> extern int __of_add_property_sysfs(struct device_node *np,
>> struct property *prop);
>> +extern struct property *__of_find_property(const struct device_node *np,
>> + const char *name, int *lenp);
>> extern int __of_remove_property(struct device_node *np, struct property *prop);
>> extern void __of_remove_property_sysfs(struct device_node *np,
>> struct property *prop);
>> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
>> index 7ae9863cb0a4..a2d5b8f0b7bf 100644
>> --- a/drivers/of/resolver.c
>> +++ b/drivers/of/resolver.c
>> @@ -20,6 +20,8 @@
>> #include <linux/errno.h>
>> #include <linux/slab.h>
>>
>> +#include "of_private.h"
>> +
>> /* illegal phandle value (set when unresolved) */
>> #define OF_PHANDLE_ILLEGAL 0xdeadbeef
>>
>> @@ -67,36 +69,43 @@ static phandle live_tree_max_phandle(void)
>> return phandle;
>> }
>>
>> -static void adjust_overlay_phandles(struct device_node *overlay,
>> +static int adjust_overlay_phandles(struct device_node *overlay,
>> int phandle_delta)
>> {
>> struct device_node *child;
>> + struct property *newprop;
>> struct property *prop;
>> phandle phandle;
>
> Some of these can move into the if statement. That will save some
> stack space on recursion (or maybe the compiler is smart enough).
Will do.
>> + int ret;
>>
>> - /* adjust node's phandle in node */
>> - if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL)
>> - overlay->phandle += phandle_delta;
>> -
>> - /* copy adjusted phandle into *phandle properties */
>> - for_each_property_of_node(overlay, prop) {
>> + if (overlay->phandle != 0 && overlay->phandle != OF_PHANDLE_ILLEGAL) {
>>
>> - if (of_prop_cmp(prop->name, "phandle") &&
>> - of_prop_cmp(prop->name, "linux,phandle"))
>> - continue;
>> -
>> - if (prop->length < 4)
>> - continue;
>> + overlay->phandle += phandle_delta;
>>
>> - phandle = be32_to_cpup(prop->value);
>> - if (phandle == OF_PHANDLE_ILLEGAL)
>> - continue;
>> + phandle = cpu_to_be32(overlay->phandle);
>> +
>> + prop = __of_find_property(overlay, "phandle", NULL);
>> + newprop = __of_prop_alloc(prop->name, &phandle, sizeof(phandle),
>> + GFP_KERNEL);
>> + if (!newprop)
>> + return -ENOMEM;
>> + __of_update_property(overlay, newprop, &prop);
>> +
>> + prop = __of_find_property(overlay, "linux,phandle", NULL);
>> + newprop = __of_prop_alloc(prop->name, &phandle, sizeof(phandle),
>> + GFP_KERNEL);
>
> There is no reason to support "linux,phandle" for overlays. That is
> legacy (pre ePAPR) which predates any overlays by a long time.
I would like to have the same behavior for non-overlays as for overlays.
The driver is the same whether the device tree description comes from
the initial device tree or from an overlay.
> Also, dtc still defaults to generating both phandle and linux,phandle
> properties which maybe we should switch off now. If anything, we're
> wasting a bit of memory storing both. I think we should only store
> "phandle" and convert any cases of only a "linux,phandle" property to
> "phandle".
Agreed. If this patch set is accepted instead of the first version, I could
do a subsequent patch to remove the "linux,phandle" property.
>
> Rob
>
--
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
* [PATCH 1/2] clk: Add bindings for the Gemini Clock Controller
From: Linus Walleij @ 2017-04-24 18:55 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, linux-clk, devicetree
Cc: Janos Laube, Paulius Zaleckas, openwrt-devel, linux-arm-kernel,
Hans Ulli Kroll, Florian Fainelli, Linus Walleij
This adds device tree bindings and a header for the Gemini SoC
Clock Controller.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
.../clock/cortina,gemini-clock-controller.txt | 25 +++++++++++++++++++
include/dt-bindings/clock/cortina,gemini-clock.h | 29 ++++++++++++++++++++++
2 files changed, 54 insertions(+)
create mode 100644 Documentation/devicetree/bindings/clock/cortina,gemini-clock-controller.txt
create mode 100644 include/dt-bindings/clock/cortina,gemini-clock.h
diff --git a/Documentation/devicetree/bindings/clock/cortina,gemini-clock-controller.txt b/Documentation/devicetree/bindings/clock/cortina,gemini-clock-controller.txt
new file mode 100644
index 000000000000..7af84acfcbce
--- /dev/null
+++ b/Documentation/devicetree/bindings/clock/cortina,gemini-clock-controller.txt
@@ -0,0 +1,25 @@
+Clock bindings for the Cortina Systems Gemini SoC Clock Controller
+
+Required properties :
+- compatible : shall contain the following:
+ "cortina,gemini-clock-controller"
+- #clock-cells should be <1>
+
+The Gemini clock controller needs to be placed as a subnode of the
+system controller.
+
+All available clocks are defined as preprocessor macros in
+dt-bindings/clock/cortina,gemini-clock.h header and can be used in device
+tree sources.
+
+Example:
+
+syscon: syscon@40000000 {
+ compatible = "cortina,gemini-syscon", "syscon", "simple-mfd";
+ reg = <0x40000000 0x1000>;
+
+ clock-controller {
+ compatible = "cortina,gemini-clock-controller";
+ #clock-cells = <1>;
+ };
+};
diff --git a/include/dt-bindings/clock/cortina,gemini-clock.h b/include/dt-bindings/clock/cortina,gemini-clock.h
new file mode 100644
index 000000000000..acf5cd550b0c
--- /dev/null
+++ b/include/dt-bindings/clock/cortina,gemini-clock.h
@@ -0,0 +1,29 @@
+#ifndef DT_BINDINGS_CORTINA_GEMINI_CLOCK_H
+#define DT_BINDINGS_CORTINA_GEMINI_CLOCK_H
+
+/* RTC, AHB, APB, CPU, PCI, TVC, UART clocks and 13 gates */
+#define GEMINI_NUM_CLKS 20
+
+#define GEMINI_CLK_RTC 0
+#define GEMINI_CLK_AHB 1
+#define GEMINI_CLK_APB 2
+#define GEMINI_CLK_CPU 3
+#define GEMINI_CLK_PCI 4
+#define GEMINI_CLK_TVC 5
+#define GEMINI_CLK_UART 6
+#define GEMINI_CLK_GATES 7
+#define GEMINI_CLK_GATE_SECURITY 7
+#define GEMINI_CLK_GATE_GMAC0 8
+#define GEMINI_CLK_GATE_GMAC1 9
+#define GEMINI_CLK_GATE_SATA0 10
+#define GEMINI_CLK_GATE_SATA1 11
+#define GEMINI_CLK_GATE_USB0 12
+#define GEMINI_CLK_GATE_USB1 13
+#define GEMINI_CLK_GATE_IDE 14
+#define GEMINI_CLK_GATE_PCI 15
+#define GEMINI_CLK_GATE_DDR 16
+#define GEMINI_CLK_GATE_FLASH 17
+#define GEMINI_CLK_GATE_TVC 18
+#define GEMINI_CLK_GATE_BOOT 19
+
+#endif /* DT_BINDINGS_CORTINA_GEMINI_CLOCK_H */
--
2.9.3
^ permalink raw reply related
* Re: [PATCH v6 4/5] i2c: aspeed: added driver for Aspeed I2C
From: Brendan Higgins @ 2017-04-24 18:56 UTC (permalink / raw)
To: Benjamin Herrenschmidt
Cc: Wolfram Sang, Rob Herring, Mark Rutland, Thomas Gleixner,
Jason Cooper, Marc Zyngier, Joel Stanley, Vladimir Zapolskiy,
Kachalov Anton, Cédric Le Goater,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Linux Kernel Mailing List,
OpenBMC Maillist
In-Reply-To: <1490945610.3177.229.camel-XVmvHMARGAS8U2dJNN8I7kB+6BGkLq7r@public.gmane.org>
>> +struct aspeed_i2c_bus {
>> + struct i2c_adapter adap;
>> + struct device *dev;
>> + void __iomem *base;
>> + /* Synchronizes I/O mem access to base. */
>> + spinlock_t lock;
>
> I am not entirely convinced we need that lock. The i2c core will
> take a mutex protecting all operations on the bus. So we only need
> to synchronize between our "xfer" code and our interrupt handler.
You are right if both having slave and master active at the same time
was not possible; however, it is. Imagine the case where the slave is
receiving a request and something in the I2C API gets called. I
suppose we could make the slave IRQ handler lock that lock, but I
think it makes more sense to have a separate lock, since we do not
control that lock making it harder to reason about. Plus, we put
ourselves in a position where an API user has access to a lock that an
interrupt handler needs to acquire, if the user does something dumb,
then we can get interrupt starvation.
>
> This probably be done without a lock if we are careful. Not a huge
> deal though as Aspeed SoC are currently not SMP so the lock compiles
> down to not much unless you have all the debug crap enabled :-)
>
>> + struct completion cmd_complete;
>> + int irq;
>> + /* Transaction state. */
>> + enum aspeed_i2c_master_state master_state;
>> + struct i2c_msg *msgs;
>> + size_t buf_index;
>> + size_t msgs_index;
>> + size_t msgs_size;
>> + bool send_stop;
...
>> + time_left = wait_for_completion_timeout(
>> + &bus->cmd_complete, bus->adap.timeout);
>> +
>> + spin_lock_irqsave(&bus->lock, flags);
>> + if (time_left == 0)
>> + ret = -ETIMEDOUT;
>> + else if (bus->cmd_err)
>> + ret = -EIO;
>> + /* Recovery failed. */
>> + else if (!(aspeed_i2c_read(bus, ASPEED_I2C_CMD_REG) &
>> + ASPEED_I2CD_SDA_LINE_STS))
>> + ret = -EIO;
>> + }
>
> Some of those error states probably also warrant a reset of the controller,
> I think aspeed does that in the SDK.
For timeout and cmd_err, I do not see any argument against it; it
sounds like we are in a very messed up, very unknown state, so full
reset is probably the best last resort. For SDA staying pulled down, I
think we can say with reasonable confidence that some device on our
bus is behaving very badly and I am not convinced that resetting the
controller is likely to do anything to help; that being said, I really
do not have any good ideas to address that. So maybe praying and
resetting the controller is *the most reasonable thing to do.* I would
like to know what you think we should do in that case.
While I was thinking about this I also realized that the SDA line
check after recovery happens in the else branch, but SCL line check
does not happen after we attempt to STOP if SCL is hung. If we decide
to make special note SDA being hung by a device that won't let go, we
might want to make a special note that SCL is hung by a device that
won't let go. Just a thought.
>
>> +out:
...
> What about I2C_M_NOSTART ?
>
> Not that I've ever seen it used... ;-)
Right now I am not doing any of the protocol mangling options, but I
can add them in if you think it is important for initial support.
>
>> + aspeed_i2c_write(bus, slave_addr, ASPEED_I2C_BYTE_BUF_REG);
>> + aspeed_i2c_write(bus, command, ASPEED_I2C_CMD_REG);
>> +}
...
>
>> + spin_lock(&bus->lock);
>> + irq_status = aspeed_i2c_read(bus, ASPEED_I2C_INTR_STS_REG);
>>
>
> I would "ack" (write back to INTR_STS_REG) immediately. Otherwise
> you have a race between status bits set as a result of what happened
> before the interrupt handler vs. as a result of what you did.
>
> For example, take TX. You get the TX bit in irq_status. You start
> a new character transmission bcs there's more to send *then* you ack
> the TX bit. That's racy. If that new transmission is fast enough,
> you'll end up acking the wrong one. Again this is extremely unlikely
> but code should be written in a way that is completely fool proof
> from such races. They can happen for stupid reasons, such as huge
> bus delays caused by a peripheral, FIQ going bonkers etc...
>
> In general, you always ACK all interrupts first. Then you handle
> the bits you have harvested.
>
The documentation says to ACK the interrupt after handling in the RX case:
<<<
S/W needs to clear this status bit to allow next data receiving.
>>>
I will double check with Ryan to make sure TX works the same way.
>> + if (irq_status & ASPEED_I2CD_INTR_ERROR ||
>> + (!bus->msgs && bus->master_state != ASPEED_I2C_MASTER_STOP)) {
>
...
>
> I would set master_state to "RECOVERY" (new state ?) and ensure
> those things are caught if they happen outside of a recovery.
Let me know if you still think we need a "RECOVERY" state.
>
>> + if (bus->master_state == ASPEED_I2C_MASTER_START) {
>
...
>
>> + dev_dbg(bus->dev,
>> + "no slave present at %02x", msg->addr);
>> + status_ack |= ASPEED_I2CD_INTR_TX_NAK;
>> + bus->cmd_err = -EIO;
>> + do_stop(bus);
>> + goto out_no_complete;
>> + } else {
>> + status_ack |= ASPEED_I2CD_INTR_TX_ACK;
>> + if (msg->flags & I2C_M_RD)
>> + bus->master_state = ASPEED_I2C_MASTER_RX;
>> + else
>> + bus->master_state = ASPEED_I2C_MASTER_TX_FIRST;
>
> What about the SMBUS_QUICK case ? (0-len transfer). Do we need
> to handle this here ? A quick look at the TX_FIRST case makes
> me think we are ok there but I'm not sure about the RX case.
I did not think that there is an SMBUS_QUICK RX. Could you point me to
an example?
>
> I'm not sure the RX case is tight also. What completion does the
> HW give you for the address cycle ? Won't you get that before it
> has received the first character ? IE. You fall through to
> the read case of the state machine with the read potentially
> not complete yet no ?
...
>> + case ASPEED_I2C_MASTER_RX:
>> + if (!(irq_status & ASPEED_I2CD_INTR_RX_DONE)) {
>> + dev_err(bus->dev, "master failed to RX");
>> + goto out_complete;
>> + }
>
> See my comment above for a bog standard i2c_read. Aren't you getting
> the completion for the address before the read is even started ?
In practice no, but it is probably best to be safe :-)
>
>> + status_ack |= ASPEED_I2CD_INTR_RX_DONE;
>> +
>> + recv_byte = aspeed_i2c_read(bus, ASPEED_I2C_BYTE_BUF_REG) >> 8;
>> + msg->buf[bus->buf_index++] = recv_byte;
>> +
>> + if (msg->flags & I2C_M_RECV_LEN &&
>> + recv_byte <= I2C_SMBUS_BLOCK_MAX) {
>> + msg->len = recv_byte +
>> + ((msg->flags & I2C_CLIENT_PEC) ? 2 : 1);
...
>> + return ((clk_high << ASPEED_I2CD_TIME_SCL_HIGH_SHIFT)
>> + & ASPEED_I2CD_TIME_SCL_HIGH_MASK)
>> + | ((clk_low << ASPEED_I2CD_TIME_SCL_LOW_SHIFT)
>> + & ASPEED_I2CD_TIME_SCL_LOW_MASK)
>> + | (base_clk & ASPEED_I2CD_TIME_BASE_DIVISOR_MASK);
>> +}
>
> As I think I mentioned earlier, the AST2500 has a slightly different
> register layout which support larger values for high and low, thus
> allowing a finer granularity.
I am developing against the 2500.
> BTW. In case you haven't, I would suggest you copy/paste the above in
> a userspace app and run it for all frequency divisors and see if your
> results match the aspeed table :)
Good call.
>
>> +static int aspeed_i2c_init_clk(struct aspeed_i2c_bus *bus,
>> + struct platform_device *pdev)
>> +{
>> + u32 clk_freq, divisor;
>> + struct clk *pclk;
>> + int ret;
>> +
>> + pclk = devm_clk_get(&pdev->dev, NULL);
>> + if (IS_ERR(pclk)) {
>> + dev_err(&pdev->dev, "clk_get failed\n");
>> + return PTR_ERR(pclk);
>> + }
>> + ret = of_property_read_u32(pdev->dev.of_node,
>> + "clock-frequency", &clk_freq);
>
> See my previous comment about calling that 'bus-frequency' rather
> than 'clock-frequency'.
>
>> + if (ret < 0) {
>> + dev_err(&pdev->dev,
>> + "Could not read clock-frequency property\n");
>> + clk_freq = 100000;
>> + }
>> + divisor = clk_get_rate(pclk) / clk_freq;
>> + /* We just need the clock rate, we don't actually use the clk object. */
>> + devm_clk_put(&pdev->dev, pclk);
>> +
>> + /* Set AC Timing */
>> + if (clk_freq / 1000 > 1000) {
>> + aspeed_i2c_write(bus, aspeed_i2c_read(bus,
>> + ASPEED_I2C_FUN_CTRL_REG) |
>> + ASPEED_I2CD_M_HIGH_SPEED_EN |
>> + ASPEED_I2CD_M_SDA_DRIVE_1T_EN |
>> + ASPEED_I2CD_SDA_DRIVE_1T_EN,
>> + ASPEED_I2C_FUN_CTRL_REG);
>> +
>> + aspeed_i2c_write(bus, 0x3, ASPEED_I2C_AC_TIMING_REG2);
>> + aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divisor),
>> + ASPEED_I2C_AC_TIMING_REG1);
>
> I already discussed by doubts about the above. I can try to scope
> it with the EVB if you don't get to it. For now I'd rather take the
> code out.
>
> We should ask aspeed from what frequency the "1T" stuff is useful.
Will do, I will try to rope Ryan in on the next review; it will be
good for him to get used to working with upstream anyway.
>
>> + } else {
>> + aspeed_i2c_write(bus, aspeed_i2c_get_clk_reg_val(divisor),
>> + ASPEED_I2C_AC_TIMING_REG1);
>> + aspeed_i2c_write(bus, ASPEED_NO_TIMEOUT_CTRL,
>> + ASPEED_I2C_AC_TIMING_REG2);
>> + }
...
>> + spin_lock_init(&bus->lock);
>> + init_completion(&bus->cmd_complete);
>> + bus->adap.owner = THIS_MODULE;
>> + bus->adap.retries = 0;
>> + bus->adap.timeout = 5 * HZ;
>> + bus->adap.algo = &aspeed_i2c_algo;
>> + bus->adap.algo_data = bus;
>> + bus->adap.dev.parent = &pdev->dev;
>> + bus->adap.dev.of_node = pdev->dev.of_node;
>> + snprintf(bus->adap.name, sizeof(bus->adap.name), "Aspeed i2c");
>
> Another trivial one, should we put some kind of bus number
> in that string ?
Whoops, looks like I missed this one; I will get to it in the next revision.
>
>> + bus->dev = &pdev->dev;
>> +
>> + /* reset device: disable master & slave functions */
>> + aspeed_i2c_write(bus, 0, ASPEED_I2C_FUN_CTRL_REG);
...
--
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
* [PATCH 1/2] reset: Add DT bindings for the Gemini reset controller
From: Linus Walleij @ 2017-04-24 19:27 UTC (permalink / raw)
To: Philipp Zabel, devicetree
Cc: openwrt-devel, Paulius Zaleckas, Janos Laube, linux-arm-kernel
This is a simple reset controller in a single 32bit
register.
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
.../bindings/reset/cortina,gemini-reset.txt | 59 ++++++++++++++++++++++
1 file changed, 59 insertions(+)
create mode 100644 Documentation/devicetree/bindings/reset/cortina,gemini-reset.txt
diff --git a/Documentation/devicetree/bindings/reset/cortina,gemini-reset.txt b/Documentation/devicetree/bindings/reset/cortina,gemini-reset.txt
new file mode 100644
index 000000000000..21aa12901774
--- /dev/null
+++ b/Documentation/devicetree/bindings/reset/cortina,gemini-reset.txt
@@ -0,0 +1,59 @@
+Cortina Gemini Reset Controller
+
+This reset controller is found in Cortina Systems CS3516 and
+the predecessor StorLink SL3516.
+
+Required properties:
+- compatible: "cortina,gemini-reset"
+- #reset-cells: Must be 1
+
+The Gemini reset controller must be a child node of the
+system controller. Apart from this it follows the standard reset
+controller bindings.
+
+Valid reset line values:
+
+0: DRAM controller
+1: Flash controller
+2: IDE controller
+3: RAID controller
+4: Security module
+5: GMAC0 (ethernet)
+6: GMAC1 (ethernet)
+7: PCI host bridge
+8: USB0 USB host controller
+9: USB1 USB host controller
+10: General DMA controller
+11: APB bridge
+12: LPC (Low Pin Count) controller
+13: LCD module
+14: Interrupt controller 0
+15: Interrupt controller 1
+16: RTC module
+17: Timer module
+18: UART controller
+19: SSP controller
+20: GPIO0 GPIO controller
+21: GPIO1 GPIO controller
+22: GPIO2 GPIO controller
+23: Watchdog timer
+24: External device reset
+25: CIR module (infrared)
+26: SATA0 SATA bridge
+27: SATA1 SATA bridge
+28: TVE TV Encoder module
+29: Reserved
+30: CPU1 reset
+31: Global soft reset
+
+Example:
+
+syscon: syscon@40000000 {
+ compatible = "cortina,gemini-syscon", "syscon", "simple-mfd";
+ reg = <0x40000000 0x1000>;
+
+ reset-controller {
+ compatible = "cortina,gemini-reset";
+ #reset-cells = <1>;
+ };
+};
--
2.9.3
_______________________________________________
openwrt-devel mailing list
openwrt-devel@lists.openwrt.org
https://lists.openwrt.org/cgi-bin/mailman/listinfo/openwrt-devel
^ permalink raw reply related
* [PATCH] ARM: dts: Add devicetree for the Raspberry Pi 3, for arm32 (v5)
From: Eric Anholt @ 2017-04-24 20:00 UTC (permalink / raw)
To: Lee Jones, Florian Fainelli, Olof Johansson, Rob Herring,
Mark Rutland, devicetree-u79uwXL29TY76Z2rM5mHXA
Cc: linux-rpi-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Stefan Wahren,
bcm-kernel-feedback-list-dY08KVG/lbpWk0Htik3J/w, Gerd Hoffmann,
Eric Anholt
Raspbian and Fedora have decided to support the Pi3 in 32-bit mode for
now, so it's useful to be able to test that mode on an upstream
kernel. It's also been useful for me to use the same board for 32-bit
and 64-bit development.
Signed-off-by: Eric Anholt <eric-WhKQ6XTQaPysTnJN9+BGXg@public.gmane.org>
---
arch/arm/boot/dts/Makefile | 1 +
arch/arm/boot/dts/bcm2837-rpi-3.b.dts | 1 +
2 files changed, 2 insertions(+)
create mode 100644 arch/arm/boot/dts/bcm2837-rpi-3.b.dts
diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 011808490fed..eded842d9978 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -72,6 +72,7 @@ dtb-$(CONFIG_ARCH_BCM2835) += \
bcm2835-rpi-b-plus.dtb \
bcm2835-rpi-a-plus.dtb \
bcm2836-rpi-2-b.dtb \
+ bcm2837-rpi-3-b.dtb \
bcm2835-rpi-zero.dtb
dtb-$(CONFIG_ARCH_BCM_5301X) += \
bcm4708-asus-rt-ac56u.dtb \
diff --git a/arch/arm/boot/dts/bcm2837-rpi-3.b.dts b/arch/arm/boot/dts/bcm2837-rpi-3.b.dts
new file mode 100644
index 000000000000..8c8aa4d1e9b3
--- /dev/null
+++ b/arch/arm/boot/dts/bcm2837-rpi-3.b.dts
@@ -0,0 +1 @@
+#include "arm64/broadcom/bcm2837-rpi-3.b.dts"
--
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
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