* [PATCH v4 00/10] Add device tree support for Exynos4 SoC camera subsystem
From: Sylwester Nawrocki @ 2014-02-20 19:40 UTC (permalink / raw)
To: linux-media, devicetree
Cc: linux-samsung-soc, linux-arm-kernel, robh+dt, mark.rutland, galak,
kyungmin.park, kgene.kim, a.hajda, Sylwester Nawrocki
In-Reply-To: <1392925237-31394-1-git-send-email-s.nawrocki@samsung.com>
This series adds devicetree support for the front and rear camera of
the Exynos4412 SoC Trats2 board. It converts related drivers to use
the v4l2-async API. The SoC output clocks are provided to external image
image sensors through the common clock API.
I'd appreciate a DT binding maintainer reviewed patches 2/11, 3/11.
With an Ack I could finally push these things upstream.
Sylwester Nawrocki (10):
Documentation: dt: Add DT binding documentation for S5K6A3 image
sensor
Documentation: dt: Add DT binding documentation for S5C73M3 camera
Documentation: devicetree: Update Samsung FIMC DT binding
V4L: Add driver for s5k6a3 image sensor
V4L: s5c73m3: Add device tree support
exynos4-is: Use external s5k6a3 sensor driver
exynos4-is: Add clock provider for the SCLK_CAM clock outputs
exynos4-is: Add support for asynchronous subdevices registration
ARM: dts: Add rear camera nodes for Exynos4412 TRATS2 board
ARM: dts: exynos4: Update clk provider part of the camera subsystem
.../devicetree/bindings/media/samsung-fimc.txt | 36 +-
.../devicetree/bindings/media/samsung-s5c73m3.txt | 97 +++++
.../devicetree/bindings/media/samsung-s5k6a3.txt | 33 ++
arch/arm/boot/dts/exynos4.dtsi | 6 +-
arch/arm/boot/dts/exynos4412-trats2.dts | 86 ++++-
drivers/media/i2c/Kconfig | 8 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/s5c73m3/s5c73m3-core.c | 207 ++++++++---
drivers/media/i2c/s5c73m3/s5c73m3-spi.c | 6 +
drivers/media/i2c/s5c73m3/s5c73m3.h | 4 +
drivers/media/i2c/s5k6a3.c | 388 ++++++++++++++++++++
drivers/media/platform/exynos4-is/fimc-is-regs.c | 2 +-
drivers/media/platform/exynos4-is/fimc-is-sensor.c | 285 +-------------
drivers/media/platform/exynos4-is/fimc-is-sensor.h | 49 +--
drivers/media/platform/exynos4-is/fimc-is.c | 97 ++---
drivers/media/platform/exynos4-is/fimc-is.h | 4 +-
drivers/media/platform/exynos4-is/media-dev.c | 329 ++++++++++++-----
drivers/media/platform/exynos4-is/media-dev.h | 32 +-
18 files changed, 1127 insertions(+), 543 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k6a3.txt
create mode 100644 drivers/media/i2c/s5k6a3.c
--
1.7.9.5
^ permalink raw reply
* [PATCH v4 00/10] Add device tree support for Exynos4 SoC camera subsystem
From: Sylwester Nawrocki @ 2014-02-20 19:40 UTC (permalink / raw)
To: linux-media, devicetree
Cc: linux-samsung-soc, linux-arm-kernel, robh+dt, mark.rutland, galak,
kyungmin.park, kgene.kim, a.hajda, Sylwester Nawrocki
This series adds devicetree support for the front and rear camera of
the Exynos4412 SoC Trats2 board. It converts related drivers to use
the v4l2-async API. The SoC output clocks are provided to external image
image sensors through the common clock API.
I'd appreciate a DT binding maintainer reviewed patches 2/10, 3/10.
With an Ack I could finally push these things upstream.
Sylwester Nawrocki (10):
Documentation: dt: Add DT binding documentation for S5K6A3 image
sensor
Documentation: dt: Add DT binding documentation for S5C73M3 camera
Documentation: devicetree: Update Samsung FIMC DT binding
V4L: Add driver for s5k6a3 image sensor
V4L: s5c73m3: Add device tree support
exynos4-is: Use external s5k6a3 sensor driver
exynos4-is: Add clock provider for the SCLK_CAM clock outputs
exynos4-is: Add support for asynchronous subdevices registration
ARM: dts: Add rear camera nodes for Exynos4412 TRATS2 board
ARM: dts: exynos4: Update clk provider part of the camera subsystem
.../devicetree/bindings/media/samsung-fimc.txt | 36 +-
.../devicetree/bindings/media/samsung-s5c73m3.txt | 97 +++++
.../devicetree/bindings/media/samsung-s5k6a3.txt | 33 ++
arch/arm/boot/dts/exynos4.dtsi | 6 +-
arch/arm/boot/dts/exynos4412-trats2.dts | 86 ++++-
drivers/media/i2c/Kconfig | 8 +
drivers/media/i2c/Makefile | 1 +
drivers/media/i2c/s5c73m3/s5c73m3-core.c | 207 ++++++++---
drivers/media/i2c/s5c73m3/s5c73m3-spi.c | 6 +
drivers/media/i2c/s5c73m3/s5c73m3.h | 4 +
drivers/media/i2c/s5k6a3.c | 388 ++++++++++++++++++++
drivers/media/platform/exynos4-is/fimc-is-regs.c | 2 +-
drivers/media/platform/exynos4-is/fimc-is-sensor.c | 285 +-------------
drivers/media/platform/exynos4-is/fimc-is-sensor.h | 49 +--
drivers/media/platform/exynos4-is/fimc-is.c | 97 ++---
drivers/media/platform/exynos4-is/fimc-is.h | 4 +-
drivers/media/platform/exynos4-is/media-dev.c | 329 ++++++++++++-----
drivers/media/platform/exynos4-is/media-dev.h | 32 +-
18 files changed, 1127 insertions(+), 543 deletions(-)
create mode 100644 Documentation/devicetree/bindings/media/samsung-s5c73m3.txt
create mode 100644 Documentation/devicetree/bindings/media/samsung-s5k6a3.txt
create mode 100644 drivers/media/i2c/s5k6a3.c
--
1.7.9.5
^ permalink raw reply
* Re: [PATCH] V4L: s5k6a3: Add DT binding documentation
From: Sylwester Nawrocki @ 2014-02-20 18:35 UTC (permalink / raw)
To: Mark Rutland
Cc: devicetree@vger.kernel.org, Rob Herring, Mauro Carvalho Chehab,
linux-media@vger.kernel.org, linux-samsung-soc@vger.kernel.org,
Kyungmin Park, Pawel Moll, Kumar Gala
In-Reply-To: <20140219174726.GE25079@e106331-lin.cambridge.arm.com>
On 19/02/14 18:47, Mark Rutland wrote:
> On Tue, Feb 18, 2014 at 03:43:11PM +0000, Sylwester Nawrocki wrote:
>> > On 22/12/13 22:27, Sylwester Nawrocki wrote:
>>> > > This patch adds DT binding documentation for the Samsung S5K6A3(YX)
>>> > > raw image sensor.
>>> > >
>>> > > Signed-off-by: Sylwester Nawrocki <s.nawrocki@samsung.com>
>>> > > Signed-off-by: Kyungmin Park <kyungmin.park@samsung.com>
>>> > > ---
>>> > > This patch adds missing documentation [1] for the "samsung,s5k6a3"
>>> > > compatible. Rob, can you please merge it through your tree if it
>>> > > looks OK ?
>> >
>> > Anyone cares to Ack this patch so it can be merged through the media
>> > tree ?
>
> It looks fine to me:
>
> Acked-by: Mark Rutland <mark.rutland@arm.com>
Thanks for the review. I'm going to post a short series including this
patch, I would appreciate your (or other DT binding maintainer) review
of the DT binding part, so this stuff can finally be pushed upstream.
Cheers,
Sylwester
^ permalink raw reply
* Re: [PATCH RFC] usb: gadget: Add xilinx axi usb2 device support
From: Felipe Balbi @ 2014-02-20 18:23 UTC (permalink / raw)
To: Subbaraya Sundeep Bhatta
Cc: Felipe Balbi, Greg Kroah-Hartman,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
michals-gjFFaj9aHVfQT0dZR+AlfA, Subbaraya Sundeep Bhatta,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <774153d4-d33f-4bb4-813b-582762bc3af9-8XeO8fnFoNFnw48GICbVzrjjLBE8jN/0@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 57540 bytes --]
Hi,
On Wed, Feb 19, 2014 at 03:10:45PM +0530, Subbaraya Sundeep Bhatta wrote:
> This patch adds xilinx axi usb2 device driver support
>
> Signed-off-by: Subbaraya Sundeep Bhatta <sbhatta-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
> ---
> .../devicetree/bindings/usb/xilinx_usb.txt | 21 +
> drivers/usb/gadget/Kconfig | 14 +
> drivers/usb/gadget/Makefile | 1 +
> drivers/usb/gadget/xilinx_udc.c | 2045 ++++++++++++++++++++
> 4 files changed, 2081 insertions(+), 0 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/usb/xilinx_usb.txt
> create mode 100644 drivers/usb/gadget/xilinx_udc.c
>
> diff --git a/Documentation/devicetree/bindings/usb/xilinx_usb.txt b/Documentation/devicetree/bindings/usb/xilinx_usb.txt
> new file mode 100644
> index 0000000..acf03ab
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/xilinx_usb.txt
> @@ -0,0 +1,21 @@
> +Xilinx AXI USB2 device controller
> +
> +Required properties:
> +- compatible : Should be "xlnx,axi-usb2-device-4.00.a"
> +- reg : Physical base address and size of the Axi USB2
> + device registers map.
> +- interrupts : Property with a value describing the interrupt
> + number.
> +- interrupt-parent : Must be core interrupt controller
> +- xlnx,include-dma : Must be 1 or 0 based on whether DMA is included
> + in IP or not.
> +
> +Example:
> + axi-usb2-device@42e00000 {
> + compatible = "xlnx,axi-usb2-device-4.00.a";
> + interrupt-parent = <0x1>;
> + interrupts = <0x0 0x39 0x1>;
> + reg = <0x42e00000 0x10000>;
> + xlnx,include-dma = <0x1>;
> + };
> +
you need to Cc devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org for this.
> diff --git a/drivers/usb/gadget/Kconfig b/drivers/usb/gadget/Kconfig
> index 8154165..0b284bf 100644
> --- a/drivers/usb/gadget/Kconfig
> +++ b/drivers/usb/gadget/Kconfig
> @@ -466,6 +466,20 @@ config USB_EG20T
> ML7213/ML7831 is companion chip for Intel Atom E6xx series.
> ML7213/ML7831 is completely compatible for Intel EG20T PCH.
>
> +config USB_GADGET_XILINX
> + tristate "Xilinx USB Driver"
> + depends on MICROBLAZE || ARCH_ZYNQ
This has the tendency to grow and I really don't like seeing a bunch of
arch-specific dependencies. At a minimum add COMPILE_TEST so I can build
test on my setup and make sure it builds fine on other architectures.
> + help
> + USB peripheral controller driver for Xilinx AXI USB2 device.
> + Xilinx AXI USB2 device is a soft IP which supports both full
> + and high speed USB 2.0 data transfers. It has seven configurable
> + endpoints(bulk or interrupt or isochronous), as well as
> + endpoint zero(for control transfers).
> +
> + Say "y" to link the driver statically, or "m" to build a
> + dynamically linked module called "xilinx_udc" and force all
> + gadget drivers to also be dynamically linked.
> +
> #
> # LAST -- dummy/emulated controller
> #
> diff --git a/drivers/usb/gadget/Makefile b/drivers/usb/gadget/Makefile
> index 5f150bc..3a55564 100644
> --- a/drivers/usb/gadget/Makefile
> +++ b/drivers/usb/gadget/Makefile
> @@ -36,6 +36,7 @@ obj-$(CONFIG_USB_FUSB300) += fusb300_udc.o
> obj-$(CONFIG_USB_FOTG210_UDC) += fotg210-udc.o
> obj-$(CONFIG_USB_MV_U3D) += mv_u3d_core.o
> obj-$(CONFIG_USB_GR_UDC) += gr_udc.o
> +obj-$(CONFIG_USB_GADGET_XILINX) += xilinx_udc.o
let's start normalizing the names here (I'll patch the others later) and
call this udc-xilinx.o
> diff --git a/drivers/usb/gadget/xilinx_udc.c b/drivers/usb/gadget/xilinx_udc.c
> new file mode 100644
> index 0000000..3ee8359
> --- /dev/null
> +++ b/drivers/usb/gadget/xilinx_udc.c
> @@ -0,0 +1,2045 @@
> +/*
> + * Xilinx USB peripheral controller driver
> + *
> + * Copyright (C) 2004 by Thomas Rathbone
> + * Copyright (C) 2005 by HP Labs
> + * Copyright (C) 2005 by David Brownell
heh! :-) Brownell was really everywhere, good to still see code from him
;-)
> + * Copyright (C) 2010 - 2014 Xilinx, Inc.
> + *
> + * Some parts of this driver code is based on the driver for at91-series
> + * USB peripheral controller (at91_udc.c).
> + *
> + * This program is free software; you can redistribute it
> + * and/or modify it under the terms of the GNU General Public
> + * License as published by the Free Software Foundation;
> + * either version 2 of the License, or (at your option) any
> + * later version.
are you sure you want to allow people ot use GPL v3 on this driver ?
> +#include <linux/interrupt.h>
> +#include <linux/device.h>
> +#include <linux/module.h>
> +#include <linux/prefetch.h>
> +#include <linux/usb/ch9.h>
> +#include <linux/usb/gadget.h>
> +#include <linux/io.h>
> +#include <linux/seq_file.h>
> +#include <linux/of_address.h>
> +#include <linux/of_device.h>
> +#include <linux/of_platform.h>
> +#include <linux/of_irq.h>
> +#include <linux/dma-mapping.h>
> +#include "gadget_chips.h"
> +
> +/* Register offsets for the USB device.*/
> +#define XUSB_EP0_CONFIG_OFFSET 0x0000 /* EP0 Config Reg Offset */
> +#define XUSB_SETUP_PKT_ADDR_OFFSET 0x0080 /* Setup Packet Address */
> +#define XUSB_ADDRESS_OFFSET 0x0100 /* Address Register */
> +#define XUSB_CONTROL_OFFSET 0x0104 /* Control Register */
> +#define XUSB_STATUS_OFFSET 0x0108 /* Status Register */
> +#define XUSB_FRAMENUM_OFFSET 0x010C /* Frame Number Register */
> +#define XUSB_IER_OFFSET 0x0110 /* Interrupt Enable Register */
> +#define XUSB_BUFFREADY_OFFSET 0x0114 /* Buffer Ready Register */
> +#define XUSB_TESTMODE_OFFSET 0x0118 /* Test Mode Register */
> +#define XUSB_DMA_RESET_OFFSET 0x0200 /* DMA Soft Reset Register */
> +#define XUSB_DMA_CONTROL_OFFSET 0x0204 /* DMA Control Register */
> +#define XUSB_DMA_DSAR_ADDR_OFFSET 0x0208 /* DMA source Address Reg */
> +#define XUSB_DMA_DDAR_ADDR_OFFSET 0x020C /* DMA destination Addr Reg */
> +#define XUSB_DMA_LENGTH_OFFSET 0x0210 /* DMA Length Register */
> +#define XUSB_DMA_STATUS_OFFSET 0x0214 /* DMA Status Register */
> +
> +/* Endpoint Configuration Space offsets */
> +#define XUSB_EP_CFGSTATUS_OFFSET 0x00 /* Endpoint Config Status */
> +#define XUSB_EP_BUF0COUNT_OFFSET 0x08 /* Buffer 0 Count */
> +#define XUSB_EP_BUF1COUNT_OFFSET 0x0C /* Buffer 1 Count */
> +
> +#define XUSB_CONTROL_USB_READY_MASK 0x80000000 /* USB ready Mask */
> +
> +/* Interrupt register related masks.*/
> +#define XUSB_STATUS_GLOBAL_INTR_MASK 0x80000000 /* Global Intr Enable */
> +#define XUSB_STATUS_RESET_MASK 0x00800000 /* USB Reset Mask */
> +#define XUSB_STATUS_SUSPEND_MASK 0x00400000 /* USB Suspend Mask */
> +#define XUSB_STATUS_DISCONNECT_MASK 0x00200000 /* USB Disconnect Mask */
> +#define XUSB_STATUS_FIFO_BUFF_RDY_MASK 0x00100000 /* FIFO Buff Ready Mask */
> +#define XUSB_STATUS_FIFO_BUFF_FREE_MASK 0x00080000 /* FIFO Buff Free Mask */
> +#define XUSB_STATUS_SETUP_PACKET_MASK 0x00040000 /* Setup packet received */
> +#define XUSB_STATUS_EP1_BUFF2_COMP_MASK 0x00000200 /* EP 1 Buff 2 Processed */
> +#define XUSB_STATUS_EP1_BUFF1_COMP_MASK 0x00000002 /* EP 1 Buff 1 Processed */
> +#define XUSB_STATUS_EP0_BUFF2_COMP_MASK 0x00000100 /* EP 0 Buff 2 Processed */
> +#define XUSB_STATUS_EP0_BUFF1_COMP_MASK 0x00000001 /* EP 0 Buff 1 Processed */
> +#define XUSB_STATUS_HIGH_SPEED_MASK 0x00010000 /* USB Speed Mask */
> +/* Suspend,Reset and Disconnect Mask */
> +#define XUSB_STATUS_INTR_EVENT_MASK 0x00E00000
> +/* Buffers completion Mask */
> +#define XUSB_STATUS_INTR_BUFF_COMP_ALL_MASK 0x0000FEFF
> +/* Mask for buffer 0 and buffer 1 completion for all Endpoints */
> +#define XUSB_STATUS_INTR_BUFF_COMP_SHIFT_MASK 0x00000101
> +#define XUSB_STATUS_EP_BUFF2_SHIFT 8 /* EP buffer offset */
> +
> +/* Endpoint Configuration Status Register */
> +#define XUSB_EP_CFG_VALID_MASK 0x80000000 /* Endpoint Valid bit */
> +#define XUSB_EP_CFG_STALL_MASK 0x40000000 /* Endpoint Stall bit */
> +#define XUSB_EP_CFG_DATA_TOGGLE_MASK 0x08000000 /* Endpoint Data toggle */
> +
> +/* USB device specific global configuration constants.*/
> +#define XUSB_MAX_ENDPOINTS 8 /* Maximum End Points */
> +#define XUSB_EP_NUMBER_ZERO 0 /* End point Zero */
> +
> +/* Test Modes (Set Feature).*/
> +#define TEST_J 1 /* Chirp J Test */
> +#define TEST_K 2 /* Chirp K Test */
> +#define TEST_SE0_NAK 3 /* Chirp SE0 Test */
> +#define TEST_PKT 4 /* Packet Test */
> +
> +#define CONFIGURATION_ONE 0x01 /* USB device configuration*/
> +#define STANDARD_OUT_DEVICE 0x00 /* Out device */
> +#define STANDARD_OUT_ENDPOINT 0x02 /* Standard Out end point */
DO NOT REDEFINE THESE, use the ones from <linux/usb/ch9.h>
> +
> +#define XUSB_DMA_READ_FROM_DPRAM 0x80000000/**< DPRAM is the source
> + address for DMA transfer
> + */
> +#define XUSB_DMA_DMASR_BUSY 0x80000000 /**< DMA busy*/
> +#define XUSB_DMA_DMASR_ERROR 0x40000000 /**< DMA Error */
> +
> +/*
> + * When this bit is set, the DMA buffer ready bit is set by hardware upon
> + * DMA transfer completion.
> + */
> +#define XUSB_DMA_BRR_CTRL 0x40000000 /**< DMA bufready ctrl bit */
> +
> +/* Phase States */
> +#define SETUP_PHASE 0x0000 /* Setup Phase */
> +#define DATA_PHASE 0x0001 /* Data Phase */
> +#define STATUS_PHASE 0x0002 /* Status Phase */
> +
> +#define EP_TRANSMIT 0 /* EP is IN endpoint */
> +#define EP0_MAX_PACKET 64 /* Endpoint 0 maximum packet length */
> +
> +/**
> + * struct xusb_request - Xilinx USB device request structure
> + * @usb_req: Linux usb request structure
> + * @queue: usb device request queue
> + */
> +struct xusb_request {
> + struct usb_request usb_req;
> + struct list_head queue;
> +};
> +
> +/**
> + * struct xusb_ep - USB end point structure.
> + * @ep_usb: usb endpoint instance
> + * @queue: endpoint message queue
> + * @udc: xilinx usb peripheral driver instance pointer
> + * @desc: pointer to the usb endpoint descriptor
> + * @data: pointer to the xusb_request structure
> + * @rambase: the endpoint buffer address
> + * @endpointoffset: the endpoint register offset value
> + * @epnumber: endpoint number
> + * @maxpacket: maximum packet size the endpoint can store
> + * @buffer0count: the size of the packet recieved in the first buffer
> + * @buffer0ready: the busy state of first buffer
> + * @buffer1count: the size of the packet received in the second buffer
> + * @buffer1ready: the busy state of second buffer
> + * @eptype: endpoint transfer type (BULK, INTERRUPT)
> + * @curbufnum: current buffer of endpoint that will be processed next
> + * @is_in: endpoint direction (IN or OUT)
> + * @stopped: endpoint active status
> + * @is_iso: endpoint type(isochronous or non isochronous)
> + * @name: name of the endpoint
> + */
> +struct xusb_ep {
> + struct usb_ep ep_usb;
> + struct list_head queue;
> + struct xusb_udc *udc;
> + const struct usb_endpoint_descriptor *desc;
> + struct xusb_request *data;
> + u32 rambase;
> + u32 endpointoffset;
> + u16 epnumber;
> + u16 maxpacket;
> + u16 buffer0count;
> + u16 buffer1count;
> + u8 buffer0ready;
> + u8 buffer1ready;
> + u8 eptype;
> + u8 curbufnum;
> + u8 is_in;
> + u8 stopped;
> + u8 is_iso;
> + char name[4];
> +};
> +
> +/**
> + * struct xusb_udc - USB peripheral driver structure
> + * @gadget: USB gadget driver instance
> + * @ep: an array of endpoint structures
> + * @driver: pointer to the usb gadget driver instance
> + * @read_fn: function pointer to read device registers
> + * @write_fn: function pointer to write to device registers
> + * @base_address: the usb device base address
> + * @lock: instance of spinlock
> + * @dma_enabled: flag indicating whether the dma is included in the system
> + * @status: status flag indicating the device cofiguration
> + */
> +struct xusb_udc {
> + struct usb_gadget gadget;
> + struct xusb_ep ep[8];
> + struct usb_gadget_driver *driver;
> + unsigned int (*read_fn)(void __iomem *);
> + void (*write_fn)(u32, void __iomem *);
> + void __iomem *base_address;
> + spinlock_t lock;
> + bool dma_enabled;
> + u8 status;
> +};
> +
> +/**
> + * struct cmdbuf - Standard USB Command Buffer Structure defined
> + * @setup: usb_ctrlrequest structure for control requests
> + * @contreadcount: read data bytes count
> + * @contwritecount: write data bytes count
> + * @setupseqtx: tx status
> + * @setupseqrx: rx status
> + * @contreadptr: pointer to endpoint0 read data
> + * @contwriteptr: pointer to endpoint0 write data
> + * @contreaddatabuffer: read data buffer for endpoint0
> + */
> +struct cmdbuf {
> + struct usb_ctrlrequest setup;
> + u32 contreadcount;
> + u32 contwritecount;
> + u32 setupseqtx;
> + u32 setupseqrx;
> + u8 *contreadptr;
> + u8 *contwriteptr;
> + u8 contreaddatabuffer[64];
> +};
> +
> +static struct cmdbuf ch9_cmdbuf;
NAK, you should support multiple instances of this device in the same
SoC.
> +/*
> + * Endpoint buffer start addresses in the core
> + */
fits in one line.
> +static u32 rambase[8] = { 0x22, 0x1000, 0x1100, 0x1200, 0x1300, 0x1400, 0x1500,
> + 0x1600 };
> +
> +static const char driver_name[] = "xilinx-udc";
> +static const char ep0name[] = "ep0";
> +
> +/* Control endpoint configuration.*/
> +static struct usb_endpoint_descriptor config_bulk_out_desc = {
> + .bLength = USB_DT_ENDPOINT_SIZE,
> + .bDescriptorType = USB_DT_ENDPOINT,
> + .bEndpointAddress = USB_DIR_OUT,
> + .bmAttributes = USB_ENDPOINT_XFER_BULK,
> + .wMaxPacketSize = __constant_cpu_to_le16(0x40),
> +};
> +
> +/**
> + * to_udc - Return the udc instance pointer
> + * @g: pointer to the usb gadget driver instance
> + *
> + * Return: xusb_udc pointer
> + */
> +static inline struct xusb_udc *to_udc(struct usb_gadget *g)
> +{
> +
remove this whiteline here. Also, this could very well be a macro
instead of a function. No strong feelings though.
> + return container_of(g, struct xusb_udc, gadget);
> +}
> +
> +/**
> + * xudc_write32 - little endian write to device registers
> + * @val: data to be written
> + * @addr: addr of device register
> + */
> +static void xudc_write32(u32 val, void __iomem *addr)
usually, people tend to pass addr, offset and value. Then you do the
quick little math internally:
iowrite32(val, addr + offset);
no strong feelings either.
> +static int xudc_eptxrx(struct xusb_ep *ep, u8 *bufferptr, u32 bufferlen,
> + u8 direction)
> +{
> + u32 *eprambase;
> + u32 bytestosend;
> + u8 *temprambase;
> + unsigned long timeout;
> + u32 srcaddr = 0;
> + u32 dstaddr = 0;
> + int rc = 0;
> + struct xusb_udc *udc = ep->udc;
> +
> + bytestosend = bufferlen;
> +
> + /* Put the transmit buffer into the correct ping-pong buffer.*/
> + if (!ep->curbufnum && !ep->buffer0ready) {
> + /* Get the Buffer address and copy the transmit data.*/
> + eprambase = (u32 __force *)(ep->udc->base_address +
> + ep->rambase);
> +
> + if (ep->udc->dma_enabled) {
> + if (direction == EP_TRANSMIT) {
> + srcaddr = dma_map_single(
> + ep->udc->gadget.dev.parent,
> + bufferptr, bufferlen, DMA_TO_DEVICE);
> + dstaddr = virt_to_phys(eprambase);
> + udc->write_fn(bufferlen,
> + ep->udc->base_address +
> + ep->endpointoffset +
> + XUSB_EP_BUF0COUNT_OFFSET);
> + udc->write_fn(XUSB_DMA_BRR_CTRL |
> + (1 << ep->epnumber),
> + ep->udc->base_address +
> + XUSB_DMA_CONTROL_OFFSET);
> + } else {
> + srcaddr = virt_to_phys(eprambase);
> + dstaddr = dma_map_single(
> + ep->udc->gadget.dev.parent,
> + bufferptr, bufferlen, DMA_FROM_DEVICE);
> + udc->write_fn(XUSB_DMA_BRR_CTRL |
> + XUSB_DMA_READ_FROM_DPRAM |
> + (1 << ep->epnumber),
> + ep->udc->base_address +
> + XUSB_DMA_CONTROL_OFFSET);
> + }
> + /*
> + * Set the addresses in the DMA source and destination
> + * registers and then set the length into the DMA length
> + * register.
> + */
> + udc->write_fn(srcaddr, ep->udc->base_address +
> + XUSB_DMA_DSAR_ADDR_OFFSET);
> + udc->write_fn(dstaddr, ep->udc->base_address +
> + XUSB_DMA_DDAR_ADDR_OFFSET);
> + udc->write_fn(bufferlen, ep->udc->base_address +
> + XUSB_DMA_LENGTH_OFFSET);
> + } else {
> +
> + while (bytestosend > 3) {
> + if (direction == EP_TRANSMIT)
> + *eprambase++ = *(u32 *)bufferptr;
> + else
> + *(u32 *)bufferptr = *eprambase++;
> + bufferptr += 4;
> + bytestosend -= 4;
> + }
> + temprambase = (u8 *)eprambase;
> + while (bytestosend--) {
> + if (direction == EP_TRANSMIT)
> + *temprambase++ = *bufferptr++;
> + else
> + *bufferptr++ = *temprambase++;
> + }
> + /*
> + * Set the Buffer count register with transmit length
> + * and enable the buffer for transmission.
> + */
> + if (direction == EP_TRANSMIT)
> + udc->write_fn(bufferlen,
> + ep->udc->base_address +
> + ep->endpointoffset +
> + XUSB_EP_BUF0COUNT_OFFSET);
> + udc->write_fn(1 << ep->epnumber,
> + ep->udc->base_address +
> + XUSB_BUFFREADY_OFFSET);
> + }
> + ep->buffer0ready = 1;
> + ep->curbufnum = 1;
> +
> + } else
> + if ((ep->curbufnum == 1) && (!ep->buffer1ready)) {
> +
> + /* Get the Buffer address and copy the transmit data.*/
> + eprambase = (u32 __force *)(ep->udc->base_address +
> + ep->rambase + ep->ep_usb.maxpacket);
> + if (ep->udc->dma_enabled) {
> + if (direction == EP_TRANSMIT) {
> + srcaddr = dma_map_single(
> + ep->udc->gadget.dev.parent,
> + bufferptr, bufferlen,
> + DMA_TO_DEVICE);
> + dstaddr = virt_to_phys(eprambase);
> + udc->write_fn(bufferlen,
> + ep->udc->base_address +
> + ep->endpointoffset +
> + XUSB_EP_BUF1COUNT_OFFSET);
> + udc->write_fn(XUSB_DMA_BRR_CTRL |
> + (1 << (ep->epnumber +
> + XUSB_STATUS_EP_BUFF2_SHIFT)),
> + ep->udc->base_address +
> + XUSB_DMA_CONTROL_OFFSET);
> + } else {
> + srcaddr = virt_to_phys(eprambase);
> + dstaddr = dma_map_single(
> + ep->udc->gadget.dev.parent,
> + bufferptr, bufferlen,
> + DMA_FROM_DEVICE);
> + udc->write_fn(XUSB_DMA_BRR_CTRL |
> + XUSB_DMA_READ_FROM_DPRAM |
> + (1 << (ep->epnumber +
> + XUSB_STATUS_EP_BUFF2_SHIFT)),
> + ep->udc->base_address +
> + XUSB_DMA_CONTROL_OFFSET);
> + }
> + /*
> + * Set the addresses in the DMA source and
> + * destination registers and then set the length
> + * into the DMA length register.
> + */
> + udc->write_fn(srcaddr,
> + ep->udc->base_address +
> + XUSB_DMA_DSAR_ADDR_OFFSET);
> + udc->write_fn(dstaddr,
> + ep->udc->base_address +
> + XUSB_DMA_DDAR_ADDR_OFFSET);
> + udc->write_fn(bufferlen,
> + ep->udc->base_address +
> + XUSB_DMA_LENGTH_OFFSET);
> + } else {
> + while (bytestosend > 3) {
> + if (direction == EP_TRANSMIT)
> + *eprambase++ =
> + *(u32 *)bufferptr;
> + else
> + *(u32 *)bufferptr =
> + *eprambase++;
> + bufferptr += 4;
> + bytestosend -= 4;
> + }
> + temprambase = (u8 *)eprambase;
> + while (bytestosend--) {
> + if (direction == EP_TRANSMIT)
> + *temprambase++ = *bufferptr++;
> + else
> + *bufferptr++ = *temprambase++;
> + }
> + /*
> + * Set the Buffer count register with transmit
> + * length and enable the buffer for
> + * transmission.
> + */
> + if (direction == EP_TRANSMIT)
> + udc->write_fn(bufferlen,
> + ep->udc->base_address +
> + ep->endpointoffset +
> + XUSB_EP_BUF1COUNT_OFFSET);
> + udc->write_fn(1 << (ep->epnumber +
> + XUSB_STATUS_EP_BUFF2_SHIFT),
> + ep->udc->base_address +
> + XUSB_BUFFREADY_OFFSET);
> + }
> + ep->buffer1ready = 1;
> + ep->curbufnum = 0;
> + } else {
> + /*
> + * None of the ping-pong buffer is free. Return a
> + * failure.
> + */
> + return 1;
> + }
> +
> + if (ep->udc->dma_enabled) {
> + /*
> + * Wait till DMA transaction is complete and
> + * check whether the DMA transaction was
> + * successful.
> + */
> + while ((udc->read_fn(ep->udc->base_address +
> + XUSB_DMA_STATUS_OFFSET) &
> + XUSB_DMA_DMASR_BUSY) == XUSB_DMA_DMASR_BUSY) {
> + timeout = jiffies + 10000;
> +
> + if (time_after(jiffies, timeout)) {
> + rc = -ETIMEDOUT;
> + goto clean;
> + }
> +
> + }
> + if ((udc->read_fn(ep->udc->base_address +
> + XUSB_DMA_STATUS_OFFSET) &
> + XUSB_DMA_DMASR_ERROR) == XUSB_DMA_DMASR_ERROR)
> + dev_dbg(&ep->udc->gadget.dev, "DMA Error\n");
> +clean:
> + if (direction == EP_TRANSMIT) {
> + dma_unmap_single(ep->udc->gadget.dev.parent,
> + srcaddr, bufferlen, DMA_TO_DEVICE);
> + } else {
> + dma_unmap_single(ep->udc->gadget.dev.parent,
> + dstaddr, bufferlen, DMA_FROM_DEVICE);
> + }
> + }
> + return rc;
> +}
what a big function. Looks like you could split it into smaller
functions to aid redability.
> +static int xudc_ep_enable(struct usb_ep *_ep,
> + const struct usb_endpoint_descriptor *desc)
> +{
> + struct xusb_ep *ep = container_of(_ep, struct xusb_ep, ep_usb);
> + struct xusb_udc *dev = ep->udc;
> + u32 tmp;
> + u8 eptype = 0;
> + unsigned long flags;
> + u32 epcfg;
> + struct xusb_udc *udc = ep->udc;
> +
> + /*
> + * The check for _ep->name == ep0name is not done as this enable i used
> + * for enabling ep0 also. In other gadget drivers, this ep name is not
> + * used.
> + */
> + if (!_ep || !desc || ep->desc ||
> + desc->bDescriptorType != USB_DT_ENDPOINT) {
> + dev_dbg(&ep->udc->gadget.dev, "first check fails\n");
you need a more descriptive message here.
> + return -EINVAL;
> + }
> +
> + if (!dev->driver || dev->gadget.speed == USB_SPEED_UNKNOWN) {
> + dev_dbg(&ep->udc->gadget.dev, "bogus device state\n");
> + return -ESHUTDOWN;
> + }
> +
> +
> + ep->is_in = ((desc->bEndpointAddress & USB_DIR_IN) != 0);
> + /*
> + * Bit 3...0: endpoint number
> + */
fits in one line, no need for multiline comment.
> + ep->epnumber = (desc->bEndpointAddress & 0x0f);
> + ep->stopped = 0;
> + ep->desc = desc;
> + ep->ep_usb.desc = desc;
> + tmp = desc->bmAttributes & USB_ENDPOINT_XFERTYPE_MASK;
> + spin_lock_irqsave(&ep->udc->lock, flags);
> + ep->ep_usb.maxpacket = le16_to_cpu(desc->wMaxPacketSize);
> +
> + switch (tmp) {
> + case USB_ENDPOINT_XFER_CONTROL:
> + dev_dbg(&ep->udc->gadget.dev, "only one control endpoint\n");
> + /* NON- ISO */
> + eptype = 0;
> + spin_unlock_irqrestore(&ep->udc->lock, flags);
> + return -EINVAL;
> + case USB_ENDPOINT_XFER_INT:
> + /* NON- ISO */
> + eptype = 0;
> + if (ep->ep_usb.maxpacket > 64)
> + goto bogus_max;
> + break;
> + case USB_ENDPOINT_XFER_BULK:
> + /* NON- ISO */
> + eptype = 0;
> + switch (ep->ep_usb.maxpacket) {
> + case 8:
> + case 16:
> + case 32:
> + case 64:
> + case 512:
> + goto ok;
bogus indentation.
> + }
> +bogus_max:
> + dev_dbg(&ep->udc->gadget.dev, "bogus maxpacket %d\n",
> + ep->ep_usb.maxpacket);
> + spin_unlock_irqrestore(&ep->udc->lock, flags);
> + return -EINVAL;
> + case USB_ENDPOINT_XFER_ISOC:
> + /* ISO */
> + eptype = 1;
> + ep->is_iso = 1;
> + break;
> + }
> +
> +ok: ep->eptype = eptype;
the label sits in a line by itself:
ok:
ep->eptype = eptype;
...
> +static int xudc_ep_disable(struct usb_ep *_ep)
> +{
> + struct xusb_ep *ep = container_of(_ep, struct xusb_ep, ep_usb);
> + unsigned long flags;
> + u32 epcfg;
> + struct xusb_udc *udc = ep->udc;
> +
> + if (ep == &ep->udc->ep[XUSB_EP_NUMBER_ZERO]) {
> + dev_dbg(&ep->udc->gadget.dev, "Ep0 disable called\n");
> + return -EINVAL;
> + }
> + spin_lock_irqsave(&ep->udc->lock, flags);
> +
> + xudc_nuke(ep, -ESHUTDOWN);
> +
> + /* Restore the endpoint's pristine config */
> + ep->desc = NULL;
> + ep->ep_usb.desc = NULL;
> +
> + ep->stopped = 1;
> +
> + dev_dbg(&ep->udc->gadget.dev, "USB Ep %d disable\n ", ep->epnumber);
> +
> + /* Disable the endpoint.*/
> + epcfg = udc->read_fn(ep->udc->base_address + ep->endpointoffset);
> + epcfg &= ~XUSB_EP_CFG_VALID_MASK;
> + udc->write_fn(epcfg, ep->udc->base_address + ep->endpointoffset);
> + spin_unlock_irqrestore(&ep->udc->lock, flags);
> + return 0;
> +}
> +
> +/**
> + * xudc_ep_alloc_request - Initializes the request queue.
> + * @_ep: pointer to the usb device endpoint structure.
> + * @gfp_flags: Flags related to the request call.
> + *
> + * Return: pointer to request structure on success and a NULL on failure.
> + */
> +static struct usb_request *xudc_ep_alloc_request(struct usb_ep *_ep,
> + gfp_t gfp_flags)
> +{
> + struct xusb_request *req;
> +
> + req = kmalloc(sizeof(*req), gfp_flags);
use kzalloc...
> + if (!req)
> + return NULL;
> +
> + memset(req, 0, sizeof(*req));
... and drop this list head.
> + INIT_LIST_HEAD(&req->queue);
remove this INIT_LIST_HEAD();
also, before returning, I suppose you probably want to link the request
to the endpoint somehow. Usually people save the endpoint pointer inside
the private request structure (iow: req->ep = ep;)
> + return &req->usb_req;
> +}
> +
> +/**
> + * xudc_free_request - Releases the request from queue.
> + * @_ep: pointer to the usb device endpoint structure.
> + * @_req: pointer to the usb request structure.
> + */
> +static void xudc_free_request(struct usb_ep *_ep, struct usb_request *_req)
> +{
> + struct xusb_ep *ep = container_of(_ep, struct xusb_ep, ep_usb);
> + struct xusb_request *req;
> +
> + req = container_of(_req, struct xusb_request, usb_req);
define helper macros for these two container_of(). It helps with
readability.
> + if (!list_empty(&req->queue))
> + dev_warn(&ep->udc->gadget.dev, "Error: No memory to free");
what did you want to do here ? Perhaps:
if (list_empty(&req->queue)) {
dev_warn(&ep->udc->gadget.dev, "Error: no request to free");
return;
}
???
> + kfree(req);
> +}
> +
> +/**
> + * xudc_ep_queue - Adds the request to the queue.
> + * @_ep: pointer to the usb device endpoint structure.
> + * @_req: pointer to the usb request structure.
> + * @gfp_flags: Flags related to the request call.
> + *
> + * Return: 0 for success and error value on failure
> + */
> +static int xudc_ep_queue(struct usb_ep *_ep, struct usb_request *_req,
> + gfp_t gfp_flags)
> +{
> + struct xusb_request *req;
> + struct xusb_ep *ep;
> + struct xusb_udc *dev;
> + unsigned long flags;
> + u32 length, count;
> + u8 *corebuf;
> + struct xusb_udc *udc;
> +
> + req = container_of(_req, struct xusb_request, usb_req);
> + ep = container_of(_ep, struct xusb_ep, ep_usb);
> + udc = ep->udc;
> +
> + if (!_req || !_req->complete || !_req->buf ||
> + !list_empty(&req->queue)) {
> + dev_dbg(&ep->udc->gadget.dev, "invalid request\n");
> + return -EINVAL;
> + }
> +
> + if (!_ep || (!ep->desc && ep->ep_usb.name != ep0name)) {
> + dev_dbg(&ep->udc->gadget.dev, "invalid ep\n");
> + return -EINVAL;
> + }
> +
> + dev = ep->udc;
> + if (!dev || !dev->driver || dev->gadget.speed == USB_SPEED_UNKNOWN) {
> + dev_dbg(&ep->udc->gadget.dev, "%s, bogus device state %p\n",
> + __func__, dev->driver);
> + return -ESHUTDOWN;
> + }
> +
> + spin_lock_irqsave(&dev->lock, flags);
> +
> + _req->status = -EINPROGRESS;
> + _req->actual = 0;
> +
> + /* Try to kickstart any empty and idle queue */
> + if (list_empty(&ep->queue)) {
> + if (!ep->epnumber) {
> + ep->data = req;
> + if (ch9_cmdbuf.setup.bRequestType & USB_DIR_IN) {
> + ch9_cmdbuf.contwriteptr = req->usb_req.buf +
> + req->usb_req.actual;
> + prefetch(ch9_cmdbuf.contwriteptr);
> + length = req->usb_req.length -
> + req->usb_req.actual;
> + corebuf = (void __force *) ((ep->rambase << 2) +
> + ep->udc->base_address);
> + ch9_cmdbuf.contwritecount = length;
> + length = count = min_t(u32, length,
> + EP0_MAX_PACKET);
> + while (length--)
> + *corebuf++ = *ch9_cmdbuf.contwriteptr++;
> + udc->write_fn(count, ep->udc->base_address +
> + XUSB_EP_BUF0COUNT_OFFSET);
> + udc->write_fn(1, ep->udc->base_address +
> + XUSB_BUFFREADY_OFFSET);
> + ch9_cmdbuf.contwritecount -= count;
> + } else {
> + if (ch9_cmdbuf.setup.wLength) {
> + ch9_cmdbuf.contreadptr =
> + req->usb_req.buf +
> + req->usb_req.actual;
> + udc->write_fn(req->usb_req.length ,
> + ep->udc->base_address +
> + XUSB_EP_BUF0COUNT_OFFSET);
> + udc->write_fn(1, ep->udc->base_address
> + + XUSB_BUFFREADY_OFFSET);
> + } else {
> + xudc_wrstatus(udc);
> + req = NULL;
> + }
> + }
> + } else {
> +
> + if (ep->is_in) {
> + dev_dbg(&ep->udc->gadget.dev,
> + "xudc_write_fifo called from queue\n");
> + if (xudc_write_fifo(ep, req) == 1)
> + req = NULL;
> + } else {
> + dev_dbg(&ep->udc->gadget.dev,
> + "xudc_read_fifo called from queue\n");
> + if (xudc_read_fifo(ep, req) == 1)
> + req = NULL;
> + }
> + }
> + }
> +
> + if (req != NULL)
> + list_add_tail(&req->queue, &ep->queue);
> + spin_unlock_irqrestore(&dev->lock, flags);
> + return 0;
> +}
> +
> +/**
> + * xudc_ep_dequeue - Removes the request from the queue.
> + * @_ep: pointer to the usb device endpoint structure.
> + * @_req: pointer to the usb request structure.
> + *
> + * Return: 0 for success and error value on failure
> + */
> +static int xudc_ep_dequeue(struct usb_ep *_ep, struct usb_request *_req)
> +{
> + struct xusb_ep *ep;
> + struct xusb_request *req;
> + unsigned long flags;
> +
> + ep = container_of(_ep, struct xusb_ep, ep_usb);
> +
> + if (!_ep || ep->ep_usb.name == ep0name)
> + return -EINVAL;
> +
> + spin_lock_irqsave(&ep->udc->lock, flags);
> + /* Make sure it's actually queued on this endpoint */
> + list_for_each_entry(req, &ep->queue, queue) {
> + if (&req->usb_req == _req)
> + break;
> + }
> + if (&req->usb_req != _req) {
> + spin_unlock_irqrestore(&ep->udc->lock, flags);
> + return -EINVAL;
> + }
> +
> + xudc_done(ep, req, -ECONNRESET);
> + spin_unlock_irqrestore(&ep->udc->lock, flags);
> +
> + return 0;
> +}
> +
> +static struct usb_ep_ops xusb_ep_ops = {
> + .enable = xudc_ep_enable,
> + .disable = xudc_ep_disable,
> +
> + .alloc_request = xudc_ep_alloc_request,
> + .free_request = xudc_free_request,
> +
> + .queue = xudc_ep_queue,
> + .dequeue = xudc_ep_dequeue,
> + .set_halt = xudc_ep_set_halt,
> +};
> +
> +/**
> + * xudc_get_frame - Reads the current usb frame number.
> + * @gadget: pointer to the usb gadget structure.
> + *
> + * Return: current frame number for success and error value on failure.
> + */
> +static int xudc_get_frame(struct usb_gadget *gadget)
> +{
> +
> + struct xusb_udc *udc = to_udc(gadget);
> + unsigned long flags;
> + int retval;
> +
> + if (!gadget)
> + return -ENODEV;
> +
> + local_irq_save(flags);
> + retval = udc->read_fn(udc->base_address + XUSB_FRAMENUM_OFFSET);
> + local_irq_restore(flags);
> + return retval;
> +}
> +
> +/**
> + * xudc_reinit - Restores inital software state.
> + * @udc: pointer to the usb device controller structure.
> + */
> +static void xudc_reinit(struct xusb_udc *udc)
> +{
> + u32 ep_number;
> + char name[4];
> +
> + INIT_LIST_HEAD(&udc->gadget.ep_list);
> + INIT_LIST_HEAD(&udc->gadget.ep0->ep_list);
> +
> + for (ep_number = 0; ep_number < XUSB_MAX_ENDPOINTS; ep_number++) {
> + struct xusb_ep *ep = &udc->ep[ep_number];
> +
> + if (ep_number) {
> + list_add_tail(&ep->ep_usb.ep_list,
> + &udc->gadget.ep_list);
> + ep->ep_usb.maxpacket = (unsigned short)~0;
> + sprintf(name, "ep%d", ep_number);
> + strcpy(ep->name, name);
> + ep->ep_usb.name = ep->name;
> + } else {
> + ep->ep_usb.name = ep0name;
> + ep->ep_usb.maxpacket = 0x40;
> + }
> +
> + ep->ep_usb.ops = &xusb_ep_ops;
> + ep->udc = udc;
> + ep->epnumber = ep_number;
> + ep->desc = NULL;
> + ep->stopped = 0;
> + /*
> + * The configuration register address offset between
> + * each endpoint is 0x10.
> + */
> + ep->endpointoffset = XUSB_EP0_CONFIG_OFFSET +
> + (ep_number * 0x10);
> + ep->is_in = 0;
> + ep->is_iso = 0;
> + ep->maxpacket = 0;
> + xudc_epconfig(ep, udc);
> + udc->status = 0;
> +
> + /* Initialize one queue per endpoint */
> + INIT_LIST_HEAD(&ep->queue);
> + }
> +}
> +
> +/**
> + * xudc_stop_activity - Stops any further activity on the device.
> + * @udc: pointer to the usb device controller structure.
> + */
> +static void xudc_stop_activity(struct xusb_udc *udc)
> +{
> + struct usb_gadget_driver *driver = udc->driver;
> + int i;
> +
> + if (udc->gadget.speed == USB_SPEED_UNKNOWN)
> + driver = NULL;
> + udc->gadget.speed = USB_SPEED_HIGH;
> +
> + for (i = 0; i < XUSB_MAX_ENDPOINTS; i++) {
> + struct xusb_ep *ep = &udc->ep[i];
> +
> + ep->stopped = 1;
> + xudc_nuke(ep, -ESHUTDOWN);
> + }
> + if (driver) {
> + spin_unlock(&udc->lock);
> + driver->disconnect(&udc->gadget);
> + spin_lock(&udc->lock);
> + }
you shouldn't be calling disconnect here, udc-core is doing that for
you.
> +
> + xudc_reinit(udc);
> +}
> +
> +/**
> + * xudc_start - Starts the device.
> + * @gadget: pointer to the usb gadget structure
> + * @driver: pointer to gadget driver structure
> + *
> + * Return: zero always
> + */
> +static int xudc_start(struct usb_gadget *gadget,
> + struct usb_gadget_driver *driver)
> +{
> + struct xusb_udc *udc = to_udc(gadget);
> + const struct usb_endpoint_descriptor *d = &config_bulk_out_desc;
> +
> + driver->driver.bus = NULL;
> + /* hook up the driver */
> + udc->driver = driver;
> + udc->gadget.dev.driver = &driver->driver;
> + udc->gadget.speed = driver->max_speed;
> +
> + /* Enable the USB device.*/
> + xudc_ep_enable(&udc->ep[XUSB_EP_NUMBER_ZERO].ep_usb, d);
> + udc->write_fn(0, (udc->base_address + XUSB_ADDRESS_OFFSET));
> + udc->write_fn(XUSB_CONTROL_USB_READY_MASK,
> + udc->base_address + XUSB_CONTROL_OFFSET);
> +
> + return 0;
> +}
> +
> +/**
> + * xudc_stop - stops the device.
> + * @gadget: pointer to the usb gadget structure
> + * @driver: pointer to usb gadget driver structure
> + *
> + * Return: zero always
> + */
> +static int xudc_stop(struct usb_gadget *gadget,
> + struct usb_gadget_driver *driver)
> +{
> + struct xusb_udc *udc = to_udc(gadget);
> + unsigned long flags;
> + u32 crtlreg;
> +
> + /* Disable USB device.*/
> + crtlreg = udc->read_fn(udc->base_address + XUSB_CONTROL_OFFSET);
> + crtlreg &= ~XUSB_CONTROL_USB_READY_MASK;
> + udc->write_fn(crtlreg, udc->base_address + XUSB_CONTROL_OFFSET);
> + spin_lock_irqsave(&udc->lock, flags);
> + udc->gadget.speed = USB_SPEED_UNKNOWN;
> + xudc_stop_activity(udc);
> + spin_unlock_irqrestore(&udc->lock, flags);
> +
> + udc->gadget.dev.driver = NULL;
> + udc->driver = NULL;
> +
> + return 0;
> +}
> +
> +static const struct usb_gadget_ops xusb_udc_ops = {
> + .get_frame = xudc_get_frame,
> + .udc_start = xudc_start,
> + .udc_stop = xudc_stop,
> +};
> +
> +/**
> + * xudc_startup_handler - The usb device controller interrupt handler.
> + * @callbackref: pointer to the reference value being passed.
> + * @intrstatus: The mask value containing the interrupt sources.
> + *
> + * This handler handles the RESET, SUSPEND and DISCONNECT interrupts.
> + */
> +static void xudc_startup_handler(void *callbackref, u32 intrstatus)
why void ? why isn't this returning irqreturn_t ?
> +{
> + struct xusb_udc *udc;
> + u32 intrreg;
> +
> + udc = (struct xusb_udc *) callbackref;
you don't need the cast here.
> + if (intrstatus & XUSB_STATUS_RESET_MASK) {
> + dev_dbg(&udc->gadget.dev, "Reset\n");
> + if (intrstatus & XUSB_STATUS_HIGH_SPEED_MASK)
> + udc->gadget.speed = USB_SPEED_HIGH;
> + else
> + udc->gadget.speed = USB_SPEED_FULL;
> +
> + if (udc->status == 1) {
> + udc->status = 0;
> + /* Set device address to 0.*/
> + udc->write_fn(0, udc->base_address +
> + XUSB_ADDRESS_OFFSET);
> + }
> + /* Disable the Reset interrupt.*/
> + intrreg = udc->read_fn(udc->base_address +
> + XUSB_IER_OFFSET);
> +
> + intrreg &= ~XUSB_STATUS_RESET_MASK;
> + udc->write_fn(intrreg, udc->base_address + XUSB_IER_OFFSET);
> + /* Enable thesuspend and disconnect.*/
> + intrreg =
> + udc->read_fn(udc->base_address + XUSB_IER_OFFSET);
> + intrreg |=
> + (XUSB_STATUS_SUSPEND_MASK |
> + XUSB_STATUS_DISCONNECT_MASK);
> + udc->write_fn(intrreg, udc->base_address + XUSB_IER_OFFSET);
> + }
> +
> + if (intrstatus & XUSB_STATUS_DISCONNECT_MASK) {
> +
> + /* Disable the Disconnect interrupt.*/
> + intrreg =
> + udc->read_fn(udc->base_address + XUSB_IER_OFFSET);
> + intrreg &= ~XUSB_STATUS_DISCONNECT_MASK;
> + udc->write_fn(intrreg, udc->base_address + XUSB_IER_OFFSET);
> + dev_dbg(&udc->gadget.dev, "Disconnect\n");
> + if (udc->status == 1) {
> + udc->status = 0;
> + /* Set device address to 0.*/
> + udc->write_fn(0, udc->base_address +
> + XUSB_ADDRESS_OFFSET);
> + /* Enable the USB device.*/
> + udc->write_fn(XUSB_CONTROL_USB_READY_MASK,
> + udc->base_address + XUSB_CONTROL_OFFSET);
> + }
> +
> + /* Enable the suspend and reset interrupts.*/
> + intrreg = udc->read_fn(udc->base_address + XUSB_IER_OFFSET) |
> + XUSB_STATUS_SUSPEND_MASK |
> + XUSB_STATUS_RESET_MASK;
> + udc->write_fn(intrreg, udc->base_address + XUSB_IER_OFFSET);
> + xudc_stop_activity(udc);
> + }
> +
> + if (intrstatus & XUSB_STATUS_SUSPEND_MASK) {
> + dev_dbg(&udc->gadget.dev, "Suspend\n");
> + /* Disable the Suspend interrupt.*/
> + intrreg = udc->read_fn(udc->base_address + XUSB_IER_OFFSET) &
> + ~XUSB_STATUS_SUSPEND_MASK;
> + udc->write_fn(intrreg, udc->base_address + XUSB_IER_OFFSET);
> + /* Enable the Disconnect and reset interrupts. */
> + intrreg = udc->read_fn(udc->base_address + XUSB_IER_OFFSET) |
> + XUSB_STATUS_DISCONNECT_MASK |
> + XUSB_STATUS_RESET_MASK;
> + udc->write_fn(intrreg, udc->base_address + XUSB_IER_OFFSET);
> + }
> +}
> +
> +/**
> + * xudc_set_clear_feature - Executes the set feature and clear feature commands.
> + * @udc: pointer to the usb device controller structure.
> + * @flag: Value deciding the set or clear action.
> + *
> + * Processes the SET_FEATURE and CLEAR_FEATURE commands.
> + */
> +static void xudc_set_clear_feature(struct xusb_udc *udc, int flag)
> +{
> + u8 endpoint;
> + u8 outinbit;
> + u32 epcfgreg;
> +
> + switch (ch9_cmdbuf.setup.bRequestType) {
> + case STANDARD_OUT_DEVICE:
use the macros from <linux/usb/ch9.h>
> + switch (ch9_cmdbuf.setup.wValue) {
> + case USB_DEVICE_TEST_MODE:
> + /*
> + * The Test Mode will be executed
> + * after the status phase.
> + */
> + break;
> +
> + default:
> + epcfgreg = udc->read_fn(udc->base_address +
> + udc->ep[XUSB_EP_NUMBER_ZERO].endpointoffset);
> + epcfgreg |= XUSB_EP_CFG_STALL_MASK;
> + udc->write_fn(epcfgreg, udc->base_address +
> + udc->ep[XUSB_EP_NUMBER_ZERO].endpointoffset);
> + break;
> + }
> + break;
> +
> + case STANDARD_OUT_ENDPOINT:
use the macros from <linux/usb/ch9.h>
> + if (!ch9_cmdbuf.setup.wValue) {
> + endpoint = ch9_cmdbuf.setup.wIndex & 0xf;
> + outinbit = ch9_cmdbuf.setup.wIndex & 0x80;
> + outinbit = outinbit >> 7;
> +
> + /* Make sure direction matches.*/
> + if (outinbit != udc->ep[endpoint].is_in) {
> + epcfgreg = udc->read_fn(udc->base_address +
> + udc->ep[XUSB_EP_NUMBER_ZERO].
> + endpointoffset);
> + epcfgreg |= XUSB_EP_CFG_STALL_MASK;
> +
> + udc->write_fn(epcfgreg, udc->base_address +
> + udc->ep[XUSB_EP_NUMBER_ZERO].
> + endpointoffset);
> + return;
> + }
> +
> + if (!endpoint) {
> + /* Clear the stall.*/
> + epcfgreg = udc->read_fn(udc->base_address +
> + udc->ep[XUSB_EP_NUMBER_ZERO].
> + endpointoffset);
> +
> + epcfgreg &= ~XUSB_EP_CFG_STALL_MASK;
> +
> + udc->write_fn(epcfgreg, udc->base_address +
> + udc->ep[XUSB_EP_NUMBER_ZERO].
> + endpointoffset);
> + break;
> + } else {
> + if (flag == 1) {
> + epcfgreg =
> + udc->read_fn(udc->base_address +
> + udc->ep[XUSB_EP_NUMBER_ZERO].
> + endpointoffset);
> + epcfgreg |= XUSB_EP_CFG_STALL_MASK;
> +
> + udc->write_fn(epcfgreg,
> + udc->base_address +
> + udc->ep[XUSB_EP_NUMBER_ZERO].
> + endpointoffset);
> + } else {
> + /* Unstall the endpoint.*/
> + epcfgreg =
> + udc->read_fn(udc->base_address +
> + udc->ep[endpoint].
> + endpointoffset);
> + epcfgreg &=
> + ~(XUSB_EP_CFG_STALL_MASK |
> + XUSB_EP_CFG_DATA_TOGGLE_MASK);
> + udc->write_fn(epcfgreg,
> + udc->base_address +
> + udc->ep[endpoint].
> + endpointoffset);
> + }
> + }
> + }
> + break;
> +
> + default:
> + epcfgreg = udc->read_fn(udc->base_address +
> + udc->ep[XUSB_EP_NUMBER_ZERO].endpointoffset);
> + epcfgreg |= XUSB_EP_CFG_STALL_MASK;
> + udc->write_fn(epcfgreg, udc->base_address +
> + udc->ep[XUSB_EP_NUMBER_ZERO].endpointoffset);
> + return;
> + }
> +
> + /* Cause and valid status phase to be issued.*/
> + xudc_wrstatus(udc);
> +
> + return;
> +}
> +
> +/**
> + * xudc_execute_cmd - Processes the USB specification chapter 9 commands.
> + * @udc: pointer to the usb device controller structure.
> + *
> + * Return: 0 for success and the same reuqest command if it is not handled.
> + */
> +static int xudc_execute_cmd(struct xusb_udc *udc)
> +{
> +
> + if ((ch9_cmdbuf.setup.bRequestType & USB_TYPE_MASK) ==
> + USB_TYPE_STANDARD) {
> + /* Process the chapter 9 command.*/
> + switch (ch9_cmdbuf.setup.bRequest) {
> +
> + case USB_REQ_CLEAR_FEATURE:
> + xudc_set_clear_feature(udc, 0);
> + break;
> +
> + case USB_REQ_SET_FEATURE:
> + xudc_set_clear_feature(udc, 1);
> + break;
> +
> + case USB_REQ_SET_ADDRESS:
> + xudc_wrstatus(udc);
> + break;
> +
> + case USB_REQ_SET_CONFIGURATION:
> + udc->status = 1;
> + return ch9_cmdbuf.setup.bRequest;
> +
> + default:
> + /*
> + * Return the same request to application for
> + * handling.
> + */
> + return ch9_cmdbuf.setup.bRequest;
> + }
> +
> + } else {
> + if ((ch9_cmdbuf.setup.bRequestType & USB_TYPE_MASK) ==
> + USB_TYPE_CLASS)
> + return ch9_cmdbuf.setup.bRequest;
> + }
> + return 0;
> +}
> +
> +/**
> + * xudc_handle_setup - Processes the setup packet.
> + * @udc: pointer to the usb device controller structure.
> + * @ctrl: pointer to the usb control endpoint structure.
> + *
> + * Return: 0 for success and request to be handled by application if
> + * is not handled by the driver.
> + */
> +static int xudc_handle_setup(struct xusb_udc *udc, struct usb_ctrlrequest *ctrl)
> +{
> + u32 *ep0rambase;
> +
> + /* Load up the chapter 9 command buffer.*/
> + ep0rambase = (u32 __force *) (udc->base_address +
> + XUSB_SETUP_PKT_ADDR_OFFSET);
> + memcpy((void *)&ch9_cmdbuf.setup, (void *)ep0rambase, 8);
> +
> + ctrl->bRequestType = ch9_cmdbuf.setup.bRequestType;
> + ctrl->bRequest = ch9_cmdbuf.setup.bRequest;
> + ctrl->wValue = ch9_cmdbuf.setup.wValue;
> + ctrl->wIndex = ch9_cmdbuf.setup.wIndex;
> + ctrl->wLength = ch9_cmdbuf.setup.wLength;
> +
> + ch9_cmdbuf.setup.wValue = cpu_to_le16(ch9_cmdbuf.setup.wValue);
> + ch9_cmdbuf.setup.wIndex = cpu_to_le16(ch9_cmdbuf.setup.wIndex);
> + ch9_cmdbuf.setup.wLength = cpu_to_le16(ch9_cmdbuf.setup.wLength);
> +
> + /* Restore ReadPtr to data buffer.*/
> + ch9_cmdbuf.contreadptr = &ch9_cmdbuf.contreaddatabuffer[0];
> + ch9_cmdbuf.contreadcount = 0;
> +
> + if (ch9_cmdbuf.setup.bRequestType & USB_DIR_IN) {
> + /* Execute the get command.*/
> + ch9_cmdbuf.setupseqrx = STATUS_PHASE;
> + ch9_cmdbuf.setupseqtx = DATA_PHASE;
> + return xudc_execute_cmd(udc);
> + } else {
> + /* Execute the put command.*/
> + ch9_cmdbuf.setupseqrx = DATA_PHASE;
> + ch9_cmdbuf.setupseqtx = STATUS_PHASE;
> + return xudc_execute_cmd(udc);
> + }
> + /* Control should never come here.*/
> + return 0;
> +}
> +
> +/**
> + * xudc_ep0_out - Processes the endpoint 0 OUT token.
> + * @udc: pointer to the usb device controller structure.
> + */
> +static void xudc_ep0_out(struct xusb_udc *udc)
> +{
> + struct xusb_ep *ep;
> + u8 count;
> + u8 *ep0rambase;
> + u16 index;
> +
> + ep = &udc->ep[0];
> + switch (ch9_cmdbuf.setupseqrx) {
> + case STATUS_PHASE:
what about the setup phase ?
> + /*
> + * This resets both state machines for the next
> + * Setup packet.
> + */
> + ch9_cmdbuf.setupseqrx = SETUP_PHASE;
> + ch9_cmdbuf.setupseqtx = SETUP_PHASE;
> + ep->data->usb_req.actual = ep->data->usb_req.length;
> + xudc_done(ep, ep->data, 0);
> + break;
> +
> + case DATA_PHASE:
> + count = udc->read_fn(udc->base_address +
> + XUSB_EP_BUF0COUNT_OFFSET);
> + /* Copy the data to be received from the DPRAM. */
> + ep0rambase =
> + (u8 __force *) (udc->base_address +
> + (udc->ep[XUSB_EP_NUMBER_ZERO].rambase << 2));
> +
> + for (index = 0; index < count; index++)
> + *ch9_cmdbuf.contreadptr++ = *ep0rambase++;
> +
> + ch9_cmdbuf.contreadcount += count;
> + if (ch9_cmdbuf.setup.wLength == ch9_cmdbuf.contreadcount) {
> + xudc_wrstatus(udc);
> + } else {
> + /* Set the Tx packet size and the Tx enable bit.*/
> + udc->write_fn(0, udc->base_address +
> + XUSB_EP_BUF0COUNT_OFFSET);
> + udc->write_fn(1, udc->base_address +
> + XUSB_BUFFREADY_OFFSET);
> + }
> + break;
> +
> + default:
> + break;
> + }
> +}
> +
> +/**
> + * xudc_ep0_in - Processes the endpoint 0 IN token.
> + * @udc: pointer to the usb device controller structure.
> + */
> +static void xudc_ep0_in(struct xusb_udc *udc)
> +{
> + struct xusb_ep *ep;
> + u32 epcfgreg;
> + u16 count;
> + u16 length;
> + u8 *ep0rambase;
> +
> + ep = &udc->ep[0];
> + switch (ch9_cmdbuf.setupseqtx) {
> + case STATUS_PHASE:
> + if (ch9_cmdbuf.setup.bRequest == USB_REQ_SET_ADDRESS) {
> + /* Set the address of the device.*/
> + udc->write_fn(ch9_cmdbuf.setup.wValue,
> + (udc->base_address +
> + XUSB_ADDRESS_OFFSET));
> + break;
> + } else {
> + if (ch9_cmdbuf.setup.bRequest == USB_REQ_SET_FEATURE) {
> + if (ch9_cmdbuf.setup.bRequestType ==
> + STANDARD_OUT_DEVICE) {
> + if (ch9_cmdbuf.setup.wValue ==
> + USB_DEVICE_TEST_MODE)
> + udc->write_fn(TEST_J,
> + udc->base_address +
> + XUSB_TESTMODE_OFFSET);
use a switch.
> + }
> + }
> + }
> + ep->data->usb_req.actual = ch9_cmdbuf.setup.wLength;
> + xudc_done(ep, ep->data, 0);
> + break;
> +
> + case DATA_PHASE:
> + if (!ch9_cmdbuf.contwritecount) {
> + /*
> + * We're done with data transfer, next
> + * will be zero length OUT with data toggle of
> + * 1. Setup data_toggle.
> + */
> + epcfgreg = udc->read_fn(udc->base_address +
> + udc->ep[XUSB_EP_NUMBER_ZERO].endpointoffset);
> + epcfgreg |= XUSB_EP_CFG_DATA_TOGGLE_MASK;
> + udc->write_fn(epcfgreg, udc->base_address +
> + udc->ep[XUSB_EP_NUMBER_ZERO].endpointoffset);
> + count = 0;
> +
> + ch9_cmdbuf.setupseqtx = STATUS_PHASE;
> +
> + } else {
> + length = count = min_t(u32, ch9_cmdbuf.contwritecount,
> + EP0_MAX_PACKET);
> + /* Copy the data to be transmitted into the DPRAM. */
> + ep0rambase = (u8 __force *) (udc->base_address +
> + (udc->ep[XUSB_EP_NUMBER_ZERO].rambase << 2));
> + while (length--)
> + *ep0rambase++ = *ch9_cmdbuf.contwriteptr++;
> +
> + ch9_cmdbuf.contwritecount -= count;
> + }
> + udc->write_fn(count, udc->base_address +
> + XUSB_EP_BUF0COUNT_OFFSET);
> + udc->write_fn(1, udc->base_address + XUSB_BUFFREADY_OFFSET);
> + break;
> +
> + default:
> + break;
> + }
> +}
> +
> +/**
> + * xudc_ctrl_ep_handler - Endpoint 0 interrupt handler.
> + * @callbackref: pointer to the call back reference passed by the
> + * main interrupt handler.
> + * @intrstatus: It's the mask value for the interrupt sources on endpoint 0.
> + *
> + * Processes the commands received during enumeration phase.
> + */
> +static void xudc_ctrl_ep_handler(void *callbackref, u32 intrstatus)
> +{
> + struct xusb_udc *udc;
> + struct usb_ctrlrequest ctrl;
> + int status;
> + int epnum;
> + u32 intrreg;
> +
> + udc = (struct xusb_udc *) callbackref;
> + /* Process the end point zero buffer interrupt.*/
> + if (intrstatus & XUSB_STATUS_EP0_BUFF1_COMP_MASK) {
> + if (intrstatus & XUSB_STATUS_SETUP_PACKET_MASK) {
> + /*
> + * Enable the Disconnect, suspend and reset
> + * interrupts.
> + */
> + intrreg = udc->read_fn(udc->base_address +
> + XUSB_IER_OFFSET);
> + intrreg |= XUSB_STATUS_DISCONNECT_MASK |
> + XUSB_STATUS_SUSPEND_MASK |
> + XUSB_STATUS_RESET_MASK;
> + udc->write_fn(intrreg,
> + udc->base_address + XUSB_IER_OFFSET);
> + status = xudc_handle_setup(udc, &ctrl);
> + if (status || ((ch9_cmdbuf.setup.bRequestType &
> + USB_TYPE_MASK) == USB_TYPE_CLASS)) {
> + /*
> + * Request is to be handled by the gadget
> + * driver.
> + */
> + spin_unlock(&udc->lock);
> + udc->driver->setup(&udc->gadget, &ctrl);
> + spin_lock(&udc->lock);
> + } else {
> + if (ctrl.bRequest == USB_REQ_CLEAR_FEATURE) {
> + epnum = ctrl.wIndex & 0xf;
> + udc->ep[epnum].stopped = 0;
> + }
> + if (ctrl.bRequest == USB_REQ_SET_FEATURE) {
> + epnum = ctrl.wIndex & 0xf;
> + udc->ep[epnum].stopped = 1;
> + }
> + }
> + } else {
> + if (intrstatus & XUSB_STATUS_FIFO_BUFF_RDY_MASK)
> + xudc_ep0_out(udc);
> + else if (intrstatus &
> + XUSB_STATUS_FIFO_BUFF_FREE_MASK)
> + xudc_ep0_in(udc);
> + }
> + }
> +}
> +
> +/**
> + * xudc_nonctrl_ep_handler - Non control endpoint interrupt handler.
> + * @callbackref: pointer to the call back reference passed by the
> + * main interrupt handler.
> + * @epnum: End point number for which the interrupt is to be processed
> + * @intrstatus: It's the mask value for the interrupt sources on endpoint 0.
> + */
> +static void xudc_nonctrl_ep_handler(void *callbackref, u8 epnum,
> + u32 intrstatus)
> +{
> +
> + struct xusb_request *req;
> + struct xusb_udc *udc;
> + struct xusb_ep *ep;
> +
> + udc = (struct xusb_udc *) callbackref;
> + ep = &udc->ep[epnum];
> + /* Process the End point interrupts.*/
> + if (intrstatus & (XUSB_STATUS_EP0_BUFF1_COMP_MASK << epnum))
> + ep->buffer0ready = 0;
> + if (intrstatus & (XUSB_STATUS_EP0_BUFF2_COMP_MASK << epnum))
> + ep->buffer1ready = 0;
> +
> + if (list_empty(&ep->queue))
> + req = NULL;
> + else
> + req = list_entry(ep->queue.next, struct xusb_request, queue);
> + if (!req)
> + return;
> + if (ep->is_in)
> + xudc_write_fifo(ep, req);
> + else
> + xudc_read_fifo(ep, req);
> +}
> +
> +/**
> + * xudc_irq - The main interrupt handler.
> + * @irq: The interrupt number.
> + * @_udc: pointer to the usb device controller structure.
> + *
> + * Return: IRQ_HANDLED after the interrupt is handled.
> + */
> +static irqreturn_t xudc_irq(int irq, void *_udc)
> +{
> + struct xusb_udc *udc = _udc;
> + u32 intrstatus;
> + u8 index;
> + u32 bufintr;
> +
> + spin_lock(&(udc->lock));
> +
> + /* Read the Interrupt Status Register.*/
> + intrstatus = udc->read_fn(udc->base_address + XUSB_STATUS_OFFSET);
> + /* Call the handler for the event interrupt.*/
> + if (intrstatus & XUSB_STATUS_INTR_EVENT_MASK) {
> + /*
> + * Check if there is any action to be done for :
> + * - USB Reset received {XUSB_STATUS_RESET_MASK}
> + * - USB Suspend received {XUSB_STATUS_SUSPEND_MASK}
what about resume ? No resume ?
> + * - USB Disconnect received {XUSB_STATUS_DISCONNECT_MASK}
> + */
> + xudc_startup_handler(udc, intrstatus);
> + }
> +
> + /* Check the buffer completion interrupts */
> + if (intrstatus & XUSB_STATUS_INTR_BUFF_COMP_ALL_MASK) {
> + if (intrstatus & XUSB_STATUS_EP0_BUFF1_COMP_MASK)
> + xudc_ctrl_ep_handler(udc, intrstatus);
> +
> + for (index = 1; index < 8; index++) {
> + bufintr = ((intrstatus &
> + (XUSB_STATUS_EP1_BUFF1_COMP_MASK <<
> + (index - 1))) ||
> + (intrstatus &
> + (XUSB_STATUS_EP1_BUFF2_COMP_MASK <<
> + (index - 1))));
> +
> + if (bufintr)
> + xudc_nonctrl_ep_handler(udc, index,
> + intrstatus);
> + }
> + }
> + spin_unlock(&(udc->lock));
> +
> + return IRQ_HANDLED;
> +}
> +
> +
> +
> +/**
> + * xudc_release - Releases device structure
> + * @dev: pointer to device structure
> + */
> +static void xudc_release(struct device *dev)
> +{
> +}
you don't need to define this, udc-core will give you a release method.
> +/**
> + * xudc_probe - The device probe function for driver initialization.
> + * @pdev: pointer to the platform device structure.
> + *
> + * Return: 0 for success and error value on failure
> + */
> +static int xudc_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct resource *res;
> + struct xusb_udc *udc;
> + int irq;
> + int ret;
> +
> + dev_dbg(&pdev->dev, "%s(%p)\n", __func__, pdev);
> +
> + udc = devm_kzalloc(&pdev->dev, sizeof(*udc), GFP_KERNEL);
> + if (!udc)
> + return -ENOMEM;
> +
> + /* Map the registers */
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + udc->base_address = devm_ioremap_nocache(&pdev->dev, res->start,
> + resource_size(res));
use devm_ioremap_resource() instead.
> + if (!udc->base_address)
> + return -ENOMEM;
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "unable to get irq\n");
> + return irq;
> + }
> + ret = request_irq(irq, xudc_irq, 0, dev_name(&pdev->dev), udc);
devm_request_irq()
> + if (ret < 0) {
> + dev_dbg(&pdev->dev, "unable to request irq %d", irq);
> + goto fail0;
> + }
> +
> + udc->dma_enabled = of_property_read_bool(np, "xlnx,include-dma");
> +
> + /* Setup gadget structure */
> + udc->gadget.ops = &xusb_udc_ops;
> + udc->gadget.max_speed = USB_SPEED_HIGH;
> + udc->gadget.speed = USB_SPEED_HIGH;
> + udc->gadget.ep0 = &udc->ep[XUSB_EP_NUMBER_ZERO].ep_usb;
> + udc->gadget.name = driver_name;
> +
> + dev_set_name(&udc->gadget.dev, "xilinx_udc");
> + udc->gadget.dev.release = xudc_release;
> + udc->gadget.dev.parent = &pdev->dev;
don't touch the gadget device directly, udc-core handles all of that for
you.
> +
> + spin_lock_init(&udc->lock);
> +
> + /* Check for IP endianness */
> + udc->write_fn = xudc_write32_be;
> + udc->read_fn = xudc_read32_be;
> + udc->write_fn(TEST_J, udc->base_address + XUSB_TESTMODE_OFFSET);
> + if ((udc->read_fn(udc->base_address + XUSB_TESTMODE_OFFSET))
> + != TEST_J) {
> + udc->write_fn = xudc_write32;
> + udc->read_fn = xudc_read32;
> + }
> + udc->write_fn(0, udc->base_address + XUSB_TESTMODE_OFFSET);
> +
> + xudc_reinit(udc);
> +
> + /* Set device address to 0.*/
> + udc->write_fn(0, udc->base_address + XUSB_ADDRESS_OFFSET);
> +
> + ret = usb_add_gadget_udc(&pdev->dev, &udc->gadget);
> + if (ret)
> + goto fail1;
> +
> + /* Enable the interrupts.*/
> + udc->write_fn(XUSB_STATUS_GLOBAL_INTR_MASK | XUSB_STATUS_RESET_MASK |
> + XUSB_STATUS_DISCONNECT_MASK | XUSB_STATUS_SUSPEND_MASK |
> + XUSB_STATUS_FIFO_BUFF_RDY_MASK |
> + XUSB_STATUS_FIFO_BUFF_FREE_MASK |
> + XUSB_STATUS_EP0_BUFF1_COMP_MASK,
> + udc->base_address + XUSB_IER_OFFSET);
> +
> + platform_set_drvdata(pdev, udc);
> +
> + dev_info(&pdev->dev, "%s #%d at 0x%08X mapped to 0x%08X\n",
> + driver_name, 0, (u32)res->start,
> + (u32 __force)udc->base_address);
make this a dev_vdbg().
> +
> + return 0;
> +
> +fail1:
> + free_irq(irq, udc);
> +fail0:
> + dev_dbg(&pdev->dev, "probe failed, %d\n", ret);
this should be dev_err().
> + return ret;
> +}
> +
> +/**
> + * xudc_remove - Releases the resources allocated during the initialization.
> + * @pdev: pointer to the platform device structure.
> + *
> + * Return: 0 for success and error value on failure
> + */
> +static int xudc_remove(struct platform_device *pdev)
> +{
> + struct xusb_udc *udc = platform_get_drvdata(pdev);
> +
> + dev_dbg(&pdev->dev, "remove\n");
> + usb_del_gadget_udc(&udc->gadget);
> + if (udc->driver)
> + return -EBUSY;
> +
> + device_unregister(&udc->gadget.dev);
remove this line.
Also, you're leaking your IRQ handler, if you switch to
devm_request_irq() then you don't need to free it, though.
From the looks of it, I doubt this was actually tested, you need a lot
of work on this driver.
cheers
--
balbi
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* [PATCH tty-next v2 4/4] Documentation: devicetree: add bindings documentation for bcm63xx-uart
From: Florian Fainelli @ 2014-02-20 18:15 UTC (permalink / raw)
To: linux-serial
Cc: devicetree, mbizon, jogo, gregkh, mark.rutland, gregory.0xf0,
Florian Fainelli
In-Reply-To: <1392920154-3642-1-git-send-email-f.fainelli@gmail.com>
Add the Device Tree binding documentation for the non-standard BCM63xx
UART hardware block found in the BCM63xx DSL SoCs.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
- update the compatible property to be "brcm,bcm6345-uart" as suggested
- reword the clocks and clock-names properties based on feedback from Mark
.../devicetree/bindings/serial/bcm63xx-uart.txt | 27 ++++++++++++++++++++++
1 file changed, 27 insertions(+)
create mode 100644 Documentation/devicetree/bindings/serial/bcm63xx-uart.txt
diff --git a/Documentation/devicetree/bindings/serial/bcm63xx-uart.txt b/Documentation/devicetree/bindings/serial/bcm63xx-uart.txt
new file mode 100644
index 0000000..f288232
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/bcm63xx-uart.txt
@@ -0,0 +1,27 @@
+Broadcom BCM63xx UART: Non standard UART used in the Broadcom BCM63xx DSL SoCs
+
+Required properties:
+- compatible: must be "brcm,bcm6345-uart"
+- reg: offset and length of the register set for the device
+- interrupts: device interrupt
+- clocks: a list of phandles and clock-specifiers, one for each entry
+ in clock-names
+- clock-names: should contain "periph" for the functional clock
+
+Example:
+
+serial0: uart@600 {
+ compatible = "brcm,bcm6345-uart";
+ reg = <0x600 0x1b>;
+ interrupts = <GIC_SPI 32 0>;
+ clocks = <&periph_clk>;
+ clock-names = "periph";
+};
+
+Note: each UART port must have an alias correctly numbered in the "aliases"
+node, e.g:
+
+aliases {
+ uart0 = &serial0;
+ uart1 = &serial1;
+};
--
1.8.3.2
^ permalink raw reply related
* [PATCH tty-next v2 3/4] tty: serial: bcm63xx_uart: add support for DT probing
From: Florian Fainelli @ 2014-02-20 18:15 UTC (permalink / raw)
To: linux-serial
Cc: devicetree, mbizon, jogo, gregkh, mark.rutland, gregory.0xf0,
Florian Fainelli
In-Reply-To: <1392920154-3642-1-git-send-email-f.fainelli@gmail.com>
Add a matching table for the the bcm63xx_uart driver on the compatible
string "brcm,bcm6345-uart" which covers all BCM63xx implementations and
reflects the fact that this block was first introduced with the BCM6345
SoC. Also make sure that we convert the id based on the uart aliases
provided by the relevant Device Tree.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
- match on the compatible property "brcm,bcm6345-uart" as suggested
drivers/tty/serial/bcm63xx_uart.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
index 37e7e33..a47421e 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -31,6 +31,7 @@
#include <linux/serial_core.h>
#include <linux/serial_bcm63xx.h>
#include <linux/io.h>
+#include <linux/of.h>
#define BCM63XX_NR_UARTS 2
@@ -806,6 +807,9 @@ static int bcm_uart_probe(struct platform_device *pdev)
struct clk *clk;
int ret;
+ if (pdev->dev.of_node)
+ pdev->id = of_alias_get_id(pdev->dev.of_node, "uart");
+
if (pdev->id < 0 || pdev->id >= BCM63XX_NR_UARTS)
return -EINVAL;
@@ -857,6 +861,12 @@ static int bcm_uart_remove(struct platform_device *pdev)
return 0;
}
+static const struct of_device_id bcm63xx_of_match[] = {
+ { .compatible = "brcm,bcm6345-uart" },
+ { /* sentinel */ }
+};
+MODULE_DEVICE_TABLE(of, bcm63xx_of_match);
+
/*
* platform driver stuff
*/
@@ -866,6 +876,7 @@ static struct platform_driver bcm_uart_platform_driver = {
.driver = {
.owner = THIS_MODULE,
.name = "bcm63xx_uart",
+ .of_match_table = bcm63xx_of_match,
},
};
--
1.8.3.2
^ permalink raw reply related
* [PATCH tty-next v2 2/4] tty: serial: bcm63xx_uart: define UART_REG_SIZE constant
From: Florian Fainelli @ 2014-02-20 18:15 UTC (permalink / raw)
To: linux-serial
Cc: devicetree, mbizon, jogo, gregkh, mark.rutland, gregory.0xf0,
Florian Fainelli
In-Reply-To: <1392920154-3642-1-git-send-email-f.fainelli@gmail.com>
The bcm63xx_uart driver uses RSET_UART_SIZE which is a constant defined
for MIPS-based BCM63xx platforms, pull this constant value from the
MIPS-specific header and put it in include/linux/serial_bcm63xx.h to
make the driver platform agnostic.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
- respin
drivers/tty/serial/bcm63xx_uart.c | 4 ++--
include/linux/serial_bcm63xx.h | 2 ++
2 files changed, 4 insertions(+), 2 deletions(-)
diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
index d71143e..37e7e33 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -589,7 +589,7 @@ static int bcm_uart_request_port(struct uart_port *port)
{
unsigned int size;
- size = RSET_UART_SIZE;
+ size = UART_REG_SIZE;
if (!request_mem_region(port->mapbase, size, "bcm63xx")) {
dev_err(port->dev, "Memory region busy\n");
return -EBUSY;
@@ -609,7 +609,7 @@ static int bcm_uart_request_port(struct uart_port *port)
*/
static void bcm_uart_release_port(struct uart_port *port)
{
- release_mem_region(port->mapbase, RSET_UART_SIZE);
+ release_mem_region(port->mapbase, UART_REG_SIZE);
iounmap(port->membase);
}
diff --git a/include/linux/serial_bcm63xx.h b/include/linux/serial_bcm63xx.h
index 570e964..a80aa1a 100644
--- a/include/linux/serial_bcm63xx.h
+++ b/include/linux/serial_bcm63xx.h
@@ -116,4 +116,6 @@
UART_FIFO_PARERR_MASK | \
UART_FIFO_BRKDET_MASK)
+#define UART_REG_SIZE 24
+
#endif /* _LINUX_SERIAL_BCM63XX_H */
--
1.8.3.2
^ permalink raw reply related
* [PATCH tty-next v2 1/4] tty: serial: bcm63xx_uart: include linux/io.h
From: Florian Fainelli @ 2014-02-20 18:15 UTC (permalink / raw)
To: linux-serial
Cc: devicetree, mbizon, jogo, gregkh, mark.rutland, gregory.0xf0,
Florian Fainelli
In-Reply-To: <1392920154-3642-1-git-send-email-f.fainelli@gmail.com>
Include linux/io.h which provides the definition for
__raw_{readl,writel}, this is not necessary on MIPS since there is an
implicit inclusion, but it is on ARM for instance.
Signed-off-by: Florian Fainelli <f.fainelli@gmail.com>
---
Changes in v2:
- respin
drivers/tty/serial/bcm63xx_uart.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/tty/serial/bcm63xx_uart.c b/drivers/tty/serial/bcm63xx_uart.c
index 78e82b0..d71143e 100644
--- a/drivers/tty/serial/bcm63xx_uart.c
+++ b/drivers/tty/serial/bcm63xx_uart.c
@@ -30,6 +30,7 @@
#include <linux/serial.h>
#include <linux/serial_core.h>
#include <linux/serial_bcm63xx.h>
+#include <linux/io.h>
#define BCM63XX_NR_UARTS 2
--
1.8.3.2
^ permalink raw reply related
* [PATCH tty-next v2 0/4] Device Tree probing for bcm63xx_uart
From: Florian Fainelli @ 2014-02-20 18:15 UTC (permalink / raw)
To: linux-serial
Cc: devicetree, mbizon, jogo, gregkh, mark.rutland, gregory.0xf0,
Florian Fainelli
Hi Greg,
This patchset make the bcm63xx_uart architecture agnostic such that it can be
built on something else than MIPS now. Finally it adds support for Device Tree
probing to the driver along with a DT binding documentation.
I would appreciate if this could go via your tree for consistency.
Thanks!
Florian Fainelli (4):
tty: serial: bcm63xx_uart: include linux/io.h
tty: serial: bcm63xx_uart: define UART_REG_SIZE constant
tty: serial: bcm63xx_uart: add support for DT probing
Documentation: devicetree: add bindings documentation for bcm63xx-uart
.../devicetree/bindings/serial/bcm63xx-uart.txt | 27 ++++++++++++++++++++++
drivers/tty/serial/bcm63xx_uart.c | 16 +++++++++++--
include/linux/serial_bcm63xx.h | 2 ++
3 files changed, 43 insertions(+), 2 deletions(-)
create mode 100644 Documentation/devicetree/bindings/serial/bcm63xx-uart.txt
--
1.8.3.2
^ permalink raw reply
* [PATCH RFC 5/5] Add sample DTS and DTSS schema
From: Tomasz Figa @ 2014-02-20 18:06 UTC (permalink / raw)
To: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Stephen Warren,
Marek Szyprowski, grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
bcousson-rdvid1DuHRBWk0Htik3J/w, olof-nZhT3qVonbNeoWH0uzbU5w,
galak-sgV2jX0FEOL9JmXXK+q4OQ, mark.rutland-5wv7dgnIgG8,
a.hajda-Sze3O3UU22JBDgjK7y7TUQ, pawel.moll-5wv7dgnIgG8,
david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+, jdl-CYoMK+44s/E,
Arnd Bergmann, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
Tomasz Figa
In-Reply-To: <1392919611-10746-1-git-send-email-t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
sample.dts | 70 +++++++++++++++++++++++++++++++++++++++++++++++++
schema.dtss | 86 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 156 insertions(+)
create mode 100644 sample.dts
create mode 100644 schema.dtss
diff --git a/sample.dts b/sample.dts
new file mode 100644
index 0000000..f962051
--- /dev/null
+++ b/sample.dts
@@ -0,0 +1,70 @@
+/dts-v1/;
+
+/ {
+ #address-cells = <1>;
+ #size-cells = <1>;
+ interrupt-parent = <&gic>;
+ compatible = "foo,bar";
+ model = "Foo Bar board";
+
+ chosen {
+ bootargs = "console=ttySAC2,115200N8";
+ };
+
+ aliases {
+ i2c0 = &i2c_0;
+ };
+
+ memory {
+ device_type = "memory";
+ reg = <0x40000000 0x40000000>;
+ };
+
+ soc {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges;
+
+ gic: interrupt-controller@10490000 {
+ compatible = "arm,cortex-a9-gic";
+ #interrupt-cells = <3>;
+ interrupt-controller;
+ reg = <0x10490000 0x1000>, <0x10480000 0x100>;
+ cpu-offset = <0x4000>;
+ };
+
+ i2c_0: i2c@13860000 {
+ compatible = "nvidia,tegra20-i2c";
+ reg = <0x13860000 0x100>;
+ interrupts = <0 57 0>;
+ clocks = <&clocks 43>, <&clocks 28>;
+ clock-names = "div-clk", "fast-clk";
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ audio-codec@33 {
+ compatible = "wlf,wm8903";
+ reg = <0x33>;
+ gpio-controller;
+ #gpio-cells = <1>;
+ };
+ };
+
+ gpio: pinctrl@11400000 {
+ compatible = "foo,bar-gpio";
+ reg = <0x11400000 0x1000>;
+ interrupts = <0 47 0>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ interrupt-controller;
+ #interrupt-cells = <2>;
+ };
+
+ clocks: clock-controller@15440000 {
+ compatible = "foo,bar-clocks";
+ reg = <0x15440000 0x1000>;
+ #clock-cells = <1>;
+ };
+ };
+};
diff --git a/schema.dtss b/schema.dtss
new file mode 100644
index 0000000..7a819fe
--- /dev/null
+++ b/schema.dtss
@@ -0,0 +1,86 @@
+/*
+ * schema.dtss - Sample Device Tree schema file.
+ *
+ * Copyright (C) 2013 Samsung Electronics Co., Ltd.
+ * Author: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation version 2.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether express or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ */
+
+/dtss-v1/;
+
+root-node {
+ /match/ path = "/";
+ /require/ mmio-bus;
+
+ compatible;
+ /optional/ model;
+};
+
+wlf,wm8903 {
+ /match/ compatible = "wlf,wm8903";
+
+ /optional/ micdet-cfg;
+ /optional/ micdet-delay;
+ /optional/ gpio-cfg;
+
+ /require/ i2c-device;
+
+ /require/ gpio-provider {
+ cells = <1>;
+ };
+
+ /use/ interrupts {
+ count = <1>;
+ };
+};
+
+nvidia,tegra20-i2c {
+ /match/ compatible = "nvidia,tegra20-i2c", "nvidia,tegra30-i2c";
+
+ resets;
+ reset-names;
+ dmas;
+ dma-names;
+
+ /require/ i2c-bus;
+
+ /require/ mmio-device {
+ reg-count = <1>;
+ };
+
+ /require/ interrupts {
+ count = <1>;
+ };
+
+ /require/ clocks {
+ names = "div-clk", "fast-clk";
+ };
+};
+
+foo,bar-gpio {
+ /match/ compatible = "foo,bar-gpio";
+
+ /require/ interrupts {
+ count = <1>;
+ };
+
+ /require/ mmio-device {
+ reg-count = <1>;
+ };
+
+ /require/ gpio-provider {
+ cells = <2>;
+ };
+
+ /require/ interrupt-controller {
+ cells = <2>;
+ };
+};
--
1.8.5.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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 RFC 4/5] Add sample C-based generic bindings
From: Tomasz Figa @ 2014-02-20 18:06 UTC (permalink / raw)
To: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Stephen Warren,
Marek Szyprowski, grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
bcousson-rdvid1DuHRBWk0Htik3J/w, olof-nZhT3qVonbNeoWH0uzbU5w,
galak-sgV2jX0FEOL9JmXXK+q4OQ, mark.rutland-5wv7dgnIgG8,
a.hajda-Sze3O3UU22JBDgjK7y7TUQ, pawel.moll-5wv7dgnIgG8,
david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+, jdl-CYoMK+44s/E,
Arnd Bergmann, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
Tomasz Figa
In-Reply-To: <1392919611-10746-1-git-send-email-t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
Makefile.dtc | 7 +-
schemas/clock/clock.c | 77 +++++
schemas/gpio/gpio.c | 93 ++++++
schemas/i2c/i2c.c | 42 +++
schemas/interrupt-controller/interrupts.c | 452 ++++++++++++++++++++++++++++++
schemas/mmio-bus.c | 97 +++++++
6 files changed, 767 insertions(+), 1 deletion(-)
create mode 100644 schemas/clock/clock.c
create mode 100644 schemas/gpio/gpio.c
create mode 100644 schemas/i2c/i2c.c
create mode 100644 schemas/interrupt-controller/interrupts.c
create mode 100644 schemas/mmio-bus.c
diff --git a/Makefile.dtc b/Makefile.dtc
index bf19564..b75da69 100644
--- a/Makefile.dtc
+++ b/Makefile.dtc
@@ -13,7 +13,12 @@ DTC_SRCS = \
srcpos.c \
treesource.c \
util.c \
- schemas/schema.c
+ schemas/mmio-bus.c \
+ schemas/schema.c \
+ schemas/clock/clock.c \
+ schemas/gpio/gpio.c \
+ schemas/i2c/i2c.c \
+ schemas/interrupt-controller/interrupts.c \
DTC_GEN_SRCS = dtc-lexer.lex.c dtc-parser.tab.c dtss-lexer.lex.c dtss-parser.tab.c
DTC_OBJS = $(DTC_SRCS:%.c=%.o) $(DTC_GEN_SRCS:%.c=%.o)
diff --git a/schemas/clock/clock.c b/schemas/clock/clock.c
new file mode 100644
index 0000000..d22b759
--- /dev/null
+++ b/schemas/clock/clock.c
@@ -0,0 +1,77 @@
+#include "dtc.h"
+#include "schema.h"
+
+static void clocks_check_names(struct node *root, struct node *node,
+ struct property *names, bool optional)
+{
+ const char *name = NULL;
+ struct property *clocks, *clock_names;
+
+ clocks = require_property(node, "clocks");
+ clock_names = require_property(node, "clock-names");
+
+ if (!clocks || !clock_names)
+ return;
+
+ for_each_propval_string(names, name) {
+ struct of_phandle_args args;
+ int ret;
+
+ ret = propval_match_string(clock_names, name);
+ if (ret < 0) {
+ if (optional)
+ continue;
+
+ pr_err("clock '%s' not specified in node '%s'\n",
+ name, node->fullpath);
+ continue;
+ }
+
+ ret = propval_parse_phandle_with_args(root, clocks,
+ "#clock-cells", ret,
+ &args);
+ if (ret < 0)
+ pr_err("failed to parse specifier of clock '%s' in node '%s'\n",
+ name, node->fullpath);
+ }
+}
+
+static void clocks_check_count(struct node *root, struct node *node,
+ struct property *count_prop)
+{
+ struct property *clocks;
+ cell_t count = propval_cell(count_prop);
+ int ret;
+
+ clocks = require_property(node, "clocks");
+ if (!clocks)
+ return;
+
+ ret = propval_count_phandle_with_args(root, clocks, "#clock-cells");
+ if (ret < 0)
+ pr_err("failed to parse clocks property\n");
+ else if (ret < count)
+ pr_err("not enough clock specifiers (expected %u, got %d)\n",
+ count, ret);
+}
+
+static void generic_checkfn_clocks(const struct generic_schema *schema,
+ struct node *root, struct node *node,
+ struct node *params, bool required)
+{
+ struct property *prop;
+
+ if (!params) {
+ pr_err("schema clocks requires arguments\n");
+ return;
+ }
+
+ prop = get_property(params, "names");
+ if (prop)
+ clocks_check_names(root, node, prop, !required);
+
+ prop = get_property(params, "count");
+ if (prop)
+ clocks_check_count(root, node, prop);
+}
+GENERIC_SCHEMA("clocks", clocks);
diff --git a/schemas/gpio/gpio.c b/schemas/gpio/gpio.c
new file mode 100644
index 0000000..9100c95
--- /dev/null
+++ b/schemas/gpio/gpio.c
@@ -0,0 +1,93 @@
+#include "schema.h"
+
+static void check_gpios_named(const struct generic_schema *schema,
+ struct node *root, struct node *node,
+ struct node *params, bool required,
+ const char *prop_name)
+{
+ struct property *prop;
+ cell_t count;
+ int ret;
+ int i;
+
+ ret = schema_get_param_cell(params, "count", &count);
+ if (ret < 0) {
+ schema_err(schema, "missing schema argument: 'count'\n");
+ return;
+ }
+
+ if (required)
+ prop = require_property(node, prop_name);
+ else
+ prop = get_property(node, prop_name);
+ if (!prop)
+ return;
+
+ for (i = 0; i < count; ++i) {
+ struct of_phandle_args args;
+
+ ret = propval_parse_phandle_with_args(root, prop,
+ "#gpio-cells", i, &args);
+ if (ret < 0)
+ pr_err("failed to parse gpio specifier %d in '%s' property of node '%s'\n",
+ i, prop_name, node->fullpath);
+ }
+}
+
+static void generic_checkfn_gpios(const struct generic_schema *schema,
+ struct node *root, struct node *node,
+ struct node *params, bool required)
+{
+ check_gpios_named(schema, root, node, params, required, "gpios");
+}
+GENERIC_SCHEMA("gpios", gpios);
+
+static void generic_checkfn_named_gpios(const struct generic_schema *schema,
+ struct node *root, struct node *node,
+ struct node *params, bool required)
+{
+ struct property *prop;
+ const char *name;
+
+ prop = schema_get_param(params, "name");
+ if (!prop) {
+ schema_err(schema, "missing schema argument: 'name'\n");
+ return;
+ }
+
+ name = propval_next_string(prop, NULL);
+
+ check_gpios_named(schema, root, node, params, required, name);
+}
+GENERIC_SCHEMA("named-gpios", named_gpios);
+
+static void generic_checkfn_gpio_provider(const struct generic_schema *schema,
+ struct node *root, struct node *node,
+ struct node *params, bool required)
+{
+ struct property *prop;
+ cell_t cells, val;
+ int ret;
+
+ ret = schema_get_param_cell(params, "cells", &cells);
+ if (ret < 0) {
+ schema_err(schema, "missing schema argument: 'cells'\n");
+ return;
+ }
+
+ prop = require_property(node, "gpio-controller");
+ if (!prop)
+ return;
+
+ prop = require_property(node, "#gpio-cells");
+ if (!prop)
+ return;
+
+ val = propval_cell(prop);
+ if (val != cells) {
+ pr_err("wrong value of #interrupt-cells property in node '%s' (expected %u, got %u)\n",
+ node->fullpath, cells, val);
+ return;
+ }
+}
+GENERIC_SCHEMA("gpio-provider", gpio_provider);
diff --git a/schemas/i2c/i2c.c b/schemas/i2c/i2c.c
new file mode 100644
index 0000000..ba8fd34
--- /dev/null
+++ b/schemas/i2c/i2c.c
@@ -0,0 +1,42 @@
+#include "schema.h"
+
+static void generic_checkfn_i2c_device(const struct generic_schema *schema,
+ struct node *root, struct node *node,
+ struct node *params, bool required)
+{
+ struct property *prop;
+ prop = require_property(node, "reg");
+ if (!prop)
+ return;
+
+ if (prop->val.len != sizeof(cell_t))
+ node_err(node, "i2c-bus expects reg property to be a single cell\n");
+
+ /* TODO: Check if parent device is an i2c bus. */
+}
+GENERIC_SCHEMA("i2c-device", i2c_device);
+
+static void generic_checkfn_i2c_bus(const struct generic_schema *schema,
+ struct node *root, struct node *node,
+ struct node *params, bool required)
+{
+ struct property *prop;
+ cell_t val;
+
+ prop = require_property(node, "#address-cells");
+ if (!prop)
+ return;
+
+ val = propval_cell(prop);
+ if (val != 1)
+ node_err(node, "i2c-bus requires #address-cells == 1\n");
+
+ prop = require_property(node, "#size-cells");
+ if (!prop)
+ return;
+
+ val = propval_cell(prop);
+ if (val != 0)
+ node_err(node, "i2c-bus requires #size-cells == 0\n");
+}
+GENERIC_SCHEMA("i2c-bus", i2c_bus);
diff --git a/schemas/interrupt-controller/interrupts.c b/schemas/interrupt-controller/interrupts.c
new file mode 100644
index 0000000..7eda441
--- /dev/null
+++ b/schemas/interrupt-controller/interrupts.c
@@ -0,0 +1,452 @@
+#include "schema.h"
+
+/*******************************************************************************
+ * Copypasta from kernel's drivers/of/irq.c starts here.
+ * (Well, maybe with some minor changes to make it compile here.)
+ ******************************************************************************/
+
+/* Adaptation glue... */
+
+#define pr_debug(...)
+typedef cell_t __be32;
+typedef cell_t phandle;
+#define of_node_get(node) (node)
+#define of_node_put(node)
+#define of_irq_workarounds (0)
+#define OF_IMAP_OLDWORLD_MAC 0x00000001
+#define OF_IMAP_NO_PHANDLE 0x00000002
+#define of_irq_dflt_pic (NULL)
+#define be32_to_cpu(val) fdt32_to_cpu((val))
+#define be32_to_cpup(ptr) be32_to_cpu(*(ptr))
+#define cpu_to_be32(val) cpu_to_fdt32(val)
+#define raw_spin_lock_irqsave(...)
+#define raw_spin_unlock_irqrestore(...)
+typedef uint32_t u32;
+#define WARN_ON(x) (x)
+#define of_irq_parse_oldworld(...) (-EINVAL)
+#define of_node_full_name(x) ((x)->fullpath)
+
+static const cell_t dummy = 0;
+
+static const void *of_get_property(struct node *np, const char *name,
+ int *lenp)
+{
+ struct property *prop;
+
+ prop = get_property(np, name);
+ if(!prop)
+ return NULL;
+
+ if (lenp)
+ *lenp = prop->val.len;
+
+ if (!prop->val.val)
+ return &dummy;
+
+ return prop->val.val;
+}
+
+static struct node *of_get_parent(const struct node *node)
+{
+ return node ? node->parent : NULL;
+}
+
+static int of_device_is_available(struct node *device)
+{
+ const char *status;
+ int statlen;
+
+ if (!device)
+ return 0;
+
+ status = of_get_property(device, "status", &statlen);
+ if (status == NULL)
+ return 1;
+
+ if (statlen > 0) {
+ if (!strcmp(status, "okay") || !strcmp(status, "ok"))
+ return 1;
+ }
+
+ return 0;
+}
+
+/* End of adaptation glue. */
+
+/**
+ * of_irq_find_parent - Given a device node, find its interrupt parent node
+ * @child: pointer to device node
+ *
+ * Returns a pointer to the interrupt parent node, or NULL if the interrupt
+ * parent could not be determined.
+ */
+static struct node *of_irq_find_parent(struct node *root, struct node *child)
+{
+ struct node *p;
+ const __be32 *parp;
+
+ if (!of_node_get(child))
+ return NULL;
+
+ do {
+ parp = of_get_property(child, "interrupt-parent", NULL);
+ if (parp == NULL)
+ p = of_get_parent(child);
+ else {
+ if (of_irq_workarounds & OF_IMAP_NO_PHANDLE)
+ p = of_node_get(of_irq_dflt_pic);
+ else
+ p = get_node_by_phandle(root,
+ be32_to_cpup(parp));
+ }
+ of_node_put(child);
+ child = p;
+ } while (p && of_get_property(p, "#interrupt-cells", NULL) == NULL);
+
+ return p;
+}
+
+/**
+ * of_irq_parse_raw - Low level interrupt tree parsing
+ * @parent: the device interrupt parent
+ * @addr: address specifier (start of "reg" property of the device) in be32 format
+ * @out_irq: structure of_irq updated by this function
+ *
+ * Returns 0 on success and a negative number on error
+ *
+ * This function is a low-level interrupt tree walking function. It
+ * can be used to do a partial walk with synthetized reg and interrupts
+ * properties, for example when resolving PCI interrupts when no device
+ * node exist for the parent. It takes an interrupt specifier structure as
+ * input, walks the tree looking for any interrupt-map properties, translates
+ * the specifier for each map, and then returns the translated map.
+ */
+static int of_irq_parse_raw(struct node *root,
+ const __be32 *addr, struct of_phandle_args *out_irq)
+{
+ struct node *ipar, *tnode, *old = NULL, *newpar = NULL;
+ __be32 initial_match_array[MAX_PHANDLE_ARGS];
+ const __be32 *match_array = initial_match_array;
+ const __be32 *tmp, *imap, *imask, dummy_imask[] = { [0 ... MAX_PHANDLE_ARGS] = ~0 };
+ u32 intsize = 1, addrsize, newintsize = 0, newaddrsize = 0;
+ int imaplen, match, i;
+
+#ifdef DEBUG
+ of_print_phandle_args("of_irq_parse_raw: ", out_irq);
+#endif
+
+ ipar = of_node_get(out_irq->np);
+
+ /* First get the #interrupt-cells property of the current cursor
+ * that tells us how to interpret the passed-in intspec. If there
+ * is none, we are nice and just walk up the tree
+ */
+ do {
+ tmp = of_get_property(ipar, "#interrupt-cells", NULL);
+ if (tmp != NULL) {
+ intsize = be32_to_cpu(*tmp);
+ break;
+ }
+ tnode = ipar;
+ ipar = of_irq_find_parent(root, ipar);
+ of_node_put(tnode);
+ } while (ipar);
+ if (ipar == NULL) {
+ pr_debug(" -> no parent found !\n");
+ goto fail;
+ }
+
+ pr_debug("of_irq_parse_raw: ipar=%s, size=%d\n", of_node_full_name(ipar), intsize);
+
+ if (out_irq->args_count != intsize)
+ return -EINVAL;
+
+ /* Look for this #address-cells. We have to implement the old linux
+ * trick of looking for the parent here as some device-trees rely on it
+ */
+ old = of_node_get(ipar);
+ do {
+ tmp = of_get_property(old, "#address-cells", NULL);
+ tnode = of_get_parent(old);
+ of_node_put(old);
+ old = tnode;
+ } while (old && tmp == NULL);
+ of_node_put(old);
+ old = NULL;
+ addrsize = (tmp == NULL) ? 2 : be32_to_cpu(*tmp);
+
+ pr_debug(" -> addrsize=%d\n", addrsize);
+
+ /* Range check so that the temporary buffer doesn't overflow */
+ if (WARN_ON(addrsize + intsize > MAX_PHANDLE_ARGS))
+ goto fail;
+
+ /* Precalculate the match array - this simplifies match loop */
+ for (i = 0; i < addrsize; i++)
+ initial_match_array[i] = addr ? addr[i] : 0;
+ for (i = 0; i < intsize; i++)
+ initial_match_array[addrsize + i] = cpu_to_be32(out_irq->args[i]);
+
+ /* Now start the actual "proper" walk of the interrupt tree */
+ while (ipar != NULL) {
+ /* Now check if cursor is an interrupt-controller and if it is
+ * then we are done
+ */
+ if (of_get_property(ipar, "interrupt-controller", NULL) !=
+ NULL) {
+ pr_debug(" -> got it !\n");
+ return 0;
+ }
+
+ /*
+ * interrupt-map parsing does not work without a reg
+ * property when #address-cells != 0
+ */
+ if (addrsize && !addr) {
+ pr_debug(" -> no reg passed in when needed !\n");
+ goto fail;
+ }
+
+ /* Now look for an interrupt-map */
+ imap = of_get_property(ipar, "interrupt-map", &imaplen);
+ /* No interrupt map, check for an interrupt parent */
+ if (imap == NULL) {
+ pr_debug(" -> no map, getting parent\n");
+ newpar = of_irq_find_parent(root, ipar);
+ goto skiplevel;
+ }
+ imaplen /= sizeof(u32);
+
+ /* Look for a mask */
+ imask = of_get_property(ipar, "interrupt-map-mask", NULL);
+ if (!imask)
+ imask = dummy_imask;
+
+ /* Parse interrupt-map */
+ match = 0;
+ while (imaplen > (addrsize + intsize + 1) && !match) {
+ /* Compare specifiers */
+ match = 1;
+ for (i = 0; i < (addrsize + intsize); i++, imaplen--)
+ match &= !((match_array[i] ^ *imap++) & imask[i]);
+
+ pr_debug(" -> match=%d (imaplen=%d)\n", match, imaplen);
+
+ /* Get the interrupt parent */
+ if (of_irq_workarounds & OF_IMAP_NO_PHANDLE)
+ newpar = of_node_get(of_irq_dflt_pic);
+ else
+ newpar = get_node_by_phandle(root,
+ be32_to_cpup(imap));
+ imap++;
+ --imaplen;
+
+ /* Check if not found */
+ if (newpar == NULL) {
+ pr_debug(" -> imap parent not found !\n");
+ goto fail;
+ }
+
+ if (!of_device_is_available(newpar))
+ match = 0;
+
+ /* Get #interrupt-cells and #address-cells of new
+ * parent
+ */
+ tmp = of_get_property(newpar, "#interrupt-cells", NULL);
+ if (tmp == NULL) {
+ pr_debug(" -> parent lacks #interrupt-cells!\n");
+ goto fail;
+ }
+ newintsize = be32_to_cpu(*tmp);
+ tmp = of_get_property(newpar, "#address-cells", NULL);
+ newaddrsize = (tmp == NULL) ? 0 : be32_to_cpu(*tmp);
+
+ pr_debug(" -> newintsize=%d, newaddrsize=%d\n",
+ newintsize, newaddrsize);
+
+ /* Check for malformed properties */
+ if (WARN_ON(newaddrsize + newintsize > MAX_PHANDLE_ARGS))
+ goto fail;
+ if (imaplen < (newaddrsize + newintsize))
+ goto fail;
+
+ imap += newaddrsize + newintsize;
+ imaplen -= newaddrsize + newintsize;
+
+ pr_debug(" -> imaplen=%d\n", imaplen);
+ }
+ if (!match)
+ goto fail;
+
+ /*
+ * Successfully parsed an interrrupt-map translation; copy new
+ * interrupt specifier into the out_irq structure
+ */
+ out_irq->np = newpar;
+
+ match_array = imap - newaddrsize - newintsize;
+ for (i = 0; i < newintsize; i++)
+ out_irq->args[i] = be32_to_cpup(imap - newintsize + i);
+ out_irq->args_count = intsize = newintsize;
+ addrsize = newaddrsize;
+
+ skiplevel:
+ /* Iterate again with new parent */
+ pr_debug(" -> new parent: %s\n", of_node_full_name(newpar));
+ of_node_put(ipar);
+ ipar = newpar;
+ newpar = NULL;
+ }
+ fail:
+ of_node_put(ipar);
+ of_node_put(newpar);
+
+ return -EINVAL;
+}
+
+/**
+ * of_irq_parse_one - Resolve an interrupt for a device
+ * @device: the device whose interrupt is to be resolved
+ * @index: index of the interrupt to resolve
+ * @out_irq: structure of_irq filled by this function
+ *
+ * This function resolves an interrupt for a node by walking the interrupt tree,
+ * finding which interrupt controller node it is attached to, and returning the
+ * interrupt specifier that can be used to retrieve a Linux IRQ number.
+ */
+static int of_irq_parse_one(struct node *root, struct node *device,
+ int index, struct of_phandle_args *out_irq)
+{
+ struct node *p;
+ const __be32 *intspec, *tmp, *addr;
+ u32 intsize, intlen;
+ int i, res = -EINVAL;
+
+ pr_debug("of_irq_parse_one: dev=%s, index=%d\n", of_node_full_name(device), index);
+
+ /* OldWorld mac stuff is "special", handle out of line */
+ if (of_irq_workarounds & OF_IMAP_OLDWORLD_MAC)
+ return of_irq_parse_oldworld(device, index, out_irq);
+
+ /* Get the reg property (if any) */
+ addr = of_get_property(device, "reg", NULL);
+
+ /* Get the interrupts property */
+ intspec = of_get_property(device, "interrupts", (int *)&intlen);
+ if (intspec == NULL) {
+ struct property *prop;
+
+ prop = get_property(device, "interrupts-extended");
+ if (!prop)
+ return -EINVAL;
+
+ /* Try the new-style interrupts-extended */
+ res = propval_parse_phandle_with_args(root, prop,
+ "#interrupt-cells",
+ index, out_irq);
+ if (res)
+ return -EINVAL;
+ return of_irq_parse_raw(root, addr, out_irq);
+ }
+ intlen /= sizeof(*intspec);
+
+ pr_debug(" intspec=%d intlen=%d\n", be32_to_cpup(intspec), intlen);
+
+ /* Look for the interrupt parent. */
+ p = of_irq_find_parent(root, device);
+ if (p == NULL)
+ return -EINVAL;
+
+ /* Get size of interrupt specifier */
+ tmp = of_get_property(p, "#interrupt-cells", NULL);
+ if (tmp == NULL)
+ goto out;
+ intsize = be32_to_cpu(*tmp);
+
+ pr_debug(" intsize=%d intlen=%d\n", intsize, intlen);
+
+ /* Check index */
+ if ((index + 1) * intsize > intlen)
+ goto out;
+
+ /* Copy intspec into irq structure */
+ intspec += index * intsize;
+ out_irq->np = p;
+ out_irq->args_count = intsize;
+ for (i = 0; i < intsize; i++)
+ out_irq->args[i] = be32_to_cpup(intspec++);
+
+ /* Check if there are any interrupt-map translations to process */
+ res = of_irq_parse_raw(root, addr, out_irq);
+ out:
+ of_node_put(p);
+ return res;
+}
+
+/*******************************************************************************
+ * Copypasta ends here.
+ ******************************************************************************/
+
+static void generic_checkfn_interrupts(const struct generic_schema *schema,
+ struct node *root, struct node *node,
+ struct node *params, bool required)
+{
+ struct of_phandle_args irq;
+ cell_t count;
+ int ret;
+ int i;
+
+ ret = schema_get_param_cell(params, "count", &count);
+ if (ret < 0) {
+ schema_err(schema, "missing schema argument: 'count'\n");
+ return;
+ }
+
+ if (!required
+ && !get_property(node, "interrupts")
+ && !get_property(node, "interrupts-extended"))
+ return;
+
+ for (i = 0; i < count; ++i) {
+ ret = of_irq_parse_one(root, node, i, &irq);
+ if (ret < 0)
+ pr_err("failed to parse interrupt entry %d of node '%s'\n",
+ i, node->fullpath);
+ }
+}
+GENERIC_SCHEMA("interrupts", interrupts);
+
+static void generic_checkfn_interrupt_controller(
+ const struct generic_schema *schema,
+ struct node *root,
+ struct node *node,
+ struct node *params,
+ bool required)
+{
+ struct property *prop;
+ cell_t cells, val;
+ int ret;
+
+ ret = schema_get_param_cell(params, "cells", &cells);
+ if (ret < 0) {
+ schema_err(schema, "missing schema argument: 'cells'\n");
+ return;
+ }
+
+ prop = require_property(node, "interrupt-controller");
+ if (!prop)
+ return;
+
+ prop = require_property(node, "#interrupt-cells");
+ if (!prop)
+ return;
+
+ val = propval_cell(prop);
+ if (val != cells) {
+ pr_err("wrong value of #interrupt-cells property in node '%s' (expected %u, got %u)\n",
+ node->fullpath, cells, val);
+ return;
+ }
+}
+GENERIC_SCHEMA("interrupt-controller", interrupt_controller);
diff --git a/schemas/mmio-bus.c b/schemas/mmio-bus.c
new file mode 100644
index 0000000..bd7888d
--- /dev/null
+++ b/schemas/mmio-bus.c
@@ -0,0 +1,97 @@
+#include "schema.h"
+
+static unsigned int get_address_cells(struct node *node)
+{
+ struct property *prop;
+
+ prop = get_property(node->parent, "#address-cells");
+ if (!prop) {
+ pr_warn("missing #address-cells property, assuming 2\n");
+ return 2;
+ }
+
+ return propval_cell(prop);
+}
+
+static unsigned int get_size_cells(struct node *node)
+{
+ struct property *prop;
+
+ prop = get_property(node->parent, "#size-cells");
+ if (!prop) {
+ pr_warn("missing #size-cells property, assuming 1\n");
+ return 1;
+ }
+
+ return propval_cell(prop);
+}
+
+static void generic_checkfn_mmio_device(const struct generic_schema *schema,
+ struct node *root, struct node *node,
+ struct node *params, bool required)
+{
+ struct property *prop;
+ cell_t count;
+ unsigned int address_cells, size_cells;
+
+ if (!params) {
+ pr_err("schema mmio-device requires arguments\n");
+ return;
+ }
+
+ prop = get_property(params, "reg-count");
+ if (!prop) {
+ pr_err("schema mmio-device requires reg-count argument\n");
+ return;
+ }
+
+ count = propval_cell(prop);
+ if (!count) {
+ pr_err("wrong number of reg entries requested\n");
+ return;
+ }
+
+ if (!node->parent) {
+ pr_err("root node can not be an mmio-device\n");
+ return;
+ }
+
+ address_cells = get_address_cells(node);
+ size_cells = get_size_cells(node);
+
+ prop = require_property(node, "reg");
+ if (!prop)
+ return;
+
+ if (prop->val.len % (address_cells + size_cells))
+ pr_err("malformed reg property - not a multiple of (#address-cells + #size-cells)\n");
+
+ if (prop->val.len < count * (address_cells + size_cells))
+ pr_err("not enough entries in reg property - expected %u\n", count);
+}
+GENERIC_SCHEMA("mmio-device", mmio_device);
+
+static void generic_checkfn_mmio_bus(const struct generic_schema *schema,
+ struct node *root, struct node *node,
+ struct node *params, bool required)
+{
+ struct property *prop;
+ cell_t val;
+
+ prop = require_property(node, "#address-cells");
+ if (!prop)
+ return;
+
+ val = propval_cell(prop);
+ if (val < 1)
+ pr_err("mmio-bus requires positive #address-cells value\n");
+
+ prop = require_property(node, "#size-cells");
+ if (!prop)
+ return;
+
+ val = propval_cell(prop);
+ if (val < 1)
+ pr_err("mmio-bus requires positive #size-cells value\n");
+}
+GENERIC_SCHEMA("mmio-bus", mmio_bus);
--
1.8.5.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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 RFC 3/5] Implement DT schema checker using hybrid approach
From: Tomasz Figa @ 2014-02-20 18:06 UTC (permalink / raw)
To: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Stephen Warren,
Marek Szyprowski, grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
bcousson-rdvid1DuHRBWk0Htik3J/w, olof-nZhT3qVonbNeoWH0uzbU5w,
galak-sgV2jX0FEOL9JmXXK+q4OQ, mark.rutland-5wv7dgnIgG8,
a.hajda-Sze3O3UU22JBDgjK7y7TUQ, pawel.moll-5wv7dgnIgG8,
david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+, jdl-CYoMK+44s/E,
Arnd Bergmann, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
Tomasz Figa
In-Reply-To: <1392919611-10746-1-git-send-email-t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
This patch adds a proof of concept framework to implement schema checker
using a combined C and DTSS based approach.
Complex and generic bindings can be implemented directly in C and then
instantiated from simple device-specific bindings using DTS-like DTSS
language.
This is based on Stephen Warren's C based DT schema checker proof of
concept patch.
[original C based DT schema checker proof of concept]
Signed-off-by: Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
[further development into hybrid solution]
Signed-off-by: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
Makefile | 2 +-
Makefile.dtc | 5 +-
checks.c | 15 +++
dtc.c | 17 ++-
dtc.h | 26 +++++
dtss-lexer.l | 291 +++++++++++++++++++++++++++++++++++++++++++++++
dtss-parser.y | 341 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
schemas/schema.c | 311 ++++++++++++++++++++++++++++++++++++++++++++++++++
schemas/schema.h | 89 +++++++++++++++
srcpos.h | 2 +
treesource.c | 22 ++++
11 files changed, 1117 insertions(+), 4 deletions(-)
create mode 100644 dtss-lexer.l
create mode 100644 dtss-parser.y
create mode 100644 schemas/schema.c
create mode 100644 schemas/schema.h
diff --git a/Makefile b/Makefile
index 86f5ab3..0625fb8 100644
--- a/Makefile
+++ b/Makefile
@@ -15,7 +15,7 @@ EXTRAVERSION =
LOCAL_VERSION =
CONFIG_LOCALVERSION =
-CPPFLAGS = -I libfdt -I .
+CPPFLAGS = -I libfdt -I . -I schemas
WARNINGS = -Werror -Wall -Wpointer-arith -Wcast-qual -Wnested-externs \
-Wstrict-prototypes -Wmissing-prototypes -Wredundant-decls -Wshadow
CFLAGS = -g -Os -fPIC -Werror $(WARNINGS)
diff --git a/Makefile.dtc b/Makefile.dtc
index bece49b..bf19564 100644
--- a/Makefile.dtc
+++ b/Makefile.dtc
@@ -12,7 +12,8 @@ DTC_SRCS = \
livetree.c \
srcpos.c \
treesource.c \
- util.c
+ util.c \
+ schemas/schema.c
-DTC_GEN_SRCS = dtc-lexer.lex.c dtc-parser.tab.c
+DTC_GEN_SRCS = dtc-lexer.lex.c dtc-parser.tab.c dtss-lexer.lex.c dtss-parser.tab.c
DTC_OBJS = $(DTC_SRCS:%.c=%.o) $(DTC_GEN_SRCS:%.c=%.o)
diff --git a/checks.c b/checks.c
index 47eda65..7c85fcf 100644
--- a/checks.c
+++ b/checks.c
@@ -19,6 +19,7 @@
*/
#include "dtc.h"
+#include "schemas/schema.h"
#ifdef TRACE_CHECKS
#define TRACE(c, ...) \
@@ -651,6 +652,18 @@ static void check_obsolete_chosen_interrupt_controller(struct check *c,
}
TREE_WARNING(obsolete_chosen_interrupt_controller, NULL);
+/*
+ * Schema checks
+ */
+
+static void check_schema(struct check *c, struct node *dt,
+ struct node *node)
+{
+ if (schema_check_node(dt, node))
+ FAIL(c, "Schema check failed for %s", node->fullpath);
+}
+NODE_ERROR(schema, NULL);
+
static struct check *check_table[] = {
&duplicate_node_names, &duplicate_property_names,
&node_name_chars, &node_name_format, &property_name_chars,
@@ -669,6 +682,8 @@ static struct check *check_table[] = {
&avoid_default_addr_size,
&obsolete_chosen_interrupt_controller,
+ &schema,
+
&always_fail,
};
diff --git a/dtc.c b/dtc.c
index d36ccdc..1a5913b 100644
--- a/dtc.c
+++ b/dtc.c
@@ -20,6 +20,7 @@
#include "dtc.h"
#include "srcpos.h"
+#include "schemas/schema.h"
/*
* Command line options
@@ -49,7 +50,7 @@ static void fill_fullpaths(struct node *tree, const char *prefix)
/* Usage related data. */
static const char usage_synopsis[] = "dtc [options] <input file>";
-static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:hv";
+static const char usage_short_opts[] = "qI:O:o:V:d:R:S:p:fb:i:H:sW:E:x:hv";
static struct option const usage_long_opts[] = {
{"quiet", no_argument, NULL, 'q'},
{"in-format", a_argument, NULL, 'I'},
@@ -67,6 +68,7 @@ static struct option const usage_long_opts[] = {
{"phandle", a_argument, NULL, 'H'},
{"warning", a_argument, NULL, 'W'},
{"error", a_argument, NULL, 'E'},
+ {"schema", a_argument, NULL, 'x'},
{"help", no_argument, NULL, 'h'},
{"version", no_argument, NULL, 'v'},
{NULL, no_argument, NULL, 0x0},
@@ -97,6 +99,7 @@ static const char * const usage_opts_help[] = {
"\t\tboth - Both \"linux,phandle\" and \"phandle\" properties",
"\n\tEnable/disable warnings (prefix with \"no-\")",
"\n\tEnable/disable errors (prefix with \"no-\")",
+ "\n\tUse schema file"
"\n\tPrint this help and exit",
"\n\tPrint version and exit",
NULL,
@@ -105,10 +108,12 @@ static const char * const usage_opts_help[] = {
int main(int argc, char *argv[])
{
struct boot_info *bi;
+ struct boot_info *bi_schema;
const char *inform = "dts";
const char *outform = "dts";
const char *outname = "-";
const char *depname = NULL;
+ const char *schema = NULL;
bool force = false, sort = false;
const char *arg;
int opt;
@@ -185,6 +190,10 @@ int main(int argc, char *argv[])
parse_checks_option(false, true, optarg);
break;
+ case 'x':
+ schema = optarg;
+ break;
+
case 'h':
usage(NULL);
default:
@@ -220,6 +229,12 @@ int main(int argc, char *argv[])
else
die("Unknown input format \"%s\"\n", inform);
+ if (schema) {
+ bi_schema = schema_from_source(schema);
+ //dt_to_source(stdout, bi_schema);
+ build_schema_list(bi_schema);
+ }
+
if (depfile) {
fputc('\n', depfile);
fclose(depfile);
diff --git a/dtc.h b/dtc.h
index 9ce9d12..19d2d24 100644
--- a/dtc.h
+++ b/dtc.h
@@ -135,21 +135,43 @@ struct label {
struct label *next;
};
+enum {
+ PROPERTY_DATA,
+ PROPERTY_USE,
+ PROPERTY_REQUIRE,
+ PROPERTY_MATCH,
+};
+
+#define PROPERTY_FLAG_OPTIONAL (1 << 0)
+
struct property {
bool deleted;
char *name;
struct data val;
+ unsigned int type;
+ unsigned int flags;
struct property *next;
struct label *labels;
};
+enum {
+ NODE_DATA,
+ NODE_USE,
+ NODE_REQUIRE,
+};
+
+#define NODE_FLAG_OPTIONAL (1 << 0)
+#define NODE_FLAG_INCOMPLETE (1 << 1)
+
struct node {
bool deleted;
char *name;
struct property *proplist;
struct node *children;
+ int type;
+ unsigned int flags;
struct node *parent;
struct node *next_sibling;
@@ -297,4 +319,8 @@ struct boot_info *dt_from_source(const char *f);
struct boot_info *dt_from_fs(const char *dirname);
+/* Schema source */
+
+struct boot_info *schema_from_source(const char *fname);
+
#endif /* _DTC_H */
diff --git a/dtss-lexer.l b/dtss-lexer.l
new file mode 100644
index 0000000..aee41f6
--- /dev/null
+++ b/dtss-lexer.l
@@ -0,0 +1,291 @@
+/*
+ * (C) Copyright David Gibson <dwg-8fk3Idey6ehBDgjK7y7TUQ@public.gmane.org>, IBM Corporation. 2005.
+ *
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This program 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
+ * USA
+ */
+
+%option noyywrap nounput noinput never-interactive prefix="dtss_yy"
+
+%x INCLUDE
+%x BYTESTRING
+%x PROPNODENAME
+%s V1
+
+PROPNODECHAR [a-zA-Z0-9,._+*$#?^@-]
+PATHCHAR ({PROPNODECHAR}|[/])
+LABEL [a-zA-Z_][a-zA-Z0-9_]*
+STRING \"([^\\"]|\\.)*\"
+CHAR_LITERAL '([^']|\\')*'
+WS [[:space:]]
+COMMENT "/*"([^*]|\*+[^*/])*\*+"/"
+LINECOMMENT "//".*\n
+
+%{
+#include "dtc.h"
+#include "srcpos.h"
+
+#define DTSS_YYLTYPE struct srcpos
+
+#include "dtss-parser.tab.h"
+
+DTSS_YYLTYPE dtss_yylloc;
+extern bool treesource_error;
+
+/* CAUTION: this will stop working if we ever use dtss_yyless() or dtss_yyunput() */
+#define YY_USER_ACTION \
+ { \
+ srcpos_update(&dtss_yylloc, dtss_yytext, dtss_yyleng); \
+ }
+
+/* #define LEXDEBUG 1 */
+
+#ifdef LEXDEBUG
+#define DPRINT(fmt, ...) fprintf(stderr, fmt, ##__VA_ARGS__)
+#else
+#define DPRINT(fmt, ...) do { } while (0)
+#endif
+
+static int dts_version = 1;
+
+#define BEGIN_DEFAULT() DPRINT("<V1>\n"); \
+ BEGIN(V1); \
+
+static void push_input_file(const char *filename);
+static bool pop_input_file(void);
+static void lexical_error(const char *fmt, ...);
+%}
+
+%%
+<*>"/include/"{WS}*{STRING} {
+ char *name = strchr(dtss_yytext, '\"') + 1;
+ dtss_yytext[dtss_yyleng-1] = '\0';
+ push_input_file(name);
+ }
+
+<*>^"#"(line)?[ \t]+[0-9]+[ \t]+{STRING}([ \t]+[0-9]+)? {
+ char *line, *tmp, *fn;
+ /* skip text before line # */
+ line = dtss_yytext;
+ while (!isdigit(*line))
+ line++;
+ /* skip digits in line # */
+ tmp = line;
+ while (!isspace(*tmp))
+ tmp++;
+ /* "NULL"-terminate line # */
+ *tmp = '\0';
+ /* start of filename */
+ fn = strchr(tmp + 1, '"') + 1;
+ /* strip trailing " from filename */
+ tmp = strchr(fn, '"');
+ *tmp = 0;
+ /* -1 since #line is the number of the next line */
+ srcpos_set_line(xstrdup(fn), atoi(line) - 1);
+ }
+
+<*><<EOF>> {
+ if (!pop_input_file()) {
+ yyterminate();
+ }
+ }
+
+<*>{STRING} {
+ DPRINT("String: %s\n", dtss_yytext);
+ dtss_yylval.data = data_copy_escape_string(dtss_yytext+1,
+ dtss_yyleng-2);
+ return DT_STRING;
+ }
+
+<*>"/dtss-v1/" {
+ DPRINT("Keyword: /dtss-v1/\n");
+ dts_version = 0x8001;
+ BEGIN_DEFAULT();
+ return DTSS_V1;
+ }
+
+<*>"/bits/" {
+ DPRINT("Keyword: /bits/\n");
+ BEGIN_DEFAULT();
+ return DT_BITS;
+ }
+
+<*>"/delete-property/" {
+ DPRINT("Keyword: /delete-property/\n");
+ DPRINT("<PROPNODENAME>\n");
+ BEGIN(PROPNODENAME);
+ return DT_DEL_PROP;
+ }
+
+<*>"/delete-node/" {
+ DPRINT("Keyword: /delete-node/\n");
+ DPRINT("<PROPNODENAME>\n");
+ BEGIN(PROPNODENAME);
+ return DT_DEL_NODE;
+ }
+
+<*>"/match/" {
+ DPRINT("Keyword: /match/\n");
+ return DTSS_MATCH;
+ }
+
+<*>"/require/" {
+ DPRINT("Keyword: /require/\n");
+ return DTSS_REQUIRE;
+ }
+
+<*>"/use/" {
+ DPRINT("Keyword: /use/\n");
+ return DTSS_USE;
+ }
+
+<*>"/incomplete/" {
+ DPRINT("Keyword: /incomplete/\n");
+ return DTSS_INCOMPLETE;
+ }
+
+<*>"/optional/" {
+ DPRINT("Keyword: /optional/\n");
+ return DTSS_OPTIONAL;
+ }
+
+<V1>([0-9]+|0[xX][0-9a-fA-F]+)(U|L|UL|LL|ULL)? {
+ char *e;
+ DPRINT("Integer Literal: '%s'\n", yytext);
+
+ errno = 0;
+ dtss_yylval.integer = strtoull(yytext, &e, 0);
+
+ assert(!(*e) || !e[strspn(e, "UL")]);
+
+ if (errno == ERANGE)
+ lexical_error("Integer literal '%s' out of range",
+ yytext);
+ else
+ /* ERANGE is the only strtoull error triggerable
+ * by strings matching the pattern */
+ assert(errno == 0);
+ return DT_LITERAL;
+ }
+
+<*>{CHAR_LITERAL} {
+ struct data d;
+ DPRINT("Character literal: %s\n", yytext);
+
+ d = data_copy_escape_string(yytext+1, yyleng-2);
+ if (d.len == 1) {
+ lexical_error("Empty character literal");
+ dtss_yylval.integer = 0;
+ return DT_CHAR_LITERAL;
+ }
+
+ dtss_yylval.integer = (unsigned char)d.val[0];
+
+ if (d.len > 2)
+ lexical_error("Character literal has %d"
+ " characters instead of 1",
+ d.len - 1);
+
+ return DT_CHAR_LITERAL;
+ }
+
+<*>\&{LABEL} { /* label reference */
+ DPRINT("Ref: %s\n", dtss_yytext+1);
+ dtss_yylval.labelref = xstrdup(dtss_yytext+1);
+ return DT_REF;
+ }
+
+<*>"&{/"{PATHCHAR}*\} { /* new-style path reference */
+ dtss_yytext[dtss_yyleng-1] = '\0';
+ DPRINT("Ref: %s\n", dtss_yytext+2);
+ dtss_yylval.labelref = xstrdup(dtss_yytext+2);
+ return DT_REF;
+ }
+
+<BYTESTRING>[0-9a-fA-F]{2} {
+ dtss_yylval.byte = strtol(dtss_yytext, NULL, 16);
+ DPRINT("Byte: %02x\n", (int)dtss_yylval.byte);
+ return DT_BYTE;
+ }
+
+<BYTESTRING>"]" {
+ DPRINT("/BYTESTRING\n");
+ BEGIN_DEFAULT();
+ return ']';
+ }
+
+<PROPNODENAME>\\?{PROPNODECHAR}+ {
+ DPRINT("PropNodeName: %s\n", dtss_yytext);
+ dtss_yylval.propnodename = xstrdup((dtss_yytext[0] == '\\') ?
+ dtss_yytext + 1 : dtss_yytext);
+ BEGIN_DEFAULT();
+ return DT_PROPNODENAME;
+ }
+
+<*>{WS}+ /* eat whitespace */
+<*>{COMMENT}+ /* eat C-style comments */
+<*>{LINECOMMENT}+ /* eat C++-style comments */
+
+<*>. {
+ DPRINT("Char: %c (\\x%02x)\n", dtss_yytext[0],
+ (unsigned)dtss_yytext[0]);
+ if (dtss_yytext[0] == '[') {
+ DPRINT("<BYTESTRING>\n");
+ BEGIN(BYTESTRING);
+ }
+ if ((dtss_yytext[0] == '{')
+ || (dtss_yytext[0] == ';')) {
+ DPRINT("<PROPNODENAME>\n");
+ BEGIN(PROPNODENAME);
+ }
+ return dtss_yytext[0];
+ }
+
+%%
+
+static void push_input_file(const char *filename)
+{
+ assert(filename);
+
+ srcfile_push(filename);
+
+ dtss_yyin = current_srcfile->f;
+
+ dtss_yypush_buffer_state(dtss_yy_create_buffer(dtss_yyin, YY_BUF_SIZE));
+}
+
+
+static bool pop_input_file(void)
+{
+ if (srcfile_pop() == 0)
+ return false;
+
+ dtss_yypop_buffer_state();
+ dtss_yyin = current_srcfile->f;
+
+ return true;
+}
+
+static void lexical_error(const char *fmt, ...)
+{
+ va_list ap;
+
+ va_start(ap, fmt);
+ srcpos_verror(&dtss_yylloc, "Lexical error", fmt, ap);
+ va_end(ap);
+
+ treesource_error = true;
+}
diff --git a/dtss-parser.y b/dtss-parser.y
new file mode 100644
index 0000000..1c807da
--- /dev/null
+++ b/dtss-parser.y
@@ -0,0 +1,341 @@
+/*
+ * (C) Copyright David Gibson <dwg-8fk3Idey6ehBDgjK7y7TUQ@public.gmane.org>, IBM Corporation. 2005.
+ *
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This program 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
+ * USA
+ */
+
+%define api.prefix dtss_yy
+
+%{
+#include <stdio.h>
+
+#include "dtc.h"
+#include "srcpos.h"
+
+#define DTSS_YYLTYPE struct srcpos
+
+extern int dtss_yylex(void);
+extern void dtss_yyerror(char const *s);
+#define ERROR(loc, ...) \
+ do { \
+ srcpos_error((loc), "Error", __VA_ARGS__); \
+ treesource_error = true; \
+ } while (0)
+
+extern struct boot_info *the_boot_info;
+extern bool treesource_error;
+%}
+
+%union {
+ char *propnodename;
+ char *labelref;
+ unsigned int cbase;
+ uint8_t byte;
+ struct data data;
+
+ struct {
+ struct data data;
+ int bits;
+ } array;
+
+ struct property *prop;
+ struct property *proplist;
+ struct node *node;
+ struct node *nodelist;
+ struct reserve_info *re;
+ uint64_t integer;
+}
+
+%token DTSS_V1
+%token DT_BITS
+%token DT_DEL_PROP
+%token DT_DEL_NODE
+%token DTSS_INCOMPLETE
+%token DTSS_MATCH
+%token DTSS_USE
+%token DTSS_REQUIRE
+%token DTSS_OPTIONAL
+%token <propnodename> DT_PROPNODENAME
+%token <integer> DT_LITERAL
+%token <integer> DT_CHAR_LITERAL
+%token <cbase> DT_BASE
+%token <byte> DT_BYTE
+%token <data> DT_STRING
+%token <data> DTSS_TYPESPEC
+%token <labelref> DT_LABEL
+%token <labelref> DT_REF
+%token DT_INCBIN
+
+%type <data> propdata
+%type <data> propdataprefix
+%type <array> arrayprefix
+%type <data> bytestring
+%type <prop> propdef
+%type <proplist> proplist
+
+%type <node> schema
+%type <node> schemaroot
+%type <node> nodedef
+%type <node> subnode
+%type <nodelist> subnodes
+
+%type <integer> integer_prim
+
+%%
+
+sourcefile:
+ DTSS_V1 ';' schema
+ {
+ the_boot_info = build_boot_info(NULL, $3, 0);
+ }
+ ;
+
+schemaroot:
+ subnodes
+ {
+ $$ = build_node(NULL, $1);
+ }
+ ;
+
+schema:
+ schemaroot
+ {
+ $$ = name_node($1, "/");
+ }
+ ;
+
+nodedef:
+ '{' proplist subnodes '}' ';'
+ {
+ $$ = build_node($2, $3);
+ }
+ ;
+
+proplist:
+ /* empty */
+ {
+ $$ = NULL;
+ }
+ | proplist propdef
+ {
+ $$ = chain_property($2, $1);
+ }
+ ;
+
+propdef:
+ DT_PROPNODENAME '=' propdata ';'
+ {
+ $$ = build_property($1, $3);
+ }
+ | DT_PROPNODENAME ';'
+ {
+ $$ = build_property($1, empty_data);
+ }
+ | DT_DEL_PROP DT_PROPNODENAME ';'
+ {
+ $$ = build_property_delete($2);
+ }
+ | DTSS_MATCH propdef
+ {
+ $2->type = PROPERTY_MATCH;
+ $$ = $2;
+ }
+ | DTSS_USE propdef
+ {
+ $2->type = PROPERTY_USE;
+ $$ = $2;
+ }
+ | DTSS_REQUIRE propdef
+ {
+ $2->type = PROPERTY_REQUIRE;
+ $$ = $2;
+ }
+ | DTSS_OPTIONAL propdef
+ {
+ $2->flags |= PROPERTY_FLAG_OPTIONAL;
+ $$ = $2;
+ }
+ ;
+
+propdata:
+ propdataprefix DT_STRING
+ {
+ $$ = data_merge($1, $2);
+ }
+ | propdataprefix arrayprefix '>'
+ {
+ $$ = data_merge($1, $2.data);
+ }
+ | propdataprefix '[' bytestring ']'
+ {
+ $$ = data_merge($1, $3);
+ }
+ | propdataprefix DT_INCBIN '(' DT_STRING ',' integer_prim ',' integer_prim ')'
+ {
+ FILE *f = srcfile_relative_open($4.val, NULL);
+ struct data d;
+
+ if ($6 != 0)
+ if (fseek(f, $6, SEEK_SET) != 0)
+ die("Couldn't seek to offset %llu in \"%s\": %s",
+ (unsigned long long)$6, $4.val,
+ strerror(errno));
+
+ d = data_copy_file(f, $8);
+
+ $$ = data_merge($1, d);
+ fclose(f);
+ }
+ | propdataprefix DT_INCBIN '(' DT_STRING ')'
+ {
+ FILE *f = srcfile_relative_open($4.val, NULL);
+ struct data d = empty_data;
+
+ d = data_copy_file(f, -1);
+
+ $$ = data_merge($1, d);
+ fclose(f);
+ }
+ ;
+
+propdataprefix:
+ /* empty */
+ {
+ $$ = empty_data;
+ }
+ | propdata ','
+ {
+ $$ = $1;
+ }
+ | propdataprefix DT_LABEL
+ {
+ $$ = data_add_marker($1, LABEL, $2);
+ }
+ ;
+
+arrayprefix:
+ DT_BITS DT_LITERAL '<'
+ {
+ unsigned long long bits;
+
+ bits = $2;
+
+ if ((bits != 8) && (bits != 16) &&
+ (bits != 32) && (bits != 64)) {
+ ERROR(&@2, "Array elements must be"
+ " 8, 16, 32 or 64-bits");
+ bits = 32;
+ }
+
+ $$.data = empty_data;
+ $$.bits = bits;
+ }
+ | '<'
+ {
+ $$.data = empty_data;
+ $$.bits = 32;
+ }
+ | arrayprefix integer_prim
+ {
+ if ($1.bits < 64) {
+ uint64_t mask = (1ULL << $1.bits) - 1;
+ /*
+ * Bits above mask must either be all zero
+ * (positive within range of mask) or all one
+ * (negative and sign-extended). The second
+ * condition is true if when we set all bits
+ * within the mask to one (i.e. | in the
+ * mask), all bits are one.
+ */
+ if (($2 > mask) && (($2 | mask) != -1ULL))
+ ERROR(&@2, "Value out of range for"
+ " %d-bit array element", $1.bits);
+ }
+
+ $$.data = data_append_integer($1.data, $2, $1.bits);
+ }
+ ;
+
+integer_prim:
+ DT_LITERAL
+ | DT_CHAR_LITERAL
+ ;
+
+bytestring:
+ /* empty */
+ {
+ $$ = empty_data;
+ }
+ | bytestring DT_BYTE
+ {
+ $$ = data_append_byte($1, $2);
+ }
+ ;
+
+subnodes:
+ /* empty */
+ {
+ $$ = NULL;
+ }
+ | subnode subnodes
+ {
+ $$ = chain_node($1, $2);
+ }
+ | subnode propdef
+ {
+ ERROR(&@2, "Properties must precede subnodes");
+ YYERROR;
+ }
+ ;
+
+subnode:
+ DT_PROPNODENAME nodedef
+ {
+ $$ = name_node($2, $1);
+ }
+ | DT_DEL_NODE DT_PROPNODENAME ';'
+ {
+ $$ = name_node(build_node_delete(), $2);
+ }
+ | DTSS_USE subnode
+ {
+ $2->type = NODE_USE;
+ $$ = $2;
+ }
+ | DTSS_REQUIRE subnode
+ {
+ $2->type = NODE_REQUIRE;
+ $$ = $2;
+ }
+ | DTSS_OPTIONAL subnode
+ {
+ $2->flags |= NODE_FLAG_OPTIONAL;
+ $$ = $2;
+ }
+ | DTSS_INCOMPLETE subnode
+ {
+ $2->flags |= NODE_FLAG_INCOMPLETE;
+ $$ = $2;
+ }
+ ;
+
+%%
+
+void dtss_yyerror(char const *s)
+{
+ ERROR(&yylloc, "%s", s);
+}
diff --git a/schemas/schema.c b/schemas/schema.c
new file mode 100644
index 0000000..e5258cf
--- /dev/null
+++ b/schemas/schema.c
@@ -0,0 +1,311 @@
+/*
+ * Copyright (C) 2013 Stephen Warren <swarren-3lzwWm7+Weoh9ZMKESR00Q@public.gmane.org>
+ *
+ * Copyright (C) 2014 Samsung Electronics Co., Ltd.
+ * Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License as
+ * published by the Free Software Foundation; either version 2 of the
+ * License, or (at your option) any later version.
+ *
+ * This program 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.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307
+ * USA
+ */
+
+#include "schema.h"
+
+static struct schema_checker schema_list = {
+ .next = &schema_list,
+};
+
+int schema_check_node(struct node *root, struct node *node)
+{
+ const struct schema_checker *checker;
+ int match;
+ int checked = 0;
+
+ checker = schema_list.next;
+ for (; checker != &schema_list; checker = checker->next) {
+ match = checker->matchfn(node, checker);
+ if (!match)
+ continue;
+
+ pr_info("Node %s matches checker %s at level %d\n",
+ node->fullpath, checker->name, match);
+
+ checker->checkfn(root, node, checker);
+ checked = 1;
+ }
+
+ /*
+ * FIXME: this is too noisy right now. Make it optional until schemas
+ * for most bindings are implemented.
+ */
+ if (!checked) {
+ pr_warn("no schema for node %s\n", node->fullpath);
+ return 0;
+ }
+
+ /*
+ * FIXME: grab validation state from global somewhere.
+ * Using global state avoids having check return values after every
+ * function call, thus making the code less verbose and appear more
+ * assertion-based.
+ */
+ return 0;
+}
+
+int schema_match_path(struct node *node, const struct schema_checker *checker)
+{
+ return !strcmp(node->fullpath, checker->u.path.path);
+}
+
+int schema_match_compatible(struct node *node,
+ const struct schema_checker *checker)
+{
+ struct property *compat_prop;
+ int index;
+ const char *node_compat;
+ const char **test_compats;
+
+ compat_prop = get_property(node, "compatible");
+ if (!compat_prop)
+ return 0;
+
+ /*
+ * Match with any compatible value of schema with any compatible
+ * value of node being verified.
+ */
+ for (node_compat = compat_prop->val.val, index = 0;
+ *node_compat;
+ node_compat += strlen(node_compat) + 1, index++) {
+ for (test_compats = checker->u.compatible.compats;
+ *test_compats; test_compats++) {
+ if (!strcmp(node_compat, *test_compats))
+ return 1;
+ }
+ }
+
+ return 0;
+}
+
+struct property *schema_get_param(struct node *params, const char *name)
+{
+ if (!params)
+ return NULL;
+
+ return get_property(params, name);
+}
+
+int schema_get_param_cell(struct node *params, const char *name, cell_t *val)
+{
+ struct property *prop;
+
+ prop = schema_get_param(params, name);
+ if (!prop)
+ return -ENOENT;
+
+ if (!prop->val.len)
+ return -EINVAL;
+
+ *val = propval_cell(prop);
+ return 0;
+}
+
+struct property *require_property(struct node *node, const char *name)
+{
+ struct property *prop;
+
+ prop = get_property(node, name);
+ if (!prop) {
+ /*
+ * FIXME: set global error state. The same comment applies
+ * everywhere.
+ */
+ pr_err("node '%s' missing '%s' property\n",
+ node->fullpath, name);
+ }
+
+ return prop;
+}
+
+static void check_required_property(struct node *node, struct property *schema)
+{
+ struct property *prop;
+
+ prop = require_property(node, schema->name);
+ if (!prop)
+ return;
+
+ if (schema->val.len
+ && (schema->val.len != prop->val.len
+ || memcmp(schema->val.val, prop->val.val, prop->val.len)))
+ pr_err("node %s with wrong constant value of property %s\n",
+ node->fullpath, schema->name);
+}
+
+static void check_optional_property(struct node *node, struct property *schema)
+{
+ struct property *prop;
+
+ prop = get_property(node, schema->name);
+ if (!prop)
+ return;
+
+ check_required_property(node, schema);
+}
+
+/*
+ * FIXME: Use a more generic solution, which does not rely on linker
+ * specific features.
+ */
+extern const struct generic_schema __start_generic_schemas;
+extern const struct generic_schema __stop_generic_schemas;
+
+static void check_generic_schema(struct node *root, struct node *node,
+ const char *name,
+ struct node *schema_node,
+ bool required)
+{
+ const struct generic_schema *gs;
+ int i;
+ bool checked = false;
+ unsigned int count = &__stop_generic_schemas - &__start_generic_schemas;
+
+ pr_info("running schema \"%s\"\n", name);
+
+ gs = &__start_generic_schemas;
+ for (i = 0; i < count; ++i, ++gs) {
+ if (strcmp(gs->name, name))
+ continue;
+
+ gs->checkfn(gs, root, node, schema_node, required);
+
+ checked = true;
+ }
+
+ if (!checked)
+ pr_err("schema \"%s\" not found\n", name);
+}
+
+static void check_dtss_schema(struct node *root, struct node *node,
+ const struct schema_checker *checker)
+{
+ struct property *prop_schema;
+ struct node *schema = checker->node, *node_schema;
+
+ for_each_property(schema, prop_schema) {
+ if (!strcmp(prop_schema->name, "compatible")
+ || !strcmp(prop_schema->name, "device_type"))
+ continue;
+
+ switch (prop_schema->type) {
+ case PROPERTY_DATA:
+ if (prop_schema->flags & PROPERTY_FLAG_OPTIONAL)
+ check_optional_property(node, prop_schema);
+ else
+ check_required_property(node, prop_schema);
+ break;
+
+ case PROPERTY_REQUIRE:
+ check_generic_schema(root, node, prop_schema->name,
+ NULL, true);
+ break;
+
+ case PROPERTY_USE:
+ check_generic_schema(root, node, prop_schema->name,
+ NULL, false);
+ break;
+ }
+ }
+
+ for_each_child(schema, node_schema) {
+ switch (node_schema->type) {
+ case NODE_DATA:
+ /* TODO: verify subnodes */
+ break;
+
+ case NODE_REQUIRE:
+ check_generic_schema(root, node, node_schema->name,
+ node_schema, true);
+ break;
+
+ case NODE_USE:
+ check_generic_schema(root, node, node_schema->name,
+ node_schema, false);
+ break;
+ }
+ }
+
+ /* TODO: detect unknown properties */
+}
+
+void build_schema_list(struct boot_info *schema_tree)
+{
+ struct node *root, *schema;
+
+ root = get_node_by_path(schema_tree->dt, "/");
+ if (!root) {
+ pr_err("schema file missing / node\n");
+ return;
+ }
+
+ for_each_child(root, schema) {
+ struct schema_checker *sc = xmalloc(sizeof(*sc));
+ struct property *prop;
+
+ sc->node = schema;
+ sc->checkfn = check_dtss_schema;
+ sc->name = schema->name;
+
+ for_each_property(schema, prop)
+ if (prop->type == PROPERTY_MATCH)
+ goto found_match;
+
+ pr_err("schema '%s' without matching key\n", sc->name);
+ free(sc);
+ continue;
+
+found_match:
+ if (!strcmp(prop->name, "compatible")) {
+ int count;
+ const char **compats;
+ const char *compat;
+
+ count = propval_string_count(schema, prop) + 1;
+ compats = xmalloc(count * sizeof(*compats));
+
+ sc->u.compatible.compats = compats;
+
+ while ((compat = propval_next_string(prop, compat))) {
+ *compats = compat;
+ ++compats;
+ }
+ *compats = NULL;
+
+ sc->matchfn = schema_match_compatible;
+ sc->next = schema_list.next;
+ schema_list.next = sc;
+ } else if (!strcmp(prop->name, "device_type")) {
+ sc->u.type.type = propval_next_string(prop, NULL);
+ sc->next = schema_list.next;
+ schema_list.next = sc;
+ } else if (!strcmp(prop->name, "path")) {
+ sc->u.path.path = propval_next_string(prop, NULL);
+ sc->matchfn = schema_match_path;
+ sc->next = schema_list.next;
+ schema_list.next = sc;
+ } else {
+ pr_err("wrong schema key type\n");
+ free(sc);
+ }
+ }
+}
diff --git a/schemas/schema.h b/schemas/schema.h
new file mode 100644
index 0000000..9972a17
--- /dev/null
+++ b/schemas/schema.h
@@ -0,0 +1,89 @@
+#ifndef _SCHEMAS_SCHEMA_H
+#define _SCHEMAS_SCHEMA_H
+
+#include "dtc.h"
+
+struct schema_checker;
+
+typedef int (schema_matcher_func)(struct node *node,
+ const struct schema_checker *checker);
+typedef void (schema_checker_func)(struct node *root, struct node *node,
+ const struct schema_checker *checker);
+
+struct schema_checker {
+ const char *name;
+ schema_matcher_func *matchfn;
+ schema_checker_func *checkfn;
+ struct node *node;
+ union {
+ struct {
+ const char *path;
+ } path;
+ struct {
+ const char **compats;
+ } compatible;
+ struct {
+ const char *type;
+ } type;
+ } u;
+ struct schema_checker *next;
+};
+
+int schema_check_node(struct node *root, struct node *node);
+
+int schema_match_path(struct node *node, const struct schema_checker *checker);
+int schema_match_compatible(struct node *node,
+ const struct schema_checker *checker);
+
+#define SCHEMA_MATCH_PATH(_name_, _path_) \
+ struct schema_checker schema_checker_##_name_ = { \
+ .name = #_name_, \
+ .matchfn = schema_match_path, \
+ .checkfn = checkfn_##_name_, \
+ .u.path.path = _path_, \
+ };
+
+#define SCHEMA_MATCH_COMPATIBLE(_name_) \
+ struct schema_checker schema_checker_##_name_ = { \
+ .name = #_name_, \
+ .matchfn = schema_match_compatible, \
+ .checkfn = checkfn_##_name_, \
+ .u.compatible.compats = compats_##_name_, \
+ };
+
+struct generic_schema;
+
+typedef void (generic_schema_checker_func)(const struct generic_schema *schema,
+ struct node *root, struct node *node,
+ struct node *params, bool required);
+
+struct generic_schema {
+ const char *name;
+ generic_schema_checker_func *checkfn;
+};
+
+#define __used __attribute__((__used__))
+#define __section(S) __attribute__ ((__section__(#S)))
+
+#define GENERIC_SCHEMA(_dt_name_, _name_) \
+ static const struct generic_schema generic_schema_##_name_ \
+ __used __section(generic_schemas) = { \
+ .name = _dt_name_, \
+ .checkfn = generic_checkfn_##_name_, \
+ };
+
+struct property *require_property(struct node *node, const char *propname);
+struct property *schema_get_param(struct node *params, const char *name);
+int schema_get_param_cell(struct node *params, const char *name, cell_t *val);
+
+void build_schema_list(struct boot_info *schema_tree);
+
+#define schema_err(s,fmt,args...) pr_err("%s: " fmt, (s)->name, ##args)
+#define schema_warn(s,fmt,args...) pr_warn("%s: " fmt, (s)->name, ##args)
+#define schema_info(s,fmt,args...) pr_info("%s: " fmt, (s)->name, ##args)
+
+#define node_err(n,fmt,args...) pr_err("%s: " fmt, (n)->fullpath, ##args)
+#define node_warn(n,fmt,args...) pr_warn("%s: " fmt, (n)->fullpath, ##args)
+#define node_info(n,fmt,args...) pr_info("%s: " fmt, (n)->fullpath, ##args)
+
+#endif
diff --git a/srcpos.h b/srcpos.h
index f81827b..b761522 100644
--- a/srcpos.h
+++ b/srcpos.h
@@ -75,7 +75,9 @@ struct srcpos {
struct srcfile_state *file;
};
+#ifndef YYLTYPE
#define YYLTYPE struct srcpos
+#endif
#define YYLLOC_DEFAULT(Current, Rhs, N) \
do { \
diff --git a/treesource.c b/treesource.c
index bf7a626..e50285c 100644
--- a/treesource.c
+++ b/treesource.c
@@ -25,6 +25,10 @@ extern FILE *yyin;
extern int yyparse(void);
extern YYLTYPE yylloc;
+extern FILE *dtss_yyin;
+extern int dtss_yyparse(void);
+extern YYLTYPE dtss_yylloc;
+
struct boot_info *the_boot_info;
bool treesource_error;
@@ -46,6 +50,24 @@ struct boot_info *dt_from_source(const char *fname)
return the_boot_info;
}
+struct boot_info *schema_from_source(const char *fname)
+{
+ the_boot_info = NULL;
+ treesource_error = false;
+
+ srcfile_push(fname);
+ dtss_yyin = current_srcfile->f;
+ dtss_yylloc.file = current_srcfile;
+
+ if (dtss_yyparse() != 0)
+ die("Unable to parse input tree\n");
+
+ if (treesource_error)
+ die("Syntax error parsing input tree\n");
+
+ return the_boot_info;
+}
+
static void write_prefix(FILE *f, int level)
{
int i;
--
1.8.5.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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 RFC 2/5] dtc: livetree: Add more tree parsing helpers
From: Tomasz Figa @ 2014-02-20 18:06 UTC (permalink / raw)
To: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Stephen Warren,
Marek Szyprowski, grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
bcousson-rdvid1DuHRBWk0Htik3J/w, olof-nZhT3qVonbNeoWH0uzbU5w,
galak-sgV2jX0FEOL9JmXXK+q4OQ, mark.rutland-5wv7dgnIgG8,
a.hajda-Sze3O3UU22JBDgjK7y7TUQ, pawel.moll-5wv7dgnIgG8,
david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+, jdl-CYoMK+44s/E,
Arnd Bergmann, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
Tomasz Figa
In-Reply-To: <1392919611-10746-1-git-send-email-t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
This patch extends the set of parsing helpers available in dtc with
string and phandle parsing.
Signed-off-by: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
dtc.h | 28 ++++++++
livetree.c | 230 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
2 files changed, 258 insertions(+)
diff --git a/dtc.h b/dtc.h
index e95bed7..9ce9d12 100644
--- a/dtc.h
+++ b/dtc.h
@@ -163,6 +163,13 @@ struct node {
struct label *labels;
};
+#define MAX_PHANDLE_ARGS 16
+struct of_phandle_args {
+ struct node *np;
+ int args_count;
+ uint32_t args[MAX_PHANDLE_ARGS];
+};
+
#define for_each_label_withdel(l0, l) \
for ((l) = (l0); (l); (l) = (l)->next)
@@ -184,6 +191,11 @@ struct node {
for_each_child_withdel(n, c) \
if (!(c)->deleted)
+#define for_each_propval_string(p, s) \
+ for (s = propval_next_string(p, NULL); \
+ s; \
+ s = propval_next_string(p, s))
+
void add_label(struct label **labels, char *label);
void delete_labels(struct label **labels);
@@ -208,6 +220,22 @@ void delete_node(struct node *node);
const char *get_unitname(struct node *node);
struct property *get_property(struct node *node, const char *propname);
cell_t propval_cell(struct property *prop);
+int propval_string_count(struct node *np, struct property *prop);
+const char *propval_next_string(struct property *prop, const char *cur);
+int propval_match_string(struct property *prop, const char *string);
+struct node *propval_parse_phandle(struct node *root,
+ struct property *prop, int index);
+int propval_parse_phandle_with_args(struct node *root,
+ struct property *prop,
+ const char *cells_name, int index,
+ struct of_phandle_args *out_args);
+int propval_parse_phandle_with_fixed_args(struct node *root,
+ struct property *prop, int cell_count,
+ int index,
+ struct of_phandle_args *out_args);
+int propval_count_phandle_with_args(struct node *root,
+ struct property *prop,
+ const char *cells_name);
struct property *get_property_by_label(struct node *tree, const char *label,
struct node **node);
struct marker *get_marker_label(struct node *tree, const char *label,
diff --git a/livetree.c b/livetree.c
index b61465f..015ed06 100644
--- a/livetree.c
+++ b/livetree.c
@@ -377,6 +377,230 @@ cell_t propval_cell(struct property *prop)
return fdt32_to_cpu(*((cell_t *)prop->val.val));
}
+int propval_string_count(struct node *np, struct property *prop)
+{
+ int i = 0;
+ size_t l = 0, total = 0;
+ const char *p;
+
+ if (!prop)
+ return -EINVAL;
+ if (!prop->val.val)
+ return -ENODATA;
+ if (strnlen(prop->val.val, prop->val.len) >= prop->val.len)
+ return -EILSEQ;
+
+ p = prop->val.val;
+
+ for (i = 0; total < prop->val.len; total += l, p += l, i++)
+ l = strlen(p) + 1;
+
+ return i;
+}
+
+const char *propval_next_string(struct property *prop, const char *cur)
+{
+ const char *curv = cur;
+
+ if (!prop)
+ return NULL;
+
+ if (!cur)
+ return prop->val.val;
+
+ curv += strlen(cur) + 1;
+ if (curv >= prop->val.val + prop->val.len)
+ return NULL;
+
+ return curv;
+}
+
+int propval_match_string(struct property *prop, const char *string)
+{
+ size_t l;
+ int i;
+ const char *p, *end;
+
+ if (!prop)
+ return -EINVAL;
+ if (!prop->val.val)
+ return -ENODATA;
+
+ p = prop->val.val;
+ end = p + prop->val.len;
+
+ for (i = 0; p < end; i++, p += l) {
+ l = strlen(p) + 1;
+ if (p + l > end)
+ return -EILSEQ;
+ if (strcmp(string, p) == 0)
+ return i; /* Found it; return index */
+ }
+ return -ENODATA;
+}
+
+static int parse_phandle_with_args(struct node *root,
+ struct property *prop,
+ const char *cells_name,
+ int cell_count, int index,
+ struct of_phandle_args *out_args)
+{
+ const cell_t *list, *list_end;
+ int rc = 0, size, cur_index = 0;
+ uint32_t count = 0;
+ struct node *node = NULL;
+ cell_t phandle;
+
+ /* Retrieve the phandle list property */
+ size = prop->val.len;
+ list = (const cell_t *)prop->val.val;
+ if (!list)
+ return -ENOENT;
+ list_end = list + size / sizeof(*list);
+
+ /* Loop over the phandles until all the requested entry is found */
+ while (list < list_end) {
+ rc = -EINVAL;
+ count = 0;
+
+ /*
+ * If phandle is 0, then it is an empty entry with no
+ * arguments. Skip forward to the next entry.
+ */
+ phandle = fdt32_to_cpu(*list++);
+ if (phandle) {
+ /*
+ * Find the provider node and parse the #*-cells
+ * property to determine the argument length.
+ *
+ * This is not needed if the cell count is hard-coded
+ * (i.e. cells_name not set, but cell_count is set),
+ * except when we're going to return the found node
+ * below.
+ */
+ if (cells_name || cur_index == index) {
+ node = get_node_by_phandle(root, phandle);
+ if (!node) {
+ pr_err("could not find phandle %u\n",
+ phandle);
+ goto err;
+ }
+ }
+
+ if (cells_name) {
+ struct property *cells_prop;
+
+ cells_prop = get_property(node, cells_name);
+ if (!cells_prop) {
+ pr_err("could not get %s for %s\n",
+ cells_name,
+ node->fullpath);
+ goto err;
+ }
+
+ count = propval_cell(cells_prop);
+ } else {
+ count = cell_count;
+ }
+
+ /*
+ * Make sure that the arguments actually fit in the
+ * remaining property data length
+ */
+ if (list + count > list_end) {
+ pr_err("arguments longer than property\n");
+ goto err;
+ }
+ }
+
+ /*
+ * All of the error cases above bail out of the loop, so at
+ * this point, the parsing is successful. If the requested
+ * index matches, then fill the out_args structure and return,
+ * or return -ENOENT for an empty entry.
+ */
+ rc = -ENOENT;
+ if (cur_index == index) {
+ if (!phandle)
+ goto err;
+
+ if (out_args) {
+ int i;
+ if (count > MAX_PHANDLE_ARGS) {
+ pr_warn("argument count higher than MAX_PHANDLE_ARGS\n");
+ count = MAX_PHANDLE_ARGS;
+ }
+ out_args->np = node;
+ out_args->args_count = count;
+ for (i = 0; i < count; i++)
+ out_args->args[i] = fdt32_to_cpu(*list++);
+ }
+
+ /* Found it! return success */
+ return 0;
+ }
+
+ node = NULL;
+ list += count;
+ cur_index++;
+ }
+
+ /*
+ * Unlock node before returning result; will be one of:
+ * -ENOENT : index is for empty phandle
+ * -EINVAL : parsing error on data
+ * [1..n] : Number of phandle (count mode; when index = -1)
+ */
+ rc = index < 0 ? cur_index : -ENOENT;
+ err:
+ return rc;
+}
+
+struct node *propval_parse_phandle(struct node *root,
+ struct property *prop, int index)
+{
+ struct of_phandle_args args;
+
+ if (index < 0)
+ return NULL;
+
+ if (parse_phandle_with_args(root, prop, NULL, 0,
+ index, &args))
+ return NULL;
+
+ return args.np;
+}
+
+int propval_parse_phandle_with_args(struct node *root,
+ struct property *prop,
+ const char *cells_name, int index,
+ struct of_phandle_args *out_args)
+{
+ if (index < 0)
+ return -EINVAL;
+ return parse_phandle_with_args(root, prop, cells_name, 0,
+ index, out_args);
+}
+
+int propval_parse_phandle_with_fixed_args(struct node *root,
+ struct property *prop, int cell_count,
+ int index,
+ struct of_phandle_args *out_args)
+{
+ if (index < 0)
+ return -EINVAL;
+ return parse_phandle_with_args(root, prop, NULL, cell_count,
+ index, out_args);
+}
+
+int propval_count_phandle_with_args(struct node *root,
+ struct property *prop,
+ const char *cells_name)
+{
+ return parse_phandle_with_args(root, prop, cells_name, 0, -1,
+ NULL);
+}
+
struct property *get_property_by_label(struct node *tree, const char *label,
struct node **node)
{
@@ -456,6 +680,12 @@ struct node *get_node_by_path(struct node *tree, const char *path)
while (path[0] == '/')
path++;
+ if (!(*path)) {
+ if (tree->deleted)
+ return NULL;
+ return tree;
+ }
+
p = strchr(path, '/');
for_each_child(tree, child) {
--
1.8.5.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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 RFC 1/5] dtc: Add helpers for various message levels
From: Tomasz Figa @ 2014-02-20 18:06 UTC (permalink / raw)
To: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Stephen Warren,
Marek Szyprowski, grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
bcousson-rdvid1DuHRBWk0Htik3J/w, olof-nZhT3qVonbNeoWH0uzbU5w,
galak-sgV2jX0FEOL9JmXXK+q4OQ, mark.rutland-5wv7dgnIgG8,
a.hajda-Sze3O3UU22JBDgjK7y7TUQ, pawel.moll-5wv7dgnIgG8,
david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+, jdl-CYoMK+44s/E,
Arnd Bergmann, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
Tomasz Figa
In-Reply-To: <1392919611-10746-1-git-send-email-t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
This patch adds three helper macros to print errors, warnings and
informational messages using standard format.
Signed-off-by: Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
---
dtc.h | 3 +++
1 file changed, 3 insertions(+)
diff --git a/dtc.h b/dtc.h
index 20de073..e95bed7 100644
--- a/dtc.h
+++ b/dtc.h
@@ -43,6 +43,9 @@
#define debug(fmt,args...)
#endif
+#define pr_err(...) fprintf (stderr, "ERROR: " __VA_ARGS__)
+#define pr_warn(...) fprintf (stderr, "WARNING: " __VA_ARGS__)
+#define pr_info(...) fprintf (stderr, "INFO: " __VA_ARGS__)
#define DEFAULT_FDT_VERSION 17
--
1.8.5.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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 RFC 0/5] Hybrid approach for DT schema checking
From: Tomasz Figa @ 2014-02-20 18:06 UTC (permalink / raw)
To: devicetree-compiler-u79uwXL29TY76Z2rM5mHXA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r, Stephen Warren,
Marek Szyprowski, grant.likely-s3s/WqlpOiPyB63q8FvJNQ,
bcousson-rdvid1DuHRBWk0Htik3J/w, olof-nZhT3qVonbNeoWH0uzbU5w,
galak-sgV2jX0FEOL9JmXXK+q4OQ, mark.rutland-5wv7dgnIgG8,
a.hajda-Sze3O3UU22JBDgjK7y7TUQ, pawel.moll-5wv7dgnIgG8,
david-xT8FGy+AXnRB3Ne2BGzF6laj5H9X9Tb+, jdl-CYoMK+44s/E,
Arnd Bergmann, jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/,
Tomasz Figa
This series adds a proof of concept framework to implement schema checker
using a combined C and DTSS based approach. Several example bindings are
also implemented using C and DTSS[1].
Complex and generic bindings can be implemented directly in C and then
instantiated from simple device-specific bindings using DTS-like DTSS
language.
A quick description of C part:
A new check is registered in dtc checks framework to perform schema checking
of each node. Checking is done by searching specified schema set for matching
schemas (by compatible, device_type or absolute path) and applying matched
schemas to the node.
Schemas for complex generic bindings (such as interrupts, gpios, clocks, i2c,
mmio-bus, etc.) can be implemented directly in C and instantiated from DTSS
schemas of particular devices. An example C schema may look like:
static void generic_checkfn_xxx_yyy(const struct generic_schema *schema,
struct node *root, struct node *node,
struct node *params, bool required)
{
/*
* Get necessary schema arguments from DTSS schema by looking
* at properties of @params node.
*
* Check whether @node node matches the schema.
*
* The @required argument may be used to check whether some error
* conditions should be ignored (e.g. unspecified interrupt).
*/
}
GENERIC_SCHEMA("xxx-yyy", xxx_yyy);
A quick description of DTSS part:
* DTSS is a DTS-like language for specification of simple bindings, e.g.
bindings of particular devices. The basic syntax is very similar to DTS,
with main elements being nodes and properties. At root level a series of
nodes should be specified representing particular bindings:
/dtss-v1/;
binding1 {
/* Definition of binding 1 */
};
binding2 {
/* Definition of binding 2 */
};
* Matching key for each binding can be specified using /match/ keyword:
root-node {
/match/ path = "/";
};
wlf,wm8903 {
/match/ compatible = "wlf,wm8903";
};
pci-bus {
/match/ device_type = "memory";
};
Currently supported matches: path, compatible, device_type.
* Bindings can be specified either by listing properties they require
(or can use) directly or by instantiating generic C-based bindings.
binding {
required-property;
/optional/ optional-property;
/require/ required-generic-schema;
/use/ optional-generic-schema {
schema-argument = <1>;
};
};
Generic schemas are implemented in C, as described above, and can use
arguments specified in DTSS as properties. /require/ calls the schema with
required=true, while /use/ with required=false.
This is based on Stephen Warren's C based DT schema checker proof of
concept patch adding C-based validation[2].
TODO:
- specification of subnodes directly from DTSS,
- specification of simple property values from DTSS (cells, strings,
phandles),
- reporting of unrecognized properties,
- probably many more...
[1] Device Tree Schema Source
[2] http://thread.gmane.org/gmane.linux.ports.arm.kernel/275896
Tomasz Figa (5):
dtc: Add helpers for various message levels
dtc: livetree: Add more tree parsing helpers
Another try of DT schema checker using hybrid C and DTSS based
approach
Add sample C-based generic bindings
Add sample DTS and DTSS schema
Makefile | 2 +-
Makefile.dtc | 10 +-
checks.c | 15 +
dtc.c | 17 +-
dtc.h | 57 ++++
dtss-lexer.l | 291 +++++++++++++++++++
dtss-parser.y | 341 ++++++++++++++++++++++
livetree.c | 230 +++++++++++++++
sample.dts | 70 +++++
schema.dtss | 86 ++++++
schemas/clock/clock.c | 77 +++++
schemas/gpio/gpio.c | 93 ++++++
schemas/i2c/i2c.c | 42 +++
schemas/interrupt-controller/interrupts.c | 452 ++++++++++++++++++++++++++++++
schemas/mmio-bus.c | 97 +++++++
schemas/schema.c | 311 ++++++++++++++++++++
schemas/schema.h | 89 ++++++
srcpos.h | 2 +
treesource.c | 22 ++
19 files changed, 2300 insertions(+), 4 deletions(-)
create mode 100644 dtss-lexer.l
create mode 100644 dtss-parser.y
create mode 100644 sample.dts
create mode 100644 schema.dtss
create mode 100644 schemas/clock/clock.c
create mode 100644 schemas/gpio/gpio.c
create mode 100644 schemas/i2c/i2c.c
create mode 100644 schemas/interrupt-controller/interrupts.c
create mode 100644 schemas/mmio-bus.c
create mode 100644 schemas/schema.c
create mode 100644 schemas/schema.h
--
1.8.5.2
--
To unsubscribe from this list: send the line "unsubscribe devicetree-compiler" 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/1] ARM: Exynos: Add generic compatible string
From: Arnd Bergmann @ 2014-02-20 17:48 UTC (permalink / raw)
To: linux-arm-kernel
Cc: Tomasz Figa, Mark Rutland, devicetree@vger.kernel.org, Kukjin Kim,
Ian Campbell, Sachin Kamat, Rob Herring, Grant Likely,
linux-samsung-soc, Olof Johansson
In-Reply-To: <53063C9F.4060307@samsung.com>
On Thursday 20 February 2014 18:34:23 Tomasz Figa wrote:
> On 20.02.2014 18:00, Arnd Bergmann wrote:
> >> Of course nothing stops you from retaining more specific compatible
> >> strings. In fact, this is probably the most appropriate solution,
> >> because in future you might find out that certain SoCs need some special
> >> differentiation, e.g. same ID value on two SoCs.
> >>
> >> So, to apply this to our case, our Exynos 5250 based Arndale board would
> >> be changed from
> >>
> >> compatible = "insignal,arndale", "samsung,exynos5260";
> >>
> >> to
> >>
> >> compatible = "insignal,arndale", "samsung,exynos5260", "samsung,exynos";
> >
> > Right, this would make sense.
> >
> >> Now, the board file will be able to match simply by "samsung,exynos"
> >> compatible string and SoC-specific code in mach-exynos (hopefully none
> >> after it gets cleaned up fully) will use soc_is_exynos*() macros (what
> >> AFAIK it is already doing at the moment).
> >
> > On principle, I would not take things out of the match list, if that
> > would break booting with old DT file that don't list "samsung,exynos"
> > as compatible. But for new SoCs that would work.
>
> My proposal was about simply adding a fully generic string, without
> removing the specific ones. For already supported SoCs this is pretty
> obvious, as existing DTBs would not have this generic string listed. But
> the specific strings should be also present in DTSes of new SoCs, even
> if not recognized by the kernel, to make sure that in future any
> SoC-specific quirks could be easily handled.
Yes, that's ideal.
> > Using soc_is_exynos*() too much of course also isn't good. A lot of
> > the things tested for should probably be checked from individual DT
> > properties, but again we have to find the right balance. I wouldn't
> > mind getting rid of the soc_is_exynos*() macros completely, because
> > a) we can't use them in device drivers
> > b) all platform code is supposed to be in drivers
> > c) both rules are enforced for arm64
>
> I fully agree. As I said, after cleaning up mach-exynos/ there should be
> no more code left using soc_is_*() macros. Ideally, the whole
> mach-exynos/ should collapse into a single, trivial file, with
> everything done in dedicated drivers.
Right.
Arnd
^ permalink raw reply
* Re: [PATCH 3/3] iio: adc: fsl,imx25-gcq driver
From: Lars-Peter Clausen @ 2014-02-20 17:46 UTC (permalink / raw)
To: Markus Pargmann
Cc: devicetree, Samuel Ortiz, linux-iio, Dmitry Torokhov,
linux-arm-kernel, kernel, linux-input, Lee Jones,
Jonathan Cameron
In-Reply-To: <1392913312-9030-4-git-send-email-mpa@pengutronix.de>
On 02/20/2014 05:21 PM, Markus Pargmann wrote:
> This is a conversion queue driver for the mx25 SoC. It uses the central
> ADC which is used by two seperate independent queues. This driver
> prepares different conversion configurations for each possible input.
> For a conversion it creates a conversionqueue of one item with the
> correct configuration for the chosen channel. It then executes the queue
> once and disables the conversion queue afterwards.
>
> The reference voltages are configurable through devicetree subnodes,
> depending on the connections of the ADC inputs.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
Looks good mostly. Just a couple of minor code-style issues. Try to run
checkpatch, sparse and friends on your driver before submission. They catch
most of the stuff that needs to be fixed in this driver.
[..]
> diff --git a/drivers/iio/adc/Kconfig b/drivers/iio/adc/Kconfig
> index 2209f28..a421445 100644
> --- a/drivers/iio/adc/Kconfig
> +++ b/drivers/iio/adc/Kconfig
> @@ -113,6 +113,13 @@ config EXYNOS_ADC
> of SoCs for drivers such as the touchscreen and hwmon to use to share
> this resource.
>
> +config MX25_ADC
> + tristate "Freescale MX25 ADC driver"
> + depends on MFD_MX25_TSADC
> + help
> + Generic Conversion Queue driver used for general purpose ADC in the
> + MX25. This driver supports single measurements using the MX25 ADC.
> +
alphabetical order
> config LP8788_ADC
> bool "LP8788 ADC driver"
> depends on MFD_LP8788
> diff --git a/drivers/iio/adc/Makefile b/drivers/iio/adc/Makefile
> index ba9a10a..63daa2c 100644
> --- a/drivers/iio/adc/Makefile
> +++ b/drivers/iio/adc/Makefile
> @@ -22,3 +22,4 @@ obj-$(CONFIG_TI_ADC081C) += ti-adc081c.o
> obj-$(CONFIG_TI_AM335X_ADC) += ti_am335x_adc.o
> obj-$(CONFIG_TWL6030_GPADC) += twl6030-gpadc.o
> obj-$(CONFIG_VIPERBOARD_ADC) += viperboard_adc.o
> +obj-$(CONFIG_MX25_ADC) += fsl-imx25-gcq.o
Here too
[...]
> +const struct iio_chan_spec mx25_gcq_channels[MX25_NUM_CFGS] = {
static
> + MX25_IIO_CHAN(0, "xp"),
> + MX25_IIO_CHAN(1, "yp"),
> + MX25_IIO_CHAN(2, "xn"),
> + MX25_IIO_CHAN(3, "yn"),
> + MX25_IIO_CHAN(4, "wiper"),
> + MX25_IIO_CHAN(5, "inaux0"),
> + MX25_IIO_CHAN(6, "inaux1"),
> + MX25_IIO_CHAN(7, "inaux2"),
> +};
> +
> +struct iio_info mx25_gcq_iio_info = {
static const
> + .read_raw = mx25_gcq_read_raw,
> +};
> +
> +static struct regmap_config mx25_gcq_regconfig = {
const
> + .fast_io = true,
You don't need to set fast_io since this is already done at the regmap_bus
level.
> + .max_register = 0x5c,
> + .reg_bits = 32,
> + .val_bits = 32,
> + .reg_stride = 4,
> +};
> +
> +static void mx25_gcq_iio_dev_setup(struct platform_device *pdev,
> + struct iio_dev *idev)
> +{
> + idev->dev.parent = &pdev->dev;
> + idev->channels = mx25_gcq_channels;
> + idev->num_channels = ARRAY_SIZE(mx25_gcq_channels);
> + idev->info = &mx25_gcq_iio_info;
Any reason why this needs to be in a separate function and can't be inlined
in probe()?
> +}
> +
> +static int mx25_gcq_setup_cfgs(struct platform_device *pdev,
> + struct mx25_gcq_priv *priv)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct device_node *child;
> + struct device *dev = &pdev->dev;
> + int ret;
> + int i;
> +
> + /* Setup all configurations registers with a default conversion
> + * configuration for each input */
> + for (i = 0; i != MX25_NUM_CFGS; ++i)
> + regmap_write(priv->regs, MX25_ADCQ_CFG(i),
> + MX25_ADCQ_CFG_YPLL_OFF |
> + MX25_ADCQ_CFG_XNUR_OFF |
> + MX25_ADCQ_CFG_XPUL_OFF |
> + MX25_ADCQ_CFG_REFP_INT |
> + (i << 4) |
> + MX25_ADCQ_CFG_REFN_NGND2);
> +
> + for_each_child_of_node(np, child) {
> + u32 reg;
> + u32 refn;
> + u32 refp;
> +
> + ret = of_property_read_u32(child, "reg", ®);
> + if (ret) {
> + dev_err(dev, "Failed to get reg property\n");
> + return ret;
> + }
> + if (reg > MX25_NUM_CFGS) {
> + dev_err(dev, "reg value is greater than the number of available configuration registers\n");
> + return -EINVAL;
> + }
> +
> + ret = of_property_read_u32(child, "fsl,adc-refn", &refn);
> + if (ret) {
> + dev_err(dev, "Failed to get fsl,adc-refn property\n");
> + return ret;
> + }
> +
> + ret = of_property_read_u32(child, "fsl,adc-refp", &refp);
> + if (ret) {
> + dev_err(dev, "Failed to get fsl,adc-refp property\n");
> + return ret;
> + }
Range check for refp and refn?
> +
> + regmap_update_bits(priv->regs, MX25_ADCQ_CFG(reg),
> + MX25_ADCQ_CFG_REFP_MASK |
> + MX25_ADCQ_CFG_REFN_MASK,
> + (refp << 7) | (refn << 2));
> + }
> + regmap_update_bits(priv->regs, MX25_ADCQ_CR,
> + MX25_ADCQ_CR_FRST | MX25_ADCQ_CR_QRST,
> + MX25_ADCQ_CR_FRST | MX25_ADCQ_CR_QRST);
> +
> + regmap_write(priv->regs, MX25_ADCQ_CR,
> + MX25_ADCQ_CR_PDMSK |
> + MX25_ADCQ_CR_QSM_FQS);
> +
> + return 0;
> +}
> +
> +static int mx25_gcq_probe(struct platform_device *pdev)
> +{
> + struct iio_dev *idev;
> + struct mx25_gcq_priv *priv;
> + struct resource *res;
> + struct device *dev = &pdev->dev;
> + int ret;
> + int irq;
> + void __iomem *mem;
> +
> + idev = devm_iio_device_alloc(&pdev->dev, sizeof(*priv));
> + if (!idev)
> + return -ENOMEM;
> +
> + priv = iio_priv(idev);
> +
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + mem = devm_ioremap_resource(dev, res);
> + if (!mem) {
> + dev_err(dev, "Failed to get iomem");
devm_ioremap_resource already prints a error message for you
> + return -ENOMEM;
> + }
> +
> + priv->regs = devm_regmap_init_mmio(dev, mem, &mx25_gcq_regconfig);
> + if (IS_ERR(priv->regs)) {
> + dev_err(dev, "Failed to initialize regmap\n");
> + return PTR_ERR(priv->regs);
> + }
> +
> + irq = platform_get_irq(pdev, 0);
> + if (irq < 0) {
0 is not a valid IRQ number either
> + dev_err(dev, "Failed to get IRQ\n");
> + return irq;
> + }
> +
> + ret = devm_request_irq(dev, irq, mx25_gcq_irq, IRQF_ONESHOT, pdev->name,
> + priv);
IRQF_ONESHOT does not make much sense for non-threaded IRQs. Also it is
probably safer to use the non managed variant of request_irq here.
> + if (ret) {
> + dev_err(dev, "Failed requesting IRQ\n");
It usually makes sense to include the error number in the message. Same for
the other error messages in probe.
> + return ret;
> + }
> +
> + ret = mx25_gcq_setup_cfgs(pdev, priv);
> + if (ret)
> + return ret;
> +
> + mx25_gcq_iio_dev_setup(pdev, idev);
> +
> + ret = iio_device_register(idev);
> + if (ret) {
> + dev_err(dev, "Failed to register iio device\n");
> + return ret;
> + }
> +
> + init_completion(&priv->completed);
Should be done before the interrupt handler is registered. You reference it
in there.
> +
> + priv->clk = mx25_tsadc_get_ipg(pdev->dev.parent);
> + ret = clk_prepare_enable(priv->clk);
The clock should probably also be enabled before the interrupt handler and
the IIO device are registered.
> + if (ret) {
> + dev_err(dev, "Failed to enable clock\n");
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, priv);
> +
> + return 0;
> +}
> +
> +static int mx25_gcq_remove(struct platform_device *pdev)
> +{
> + struct mx25_gcq_priv *priv = platform_get_drvdata(pdev);
> +
iio_device_unregister().
> + clk_disable_unprepare(priv->clk);
> +
> + return 0;
> +}
^ permalink raw reply
* Re: [PATCH 1/1] ARM: Exynos: Add generic compatible string
From: Tomasz Figa @ 2014-02-20 17:34 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Sachin Kamat, linux-samsung-soc, Kukjin Kim,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
Olof Johansson,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Mark Rutland, Grant Likely, Ian Campbell
In-Reply-To: <201402201800.49425.arnd-r2nGTMty4D4@public.gmane.org>
On 20.02.2014 18:00, Arnd Bergmann wrote:
> On Thursday 20 February 2014, Tomasz Figa wrote:
>> On 20.02.2014 05:14, Sachin Kamat wrote:
>>> Hi Tomasz,
>>>
>>> On 19 February 2014 18:15, Tomasz Figa <t.figa-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org> wrote:
>>>> Hi Sachin,
>>>>
>>>> [adding linux-arm-kernel ML to CC list]
>>>>
>>>>
>>>> On 19.02.2014 12:34, Sachin Kamat wrote:
>>>>>
>>>>> To avoid modifying the kernel every time a new SoC variant
>>>>> comes out.
>>> <snip>
>>>>
>>>> Since all Exynos chips can be easily recognized using dedicated chip ID
>>>> register, I wonder whether we really need to maintain two distinct board
>>>> files for Exynos 4 and 5 series, especially when both of them are doing
>>>> mostly the same set up, which can be simply generalized to cover all the
>>>> cases.
>>>>
>>>> Instead of adding just another level of artificially fine grained compatible
>>>> strings, I'd rather suggest merging both board files together and adding a
>>>> single compatible string identifying all SoCs that can be further
>>>> differentiated by using hardware chip ID register.
>>>>
>>>> What do you think?
>>>
>>> I agree with your idea of merging both the files as there is very little that is
>>> different for now. However I am not really sure if having a single compatible
>>> string for all SoCs would be good. What is achieved through compatible string
>>> can very well be done using chip ID too. But wouldn't we need to maintain some
>>> unique identity for the SoCs in human readable form at the DT level?
>>
>> Well, my understanding of Device Tree is that it should provide the
>> information that can't be automatically retrieved from the hardware, not
>> more.
>>
>> If you have a PCI or USB bus with enumerable devices, you don't list
>> them in DT, but instead limit the description to just the host
>> controller, if it can't be enumerated.
>>
>> Same goes for compatible string. My interpretation of it is that if you
>> can identify the hardware by some automatic means, e.g. querying some ID
>> register, then the compatible should be specific enough to identify the
>> class of devices with the same method of querying such register, with no
>> need for any additional redundant data in DT.
>
> There are some limitations to that, and you also have to apply common
> sense and taste. In theory you could argue that a compatible string can
> identify a board uniquely the same way that a board number did, and
> everything else is implied by that. Clearly that's not what we want.
I'd argue about relevance of this example. What I'm talking about is
kind of data that can be retrieved directly from the hardware. Both
compatible string and board number are just artificial identifiers that
can be used by the kernel to decide (in a greatly simplified view) which
set of static (i.e. hardcoded) data (and code) to use.
If it happens that a new piece of hardware requires just exactly the
same set, because any possible differences can be obtained by reading
some registers, then there is no need for the kernel to support any new
compatible string and such hardware will work on older kernels as well.
> On the other end of the scale, you could eliminate a lot of compatible
> strings if you describe each register or each bit in some cases in
> DT properties, and that would be just as wrong.
I don't mind fine grained compatible strings. In fact, making them fine
grained is most likely to be the best choice most of the time. Still,
the simple fact is, though, that data which can be retrieved from the
hardware automatically should not be repeated in DT. This is called
redundancy and this is bad, from obvious reasons.
>> Of course nothing stops you from retaining more specific compatible
>> strings. In fact, this is probably the most appropriate solution,
>> because in future you might find out that certain SoCs need some special
>> differentiation, e.g. same ID value on two SoCs.
>>
>> So, to apply this to our case, our Exynos 5250 based Arndale board would
>> be changed from
>>
>> compatible = "insignal,arndale", "samsung,exynos5260";
>>
>> to
>>
>> compatible = "insignal,arndale", "samsung,exynos5260", "samsung,exynos";
>
> Right, this would make sense.
>
>> Now, the board file will be able to match simply by "samsung,exynos"
>> compatible string and SoC-specific code in mach-exynos (hopefully none
>> after it gets cleaned up fully) will use soc_is_exynos*() macros (what
>> AFAIK it is already doing at the moment).
>
> On principle, I would not take things out of the match list, if that
> would break booting with old DT file that don't list "samsung,exynos"
> as compatible. But for new SoCs that would work.
My proposal was about simply adding a fully generic string, without
removing the specific ones. For already supported SoCs this is pretty
obvious, as existing DTBs would not have this generic string listed. But
the specific strings should be also present in DTSes of new SoCs, even
if not recognized by the kernel, to make sure that in future any
SoC-specific quirks could be easily handled.
> Using soc_is_exynos*() too much of course also isn't good. A lot of
> the things tested for should probably be checked from individual DT
> properties, but again we have to find the right balance. I wouldn't
> mind getting rid of the soc_is_exynos*() macros completely, because
> a) we can't use them in device drivers
> b) all platform code is supposed to be in drivers
> c) both rules are enforced for arm64
I fully agree. As I said, after cleaning up mach-exynos/ there should be
no more code left using soc_is_*() macros. Ideally, the whole
mach-exynos/ should collapse into a single, trivial file, with
everything done in dedicated drivers.
>> Another benefit of this would be increased safety, because you are
>> reading SoC type from actual hardware, not from externally supplied
>> data. In conjunction with the more specific compatible string (e.g.
>> "samsung,exynos5260") some validation could be performed at boot-up time
>> to make sure that DT for correct SoC is used.
>
> I don't think that's necessary. If you get that part wrong, normally
> all your hopes are lost with the rest of the information in DT. ;-)
This would let you detect the problem earlier, though, without trying to
probe incorrect devices and activating unsuitable configurations on
devices that may happen to match on both SoCs. I agree, though, that
this is not anything essential.
Best regards,
Tomasz
--
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/4] ASoC: simple-card: Fix device node locks
From: Jean-Francois Moine @ 2014-02-20 17:25 UTC (permalink / raw)
To: Mark Brown
Cc: alsa-devel, Kuninori Morimoto, linux-kernel, Xiubo Li, devicetree
In-Reply-To: <cover.1392995566.git.moinejf@free.fr>
Some device nodes stay locked and some other ones are not locked
while being used during the card lifetime.
Signed-off-by: Jean-Francois Moine <moinejf@free.fr>
---
sound/soc/generic/simple-card.c | 51 +++++++++++++++++++++++++++++++----------
1 file changed, 39 insertions(+), 12 deletions(-)
diff --git a/sound/soc/generic/simple-card.c b/sound/soc/generic/simple-card.c
index 4fe8abc..8809ab4 100644
--- a/sound/soc/generic/simple-card.c
+++ b/sound/soc/generic/simple-card.c
@@ -92,7 +92,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
/* get dai->name */
ret = snd_soc_of_get_dai_name(np, name);
if (ret < 0)
- goto parse_error;
+ return ret;
/*
* bitclock-inversion, frame-inversion
@@ -112,7 +112,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
clk = of_clk_get(np, 0);
if (IS_ERR(clk)) {
ret = PTR_ERR(clk);
- goto parse_error;
+ return ret;
}
dai->sysclk = clk_get_rate(clk);
@@ -126,12 +126,7 @@ asoc_simple_card_sub_parse_of(struct device_node *np,
dai->sysclk = clk_get_rate(clk);
}
- ret = 0;
-
-parse_error:
- of_node_put(node);
-
- return ret;
+ return 0;
}
static int asoc_simple_card_parse_of(struct device_node *node,
@@ -169,22 +164,26 @@ static int asoc_simple_card_parse_of(struct device_node *node,
/* CPU sub-node */
ret = -EINVAL;
np = of_get_child_by_name(node, "simple-audio-card,cpu");
- if (np)
+ if (np) {
ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
&priv->cpu_dai,
&dai_link->cpu_of_node,
&dai_link->cpu_dai_name);
+ of_node_put(np);
+ }
if (ret < 0)
return ret;
/* CODEC sub-node */
ret = -EINVAL;
np = of_get_child_by_name(node, "simple-audio-card,codec");
- if (np)
+ if (np) {
ret = asoc_simple_card_sub_parse_of(np, priv->daifmt,
&priv->codec_dai,
&dai_link->codec_of_node,
&dai_link->codec_dai_name);
+ of_node_put(np);
+ }
if (ret < 0)
return ret;
@@ -219,6 +218,26 @@ static int asoc_simple_card_parse_of(struct device_node *node,
return 0;
}
+static int asoc_simple_card_purge(struct platform_device *pdev)
+{
+ struct snd_soc_card *card = platform_get_drvdata(pdev);
+ struct snd_soc_dai_link *dai_link;
+ struct device_node *np;
+ int num_links;
+
+ for (num_links = 0, dai_link = card->dai_link;
+ num_links < card->num_links;
+ num_links++, dai_link++) {
+ np = (struct device_node *) dai_link->cpu_of_node;
+ if (np)
+ of_node_put(np);
+ np = (struct device_node *) dai_link->codec_of_node;
+ if (np)
+ of_node_put(np);
+ }
+ return 0;
+}
+
static int asoc_simple_card_probe(struct platform_device *pdev)
{
struct simple_card_data *priv;
@@ -246,7 +265,7 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
if (ret < 0) {
if (ret != -EPROBE_DEFER)
dev_err(dev, "parse error %d\n", ret);
- return ret;
+ goto err;
}
} else {
struct asoc_simple_card_info *cinfo;
@@ -287,7 +306,14 @@ static int asoc_simple_card_probe(struct platform_device *pdev)
snd_soc_card_set_drvdata(&priv->snd_card, priv);
- return devm_snd_soc_register_card(&pdev->dev, &priv->snd_card);
+ ret = devm_snd_soc_register_card(&pdev->dev, &priv->snd_card);
+ if (ret < 0)
+ goto err;
+ return 0;
+
+err:
+ asoc_simple_card_purge(pdev);
+ return ret;
}
static const struct of_device_id asoc_simple_of_match[] = {
@@ -303,6 +329,7 @@ static struct platform_driver asoc_simple_card = {
.of_match_table = asoc_simple_of_match,
},
.probe = asoc_simple_card_probe,
+ .remove = asoc_simple_card_purge,
};
module_platform_driver(asoc_simple_card);
--
1.9.0
^ permalink raw reply related
* [PATCH v6 1/4] ASoC: tlv320aic32x4: Support for master clock
From: Markus Pargmann @ 2014-02-20 17:22 UTC (permalink / raw)
To: Mark Brown
Cc: Liam Girdwood, Lars-Peter Clausen,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, kernel-bIcnvbaLZ9MEGnE8C9+IrQ,
Markus Pargmann, Rob Herring, Pawel Moll, Mark Rutland,
Ian Campbell, Kumar Gala, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1392916981-9333-1-git-send-email-mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
Add support for a master clock passed through DT. The master clock of
the codec is only active when the codec is in use.
Cc: Rob Herring <robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: Pawel Moll <pawel.moll-5wv7dgnIgG8@public.gmane.org>
Cc: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Cc: Ian Campbell <ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>
Cc: Kumar Gala <galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Signed-off-by: Markus Pargmann <mpa-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
---
.../devicetree/bindings/sound/tlv320aic32x4.txt | 4 ++++
sound/soc/codecs/tlv320aic32x4.c | 21 +++++++++++++++++++++
2 files changed, 25 insertions(+)
diff --git a/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt b/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt
index db05510..352be7b 100644
--- a/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt
+++ b/Documentation/devicetree/bindings/sound/tlv320aic32x4.txt
@@ -8,6 +8,8 @@ Required properties:
Optional properties:
- reset-gpios: Reset-GPIO phandle with args as described in gpio/gpio.txt
+ - clocks/clock-names: Clock named 'mclk' for the master clock of the codec.
+ See clock/clock-bindings.txt for information about the detailed format.
Example:
@@ -15,4 +17,6 @@ Example:
codec: tlv320aic32x4@18 {
compatible = "ti,tlv320aic32x4";
reg = <0x18>;
+ clocks = <&clks 201>;
+ clock-names = "mclk";
};
diff --git a/sound/soc/codecs/tlv320aic32x4.c b/sound/soc/codecs/tlv320aic32x4.c
index 1dd50e4..643fa53 100644
--- a/sound/soc/codecs/tlv320aic32x4.c
+++ b/sound/soc/codecs/tlv320aic32x4.c
@@ -33,6 +33,7 @@
#include <linux/i2c.h>
#include <linux/cdev.h>
#include <linux/slab.h>
+#include <linux/clk.h>
#include <sound/tlv320aic32x4.h>
#include <sound/core.h>
@@ -67,6 +68,7 @@ struct aic32x4_priv {
u32 micpga_routing;
bool swapdacs;
int rstn_gpio;
+ struct clk *mclk;
};
/* 0dB min, 0.5dB steps */
@@ -487,8 +489,18 @@ static int aic32x4_mute(struct snd_soc_dai *dai, int mute)
static int aic32x4_set_bias_level(struct snd_soc_codec *codec,
enum snd_soc_bias_level level)
{
+ struct aic32x4_priv *aic32x4 = snd_soc_codec_get_drvdata(codec);
+ int ret;
+
switch (level) {
case SND_SOC_BIAS_ON:
+ /* Switch on master clock */
+ ret = clk_prepare_enable(aic32x4->mclk);
+ if (ret) {
+ dev_err(codec->dev, "Failed to enable master clock\n");
+ return ret;
+ }
+
/* Switch on PLL */
snd_soc_update_bits(codec, AIC32X4_PLLPR,
AIC32X4_PLLEN, AIC32X4_PLLEN);
@@ -539,6 +551,9 @@ static int aic32x4_set_bias_level(struct snd_soc_codec *codec,
/* Switch off BCLK_N Divider */
snd_soc_update_bits(codec, AIC32X4_BCLKN,
AIC32X4_BCLKEN, 0);
+
+ /* Switch off master clock */
+ clk_disable_unprepare(aic32x4->mclk);
break;
case SND_SOC_BIAS_OFF:
break;
@@ -717,6 +732,12 @@ static int aic32x4_i2c_probe(struct i2c_client *i2c,
aic32x4->rstn_gpio = -1;
}
+ aic32x4->mclk = devm_clk_get(&i2c->dev, "mclk");
+ if (IS_ERR(aic32x4->mclk)) {
+ dev_err(&i2c->dev, "Failed getting the mclk. The current implementation does not support the usage of this codec without mclk\n");
+ return PTR_ERR(aic32x4->mclk);
+ }
+
if (gpio_is_valid(aic32x4->rstn_gpio)) {
ret = devm_gpio_request_one(&i2c->dev, aic32x4->rstn_gpio,
GPIOF_OUT_INIT_LOW, "tlv320aic32x4 rstn");
--
1.8.5.3
--
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 1/3] mfd: fsl imx25 Touchscreen ADC driver
From: Fabio Estevam @ 2014-02-20 17:17 UTC (permalink / raw)
To: Markus Pargmann
Cc: devicetree@vger.kernel.org, linux-input, linux-iio,
Dmitry Torokhov, Samuel Ortiz, Lee Jones, Jonathan Cameron,
linux-arm-kernel@lists.infradead.org, Sascha Hauer
In-Reply-To: <1392913312-9030-2-git-send-email-mpa@pengutronix.de>
Hi Markus,
On Thu, Feb 20, 2014 at 1:21 PM, Markus Pargmann <mpa@pengutronix.de> wrote:
> This is the core driver for imx25 touchscreen/adc driver. The module
> has one shared ADC and two different conversion queues which use the
> ADC. The two queues are identical. Both can be used for general purpose
> ADC but one is meant to be used for touchscreens.
>
> This driver is the core which manages the central components and
> registers of the TSC/ADC unit. It manages the IRQs and forwards them to
> the correct components.
>
> Signed-off-by: Markus Pargmann <mpa@pengutronix.de>
That's great :-) Nice work!
> ---
> .../devicetree/bindings/mfd/fsl-imx25-tsadc.txt | 46 ++++
> drivers/mfd/Kconfig | 9 +
> drivers/mfd/Makefile | 2 +
> drivers/mfd/fsl-imx25-tsadc.c | 234 +++++++++++++++++++++
> include/linux/mfd/imx25-tsadc.h | 138 ++++++++++++
> 5 files changed, 429 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt
> create mode 100644 drivers/mfd/fsl-imx25-tsadc.c
> create mode 100644 include/linux/mfd/imx25-tsadc.h
>
> diff --git a/Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt b/Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt
> new file mode 100644
> index 0000000..a857af0e
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mfd/fsl-imx25-tsadc.txt
> @@ -0,0 +1,46 @@
> +Freescale mx25 ADC/TSC multifunction device
> +
> +This device combines two general purpose conversion queues one used for general
> +ADC and the other used for touchscreens.
> +
> +Required properties:
> + - compatible: Should be "fsl,imx25-tsadc".
> + - reg: Memory range of the device.
> + - interrupts: Interrupt for this device as described in
> + interrupts/interrupts.txt
> + - clocks: An 'ipg' clock defined as described in clocks/clock.txt
> + - interrupt-controller: This device is an interrupt controller. It controls
> + the interrupts of both conversion queues.
> + - #interrupt-cells: Should be '<1>'.
> + - #address-cells: Should be '<1>'.
> + - #size-cells: Should be '<1>'.
> + - ranges
> +
> +This device includes two conversion queues which can be added as subnodes.
> +The first queue is for the touchscreen, the second for general purpose ADC.
> +
> +Example:
> + tscadc: tscadc@50030000 {
> + compatible = "fsl,imx25-tsadc";
> + reg = <0x50030000 0xc>;
> + interrupts = <46>;
> + clocks = <&clks 119>;
> + clock-names = "ipg";
> + interrupt-controller;
> + #interrupt-cells = <1>;
> + #address-cells = <1>;
> + #size-cells = <1>;
> + ranges;
> +
> + tsc: tcq@50030400 {
> + compatible = "fsl,imx25-tcq";
> + reg = <0x50030400 0x60>;
> + ...
> + };
> +
> + adc: gcq@50030800 {
> + compatible = "fsl,imx25-gcq";
> + reg = <0x50030800 0x60>;
> + ...
> + };
> + };
The meaning of 'tcq' and 'gcq' acronyms are not obvious. Could they be
written more explicitily?
Also, what does the '...' mean?
> +static int mx25_tsadc_domain_map(struct irq_domain *d, unsigned int irq,
> + irq_hw_number_t hwirq)
> +{
> + struct mx25_tsadc_priv *priv = d->host_data;
> +
> + irq_set_chip_data(irq, priv);
> + irq_set_chip_and_handler(irq, &mx25_tsadc_irq_chip,
> + handle_level_irq);
> +
> +#ifdef CONFIG_ARM
> + set_irq_flags(irq, IRQF_VALID);
> +#else
> + irq_set_noprobe(irq);
> +#endif
Do we really need these ifdef's? Can't we just assume that CONFIG_ARM
is always selected for this driver?
> +static int mx25_tsadc_probe(struct platform_device *pdev)
> +{
> + struct device *dev = &pdev->dev;
> + struct device_node *np = dev->of_node;
> + struct mx25_tsadc_priv *priv;
> + struct resource *iores;
> + int ret;
> + void __iomem *iomem;
> +
> + priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
> + if (!priv)
> + return -ENOMEM;
> +
> + iores = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + iomem = devm_ioremap_resource(dev, iores);
> + if (IS_ERR(iomem)) {
> + dev_err(dev, "Failed to remap iomem\n");
> + return PTR_ERR(iomem);
> + }
> +
> + priv->regs = regmap_init_mmio(dev, iomem, &mx25_tsadc);
> + if (IS_ERR(priv->regs)) {
> + dev_err(dev, "Failed to initialize regmap\n");
> + return PTR_ERR(priv->regs);
> + }
> +
> + priv->clk = devm_clk_get(dev, "ipg");
> + if (IS_ERR(priv->clk)) {
> + dev_err(dev, "Failed to get ipg clock\n");
> + return PTR_ERR(priv->clk);
> + }
> +
> + /* Enable clock and reset the component */
> + regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_CLK_EN,
> + MX25_TGCR_CLK_EN);
> + regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_TSC_RST,
> + MX25_TGCR_TSC_RST);
> +
> + /* Setup powersaving mode, but enable internal reference voltage */
> + regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_POWERMODE_MASK,
> + MX25_TGCR_POWERMODE_SAVE);
> + regmap_update_bits(priv->regs, MX25_TSC_TGCR, MX25_TGCR_INTREFEN,
> + MX25_TGCR_INTREFEN);
> +
> + ret = mx25_tsadc_setup_irq(pdev, priv);
> + if (ret) {
> + dev_err(dev, "Failed to setup irqs\n");
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, priv);
> +
> + of_platform_populate(np, NULL, NULL, dev);
> +
> + dev_info(dev, "i.MX25/25 Touchscreen and ADC core driver loaded\n");
You could remove the double '25/25'.
> +
> + return 0;
> +}
> +
> +static const struct of_device_id mx25_tsadc_ids[] = {
> + { .compatible = "fsl,imx25-tsadc", },
> + { /* Sentinel */ }
> +};
> +
> +static struct platform_driver mx25_tsadc_driver = {
> + .driver = {
> + .name = "mx25-tsadc",
> + .owner = THIS_MODULE,
> + .of_match_table = mx25_tsadc_ids,
> + },
> + .probe = mx25_tsadc_probe,
> +};
> +module_platform_driver(mx25_tsadc_driver);
> +
> +MODULE_DESCRIPTION("MFD for ADC/TSC for Freescale mx25");
> +MODULE_AUTHOR("Markus Pargmann <mpa@pengutronix.de>");
> +MODULE_LICENSE("GPL v2");
MODULE_ALIAS() as well?
^ permalink raw reply
* Re: [PATCH 2/2] arm/xen: Don't use xen DMA ops when the device is protected by an IOMMU
From: Ian Campbell @ 2014-02-20 17:13 UTC (permalink / raw)
To: Julien Grall, devicetree-spec-u79uwXL29TY76Z2rM5mHXA
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
stefano.stabellini-mvvWK6WmYclDPfheJLI6IQ,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
xen-devel-GuqFBffKawtpuQazS67q72D2FQJk+8+b, Rob Herring,
Pawel Moll, Mark Rutland, Ian Campbell, Kumar Gala, Rob Landley,
Russell King, devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1392913301-25524-1-git-send-email-julien.grall-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Adding the -spec list for the generic IOMMU binding question.
On Thu, 2014-02-20 at 16:21 +0000, Julien Grall wrote:
> diff --git a/Documentation/devicetree/bindings/arm/xen.txt b/Documentation/devicetree/bindings/arm/xen.txt
> index 0f7b9c2..ee25a57 100644
> --- a/Documentation/devicetree/bindings/arm/xen.txt
> +++ b/Documentation/devicetree/bindings/arm/xen.txt
> @@ -15,6 +15,8 @@ the following properties:
> - interrupts: the interrupt used by Xen to inject event notifications.
> A GIC node is also required.
>
> +- protected-devices: (optional) List of phandles to device node where the
> +IOMMU has been programmed by Xen.
Is there some common/generic IOMMU binding which we can reuse here?
Although this isn't exactly an IOMMU it certainly has some similarities
-- it is providing IOMMU like functionality (albeit a very inflexible
IOMMU which you don't need to/can't actually program).
I'd far rather we followed existing patterns rather than invent our own
things.
I'm wondering if perhaps we didn't ought to integrate this as an actual
IOMMU driver, although I'm not convinced this would make sense.
I'm also not sure about shovelling everything as properties under a
single Xen node, should this not be its own node with compatible =
"xen,(pv)iommu"?
Ian.
--
To unsubscribe from this list: send the line "unsubscribe devicetree-spec" 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 RESEND] ARM: dts: qcom-msm8960-cdp: Add RNG device tree node
From: Kumar Gala @ 2014-02-20 17:06 UTC (permalink / raw)
To: Stanimir Varbanov
Cc: devicetree, linux-arm-kernel, linux-arm-msm, Mark Rutland,
Rob Herring, Pawel Moll, Ian Campbell, David Brown, Stephen Boyd
In-Reply-To: <1392914942-23191-1-git-send-email-svarbanov@mm-sol.com>
On Feb 20, 2014, at 10:49 AM, Stanimir Varbanov <svarbanov@mm-sol.com> wrote:
> Add the necessary DT node to probe the rng driver on
> msm8960-cdp platform.
>
> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> Tested-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
> arch/arm/boot/dts/qcom-msm8960-cdp.dts | 7 +++++++
> 1 file changed, 7 insertions(+)
applied to linux-qcom
(fixed up to apply to qcom-msm8960.dtsi)
- k
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH RESEND] ARM: dts: qcom-msm8974: Add RNG device tree node
From: Kumar Gala @ 2014-02-20 17:05 UTC (permalink / raw)
To: Stanimir Varbanov
Cc: devicetree, linux-arm-kernel, linux-arm-msm, Mark Rutland,
Rob Herring, Pawel Moll, Ian Campbell, David Brown, Stephen Boyd
In-Reply-To: <1392914932-23145-1-git-send-email-svarbanov@mm-sol.com>
On Feb 20, 2014, at 10:48 AM, Stanimir Varbanov <svarbanov@mm-sol.com> wrote:
> Add the necessary DT node to probe the rng driver on
> msm8974 platforms.
>
> Signed-off-by: Stanimir Varbanov <svarbanov@mm-sol.com>
> Acked-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
> arch/arm/boot/dts/qcom-msm8974.dtsi | 7 +++++++
> 1 file changed, 7 insertions(+)
applied to linux-qcom
- k
--
Employee of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
^ permalink raw reply
* Re: [PATCH 1/1] ARM: Exynos: Add generic compatible string
From: Arnd Bergmann @ 2014-02-20 17:00 UTC (permalink / raw)
To: Tomasz Figa
Cc: Sachin Kamat, linux-samsung-soc, Kukjin Kim,
linux-arm-kernel@lists.infradead.org, Olof Johansson,
devicetree@vger.kernel.org, Rob Herring, Mark Rutland,
Grant Likely, Ian Campbell
In-Reply-To: <5306168F.1080400@samsung.com>
On Thursday 20 February 2014, Tomasz Figa wrote:
> On 20.02.2014 05:14, Sachin Kamat wrote:
> > Hi Tomasz,
> >
> > On 19 February 2014 18:15, Tomasz Figa <t.figa@samsung.com> wrote:
> >> Hi Sachin,
> >>
> >> [adding linux-arm-kernel ML to CC list]
> >>
> >>
> >> On 19.02.2014 12:34, Sachin Kamat wrote:
> >>>
> >>> To avoid modifying the kernel every time a new SoC variant
> >>> comes out.
> > <snip>
> >>
> >> Since all Exynos chips can be easily recognized using dedicated chip ID
> >> register, I wonder whether we really need to maintain two distinct board
> >> files for Exynos 4 and 5 series, especially when both of them are doing
> >> mostly the same set up, which can be simply generalized to cover all the
> >> cases.
> >>
> >> Instead of adding just another level of artificially fine grained compatible
> >> strings, I'd rather suggest merging both board files together and adding a
> >> single compatible string identifying all SoCs that can be further
> >> differentiated by using hardware chip ID register.
> >>
> >> What do you think?
> >
> > I agree with your idea of merging both the files as there is very little that is
> > different for now. However I am not really sure if having a single compatible
> > string for all SoCs would be good. What is achieved through compatible string
> > can very well be done using chip ID too. But wouldn't we need to maintain some
> > unique identity for the SoCs in human readable form at the DT level?
>
> Well, my understanding of Device Tree is that it should provide the
> information that can't be automatically retrieved from the hardware, not
> more.
>
> If you have a PCI or USB bus with enumerable devices, you don't list
> them in DT, but instead limit the description to just the host
> controller, if it can't be enumerated.
>
> Same goes for compatible string. My interpretation of it is that if you
> can identify the hardware by some automatic means, e.g. querying some ID
> register, then the compatible should be specific enough to identify the
> class of devices with the same method of querying such register, with no
> need for any additional redundant data in DT.
There are some limitations to that, and you also have to apply common
sense and taste. In theory you could argue that a compatible string can
identify a board uniquely the same way that a board number did, and
everything else is implied by that. Clearly that's not what we want.
On the other end of the scale, you could eliminate a lot of compatible
strings if you describe each register or each bit in some cases in
DT properties, and that would be just as wrong.
> Of course nothing stops you from retaining more specific compatible
> strings. In fact, this is probably the most appropriate solution,
> because in future you might find out that certain SoCs need some special
> differentiation, e.g. same ID value on two SoCs.
>
> So, to apply this to our case, our Exynos 5250 based Arndale board would
> be changed from
>
> compatible = "insignal,arndale", "samsung,exynos5260";
>
> to
>
> compatible = "insignal,arndale", "samsung,exynos5260", "samsung,exynos";
Right, this would make sense.
> Now, the board file will be able to match simply by "samsung,exynos"
> compatible string and SoC-specific code in mach-exynos (hopefully none
> after it gets cleaned up fully) will use soc_is_exynos*() macros (what
> AFAIK it is already doing at the moment).
On principle, I would not take things out of the match list, if that
would break booting with old DT file that don't list "samsung,exynos"
as compatible. But for new SoCs that would work.
Using soc_is_exynos*() too much of course also isn't good. A lot of
the things tested for should probably be checked from individual DT
properties, but again we have to find the right balance. I wouldn't
mind getting rid of the soc_is_exynos*() macros completely, because
a) we can't use them in device drivers
b) all platform code is supposed to be in drivers
c) both rules are enforced for arm64
> Another benefit of this would be increased safety, because you are
> reading SoC type from actual hardware, not from externally supplied
> data. In conjunction with the more specific compatible string (e.g.
> "samsung,exynos5260") some validation could be performed at boot-up time
> to make sure that DT for correct SoC is used.
I don't think that's necessary. If you get that part wrong, normally
all your hopes are lost with the rest of the information in DT. ;-)
Arnd
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox