* RE: RFC: extend iommu-map binding to support #iommu-cells > 1
From: Stuart Yoder @ 2016-12-16 15:06 UTC (permalink / raw)
To: Bharat Bhushan, Mark Rutland,
robin.murphy-5wv7dgnIgG8@public.gmane.org,
will.deacon-5wv7dgnIgG8@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
In-Reply-To: <AM5PR0401MB25455AE3850ED6E724B025EF9A9C0-oQ3wXcTHOqrg6d/1FbYcvI3W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
> -----Original Message-----
> From: Bharat Bhushan
> Sent: Thursday, December 15, 2016 9:46 PM
> To: Stuart Yoder <stuart.yoder-3arQi8VN3Tc@public.gmane.org>; Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>; robin.murphy-5wv7dgnIgG8@public.gmane.org;
> will.deacon-5wv7dgnIgG8@public.gmane.org
> Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; Nipun Gupta <nipun.gupta-3arQi8VN3Tc@public.gmane.org>; Diana Madalina Craciun
> <diana.craciun-3arQi8VN3Tc@public.gmane.org>; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> Subject: RE: RFC: extend iommu-map binding to support #iommu-cells > 1
>
>
>
> > -----Original Message-----
> > From: Stuart Yoder
> > Sent: Friday, December 16, 2016 8:07 AM
> > To: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>; robin.murphy-5wv7dgnIgG8@public.gmane.org;
> > will.deacon-5wv7dgnIgG8@public.gmane.org
> > Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; Bharat Bhushan <bharat.bhushan-3arQi8VN3Tc@public.gmane.org>;
> > Nipun Gupta <nipun.gupta-3arQi8VN3Tc@public.gmane.org>; Diana Madalina Craciun
> > <diana.craciun-3arQi8VN3Tc@public.gmane.org>; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs/ROKNJybVBZg@public.gmane.org
> > foundation.org
> > Subject: RFC: extend iommu-map binding to support #iommu-cells > 1
> >
> > For context, please see the thread:
> > https://www.spinics.net/lists/arm-kernel/msg539066.html
> >
> > The existing iommu-map binding did not account for the situation where
> > #iommu-cells == 2, as permitted in the ARM SMMU binding. The 2nd cell of
> > the IOMMU specifier being the SMR mask. The existing binding defines the
> > mapping as:
> > Any RID r in the interval [rid-base, rid-base + length) is associated with
> > the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base).
> >
> > ...and that does not work if iommu-base is 2 cells, the second being the SMR
> > mask.
> >
> > While this can be worked around by always having length=1, it seems we can
> > get this cleaned up by updating the binding definition for iommu-map.
> >
> > See patch below. Thoughts?
> >
> > Thanks,
> > Stuart
> >
> > -------------------------------------------------------------------------
> >
> > diff --git a/Documentation/devicetree/bindings/pci/pci-iommu.txt
> > b/Documentation/devicetree/bindings/pci/pci-iommu.txt
> > index 56c8296..e81b461 100644
> > --- a/Documentation/devicetree/bindings/pci/pci-iommu.txt
> > +++ b/Documentation/devicetree/bindings/pci/pci-iommu.txt
> > @@ -38,8 +38,20 @@ Optional properties
> > The property is an arbitrary number of tuples of
> > (rid-base,iommu,iommu-base,length).
> >
> > - Any RID r in the interval [rid-base, rid-base + length) is associated with
> > - the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base).
> > + If the associated IOMMU has an #iommu-cells value of 1, any RID r in
> > + the interval [rid-base, rid-base + length) is associated with the
> > + listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base).
> > +
> > + ARM SMMU Note:
> > + The ARM SMMU binding permits an #iommu-cells value of 2 and in this
> > + case defines an IOMMU specifier to be: (stream-id,smr-mask)
> > +
> > + In an iommu-map this means the iommu-base consists of 2 cells:
> > + (rid-base,iommu,[stream-id,smr-mask],length).
> > +
> > + In this case the RID to IOMMU specifier mapping is defined to be:
> > + any RID r in the interval [rid-base, rid-base + length) is associated
> > + with the listed IOMMU, with the iommu-specifier (r - rid-base + stream-
> > id).
>
> Should not this be (r - rid-base + (stream-id & smr-mask)) ?
No, the SMR mask is not applied here-- that is programmed in the SMMU
hardware and not applied by software. The SMR mask in the case of NXP
is 0x7C00, and so is the inverse of a typical mask of 1 bits.
If the map was defined as: <0x0 &smmu 0x10 0x7c00 10>;
An RID value of 0x2 would get mapped to the 2-cell specifier: <0x12 0x7c00>
Stuart
^ permalink raw reply
* Re: [PATCH v4 1/4] mtd: spi-nor: add memory controllers for the Aspeed AST2500 SoC
From: Cédric Le Goater @ 2016-12-16 14:56 UTC (permalink / raw)
To: Cyrille Pitchen, linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Mark Rutland, Boris Brezillon, devicetree-u79uwXL29TY76Z2rM5mHXA,
Richard Weinberger, Marek Vasut, Rob Herring, Joel Stanley,
Cyrille Pitchen, Brian Norris, David Woodhouse
In-Reply-To: <5566c62d-cc72-f207-e1dd-5a59e6947c24-yU5RGvR974pGWvitb5QawA@public.gmane.org>
Hello Cyrille,
On 12/16/2016 12:15 AM, Cyrille Pitchen wrote:
> Hi Cedric,
>
> Le 12/12/2016 à 16:40, Cédric Le Goater a écrit :
>> This driver adds mtd support for the Aspeed AST2500 SoC static memory
>> controllers :
>>
>> * Firmware SPI Memory Controller (FMC)
>> . BMC firmware
>> . 3 chip select pins (CE0 ~ CE2)
>> . supports SPI type flash memory (CE0-CE1)
>> . CE2 can be of NOR type flash but this is not supported by the
>> driver
>>
>> * SPI Flash Controller (SPI1 and SPI2)
>> . host firmware
>> . 2 chip select pins (CE0 ~ CE1)
>> . supports SPI type flash memory
>>
>> Each controller has a memory range on which it maps its flash module
>> slaves. Each slave is assigned a memory window for its mapping that
>> can be changed at bootime with the Segment Address Register.
>>
>> Each SPI flash slave can then be accessed in two modes: Command and
>> User. When in User mode, accesses to the memory segment of the slaves
>> are translated in SPI transfers. When in Command mode, the HW
>> generates the SPI commands automatically and the memory segment is
>> accessed as if doing a MMIO.
>>
>> Currently, only the User mode is supported. Command mode needs a
>> little more work to check that the memory window on the AHB bus fits
>> the module size.
>>
>> Based on previous work from Milton D. Miller II <miltonm-r/Jw6+rmf7HQT0dZR+AlfA@public.gmane.org>
>>
>> Signed-off-by: Cédric Le Goater <clg-Bxea+6Xhats@public.gmane.org>
>> Reviewed-by: Joel Stanley <joel-U3u1mxZcP9KHXe+LvDLADg@public.gmane.org>
>> ---
>>
>> Changes since v3:
>> - reworked IO routines to use io{read,write}32_rep
>> - changed config option to SPI_ASPEED_SMC
>> - fixed aspeed_smc_chip_setup_init() returned value
>> - merged the use of the "label" property"
>>
>> drivers/mtd/spi-nor/Kconfig | 10 +
>> drivers/mtd/spi-nor/Makefile | 1 +
>> drivers/mtd/spi-nor/aspeed-smc.c | 719 +++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 730 insertions(+)
>> create mode 100644 drivers/mtd/spi-nor/aspeed-smc.c
>>
>> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig
>> index 4a682ee0f632..42168e9d6097 100644
>> --- a/drivers/mtd/spi-nor/Kconfig
>> +++ b/drivers/mtd/spi-nor/Kconfig
>> @@ -29,6 +29,16 @@ config MTD_SPI_NOR_USE_4K_SECTORS
>> Please note that some tools/drivers/filesystems may not work with
>> 4096 B erase size (e.g. UBIFS requires 15 KiB as a minimum).
>>
>> +config SPI_ASPEED_SMC
>> + tristate "Aspeed flash controllers in SPI mode"
>> + depends on ARCH_ASPEED || COMPILE_TEST
>> + depends on HAS_IOMEM && OF
>> + help
>> + This enables support for the Firmware Memory controller (FMC)
>> + in the Aspeed AST2500 SoC when attached to SPI NOR chips,
>> + and support for the SPI flash memory controller (SPI) for
>> + the host firmware. The implementation only supports SPI NOR.
>> +
>> config SPI_ATMEL_QUADSPI
>> tristate "Atmel Quad SPI Controller"
>> depends on ARCH_AT91 || (ARM && COMPILE_TEST)
>> diff --git a/drivers/mtd/spi-nor/Makefile b/drivers/mtd/spi-nor/Makefile
>> index 121695e83542..6ff64bc7fa0e 100644
>> --- a/drivers/mtd/spi-nor/Makefile
>> +++ b/drivers/mtd/spi-nor/Makefile
>> @@ -1,4 +1,5 @@
>> obj-$(CONFIG_MTD_SPI_NOR) += spi-nor.o
>> +obj-$(CONFIG_SPI_ASPEED_SMC) += aspeed-smc.o
>> obj-$(CONFIG_SPI_ATMEL_QUADSPI) += atmel-quadspi.o
>> obj-$(CONFIG_SPI_CADENCE_QUADSPI) += cadence-quadspi.o
>> obj-$(CONFIG_SPI_FSL_QUADSPI) += fsl-quadspi.o
>> diff --git a/drivers/mtd/spi-nor/aspeed-smc.c b/drivers/mtd/spi-nor/aspeed-smc.c
>> new file mode 100644
>> index 000000000000..2667ab7aeb9b
>> --- /dev/null
>> +++ b/drivers/mtd/spi-nor/aspeed-smc.c
>> @@ -0,0 +1,719 @@
>> +/*
>> + * ASPEED Static Memory Controller driver
>> + *
>> + * Copyright (c) 2015-2016, IBM Corporation.
>> + *
>> + * 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.
>> + */
>> +
>> +#include <linux/bug.h>
>> +#include <linux/device.h>
>> +#include <linux/io.h>
>> +#include <linux/module.h>
>> +#include <linux/mutex.h>
>> +#include <linux/mtd/mtd.h>
>> +#include <linux/mtd/partitions.h>
>> +#include <linux/mtd/spi-nor.h>
>> +#include <linux/of.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/sysfs.h>
>> +
>> +#define DEVICE_NAME "aspeed-smc"
>> +
>> +/*
>> + * The driver only support SPI flash
>> + */
>> +enum aspeed_smc_flash_type {
>> + smc_type_nor = 0,
>> + smc_type_nand = 1,
>> + smc_type_spi = 2,
>> +};
>> +
>> +struct aspeed_smc_chip;
>> +
>> +struct aspeed_smc_info {
>> + u32 maxsize; /* maximum size of chip window */
>> + u8 nce; /* number of chip enables */
>> + bool hastype; /* flash type field exists in config reg */
>> + u8 we0; /* shift for write enable bit for CE0 */
>> + u8 ctl0; /* offset in regs of ctl for CE0 */
>> +
>> + void (*set_4b)(struct aspeed_smc_chip *chip);
>> +};
>> +
>> +static void aspeed_smc_chip_set_4b(struct aspeed_smc_chip *chip);
>> +
>> +static const struct aspeed_smc_info fmc_2500_info = {
>> + .maxsize = 256 * 1024 * 1024,
>> + .nce = 3,
>> + .hastype = true,
>> + .we0 = 16,
>> + .ctl0 = 0x10,
>> + .set_4b = aspeed_smc_chip_set_4b,
>> +};
>> +
>> +static const struct aspeed_smc_info spi_2500_info = {
>> + .maxsize = 128 * 1024 * 1024,
>> + .nce = 2,
>> + .hastype = false,
>> + .we0 = 16,
>> + .ctl0 = 0x10,
>> + .set_4b = aspeed_smc_chip_set_4b,
>> +};
>> +
>> +enum aspeed_smc_ctl_reg_value {
>> + smc_base, /* base value without mode for other commands */
>> + smc_read, /* command reg for (maybe fast) reads */
>> + smc_write, /* command reg for writes */
>> + smc_max,
>> +};
>> +
>> +struct aspeed_smc_controller;
>> +
>> +struct aspeed_smc_chip {
>> + int cs;
>> + struct aspeed_smc_controller *controller;
>> + void __iomem *ctl; /* control register */
>> + void __iomem *ahb_base; /* base of chip window */
>> + u32 ctl_val[smc_max]; /* control settings */
>> + enum aspeed_smc_flash_type type; /* what type of flash */
>> + struct spi_nor nor;
>> +};
>> +
>> +struct aspeed_smc_controller {
>> + struct device *dev;
>> +
>> + struct mutex mutex; /* controller access mutex */
>> + const struct aspeed_smc_info *info; /* type info of controller */
>> + void __iomem *regs; /* controller registers */
>> + void __iomem *ahb_base; /* per-chip windows resource */
>> +
>> + struct aspeed_smc_chip *chips[0]; /* pointers to attached chips */
>> +};
>> +
>> +/*
>> + * SPI Flash Configuration Register (AST2500 SPI)
>> + * or
>> + * Type setting Register (AST2500 FMC).
>> + * CE0 and CE1 can only be of type SPI. CE2 can be of type NOR but the
>> + * driver does not support it.
>> + */
>> +#define CONFIG_REG 0x0
>> +#define CONFIG_DISABLE_LEGACY BIT(31) /* 1 */
>> +
>> +#define CONFIG_CE2_WRITE BIT(18)
>> +#define CONFIG_CE1_WRITE BIT(17)
>> +#define CONFIG_CE0_WRITE BIT(16)
>> +
>> +#define CONFIG_CE2_TYPE BIT(4) /* AST2500 FMC only */
>> +#define CONFIG_CE1_TYPE BIT(2) /* AST2500 FMC only */
>> +#define CONFIG_CE0_TYPE BIT(0) /* AST2500 FMC only */
>> +
>> +/*
>> + * CE Control Register
>> + */
>> +#define CE_CONTROL_REG 0x4
>> +
>> +/*
>> + * CEx Control Register
>> + */
>> +#define CONTROL_AAF_MODE BIT(31)
>> +#define CONTROL_IO_MODE_MASK GENMASK(30, 28)
>> +#define CONTROL_IO_DUAL_DATA BIT(29)
>> +#define CONTROL_IO_DUAL_ADDR_DATA (BIT(29) | BIT(28))
>> +#define CONTROL_IO_QUAD_DATA BIT(30)
>> +#define CONTROL_IO_QUAD_ADDR_DATA (BIT(30) | BIT(28))
>> +#define CONTROL_CE_INACTIVE_SHIFT 24
>> +#define CONTROL_CE_INACTIVE_MASK GENMASK(27, \
>> + CONTROL_CE_INACTIVE_SHIFT)
>> +/* 0 = 16T ... 15 = 1T T=HCLK */
>> +#define CONTROL_COMMAND_SHIFT 16
>> +#define CONTROL_DUMMY_COMMAND_OUT BIT(15)
>> +#define CONTROL_IO_DUMMY_HI BIT(14)
>> +#define CONTROL_IO_DUMMY_HI_SHIFT 14
>> +#define CONTROL_CLK_DIV4 BIT(13) /* others */
>> +#define CONTROL_RW_MERGE BIT(12)
>> +#define CONTROL_IO_DUMMY_LO_SHIFT 6
>> +#define CONTROL_IO_DUMMY_LO GENMASK(7, \
>> + CONTROL_IO_DUMMY_LO_SHIFT)
>> +#define CONTROL_IO_DUMMY_MASK (CONTROL_IO_DUMMY_HI | \
>> + CONTROL_IO_DUMMY_LO)
>> +#define CONTROL_IO_DUMMY_SET(dummy) \
>> + (((((dummy) >> 2) & 0x1) << CONTROL_IO_DUMMY_HI_SHIFT) | \
>> + (((dummy) & 0x3) << CONTROL_IO_DUMMY_LO_SHIFT))
>> +
>> +#define CONTROL_CLOCK_FREQ_SEL_SHIFT 8
>> +#define CONTROL_CLOCK_FREQ_SEL_MASK GENMASK(11, \
>> + CONTROL_CLOCK_FREQ_SEL_SHIFT)
>> +#define CONTROL_LSB_FIRST BIT(5)
>> +#define CONTROL_CLOCK_MODE_3 BIT(4)
>> +#define CONTROL_IN_DUAL_DATA BIT(3)
>> +#define CONTROL_CE_STOP_ACTIVE_CONTROL BIT(2)
>> +#define CONTROL_COMMAND_MODE_MASK GENMASK(1, 0)
>> +#define CONTROL_COMMAND_MODE_NORMAL 0
>> +#define CONTROL_COMMAND_MODE_FREAD 1
>> +#define CONTROL_COMMAND_MODE_WRITE 2
>> +#define CONTROL_COMMAND_MODE_USER 3
>> +
>> +#define CONTROL_KEEP_MASK \
>> + (CONTROL_AAF_MODE | CONTROL_CE_INACTIVE_MASK | CONTROL_CLK_DIV4 | \
>> + CONTROL_IO_DUMMY_MASK | CONTROL_CLOCK_FREQ_SEL_MASK | \
>> + CONTROL_LSB_FIRST | CONTROL_CLOCK_MODE_3)
>> +
>> +/*
>> + * The Segment Register uses a 8MB unit to encode the start address
>> + * and the end address of the mapping window of a flash SPI slave :
>> + *
>> + * | byte 1 | byte 2 | byte 3 | byte 4 |
>> + * +--------+--------+--------+--------+
>> + * | end | start | 0 | 0 |
>> + */
>> +#define SEGMENT_ADDR_REG0 0x30
>> +#define SEGMENT_ADDR_START(_r) ((((_r) >> 16) & 0xFF) << 23)
>> +#define SEGMENT_ADDR_END(_r) ((((_r) >> 24) & 0xFF) << 23)
>> +
>> +/*
>> + * In user mode all data bytes read or written to the chip decode address
>> + * range are transferred to or from the SPI bus. The range is treated as a
>> + * fifo of arbitratry 1, 2, or 4 byte width but each write has to be aligned
>> + * to its size. The address within the multiple 8kB range is ignored when
>> + * sending bytes to the SPI bus.
>> + *
>> + * On the arm architecture, as of Linux version 4.3, memcpy_fromio and
>> + * memcpy_toio on little endian targets use the optimized memcpy routines
>> + * that were designed for well behavied memory storage. These routines
>> + * have a stutter if the source and destination are not both word aligned,
>> + * once with a duplicate access to the source after aligning to the
>> + * destination to a word boundary, and again with a duplicate access to
>> + * the source when the final byte count is not word aligned.
>> + *
>> + * When writing or reading the fifo this stutter discards data or sends
>> + * too much data to the fifo and can not be used by this driver.
>> + *
>> + * While the low level io string routines that implement the insl family do
>> + * the desired accesses and memory increments, the cross architecture io
>> + * macros make them essentially impossible to use on a memory mapped address
>> + * instead of a a token from the call to iomap of an io port.
>> + *
>> + * These fifo routines use readl and friends to a constant io port and update
>> + * the memory buffer pointer and count via explicit code. The final updates
>> + * to len are optimistically suppressed.
>> + */
>> +static int aspeed_smc_read_from_ahb(void *buf, const void __iomem *src,
>> + size_t len)
>> +{
>> + if (IS_ALIGNED((uintptr_t)src, sizeof(uintptr_t)) &&
>> + IS_ALIGNED((uintptr_t)buf, sizeof(uintptr_t)) &&
>> + IS_ALIGNED(len, sizeof(u32))) {
>> + ioread32_rep(src, buf, len >> 2);
>> + } else {
>> + ioread8_rep(src, buf, len);
>> + }
>> + return 0;
>> +}
>> +
>
> Maybe It might be something like:
>
> size_t offset = 0;
>
> if (IS_ALIGNED((uintptr_t)src, sizeof(uintptr_t)) &&
> IS_ALIGNED((uintptr_t)buf, sizeof(uintptr_t))) {
> ioread32_rep(src, buf, len >> 2);
> offset = len & ~0x3;
> len -= offset;
> }
> ioread8_rep(src, (const u8 *)buf + offset, len);
yes. We had an earlier version doing something similar, not as well
crafted tough.
> I assume the Aspeed SPI controller allows to read as much 32-bit words
> as possible before reading the remaining bytes.
yes.
> This is just a suggested optimization, no need to use it if you don't
> want to.
well, it depends if there is a v4. If so, I will.
> In v3, with readl()/readb(), you used to increment both src and buf in
> your while() loop but now with ioreadX_rep() only buf is incremented: it
> always reads from src without incrementing this latest address.
>
> I guess it means that the Aspeed SPI controller doesn't care of the
> actual value of src as long as it lays inside the chip address range.
yes :) in 'User' mode, the address has no meaning.
The previous routine was practical to cover both mode: the currently
supported 'User' mode in which we read/write the ops in the memory window
of the flash, and the 'Command' mode in which we read/write directly the
flash contents from the AHB bus. This mode requires a preliminary setup
of the CEx Control Register, which is what the ctl_val field is for.
We have postponed 'Command' mode for the moment because we have flash
modules which exceed the maximum window size on some boards, and 'Command'
mode does not work in that case. Covering this mode and these special
cases needs more work. 'User' mode is simpler to start with but the code
prepares ground for the other mode.
> This is what you explain in the 1st paragraph of the comment, isn't it?
yes. That might be a little outdated.
>> +static int aspeed_smc_write_to_ahb(void __iomem *dst, const void *buf,
>> + size_t len)
>> +{
>> + if (IS_ALIGNED((uintptr_t)dst, sizeof(uintptr_t)) &&
>> + IS_ALIGNED((uintptr_t)buf, sizeof(uintptr_t)) &&
>> + IS_ALIGNED(len, sizeof(u32))) {
>> + iowrite32_rep(dst, buf, len >> 2);
>> + } else {
>> + iowrite8_rep(dst, buf, len);
>> + }
>> + return 0;
>> +}
>> +
>> +static inline u32 aspeed_smc_chip_write_bit(struct aspeed_smc_chip *chip)
>> +{
>> + return BIT(chip->controller->info->we0 + chip->cs);
>> +}
>> +
>> +static void aspeed_smc_chip_check_config(struct aspeed_smc_chip *chip)
>> +{
>> + struct aspeed_smc_controller *controller = chip->controller;
>> + u32 reg;
>> +
>> + reg = readl(controller->regs + CONFIG_REG);
>> +
>> + if (reg & aspeed_smc_chip_write_bit(chip))
>> + return;
>> +
>> + dev_dbg(controller->dev, "config write is not set ! @%p: 0x%08x\n",
>> + controller->regs + CONFIG_REG, reg);
>> + reg |= aspeed_smc_chip_write_bit(chip);
>> + writel(reg, controller->regs + CONFIG_REG);
>> +}
>> +
>> +static void aspeed_smc_start_user(struct spi_nor *nor)
>> +{
>> + struct aspeed_smc_chip *chip = nor->priv;
>> + u32 ctl = chip->ctl_val[smc_base];
>> +
>> + /*
>> + * When the chip is controlled in user mode, we need write
>> + * access to send the opcodes to it. So check the config.
>> + */
>> + aspeed_smc_chip_check_config(chip);
>> +
>> + ctl |= CONTROL_COMMAND_MODE_USER |
>> + CONTROL_CE_STOP_ACTIVE_CONTROL;
>> + writel(ctl, chip->ctl);
>> +
>> + ctl &= ~CONTROL_CE_STOP_ACTIVE_CONTROL;
>> + writel(ctl, chip->ctl);
>> +}
>> +
>> +static void aspeed_smc_stop_user(struct spi_nor *nor)
>> +{
>> + struct aspeed_smc_chip *chip = nor->priv;
>> +
>> + u32 ctl = chip->ctl_val[smc_read];
>> + u32 ctl2 = ctl | CONTROL_COMMAND_MODE_USER |
>> + CONTROL_CE_STOP_ACTIVE_CONTROL;
>> +
>> + writel(ctl2, chip->ctl); /* stop user CE control */
>> + writel(ctl, chip->ctl); /* default to fread or read mode */
>> +}
>> +
>
> This driver seems to use only the "USER" mode so why do you go back the
> the "FREAD" or "READ" modes at the very end of aspeed_smc_stop_user() as
> the comment suggests?
>
> Do you plan to implement other modes later?
yes. I would like to, I have some code for it already but there some
little issues as said above.
> Can't you just stay in "USER" mode?
well, yes. we are also preparing ground for the Command mode and
the DMA support.
> I guess you just need the chip-select control part.
yes.
>> +static int aspeed_smc_prep(struct spi_nor *nor, enum spi_nor_ops ops)
>> +{
>> + struct aspeed_smc_chip *chip = nor->priv;
>> +
>> + mutex_lock(&chip->controller->mutex);
>> + return 0;
>> +}
>> +
>> +static void aspeed_smc_unprep(struct spi_nor *nor, enum spi_nor_ops ops)
>> +{
>> + struct aspeed_smc_chip *chip = nor->priv;
>> +
>> + mutex_unlock(&chip->controller->mutex);
>> +}
>> +
>> +static int aspeed_smc_read_reg(struct spi_nor *nor, u8 opcode, u8 *buf, int len)
>> +{
>> + struct aspeed_smc_chip *chip = nor->priv;
>> +
>> + aspeed_smc_start_user(nor);
>> + aspeed_smc_write_to_ahb(chip->ahb_base, &opcode, 1);
>> + aspeed_smc_read_from_ahb(buf, chip->ahb_base, len);
>> + aspeed_smc_stop_user(nor);
>> + return 0;
>> +}
>> +
>> +static int aspeed_smc_write_reg(struct spi_nor *nor, u8 opcode, u8 *buf,
>> + int len)
>> +{
>> + struct aspeed_smc_chip *chip = nor->priv;
>> +
>> + aspeed_smc_start_user(nor);
>> + aspeed_smc_write_to_ahb(chip->ahb_base, &opcode, 1);
>> + aspeed_smc_write_to_ahb(chip->ahb_base, buf, len);
>> + aspeed_smc_stop_user(nor);
>> + return 0;
>> +}
>> +
>> +static void aspeed_smc_send_cmd_addr(struct spi_nor *nor, u8 cmd, u32 addr)
>> +{
>> + struct aspeed_smc_chip *chip = nor->priv;
>> + __be32 temp;
>> + u32 cmdaddr;
>> +
>> + switch (nor->addr_width) {
>> + default:
>> + WARN_ONCE(1, "Unexpected address width %u, defaulting to 3\n",
>> + nor->addr_width);
>> + /* FALLTHROUGH */
>> + case 3:
>> + cmdaddr = addr & 0xFFFFFF;
>> + cmdaddr |= cmd << 24;
>> +
>> + temp = cpu_to_be32(cmdaddr);
>> + aspeed_smc_write_to_ahb(chip->ahb_base, &temp, 4);
>> + break;
>> + case 4:
>> + temp = cpu_to_be32(addr);
>> + aspeed_smc_write_to_ahb(chip->ahb_base, &cmd, 1);
>> + aspeed_smc_write_to_ahb(chip->ahb_base, &temp, 4);
>> + break;
>> + }
>> +}
>> +
>> +static ssize_t aspeed_smc_read_user(struct spi_nor *nor, loff_t from,
>> + size_t len, u_char *read_buf)
>> +{
>> + struct aspeed_smc_chip *chip = nor->priv;
>> +
>> + aspeed_smc_start_user(nor);
>> + aspeed_smc_send_cmd_addr(nor, nor->read_opcode, from);
>
> Here, please check nor->read_dummy to write the relevant number dummy
> bytes between the address and data cycles.
>
> It should not need too much work to add support to the dummy clock
> cycles and it's more reliable/safe.
>
> Indeed, even if you call the current spi_nor_scan() function with the
> enum read_mode SPI_NOR_NORMAL value, this function just doesn't care and
> selects the Fast Read (0Bh) command instead of the Read (03h) command
> for nor->read_opcode if the "m25p,fast-read" DT property is set.
>
> So if any end user sets this property in a custom DT,
> aspeed_smc_read_user() would just fail.
>
> Hence I think it's worth dealing with dummy cycles now rather than later.
>
> Actually all (Fast) Read commands but the legacy Read (03h) command need
> dummy cycles. So the Read SFDP (5Ah) command does.
>
> For all the (Q)SPI memories I've seen till now, the default factory
> settings for the number of dummy cycles are chosen so it always
> corresponds to entire bytes, whatever the SPI protocol is (SPI 1-1-2,
> 1-2-2, 1-1-4, 1-4-4, ...).
>
> Besides, I recommend you use the 0xFF value for dummy cycles: this value
> prevents the memory from entering its continuous mode by mistake.
> The 0xFF value works for all manufacturers. The SFDP specification seems
> to confirm that.
OK. I will take a closer look at that.
>
>> + aspeed_smc_read_from_ahb(read_buf, chip->ahb_base, len);
>> + aspeed_smc_stop_user(nor);
>> + return len;
>> +}
>> +
>> +static ssize_t aspeed_smc_write_user(struct spi_nor *nor, loff_t to,
>> + size_t len, const u_char *write_buf)
>> +{
>> + struct aspeed_smc_chip *chip = nor->priv;
>> +
>> + aspeed_smc_start_user(nor);
>> + aspeed_smc_send_cmd_addr(nor, nor->program_opcode, to);
>> + aspeed_smc_write_to_ahb(chip->ahb_base, write_buf, len);
>> + aspeed_smc_stop_user(nor);
>> + return len;
>> +}
>> +
>> +static int aspeed_smc_unregister(struct aspeed_smc_controller *controller)
>> +{
>> + struct aspeed_smc_chip *chip;
>> + int n;
>> +
>> + for (n = 0; n < controller->info->nce; n++) {
>> + chip = controller->chips[n];
>> + if (chip)
>> + mtd_device_unregister(&chip->nor.mtd);
>> + }
>> +
>> + return 0;
>> +}
>> +
>> +static int aspeed_smc_remove(struct platform_device *dev)
>> +{
>> + return aspeed_smc_unregister(platform_get_drvdata(dev));
>> +}
>> +
>> +static const struct of_device_id aspeed_smc_matches[] = {
>> + { .compatible = "aspeed,ast2500-fmc", .data = &fmc_2500_info },
>> + { .compatible = "aspeed,ast2500-spi", .data = &spi_2500_info },
>> + { }
>> +};
>> +MODULE_DEVICE_TABLE(of, aspeed_smc_matches);
>> +
>> +/*
>> + * Each chip has a mapping window defined by a segment address
>> + * register defining a start and an end address on the AHB bus. These
>> + * addresses can be configured to fit the chip size and offer a
>> + * contiguous memory region across chips. For the moment, we only
>> + * check that each chip segment is valid.
>> + */
>> +static void __iomem *aspeed_smc_chip_base(struct aspeed_smc_chip *chip,
>> + struct resource *res)
>> +{
>> + struct aspeed_smc_controller *controller = chip->controller;
>> + u32 offset = 0;
>> + u32 reg;
>> +
>> + if (controller->info->nce > 1) {
>> + reg = readl(controller->regs + SEGMENT_ADDR_REG0 +
>> + chip->cs * 4);
>> +
>> + if (SEGMENT_ADDR_START(reg) >= SEGMENT_ADDR_END(reg))
>> + return NULL;
>> +
>> + offset = SEGMENT_ADDR_START(reg) - res->start;
>> + }
>> +
>> + return controller->ahb_base + offset;
>> +}
>> +
>> +static void aspeed_smc_chip_enable_write(struct aspeed_smc_chip *chip)
>> +{
>> + struct aspeed_smc_controller *controller = chip->controller;
>> + u32 reg;
>> +
>> + reg = readl(controller->regs + CONFIG_REG);
>> +
>> + reg |= aspeed_smc_chip_write_bit(chip);
>> + writel(reg, controller->regs + CONFIG_REG);
>> +}
>> +
>> +static void aspeed_smc_chip_set_type(struct aspeed_smc_chip *chip, int type)
>> +{
>> + struct aspeed_smc_controller *controller = chip->controller;
>> + u32 reg;
>> +
>> + chip->type = type;
>> +
>> + reg = readl(controller->regs + CONFIG_REG);
>> + reg &= ~(3 << (chip->cs * 2));
>> + reg |= chip->type << (chip->cs * 2);
>> + writel(reg, controller->regs + CONFIG_REG);
>> +}
>> +
>> +/*
>> + * The AST2500 FMC flash controller should be strapped by hardware, or
>> + * autodetected, but the AST2500 SPI flash needs to be set.
>> + */
>> +static void aspeed_smc_chip_set_4b(struct aspeed_smc_chip *chip)
>> +{
>> + struct aspeed_smc_controller *controller = chip->controller;
>> + u32 reg;
>> +
>> + if (chip->controller->info == &spi_2500_info) {
>> + reg = readl(controller->regs + CE_CONTROL_REG);
>> + reg |= 1 << chip->cs;
>> + writel(reg, controller->regs + CE_CONTROL_REG);
>> + }
>> +}
>> +
>> +static int aspeed_smc_chip_setup_init(struct aspeed_smc_chip *chip,
>> + struct resource *res)
>> +{
>> + struct aspeed_smc_controller *controller = chip->controller;
>> + const struct aspeed_smc_info *info = controller->info;
>> + u32 reg, base_reg;
>> +
>> + /*
>> + * Always turn on the write enable bit to allow opcodes to be
>> + * sent in user mode.
>> + */
>> + aspeed_smc_chip_enable_write(chip);
>> +
>> + /* The driver only supports SPI type flash */
>> + if (info->hastype)
>> + aspeed_smc_chip_set_type(chip, smc_type_spi);
>> +
>> + /*
>> + * Configure chip base address in memory
>> + */
>> + chip->ahb_base = aspeed_smc_chip_base(chip, res);
>> + if (!chip->ahb_base) {
>> + dev_warn(chip->nor.dev, "CE segment window closed.\n");
>> + return -EINVAL;
>> + }
>> +
>> + /*
>> + * Get value of the inherited control register. U-Boot usually
>> + * does some timing calibration on the FMC chip, so it's good
>> + * to keep them. In the future, we should handle calibration
>> + * from Linux.
>> + */
>> + reg = readl(chip->ctl);
>> + dev_dbg(controller->dev, "control register: %08x\n", reg);
>> +
>> + base_reg = reg & CONTROL_KEEP_MASK;
>> + if (base_reg != reg) {
>> + dev_info(controller->dev,
>> + "control register changed to: %08x\n",
>> + base_reg);
>
> dev_dbg() should be enough: end users don't know what to do with the new
> control register value, do they?
>
> This is just a suggestion, you can keep dev_info() if you want, I don't
> mind :)
No, you are right. This is debug stuff. We had a bunch of user space tools
poking in the SMC controller MMIO region in early days and it was nice to
know what was the initial setup.
>> + }
>> + chip->ctl_val[smc_base] = base_reg;
>> +
>> + /*
>> + * Retain the prior value of the control register as the
>> + * default if it was normal access mode. Otherwise start with
>> + * the sanitized base value set to read mode.
>> + */
>> + if ((reg & CONTROL_COMMAND_MODE_MASK) ==
>> + CONTROL_COMMAND_MODE_NORMAL)
>> + chip->ctl_val[smc_read] = reg;
>> + else
>> + chip->ctl_val[smc_read] = chip->ctl_val[smc_base] |
>> + CONTROL_COMMAND_MODE_NORMAL;
>> +
>> + dev_dbg(controller->dev, "default control register: %08x\n",
>> + chip->ctl_val[smc_read]);
>> + return 0;
>> +}
>> +
>> +static int aspeed_smc_chip_setup_finish(struct aspeed_smc_chip *chip)
>> +{
>> + struct aspeed_smc_controller *controller = chip->controller;
>> + const struct aspeed_smc_info *info = controller->info;
>> + u32 cmd;
>> +
>> + if (chip->nor.addr_width == 4 && info->set_4b)
>> + info->set_4b(chip);
>> +
>> + /*
>> + * base mode has not been optimized yet. use it for writes.
>> + */
>> + chip->ctl_val[smc_write] = chip->ctl_val[smc_base] |
>> + chip->nor.program_opcode << CONTROL_COMMAND_SHIFT |
>> + CONTROL_COMMAND_MODE_WRITE;
>> +
>> + dev_dbg(controller->dev, "write control register: %08x\n",
>> + chip->ctl_val[smc_write]);
>> +
>> + /*
>> + * TODO: Adjust clocks if fast read is supported and interpret
>> + * SPI-NOR flags to adjust controller settings.
>> + */
>> + switch (chip->nor.flash_read) {
>> + case SPI_NOR_NORMAL:
>> + cmd = CONTROL_COMMAND_MODE_NORMAL;
>> + break;
>> + case SPI_NOR_FAST:
>> + cmd = CONTROL_COMMAND_MODE_FREAD;
>> + break;
>> + default:
>> + dev_err(chip->nor.dev, "unsupported SPI read mode\n");
>> + return -EINVAL;
>> + }
>> +
>> + chip->ctl_val[smc_read] |= cmd |
>> + CONTROL_IO_DUMMY_SET(chip->nor.read_dummy / 8);
>> +
>> + dev_dbg(controller->dev, "base control register: %08x\n",
>> + chip->ctl_val[smc_read]);
>> + return 0;
>> +}
>> +
>
> Why do you configure both chip->ctrl_val[smc_write] and
> chip->ctrl_val[smc_read] if the driver actually only uses
> chip->ctrl_val[smc_base] ?
we expect Command mode support and DMAs will use it.
> all aspeed_smc_[read|write]_[reg|user]() functions call
> aspeed_smc_[start|stop]_user(), so this driver always selects the "USER"
> mode and configures the control register based on chip->ctrl_val[smc_base].
yes.
Maybe you think it is too early to prepare ground for the other
mode ? Is that really confusing in the code ? Do you think I should
remove it for the initial support in the driver and stick to 'User'
mode.
>> +static int aspeed_smc_setup_flash(struct aspeed_smc_controller *controller,
>> + struct device_node *np, struct resource *r)
>> +{
>> + const struct aspeed_smc_info *info = controller->info;
>> + struct device *dev = controller->dev;
>> + struct device_node *child;
>> + unsigned int cs;
>> + int ret = -ENODEV;
>> +
>> + for_each_available_child_of_node(np, child) {
>> + struct aspeed_smc_chip *chip;
>> + struct spi_nor *nor;
>> + struct mtd_info *mtd;
>> +
>> + /* This driver does not support NAND or NOR flash devices. */
>> + if (!of_device_is_compatible(child, "jedec,spi-nor"))
>> + continue;
>> +
>> + ret = of_property_read_u32(child, "reg", &cs);
>> + if (ret) {
>> + dev_err(dev, "Couldn't not read chip select.\n");
>> + break;
>> + }
>> +
>> + if (cs >= info->nce) {
>> + dev_err(dev, "Chip select %d out of range.\n",
>> + cs);
>> + ret = -ERANGE;
>> + break;
>> + }
>> +
>> + if (controller->chips[cs]) {
>> + dev_err(dev, "Chip select %d already in use by %s\n",
>> + cs, dev_name(controller->chips[cs]->nor.dev));
>> + ret = -EBUSY;
>> + break;
>> + }
>> +
>> + chip = devm_kzalloc(controller->dev, sizeof(*chip), GFP_KERNEL);
>> + if (!chip) {
>> + ret = -ENOMEM;
>> + break;
>> + }
>> +
>> + chip->controller = controller;
>> + chip->ctl = controller->regs + info->ctl0 + cs * 4;
>> + chip->cs = cs;
>> +
>> + nor = &chip->nor;
>> + mtd = &nor->mtd;
>> +
>> + nor->dev = dev;
>> + nor->priv = chip;
>> + spi_nor_set_flash_node(nor, child);
>> + nor->read = aspeed_smc_read_user;
>> + nor->write = aspeed_smc_write_user;
>> + nor->read_reg = aspeed_smc_read_reg;
>> + nor->write_reg = aspeed_smc_write_reg;
>> + nor->prepare = aspeed_smc_prep;
>> + nor->unprepare = aspeed_smc_unprep;
>> +
>> + mtd->name = of_get_property(child, "label", NULL);
>
> This new "label" DT property should be removed from this patch and send
> in a dedicated patch to be discussed separately. However I agree with
> Marek: it looks generic so maybe it could be managed directly from
> mtd_device_register() since setting such as name could also be done when
> using a NAND flash. Anyway, I don't see the link between this name and
> spi-nor. Hence I don't think the DT property should be documented in
> jedec,spi-nor.txt.
OK. I will remove it in the next version.
> Be patient because I expect such a topic to be discussed quite a long
> time before we all agree, even if this is "just" a new DT property ;)
yeah. I expected that also :) But it is quite pratical to give user
space a hint on the flash nature. Systems can have up to 4 different
ones. So there is need for it IMO.
How should I proceed then ? Shall I start a discussion with a single
patch changing mtd_device_register() ? but I need to know which binding
would be the more consensual for such a prop.
Thanks,
C.
>
> Best regards,
>
> Cyrille
>
>
>> +
>> + ret = aspeed_smc_chip_setup_init(chip, r);
>> + if (ret)
>> + break;
>> +
>> + /*
>> + * TODO: Add support for SPI_NOR_QUAD and SPI_NOR_DUAL
>> + * attach when board support is present as determined
>> + * by of property.
>> + */
>> + ret = spi_nor_scan(nor, NULL, SPI_NOR_NORMAL);
>> + if (ret)
>> + break;
>> +
>> + ret = aspeed_smc_chip_setup_finish(chip);
>> + if (ret)
>> + break;
>> +
>> + ret = mtd_device_register(mtd, NULL, 0);
>> + if (ret)
>> + break;
>> +
>> + controller->chips[cs] = chip;
>> + }
>> +
>> + if (ret)
>> + aspeed_smc_unregister(controller);
>> +
>> + return ret;
>> +}
>> +
>> +static int aspeed_smc_probe(struct platform_device *pdev)
>> +{
>> + struct device_node *np = pdev->dev.of_node;
>> + struct device *dev = &pdev->dev;
>> + struct aspeed_smc_controller *controller;
>> + const struct of_device_id *match;
>> + const struct aspeed_smc_info *info;
>> + struct resource *res;
>> + int ret;
>> +
>> + match = of_match_device(aspeed_smc_matches, &pdev->dev);
>> + if (!match || !match->data)
>> + return -ENODEV;
>> + info = match->data;
>> +
>> + controller = devm_kzalloc(&pdev->dev, sizeof(*controller) +
>> + info->nce * sizeof(controller->chips[0]), GFP_KERNEL);
>> + if (!controller)
>> + return -ENOMEM;
>> + controller->info = info;
>> + controller->dev = dev;
>> +
>> + mutex_init(&controller->mutex);
>> + platform_set_drvdata(pdev, controller);
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + controller->regs = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(controller->regs)) {
>> + dev_err(dev, "Cannot remap controller address.\n");
>> + return PTR_ERR(controller->regs);
>> + }
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1);
>> + controller->ahb_base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(controller->ahb_base)) {
>> + dev_err(dev, "Cannot remap controller address.\n");
>> + return PTR_ERR(controller->ahb_base);
>> + }
>> +
>> + ret = aspeed_smc_setup_flash(controller, np, res);
>> + if (ret)
>> + dev_err(dev, "Aspeed SMC probe failed %d\n", ret);
>> +
>> + return ret;
>> +}
>> +
>> +static struct platform_driver aspeed_smc_driver = {
>> + .probe = aspeed_smc_probe,
>> + .remove = aspeed_smc_remove,
>> + .driver = {
>> + .name = DEVICE_NAME,
>> + .of_match_table = aspeed_smc_matches,
>> + }
>> +};
>> +
>> +module_platform_driver(aspeed_smc_driver);
>> +
>> +MODULE_DESCRIPTION("ASPEED Static Memory Controller Driver");
>> +MODULE_AUTHOR("Cedric Le Goater <clg-Bxea+6Xhats@public.gmane.org>");
>> +MODULE_LICENSE("GPL v2");
>>
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Docs: dt: Be explicit and consistent in reference to IOMMU specifiers
From: Mark Rutland @ 2016-12-16 14:56 UTC (permalink / raw)
To: Stuart Yoder, treding-DDmLM1+adcrQT0dZR+AlfA
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
will.deacon-5wv7dgnIgG8@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
In-Reply-To: <VI1PR0401MB263816D0AA09CF94E2B5C7E58D9C0-9IDQY6o3qQjcXZ0H4ZLnAo3W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
On Fri, Dec 16, 2016 at 02:08:09PM +0000, Stuart Yoder wrote:
>
> > -----Original Message-----
> > From: Mark Rutland [mailto:mark.rutland-5wv7dgnIgG8@public.gmane.org]
> >
> > On Thu, Dec 15, 2016 at 06:16:13PM -0600, Stuart Yoder wrote:
> > > In the iommu-map binding change references to iommu-specifier to
> > > "IOMMU specifier" so we are 100% consistent everywhere with terminology
> > > and capitalization.
> >
> > Elsewhere, we always use lower case "xxx-specifier" or "xxx specifier",
> > e.g. Documentation/devicetree/bindings/gpio/gpio.txt defines
> > "gpio-specifier", ePAPR defines "interrupt specifier".
> >
> > Given we're morstly consistent on "iommu-specifier" today,could we
> > please jsut update the ARM SMMU binding to match that? If we're going to
> > fix the dash mismatch, that's a more general, cross-binding thing.
>
> The notable place where we don't use "iommu-specifier" in in the generic
> IOMMU binding itself where we use "IOMMU specifier".
True; I failed to notice that. You are right in that the pci-iommu
binding is the odd one out. Sorry for the misinformation above. :/
> You're suggesting using "iommu-specifier" everywhere including the
> generic binding? Sounds fine to me. It's a nit but would like to see
> it consistent everywhere.
I certainly agree that we should be consistent.
So FWIW, for this patch (as-is):
Acked-by: Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org>
Thanks,
Mark.
^ permalink raw reply
* Re: RFC: extend iommu-map binding to support #iommu-cells > 1
From: Robin Murphy @ 2016-12-16 14:50 UTC (permalink / raw)
To: Stuart Yoder, Mark Rutland
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
will.deacon-5wv7dgnIgG8@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
In-Reply-To: <VI1PR0401MB26387C2712FC4BF74B4B96028D9C0-9IDQY6o3qQjcXZ0H4ZLnAo3W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
On 16/12/16 14:21, Stuart Yoder wrote:
>
>
>> -----Original Message-----
>> From: Mark Rutland [mailto:mark.rutland-5wv7dgnIgG8@public.gmane.org]
>> Sent: Friday, December 16, 2016 5:39 AM
>> To: Stuart Yoder <stuart.yoder-3arQi8VN3Tc@public.gmane.org>
>> Cc: robin.murphy-5wv7dgnIgG8@public.gmane.org; will.deacon-5wv7dgnIgG8@public.gmane.org; robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; Bharat Bhushan
>> <bharat.bhushan-3arQi8VN3Tc@public.gmane.org>; Nipun Gupta <nipun.gupta-3arQi8VN3Tc@public.gmane.org>; Diana Madalina Craciun
>> <diana.craciun-3arQi8VN3Tc@public.gmane.org>; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
>> Subject: Re: RFC: extend iommu-map binding to support #iommu-cells > 1
>>
>> On Fri, Dec 16, 2016 at 02:36:57AM +0000, Stuart Yoder wrote:
>>> For context, please see the thread:
>>> https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-
>> kernel%2Fmsg539066.html&data=01%7C01%7Cstuart.yoder%40nxp.com%7C0464e12bddfd42e0f0a508d425a847cb%7C686ea
>> 1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=KnzeOlbts271GwD1cpx2FLMsKnxw%2FOdiGVTp6%2BKFrbM%3D&reserved=0
>>>
>>> The existing iommu-map binding did not account for the situation where
>>> #iommu-cells == 2, as permitted in the ARM SMMU binding. The 2nd cell
>>> of the IOMMU specifier being the SMR mask. The existing binding defines
>>> the mapping as:
>>> Any RID r in the interval [rid-base, rid-base + length) is associated with
>>> the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base).
>>>
>>> ...and that does not work if iommu-base is 2 cells, the second being the
>>> SMR mask.
>>>
>>> While this can be worked around by always having length=1, it seems we
>>> can get this cleaned up by updating the binding definition for iommu-map.
>>
>> To reiterate, I'm very much not keen on the pci-iommu binding having
>> knowledge of the decomposition of iommu-specifiers from other bindings.
>
> With the current definition of iommu-map we already have baked in an
> assumption that an iommu-specifier is a value that can be incremented
> by 1 to get to the next sequential specifier. So the binding already
> has "knowledge" that applies in most, but not all cases.
>
> The generic iommu binding also defines a case where #iommu-cells=4
> for some IOMMUs.
>
> Since the ARM SMMU is a special case, perhaps the intepretation
> of an iommu-specifier in the context of iommu-map should be moved
> into the SMMU binding.
>
>> As mentioned previously, there's an intended interpretation [1] that we
>> need to fix up the pci-iommu binding with. With that, I don't think it's
>> even necessary to bodge iommu-cells = <1> AFAICT.
>
> You had said in the previous thread:
>
> > I had intended that the offset was added to the final cell of the
> > iommu-specifier (i.e. that the iommu-specifier was treated as a large
> > number).
>
> > You can handle this case by adding additional entries in the map table,
> > with a single entry length.
>
> I understand that, and it works, but I don't see why the definition has
> to be that the offset is added to the "final cell".
Because the cells of a specifier form a single opaque big-endian value.
Were DT little-endian it would be the "first cell". To be pedantic, both
of those descriptions are technically wrong because they fail to account
for overflow and carry up into the next cell (in whichever direction).
> Why can't it be
> an iommu specific definition that makes sense for a given IOMMU?
Because the implementation would then become a nightmarish rabbit-warren
of special-cases, largely defeating the point of a *generic* binding. At
the very least it'd have to chase every phandle and determine its
compatible just to work out what to do with any given specifier - merely
thinking about the complexity of the error handling for the number of
additional failure modes that introduces is enough to put me off.
Robin.
>
> Stuart
>
^ permalink raw reply
* Re: [PATCH] ARM: dts: sun8i-q8-common: enable bluetooth on SDIO Wi-Fi
From: Icenowy Zheng @ 2016-12-16 14:40 UTC (permalink / raw)
To: Maxime Ripard
Cc: Hans de Goede, Chen-Yu Tsai, linux-kernel,
linux-arm-kernel@lists.infradead.org, devicetree@vger.kernel.org
In-Reply-To: <20161216124748.rkvnnlo4x5onzpvk@lukather>
16.12.2016, 20:47, "Maxime Ripard" <maxime.ripard@free-electrons.com>:
> On Fri, Dec 09, 2016 at 07:49:00PM +0800, Icenowy Zheng wrote:
>> 2016年12月9日 下午4:07于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
>> >
>> > On Tue, Dec 06, 2016 at 04:08:38PM +0800, Icenowy Zheng wrote:
>> > > Some SDIO Wi-Fi chips (such as RTL8703AS) have a UART bluetooth, which
>> > > has a dedicated enable pin (PL8 in the reference design).
>> > >
>> > > Enable the pin in the same way as the WLAN enable pins.
>> > >
>> > > Tested on an A33 Q8 tablet with RTL8703AS.
>> > >
>> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
>> > > ---
>> > >
>> > > This patch should be coupled with the uart1 node patch I send before:
>> > > http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/471997.html
>> > >
>> > > For RTL8703AS, the rtl8723bs bluetooth code is used, which can be retrieve from:
>> > > https://github.com/lwfinger/rtl8723bs_bt
>> > >
>> > > arch/arm/boot/dts/sun8i-q8-common.dtsi | 2 +-
>> > > 1 file changed, 1 insertion(+), 1 deletion(-)
>> > >
>> > > diff --git a/arch/arm/boot/dts/sun8i-q8-common.dtsi b/arch/arm/boot/dts/sun8i-q8-common.dtsi
>> > > index c676940..4aeb5bb 100644
>> > > --- a/arch/arm/boot/dts/sun8i-q8-common.dtsi
>> > > +++ b/arch/arm/boot/dts/sun8i-q8-common.dtsi
>> > > @@ -88,7 +88,7 @@
>> > >
>> > > &r_pio {
>> > > wifi_pwrseq_pin_q8: wifi_pwrseq_pin@0 {
>> > > - pins = "PL6", "PL7", "PL11";
>> > > + pins = "PL6", "PL7", "PL8", "PL11";
>> > > function = "gpio_in";
>> > > bias-pull-up;
>> > > };
>> >
>> > There's several things wrong here. The first one is that you rely
>> > solely on the pinctrl state to maintain a reset line. This is very
>> > fragile (especially since the GPIO pinctrl state are likely to go away
>> > at some point), but it also means that if your driver wants to recover
>> > from that situation at some point, it won't work.
>> >
>> > The other one is that the bluetooth and wifi chips are two devices in
>> > linux, and you assign that pin to the wrong device (wifi).
>> >
>> > rfkill-gpio is made just for that, so please use it.
>>
>> The GPIO is not for the radio, but for the full Bluetooth part.
>
> I know.
>
>> If it's set to 0, then the bluetooth part will reset, and the
>> hciattach will fail.
>
> Both rfkill-gpio and rfkill-regulator will shutdown when called
> (either by poking the reset pin or shutting down the regulator), so
> that definitely seems like an expected behavior to put the device in
> reset.
>
>> The BSP uses this as a rfkill, and the result is that the bluetooth
>> on/off switch do not work properly.
>
> Then rfkill needs fixing, but working around it by hoping that the
> core will probe an entirely different device, and enforcing a default
> that the rest of the kernel might or might not change is both fragile
> and wrong.
I think a rfkill-gpio here works just like the BSP rfkill...
The real problem is that the Realtek UART bluetooth driver is a userspace
program (a modified hciattach), which is not capable of the GPIO reset...
>
> Maxime
>
> --
> Maxime Ripard, Free Electrons
> Embedded Linux and Kernel engineering
> http://free-electrons.com
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] Documentation: dt: Explicitly mark Samsung Exynos SoC bindings as unstable
From: Bartlomiej Zolnierkiewicz @ 2016-12-16 14:27 UTC (permalink / raw)
To: Marek Szyprowski
Cc: linux-samsung-soc, devicetree, linux-arm-kernel, Rob Herring,
Mark Rutland, Krzysztof Kozlowski, Javier Martinez Canillas,
Kukjin Kim, Inki Dae, Seung-Woo Kim, Chanwoo Choi,
Sylwester Nawrocki
In-Reply-To: <1481897676-13578-1-git-send-email-m.szyprowski@samsung.com>
Hi,
On Friday, December 16, 2016 03:14:36 PM Marek Szyprowski wrote:
> Samsung Exynos SoCs and boards related bindings evolved since the initial
> introduction, but initially the bindings were minimal and a bit incomplete
> (they never described all the hardware modules available in the SoCs).
> Since then some significant (not fully compatible) changes have been
> already committed a few times (like gpio replaced by pinctrl, display ddc,
> mfc reserved memory, some core clocks added to various hardware modules,
> added more required nodes).
>
> On the other side there are no boards which have device tree embedded in
> the bootloader. Device tree blob is always compiled from the kernel tree
> and updated together with the kernel image.
>
> Thus to avoid further adding a bunch of workarounds for old/missing
> bindings and allow to make cleanup of the existing code and device tree
> files, lets mark Samsung Exynos SoC platform bindings as unstable. This
> means that bindings can may change at any time and users should use the
> dtb file compiled from the same kernel source tree as the kernel image.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
This change is long overdue..
Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> ---
> Documentation/devicetree/bindings/arm/samsung/exynos.txt | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/arm/samsung/exynos.txt
>
> diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos.txt b/Documentation/devicetree/bindings/arm/samsung/exynos.txt
> new file mode 100644
> index 000000000000..0c606f4c6e85
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/arm/samsung/exynos.txt
> @@ -0,0 +1,12 @@
> +Samsung Exynos SoC Family Device Tree Bindings
> +---------------------------------------------------------------
> +
> +Work in progress statement:
> +
> +Device tree files and bindings applying to Samsung Exynos SoCs and boards are
> +considered "unstable". Any Samsung Exynos device tree binding may change at any
> +time. Be sure to use a device tree binary and a kernel image generated from the
> +same source tree.
> +
> +Please refer to Documentation/devicetree/bindings/ABI.txt for a definition of a
> +stable binding/ABI.
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics
^ permalink raw reply
* RE: RFC: extend iommu-map binding to support #iommu-cells > 1
From: Stuart Yoder @ 2016-12-16 14:21 UTC (permalink / raw)
To: Mark Rutland
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
will.deacon-5wv7dgnIgG8@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
In-Reply-To: <20161216113914.GC20265@leverpostej>
> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland-5wv7dgnIgG8@public.gmane.org]
> Sent: Friday, December 16, 2016 5:39 AM
> To: Stuart Yoder <stuart.yoder-3arQi8VN3Tc@public.gmane.org>
> Cc: robin.murphy-5wv7dgnIgG8@public.gmane.org; will.deacon-5wv7dgnIgG8@public.gmane.org; robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; Bharat Bhushan
> <bharat.bhushan-3arQi8VN3Tc@public.gmane.org>; Nipun Gupta <nipun.gupta-3arQi8VN3Tc@public.gmane.org>; Diana Madalina Craciun
> <diana.craciun-3arQi8VN3Tc@public.gmane.org>; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> Subject: Re: RFC: extend iommu-map binding to support #iommu-cells > 1
>
> On Fri, Dec 16, 2016 at 02:36:57AM +0000, Stuart Yoder wrote:
> > For context, please see the thread:
> > https://emea01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.spinics.net%2Flists%2Farm-
> kernel%2Fmsg539066.html&data=01%7C01%7Cstuart.yoder%40nxp.com%7C0464e12bddfd42e0f0a508d425a847cb%7C686ea
> 1d3bc2b4c6fa92cd99c5c301635%7C0&sdata=KnzeOlbts271GwD1cpx2FLMsKnxw%2FOdiGVTp6%2BKFrbM%3D&reserved=0
> >
> > The existing iommu-map binding did not account for the situation where
> > #iommu-cells == 2, as permitted in the ARM SMMU binding. The 2nd cell
> > of the IOMMU specifier being the SMR mask. The existing binding defines
> > the mapping as:
> > Any RID r in the interval [rid-base, rid-base + length) is associated with
> > the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base).
> >
> > ...and that does not work if iommu-base is 2 cells, the second being the
> > SMR mask.
> >
> > While this can be worked around by always having length=1, it seems we
> > can get this cleaned up by updating the binding definition for iommu-map.
>
> To reiterate, I'm very much not keen on the pci-iommu binding having
> knowledge of the decomposition of iommu-specifiers from other bindings.
With the current definition of iommu-map we already have baked in an
assumption that an iommu-specifier is a value that can be incremented
by 1 to get to the next sequential specifier. So the binding already
has "knowledge" that applies in most, but not all cases.
The generic iommu binding also defines a case where #iommu-cells=4
for some IOMMUs.
Since the ARM SMMU is a special case, perhaps the intepretation
of an iommu-specifier in the context of iommu-map should be moved
into the SMMU binding.
> As mentioned previously, there's an intended interpretation [1] that we
> need to fix up the pci-iommu binding with. With that, I don't think it's
> even necessary to bodge iommu-cells = <1> AFAICT.
You had said in the previous thread:
> I had intended that the offset was added to the final cell of the
> iommu-specifier (i.e. that the iommu-specifier was treated as a large
> number).
> You can handle this case by adding additional entries in the map table,
> with a single entry length.
I understand that, and it works, but I don't see why the definition has
to be that the offset is added to the "final cell". Why can't it be
an iommu specific definition that makes sense for a given IOMMU?
Stuart
^ permalink raw reply
* Re: [PATCH] Documentation: dt: Explicitly mark Samsung Exynos SoC bindings as unstable
From: Javier Martinez Canillas @ 2016-12-16 14:18 UTC (permalink / raw)
To: Marek Szyprowski, linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Rob Herring, Mark Rutland, Krzysztof Kozlowski,
Bartlomiej Zolnierkiewicz, Kukjin Kim, Inki Dae, Seung-Woo Kim,
Chanwoo Choi, Sylwester Nawrocki
In-Reply-To: <1481897676-13578-1-git-send-email-m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
Hello Marek,
On 12/16/2016 11:14 AM, Marek Szyprowski wrote:
> Samsung Exynos SoCs and boards related bindings evolved since the initial
> introduction, but initially the bindings were minimal and a bit incomplete
> (they never described all the hardware modules available in the SoCs).
> Since then some significant (not fully compatible) changes have been
> already committed a few times (like gpio replaced by pinctrl, display ddc,
> mfc reserved memory, some core clocks added to various hardware modules,
> added more required nodes).
>
> On the other side there are no boards which have device tree embedded in
> the bootloader. Device tree blob is always compiled from the kernel tree
> and updated together with the kernel image.
>
> Thus to avoid further adding a bunch of workarounds for old/missing
> bindings and allow to make cleanup of the existing code and device tree
> files, lets mark Samsung Exynos SoC platform bindings as unstable. This
> means that bindings can may change at any time and users should use the
> dtb file compiled from the same kernel source tree as the kernel image.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> ---
I completely agree with you on this.
Reviewed-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 2/6] [media] rc-main: split setup and unregister functions
From: Sean Young @ 2016-12-16 14:16 UTC (permalink / raw)
To: Andi Shyti
Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Richard Purdie,
Jacek Anaszewski, Heiner Kallweit, linux-media, devicetree,
linux-leds, linux-kernel, Andi Shyti
In-Reply-To: <20161216121026.GA31618@gofer.mess.org>
Hi Andi,
On Fri, Dec 16, 2016 at 12:10:26PM +0000, Sean Young wrote:
> Sorry to add to your woes, but there are some checkpatch warnings and
> errors. Please can you correct these. One is below.
Actually, the changes are pretty minor, I can fix them up before sending
them to Mauro. Sorry for bothering you.
Sean
^ permalink raw reply
* [PATCH] Documentation: dt: Explicitly mark Samsung Exynos SoC bindings as unstable
From: Marek Szyprowski @ 2016-12-16 14:14 UTC (permalink / raw)
To: linux-samsung-soc, devicetree, linux-arm-kernel
Cc: Mark Rutland, Chanwoo Choi, Bartlomiej Zolnierkiewicz,
Seung-Woo Kim, Krzysztof Kozlowski, Inki Dae,
Javier Martinez Canillas, Rob Herring, Kukjin Kim,
Sylwester Nawrocki, Marek Szyprowski
Samsung Exynos SoCs and boards related bindings evolved since the initial
introduction, but initially the bindings were minimal and a bit incomplete
(they never described all the hardware modules available in the SoCs).
Since then some significant (not fully compatible) changes have been
already committed a few times (like gpio replaced by pinctrl, display ddc,
mfc reserved memory, some core clocks added to various hardware modules,
added more required nodes).
On the other side there are no boards which have device tree embedded in
the bootloader. Device tree blob is always compiled from the kernel tree
and updated together with the kernel image.
Thus to avoid further adding a bunch of workarounds for old/missing
bindings and allow to make cleanup of the existing code and device tree
files, lets mark Samsung Exynos SoC platform bindings as unstable. This
means that bindings can may change at any time and users should use the
dtb file compiled from the same kernel source tree as the kernel image.
Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
---
Documentation/devicetree/bindings/arm/samsung/exynos.txt | 12 ++++++++++++
1 file changed, 12 insertions(+)
create mode 100644 Documentation/devicetree/bindings/arm/samsung/exynos.txt
diff --git a/Documentation/devicetree/bindings/arm/samsung/exynos.txt b/Documentation/devicetree/bindings/arm/samsung/exynos.txt
new file mode 100644
index 000000000000..0c606f4c6e85
--- /dev/null
+++ b/Documentation/devicetree/bindings/arm/samsung/exynos.txt
@@ -0,0 +1,12 @@
+Samsung Exynos SoC Family Device Tree Bindings
+---------------------------------------------------------------
+
+Work in progress statement:
+
+Device tree files and bindings applying to Samsung Exynos SoCs and boards are
+considered "unstable". Any Samsung Exynos device tree binding may change at any
+time. Be sure to use a device tree binary and a kernel image generated from the
+same source tree.
+
+Please refer to Documentation/devicetree/bindings/ABI.txt for a definition of a
+stable binding/ABI.
--
1.9.1
^ permalink raw reply related
* RE: [PATCH] Docs: dt: Be explicit and consistent in reference to IOMMU specifiers
From: Stuart Yoder @ 2016-12-16 14:08 UTC (permalink / raw)
To: Mark Rutland
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
will.deacon-5wv7dgnIgG8@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
In-Reply-To: <20161216113317.GB20265@leverpostej>
> -----Original Message-----
> From: Mark Rutland [mailto:mark.rutland-5wv7dgnIgG8@public.gmane.org]
> Sent: Friday, December 16, 2016 5:33 AM
> To: Stuart Yoder <stuart.yoder-3arQi8VN3Tc@public.gmane.org>
> Cc: robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org; devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org; joro-zLv9SwRftAIdnm+yROfE0A@public.gmane.org;
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org; will.deacon-5wv7dgnIgG8@public.gmane.org
> Subject: Re: [PATCH] Docs: dt: Be explicit and consistent in reference to IOMMU specifiers
>
> Hi Stuart,
>
> On Thu, Dec 15, 2016 at 06:16:13PM -0600, Stuart Yoder wrote:
> > The generic IOMMU binding says that the meaning of an 'IOMMU specifier'
> > is defined by the binding of a specific SMMU. The ARM SMMU binding
> > never explicitly uses the term 'specifier' at all. Update implicit
> > references to use the explicit term.
> >
> > In the iommu-map binding change references to iommu-specifier to
> > "IOMMU specifier" so we are 100% consistent everywhere with terminology
> > and capitalization.
>
> Elsewhere, we always use lower case "xxx-specifier" or "xxx specifier",
> e.g. Documentation/devicetree/bindings/gpio/gpio.txt defines
> "gpio-specifier", ePAPR defines "interrupt specifier".
>
> Given we're morstly consistent on "iommu-specifier" today,could we
> please jsut update the ARM SMMU binding to match that? If we're going to
> fix the dash mismatch, that's a more general, cross-binding thing.
The notable place where we don't use "iommu-specifier" in in the generic
IOMMU binding itself where we use "IOMMU specifier". You're suggesting
using "iommu-specifier" everywhere including the generic binding? Sounds
fine to me. It's a nit but would like to see it consistent everywhere.
Thanks,
Stuart
^ permalink raw reply
* Re: [PATCH v7 1/2] mtd: nand: add tango NFC dt bindings doc
From: Mark Rutland @ 2016-12-16 13:33 UTC (permalink / raw)
To: Marc Gonzalez
Cc: Arnd Bergmann, Boris Brezillon, linux-mtd, Richard Weinberger, DT,
Rob Herring, Mason, Sebastian Frias, Mans Rullgard, Mark Brown
In-Reply-To: <ee9efd0a-59f5-8c21-0cad-40ca30e3a3e7-y1yR0Z3OICC7zZZRDBGcUA@public.gmane.org>
On Fri, Dec 16, 2016 at 02:26:33PM +0100, Marc Gonzalez wrote:
> On 16/12/2016 11:56, Marc Gonzalez wrote:
> > Should I just use "rxtx" for my driver?
>
> On IRC, Arnd wrote:
>
> "The string for dma-names is not important, it just needs to be documented
> in the binding. If you have hardware specifications, use the name that is
> next to the wire for the dma-request line. "dma-names" is only required so
> you can connect a single dmarq to multiple dma engines (most chips only
> connect each rq to one dmaengine though)."
>
> In my system, peripheral devices are connected to DMA channels via
> a multiplexer called the "host switch box". The documentation calls
> the ports FOO_SBOX and SBOX_FOO.
>
> For MLC NAND Flash controller 0, the ports are mlc_flash0_sbox
> and sbox_mlc_flash0.
>
> Additionally, the DMA driver handles both directions, so the DT only
> defines a single duplex channel.
>
> I originally used "nfc_sbox" (nfc = NAND Flash controller) to stick to
> the HW naming, but I am willing to change it to "rxtx" if that's what
> Boris prefers, (or just "sbox", but Mans pointed out that this was too
> specific, and future HW might do away with the switch box).
"rxtx" is the best option, if you need a name at all.
The name is relative to the device instance, so "nfc_" is unnecessary;
we know the node is an NFC controller. The sbox is part of the SoC
wiring rather than being part of the NFC controller, so the "sbox" part
also shouldn't be part of the name.
Likewise for the NAND flash controller. There, the "0" instance number
also shouldn't have been there -- the name is relative to the instance,
and two instances should use the same names. Too late now, I guess. :(
Thanks,
Mark.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 1/2] mtd: nand: add tango NFC dt bindings doc
From: Marc Gonzalez @ 2016-12-16 13:26 UTC (permalink / raw)
To: Arnd Bergmann, Boris Brezillon
Cc: Mark Rutland, DT, Mans Rullgard, Mason, Richard Weinberger,
Mark Brown, Rob Herring, linux-mtd, Sebastian Frias
In-Reply-To: <d3cbfda6-23bf-4ad1-b1ce-c179b765272f@sigmadesigns.com>
On 16/12/2016 11:56, Marc Gonzalez wrote:
> On 07/11/2016 10:18, Arnd Bergmann wrote:
>> On Tuesday, October 25, 2016 3:15:50 PM CET Marc Gonzalez wrote:
>>> Add the tango NAND Flash Controller dt bindings documentation.
>>>
>>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>>> ---
>>> Documentation/devicetree/bindings/mtd/tango-nand.txt | 38 ++++++++++++++++++++++++++++++
>>> 1 file changed, 38 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/tango-nand.txt b/Documentation/devicetree/bindings/mtd/tango-nand.txt
>>> new file mode 100644
>>> index 000000000000..3cbf95d6595a
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/mtd/tango-nand.txt
>>> @@ -0,0 +1,38 @@
>>> +Sigma Designs Tango4 NAND Flash Controller (NFC)
>>> +
>>> +Required properties:
>>> +
>>> +- compatible: "sigma,smp8758-nand"
>>> +- reg: address/size of nfc_reg, nfc_mem, and pbus_reg
>>> +- dmas: reference to the DMA channel used by the controller
>>> +- dma-names: "nfc_sbox"
>>
>> Drop the "nfc_" prefix here, it seems redundant.
>
> I took a closer look at Documentation/devicetree/bindings and
> arch/arm/boot/dts and it seems the overwhelming nomenclature
> is "rx", "tx" (with variants around these two).
>
> Should I just use "rxtx" for my driver?
On IRC, Arnd wrote:
"The string for dma-names is not important, it just needs to be documented
in the binding. If you have hardware specifications, use the name that is
next to the wire for the dma-request line. "dma-names" is only required so
you can connect a single dmarq to multiple dma engines (most chips only
connect each rq to one dmaengine though)."
In my system, peripheral devices are connected to DMA channels via
a multiplexer called the "host switch box". The documentation calls
the ports FOO_SBOX and SBOX_FOO.
For MLC NAND Flash controller 0, the ports are mlc_flash0_sbox
and sbox_mlc_flash0.
Additionally, the DMA driver handles both directions, so the DT only
defines a single duplex channel.
I originally used "nfc_sbox" (nfc = NAND Flash controller) to stick to
the HW naming, but I am willing to change it to "rxtx" if that's what
Boris prefers, (or just "sbox", but Mans pointed out that this was too
specific, and future HW might do away with the switch box).
I'm open to comments.
Regards.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* [RFC PATCH] iommu/arm-smmu: Add global SMR masking property
From: Robin Murphy @ 2016-12-16 13:19 UTC (permalink / raw)
To: iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: will.deacon-5wv7dgnIgG8, mark.rutland-5wv7dgnIgG8,
stuart.yoder-3arQi8VN3Tc
The current SMR masking support using a 2-cell iommu-specifier is
primarily intended to handle individual masters with large and/or
complex Stream ID assignments; it quickly gets a bit clunky in other SMR
use-cases where we just want to consistently mask out the same part of
every Stream ID (e.g. for MMU-500 configurations where the appended TBU
number gets in the way unnecessarily). Let's add a new property to allow
a single global mask value to better fit the latter situation.
CC: Stuart Yoder <stuart.yoder-3arQi8VN3Tc@public.gmane.org>
Signed-off-by: Robin Murphy <robin.murphy-5wv7dgnIgG8@public.gmane.org>
---
Compile-tested only...
Documentation/devicetree/bindings/iommu/arm,smmu.txt | 8 ++++++++
drivers/iommu/arm-smmu.c | 4 +++-
2 files changed, 11 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
index e862d1485205..98f5cbe5fdb4 100644
--- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
+++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
@@ -60,6 +60,14 @@ conditions.
aliases of secure registers have to be used during
SMMU configuration.
+- stream-match-mask : Specifies a fixed SMR mask value to combine with
+ the Stream ID value from every iommu-specifier. This
+ may be used instead of an "#iommu-cells" value of 2
+ when there is no need for per-master SMR masks, but
+ it is still desired to mask some portion of every
+ Stream ID (e.g. for certain MMU-500 configurations
+ given globally unique external IDs).
+
** Deprecated properties:
- mmu-masters (deprecated in favour of the generic "iommus" binding) :
diff --git a/drivers/iommu/arm-smmu.c b/drivers/iommu/arm-smmu.c
index 8f7281444551..f1abcb7dde36 100644
--- a/drivers/iommu/arm-smmu.c
+++ b/drivers/iommu/arm-smmu.c
@@ -1534,13 +1534,15 @@ static int arm_smmu_domain_set_attr(struct iommu_domain *domain,
static int arm_smmu_of_xlate(struct device *dev, struct of_phandle_args *args)
{
- u32 fwid = 0;
+ u32 mask, fwid = 0;
if (args->args_count > 0)
fwid |= (u16)args->args[0];
if (args->args_count > 1)
fwid |= (u16)args->args[1] << SMR_MASK_SHIFT;
+ else if (!of_property_read_u32(args->np, "stream-match-mask", &mask))
+ fwid |= (u16)mask << SMR_MASK_SHIFT;
return iommu_fwspec_add_ids(dev, &fwid, 1);
}
--
2.10.2.dirty
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* Re: [PATCH] ARM: dts: sun8i-q8-common: enable bluetooth on SDIO Wi-Fi
From: Maxime Ripard @ 2016-12-16 12:47 UTC (permalink / raw)
To: Icenowy Zheng
Cc: Hans de Goede, Chen-Yu Tsai, linux-kernel, linux-arm-kernel,
devicetree
In-Reply-To: <20161213233658.atGuNCNY@smtp1h.mail.yandex.net>
[-- Attachment #1.1: Type: text/plain, Size: 2945 bytes --]
On Fri, Dec 09, 2016 at 07:49:00PM +0800, Icenowy Zheng wrote:
>
> 2016年12月9日 下午4:07于 Maxime Ripard <maxime.ripard@free-electrons.com>写道：
> >
> > On Tue, Dec 06, 2016 at 04:08:38PM +0800, Icenowy Zheng wrote:
> > > Some SDIO Wi-Fi chips (such as RTL8703AS) have a UART bluetooth, which
> > > has a dedicated enable pin (PL8 in the reference design).
> > >
> > > Enable the pin in the same way as the WLAN enable pins.
> > >
> > > Tested on an A33 Q8 tablet with RTL8703AS.
> > >
> > > Signed-off-by: Icenowy Zheng <icenowy@aosc.xyz>
> > > ---
> > >
> > > This patch should be coupled with the uart1 node patch I send before:
> > > http://lists.infradead.org/pipermail/linux-arm-kernel/2016-December/471997.html
> > >
> > > For RTL8703AS, the rtl8723bs bluetooth code is used, which can be retrieve from:
> > > https://github.com/lwfinger/rtl8723bs_bt
> > >
> > > arch/arm/boot/dts/sun8i-q8-common.dtsi | 2 +-
> > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/arch/arm/boot/dts/sun8i-q8-common.dtsi b/arch/arm/boot/dts/sun8i-q8-common.dtsi
> > > index c676940..4aeb5bb 100644
> > > --- a/arch/arm/boot/dts/sun8i-q8-common.dtsi
> > > +++ b/arch/arm/boot/dts/sun8i-q8-common.dtsi
> > > @@ -88,7 +88,7 @@
> > >
> > > &r_pio {
> > > wifi_pwrseq_pin_q8: wifi_pwrseq_pin@0 {
> > > - pins = "PL6", "PL7", "PL11";
> > > + pins = "PL6", "PL7", "PL8", "PL11";
> > > function = "gpio_in";
> > > bias-pull-up;
> > > };
> >
> > There's several things wrong here. The first one is that you rely
> > solely on the pinctrl state to maintain a reset line. This is very
> > fragile (especially since the GPIO pinctrl state are likely to go away
> > at some point), but it also means that if your driver wants to recover
> > from that situation at some point, it won't work.
> >
> > The other one is that the bluetooth and wifi chips are two devices in
> > linux, and you assign that pin to the wrong device (wifi).
> >
> > rfkill-gpio is made just for that, so please use it.
>
> The GPIO is not for the radio, but for the full Bluetooth part.
I know.
> If it's set to 0, then the bluetooth part will reset, and the
> hciattach will fail.
Both rfkill-gpio and rfkill-regulator will shutdown when called
(either by poking the reset pin or shutting down the regulator), so
that definitely seems like an expected behavior to put the device in
reset.
> The BSP uses this as a rfkill, and the result is that the bluetooth
> on/off switch do not work properly.
Then rfkill needs fixing, but working around it by hoping that the
core will probe an entirely different device, and enforcing a default
that the rest of the kernel might or might not change is both fragile
and wrong.
Maxime
--
Maxime Ripard, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v5 2/6] [media] rc-main: split setup and unregister functions
From: Sean Young @ 2016-12-16 12:10 UTC (permalink / raw)
To: Andi Shyti
Cc: Mauro Carvalho Chehab, Rob Herring, Mark Rutland, Richard Purdie,
Jacek Anaszewski, Heiner Kallweit, linux-media, devicetree,
linux-leds, linux-kernel, Andi Shyti
In-Reply-To: <20161216061218.5906-3-andi.shyti@samsung.com>
Hi Andi,
Sorry to add to your woes, but there are some checkpatch warnings and
errors. Please can you correct these. One is below.
Thanks
Sean
On Fri, Dec 16, 2016 at 03:12:14PM +0900, Andi Shyti wrote:
> Move the input device allocation, map and protocol handling to
> different functions.
>
> Signed-off-by: Andi Shyti <andi.shyti@samsung.com>
> Reviewed-by: Sean Young <sean@mess.org>
> ---
> drivers/media/rc/rc-main.c | 143 +++++++++++++++++++++++++--------------------
> 1 file changed, 81 insertions(+), 62 deletions(-)
>
> diff --git a/drivers/media/rc/rc-main.c b/drivers/media/rc/rc-main.c
> index a6bbceb..7cc700d 100644
> --- a/drivers/media/rc/rc-main.c
> +++ b/drivers/media/rc/rc-main.c
> @@ -1436,16 +1436,12 @@ struct rc_dev *devm_rc_allocate_device(struct device *dev,
> }
> EXPORT_SYMBOL_GPL(devm_rc_allocate_device);
>
> -int rc_register_device(struct rc_dev *dev)
> +static int rc_setup_rx_device(struct rc_dev *dev)
> {
> - static bool raw_init = false; /* raw decoders loaded? */
> - struct rc_map *rc_map;
> - const char *path;
> - int attr = 0;
> - int minor;
> int rc;
> + struct rc_map *rc_map;
>
> - if (!dev || !dev->map_name)
> + if (!dev->map_name)
> return -EINVAL;
>
> rc_map = rc_map_get(dev->map_name);
> @@ -1454,6 +1450,19 @@ int rc_register_device(struct rc_dev *dev)
> if (!rc_map || !rc_map->scan || rc_map->size == 0)
> return -EINVAL;
>
> + rc = ir_setkeytable(dev, rc_map);
> + if (rc)
> + return rc;
> +
> + if (dev->change_protocol) {
> + u64 rc_type = (1ll << rc_map->rc_type);
> +
> + rc = dev->change_protocol(dev, &rc_type);
> + if (rc < 0)
> + goto out_table;
> + dev->enabled_protocols = rc_type;
> + }
> +
> set_bit(EV_KEY, dev->input_dev->evbit);
> set_bit(EV_REP, dev->input_dev->evbit);
> set_bit(EV_MSC, dev->input_dev->evbit);
> @@ -1463,6 +1472,61 @@ int rc_register_device(struct rc_dev *dev)
> if (dev->close)
> dev->input_dev->close = ir_close;
>
> + /*
> + * Default delay of 250ms is too short for some protocols, especially
> + * since the timeout is currently set to 250ms. Increase it to 500ms,
> + * to avoid wrong repetition of the keycodes. Note that this must be
> + * set after the call to input_register_device().
> + */
> + dev->input_dev->rep[REP_DELAY] = 500;
> +
> + /*
> + * As a repeat event on protocols like RC-5 and NEC take as long as
> + * 110/114ms, using 33ms as a repeat period is not the right thing
> + * to do.
> + */
> + dev->input_dev->rep[REP_PERIOD] = 125;
> +
> + dev->input_dev->dev.parent = &dev->dev;
> + memcpy(&dev->input_dev->id, &dev->input_id, sizeof(dev->input_id));
> + dev->input_dev->phys = dev->input_phys;
> + dev->input_dev->name = dev->input_name;
> +
> + /* rc_open will be called here */
> + rc = input_register_device(dev->input_dev);
> + if (rc)
> + goto out_table;
> +
> + return 0;
> +
> +out_table:
> + ir_free_table(&dev->rc_map);
> +
> + return rc;
> +}
> +
> +static void rc_free_rx_device(struct rc_dev *dev)
> +{
> + if (!dev)
> + return;
> +
> + ir_free_table(&dev->rc_map);
> +
> + input_unregister_device(dev->input_dev);
> + dev->input_dev = NULL;
> +}
> +
> +int rc_register_device(struct rc_dev *dev)
> +{
> + static bool raw_init = false; /* raw decoders loaded? */
ERROR: do not initialise statics to false
#110: FILE: drivers/media/rc/rc-main.c:1741:
+ static bool raw_init = false; /* raw decoders loaded? */
> + const char *path;
> + int attr = 0;
> + int minor;
> + int rc;
> +
> + if (!dev)
> + return -EINVAL;
> +
> minor = ida_simple_get(&rc_ida, 0, RC_DEV_MAX, GFP_KERNEL);
> if (minor < 0)
> return minor;
> @@ -1486,39 +1550,15 @@ int rc_register_device(struct rc_dev *dev)
> if (rc)
> goto out_unlock;
>
> - rc = ir_setkeytable(dev, rc_map);
> - if (rc)
> - goto out_dev;
> -
> - dev->input_dev->dev.parent = &dev->dev;
> - memcpy(&dev->input_dev->id, &dev->input_id, sizeof(dev->input_id));
> - dev->input_dev->phys = dev->input_phys;
> - dev->input_dev->name = dev->input_name;
> -
> - rc = input_register_device(dev->input_dev);
> - if (rc)
> - goto out_table;
> -
> - /*
> - * Default delay of 250ms is too short for some protocols, especially
> - * since the timeout is currently set to 250ms. Increase it to 500ms,
> - * to avoid wrong repetition of the keycodes. Note that this must be
> - * set after the call to input_register_device().
> - */
> - dev->input_dev->rep[REP_DELAY] = 500;
> -
> - /*
> - * As a repeat event on protocols like RC-5 and NEC take as long as
> - * 110/114ms, using 33ms as a repeat period is not the right thing
> - * to do.
> - */
> - dev->input_dev->rep[REP_PERIOD] = 125;
> -
> path = kobject_get_path(&dev->dev.kobj, GFP_KERNEL);
> dev_info(&dev->dev, "%s as %s\n",
> dev->input_name ?: "Unspecified device", path ?: "N/A");
> kfree(path);
>
> + rc = rc_setup_rx_device(dev);
> + if (rc)
> + goto out_dev;
> +
> if (dev->driver_type == RC_DRIVER_IR_RAW) {
> if (!raw_init) {
> request_module_nowait("ir-lirc-codec");
> @@ -1526,36 +1566,20 @@ int rc_register_device(struct rc_dev *dev)
> }
> rc = ir_raw_event_register(dev);
> if (rc < 0)
> - goto out_input;
> - }
> -
> - if (dev->change_protocol) {
> - u64 rc_type = (1ll << rc_map->rc_type);
> - rc = dev->change_protocol(dev, &rc_type);
> - if (rc < 0)
> - goto out_raw;
> - dev->enabled_protocols = rc_type;
> + goto out_rx;
> }
>
> /* Allow the RC sysfs nodes to be accessible */
> atomic_set(&dev->initialized, 1);
>
> - IR_dprintk(1, "Registered rc%u (driver: %s, remote: %s, mode %s)\n",
> + IR_dprintk(1, "Registered rc%u (driver: %s)\n",
> dev->minor,
> - dev->driver_name ? dev->driver_name : "unknown",
> - rc_map->name ? rc_map->name : "unknown",
> - dev->driver_type == RC_DRIVER_IR_RAW ? "raw" : "cooked");
> + dev->driver_name ? dev->driver_name : "unknown");
>
> return 0;
>
> -out_raw:
> - if (dev->driver_type == RC_DRIVER_IR_RAW)
> - ir_raw_event_unregister(dev);
> -out_input:
> - input_unregister_device(dev->input_dev);
> - dev->input_dev = NULL;
> -out_table:
> - ir_free_table(&dev->rc_map);
> +out_rx:
> + rc_free_rx_device(dev);
> out_dev:
> device_del(&dev->dev);
> out_unlock:
> @@ -1601,12 +1625,7 @@ void rc_unregister_device(struct rc_dev *dev)
> if (dev->driver_type == RC_DRIVER_IR_RAW)
> ir_raw_event_unregister(dev);
>
> - /* Freeing the table should also call the stop callback */
> - ir_free_table(&dev->rc_map);
> - IR_dprintk(1, "Freed keycode table\n");
> -
> - input_unregister_device(dev->input_dev);
> - dev->input_dev = NULL;
> + rc_free_rx_device(dev);
>
> device_del(&dev->dev);
>
> --
> 2.10.2
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-media" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 3/6] [media] rc-core: add support for IR raw transmitters
From: kbuild test robot @ 2016-12-16 11:41 UTC (permalink / raw)
Cc: kbuild-all-JC7UmRfGjtg, Mauro Carvalho Chehab, Sean Young,
Rob Herring, Mark Rutland, Richard Purdie, Jacek Anaszewski,
Heiner Kallweit, linux-media-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-leds-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andi Shyti, Andi Shyti
In-Reply-To: <20161216061218.5906-4-andi.shyti-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 4574 bytes --]
Hi Andi,
[auto build test WARNING on linuxtv-media/master]
[also build test WARNING on next-20161216]
[cannot apply to v4.9]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Andi-Shyti/Add-support-for-IR-transmitters/20161216-144204
base: git://linuxtv.org/media_tree.git master
reproduce: make htmldocs
All warnings (new ones prefixed by >>):
make[3]: warning: jobserver unavailable: using -j1. Add '+' to parent make rule.
include/linux/init.h:1: warning: no structured comments found
include/linux/workqueue.h:392: warning: No description found for parameter '...'
include/linux/workqueue.h:392: warning: Excess function parameter 'args' description in 'alloc_workqueue'
include/linux/workqueue.h:413: warning: No description found for parameter '...'
include/linux/workqueue.h:413: warning: Excess function parameter 'args' description in 'alloc_ordered_workqueue'
include/linux/kthread.h:26: warning: No description found for parameter '...'
kernel/sys.c:1: warning: no structured comments found
drivers/dma-buf/seqno-fence.c:1: warning: no structured comments found
include/linux/fence-array.h:61: warning: No description found for parameter 'fence'
include/sound/core.h:324: warning: No description found for parameter '...'
include/sound/core.h:335: warning: No description found for parameter '...'
include/sound/core.h:388: warning: No description found for parameter '...'
drivers/media/dvb-core/dvb_frontend.h:677: warning: No description found for parameter 'refcount'
include/media/media-entity.h:1054: warning: No description found for parameter '...'
>> include/media/rc-core.h:39: warning: bad line: driver requires pulse/space data sequence.
include/net/mac80211.h:3207: ERROR: Unexpected indentation.
include/net/mac80211.h:3210: WARNING: Block quote ends without a blank line; unexpected unindent.
include/net/mac80211.h:3212: ERROR: Unexpected indentation.
include/net/mac80211.h:3213: WARNING: Block quote ends without a blank line; unexpected unindent.
include/net/mac80211.h:1772: ERROR: Unexpected indentation.
include/net/mac80211.h:1776: WARNING: Block quote ends without a blank line; unexpected unindent.
kernel/sched/fair.c:7259: WARNING: Inline emphasis start-string without end-string.
kernel/time/timer.c:1240: ERROR: Unexpected indentation.
kernel/time/timer.c:1242: ERROR: Unexpected indentation.
kernel/time/timer.c:1243: WARNING: Block quote ends without a blank line; unexpected unindent.
include/linux/wait.h:121: WARNING: Block quote ends without a blank line; unexpected unindent.
include/linux/wait.h:124: ERROR: Unexpected indentation.
include/linux/wait.h:126: WARNING: Block quote ends without a blank line; unexpected unindent.
kernel/time/hrtimer.c:1021: WARNING: Block quote ends without a blank line; unexpected unindent.
kernel/signal.c:317: WARNING: Inline literal start-string without end-string.
drivers/base/firmware_class.c:1348: WARNING: Bullet list ends without a blank line; unexpected unindent.
drivers/message/fusion/mptbase.c:5054: WARNING: Definition list ends without a blank line; unexpected unindent.
drivers/tty/serial/serial_core.c:1893: WARNING: Definition list ends without a blank line; unexpected unindent.
include/linux/spi/spi.h:369: ERROR: Unexpected indentation.
WARNING: dvipng command 'dvipng' cannot be run (needed for math display), check the imgmath_dvipng setting
vim +39 include/media/rc-core.h
23 #include <media/rc-map.h>
24
25 extern int rc_core_debug;
26 #define IR_dprintk(level, fmt, ...) \
27 do { \
28 if (rc_core_debug >= level) \
29 printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); \
30 } while (0)
31
32 /**
33 * enum rc_driver_type - type of the RC output
34 *
35 * @RC_DRIVER_SCANCODE: Driver or hardware generates a scancode
36 * @RC_DRIVER_IR_RAW: Driver or hardware generates pulse/space sequences.
37 * It needs a Infra-Red pulse/space decoder
38 * @RC_DRIVER_IR_RAW_TX: Device transmitter only,
> 39 driver requires pulse/space data sequence.
40 */
41 enum rc_driver_type {
42 RC_DRIVER_SCANCODE = 0,
43 RC_DRIVER_IR_RAW,
44 RC_DRIVER_IR_RAW_TX,
45 };
46
47 /**
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 6425 bytes --]
^ permalink raw reply
* Re: RFC: extend iommu-map binding to support #iommu-cells > 1
From: Mark Rutland @ 2016-12-16 11:39 UTC (permalink / raw)
To: Stuart Yoder
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
will.deacon-5wv7dgnIgG8@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
In-Reply-To: <VI1PR0401MB2638DE2D30F8423F6F8C7A6E8D9C0-9IDQY6o3qQjcXZ0H4ZLnAo3W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
On Fri, Dec 16, 2016 at 02:36:57AM +0000, Stuart Yoder wrote:
> For context, please see the thread:
> https://www.spinics.net/lists/arm-kernel/msg539066.html
>
> The existing iommu-map binding did not account for the situation where
> #iommu-cells == 2, as permitted in the ARM SMMU binding. The 2nd cell
> of the IOMMU specifier being the SMR mask. The existing binding defines
> the mapping as:
> Any RID r in the interval [rid-base, rid-base + length) is associated with
> the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base).
>
> ...and that does not work if iommu-base is 2 cells, the second being the
> SMR mask.
>
> While this can be worked around by always having length=1, it seems we
> can get this cleaned up by updating the binding definition for iommu-map.
To reiterate, I'm very much not keen on the pci-iommu binding having
knowledge of the decomposition of iommu-specifiers from other bindings.
As mentioned previously, there's an intended interpretation [1] that we
need to fix up the pci-iommu binding with. With that, I don't think it's
even necessary to bodge iommu-cells = <1> AFAICT.
I'm sorry that the patch I suggested never materialized; let me figure
out the wording now...
Thanks,
Mark.
[1] https://www.spinics.net/lists/arm-kernel/msg539357.html
>
> See patch below. Thoughts?
>
> Thanks,
> Stuart
>
> -------------------------------------------------------------------------
>
> diff --git a/Documentation/devicetree/bindings/pci/pci-iommu.txt b/Documentation/devicetree/bindings/pci/pci-iommu.txt
> index 56c8296..e81b461 100644
> --- a/Documentation/devicetree/bindings/pci/pci-iommu.txt
> +++ b/Documentation/devicetree/bindings/pci/pci-iommu.txt
> @@ -38,8 +38,20 @@ Optional properties
> The property is an arbitrary number of tuples of
> (rid-base,iommu,iommu-base,length).
>
> - Any RID r in the interval [rid-base, rid-base + length) is associated with
> - the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base).
> + If the associated IOMMU has an #iommu-cells value of 1, any RID r in the
> + interval [rid-base, rid-base + length) is associated with the listed IOMMU,
> + with the iommu-specifier (r - rid-base + iommu-base).
> +
> + ARM SMMU Note:
> + The ARM SMMU binding permits an #iommu-cells value of 2 and in this
> + case defines an IOMMU specifier to be: (stream-id,smr-mask)
> +
> + In an iommu-map this means the iommu-base consists of 2 cells:
> + (rid-base,iommu,[stream-id,smr-mask],length).
> +
> + In this case the RID to IOMMU specifier mapping is defined to be:
> + any RID r in the interval [rid-base, rid-base + length) is associated
> + with the listed IOMMU, with the iommu-specifier (r - rid-base + stream-id).
>
> - iommu-map-mask: A mask to be applied to each Requester ID prior to being
> mapped to an iommu-specifier per the iommu-map property.
>
>
>
^ permalink raw reply
* Re: RFC: extend iommu-map binding to support #iommu-cells > 1
From: Robin Murphy @ 2016-12-16 11:34 UTC (permalink / raw)
To: Stuart Yoder, Mark Rutland,
will.deacon-5wv7dgnIgG8@public.gmane.org
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org
In-Reply-To: <VI1PR0401MB2638DE2D30F8423F6F8C7A6E8D9C0-9IDQY6o3qQjcXZ0H4ZLnAo3W/0Ik+aLCnBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
On 16/12/16 02:36, Stuart Yoder wrote:
> For context, please see the thread:
> https://www.spinics.net/lists/arm-kernel/msg539066.html
>
> The existing iommu-map binding did not account for the situation where
> #iommu-cells == 2, as permitted in the ARM SMMU binding. The 2nd cell
> of the IOMMU specifier being the SMR mask. The existing binding defines
> the mapping as:
> Any RID r in the interval [rid-base, rid-base + length) is associated with
> the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base).
>
> ...and that does not work if iommu-base is 2 cells, the second being the
> SMR mask.
>
> While this can be worked around by always having length=1, it seems we
> can get this cleaned up by updating the binding definition for iommu-map.
>
> See patch below. Thoughts?
I really don't think defining a generic binding to have a magic
non-standard meaning for one specific use-case is the right way to go.
Give me a moment to spin the patch I reckon you actually want...
Robin.
>
> Thanks,
> Stuart
>
> -------------------------------------------------------------------------
>
> diff --git a/Documentation/devicetree/bindings/pci/pci-iommu.txt b/Documentation/devicetree/bindings/pci/pci-iommu.txt
> index 56c8296..e81b461 100644
> --- a/Documentation/devicetree/bindings/pci/pci-iommu.txt
> +++ b/Documentation/devicetree/bindings/pci/pci-iommu.txt
> @@ -38,8 +38,20 @@ Optional properties
> The property is an arbitrary number of tuples of
> (rid-base,iommu,iommu-base,length).
>
> - Any RID r in the interval [rid-base, rid-base + length) is associated with
> - the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base).
> + If the associated IOMMU has an #iommu-cells value of 1, any RID r in the
> + interval [rid-base, rid-base + length) is associated with the listed IOMMU,
> + with the iommu-specifier (r - rid-base + iommu-base).
> +
> + ARM SMMU Note:
> + The ARM SMMU binding permits an #iommu-cells value of 2 and in this
> + case defines an IOMMU specifier to be: (stream-id,smr-mask)
> +
> + In an iommu-map this means the iommu-base consists of 2 cells:
> + (rid-base,iommu,[stream-id,smr-mask],length).
> +
> + In this case the RID to IOMMU specifier mapping is defined to be:
> + any RID r in the interval [rid-base, rid-base + length) is associated
> + with the listed IOMMU, with the iommu-specifier (r - rid-base + stream-id).
>
> - iommu-map-mask: A mask to be applied to each Requester ID prior to being
> mapped to an iommu-specifier per the iommu-map property.
>
>
>
> _______________________________________________
> iommu mailing list
> iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linuxfoundation.org/mailman/listinfo/iommu
>
^ permalink raw reply
* Re: [PATCH] Docs: dt: Be explicit and consistent in reference to IOMMU specifiers
From: Mark Rutland @ 2016-12-16 11:33 UTC (permalink / raw)
To: Stuart Yoder
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA, will.deacon-5wv7dgnIgG8,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
iommu-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A
In-Reply-To: <1481847373-2602-1-git-send-email-stuart.yoder-3arQi8VN3Tc@public.gmane.org>
Hi Stuart,
On Thu, Dec 15, 2016 at 06:16:13PM -0600, Stuart Yoder wrote:
> The generic IOMMU binding says that the meaning of an 'IOMMU specifier'
> is defined by the binding of a specific SMMU. The ARM SMMU binding
> never explicitly uses the term 'specifier' at all. Update implicit
> references to use the explicit term.
>
> In the iommu-map binding change references to iommu-specifier to
> "IOMMU specifier" so we are 100% consistent everywhere with terminology
> and capitalization.
Elsewhere, we always use lower case "xxx-specifier" or "xxx specifier",
e.g. Documentation/devicetree/bindings/gpio/gpio.txt defines
"gpio-specifier", ePAPR defines "interrupt specifier".
Given we're morstly consistent on "iommu-specifier" today, could we
please jsut update the ARM SMMU binding to match that? If we're going to
fix the dash mismatch, that's a more general, cross-binding thing.
Thanks,
Mark.
> Signed-off-by: Stuart Yoder <stuart.yoder-3arQi8VN3Tc@public.gmane.org>
> ---
> Documentation/devicetree/bindings/iommu/arm,smmu.txt | 10 +++++-----
> Documentation/devicetree/bindings/pci/pci-iommu.txt | 6 +++---
> 2 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/iommu/arm,smmu.txt b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> index e862d148..6cdf32d 100644
> --- a/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> +++ b/Documentation/devicetree/bindings/iommu/arm,smmu.txt
> @@ -36,15 +36,15 @@ conditions.
> combined interrupt, it must be listed multiple times.
>
> - #iommu-cells : See Documentation/devicetree/bindings/iommu/iommu.txt
> - for details. With a value of 1, each "iommus" entry
> + for details. With a value of 1, each IOMMU specifier
> represents a distinct stream ID emitted by that device
> into the relevant SMMU.
>
> SMMUs with stream matching support and complex masters
> - may use a value of 2, where the second cell represents
> - an SMR mask to combine with the ID in the first cell.
> - Care must be taken to ensure the set of matched IDs
> - does not result in conflicts.
> + may use a value of 2, where the second cell of the
> + IOMMU specifier represents an SMR mask to combine with
> + the ID in the first cell. Care must be taken to ensure
> + the set of matched IDs does not result in conflicts.
>
> ** System MMU optional properties:
>
> diff --git a/Documentation/devicetree/bindings/pci/pci-iommu.txt b/Documentation/devicetree/bindings/pci/pci-iommu.txt
> index 56c8296..0def586 100644
> --- a/Documentation/devicetree/bindings/pci/pci-iommu.txt
> +++ b/Documentation/devicetree/bindings/pci/pci-iommu.txt
> @@ -32,17 +32,17 @@ PCI root complex
> Optional properties
> -------------------
>
> -- iommu-map: Maps a Requester ID to an IOMMU and associated iommu-specifier
> +- iommu-map: Maps a Requester ID to an IOMMU and associated IOMMU specifier
> data.
>
> The property is an arbitrary number of tuples of
> (rid-base,iommu,iommu-base,length).
>
> Any RID r in the interval [rid-base, rid-base + length) is associated with
> - the listed IOMMU, with the iommu-specifier (r - rid-base + iommu-base).
> + the listed IOMMU, with the IOMMU specifier (r - rid-base + iommu-base).
>
> - iommu-map-mask: A mask to be applied to each Requester ID prior to being
> - mapped to an iommu-specifier per the iommu-map property.
> + mapped to an IOMMU specifier per the iommu-map property.
>
>
> Example (1)
> --
> 1.9.0
>
^ permalink raw reply
* Re: [PATCH v2] mmc: sdhci-cadence: add Socionext UniPhier specific compatible string
From: Ulf Hansson @ 2016-12-16 11:09 UTC (permalink / raw)
To: Masahiro Yamada, Rob Herring
Cc: linux-mmc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Adrian Hunter,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Rob Herring,
Mark Rutland
In-Reply-To: <1481681446-29832-1-git-send-email-yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
On 14 December 2016 at 03:10, Masahiro Yamada
<yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org> wrote:
> Add a Socionext SoC specific compatible (suggested by Rob Herring).
>
> No SoC specific data are associated with the compatible strings for
> now, but other SoC vendors may use this IP and want to differentiate
> IP variants in the future.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro-uWyLwvC0a2jby3iVrkZq2A@public.gmane.org>
This looks good to me. I intend to apply this as fix, although
deferring to first get an ack from Rob - to make sure we get it right
this time. :-)
Kind regards
Uffe
> ---
>
> Changes in v2:
> - Add "uniphier" to the compatible to make it more SoC-specific
>
> Documentation/devicetree/bindings/mmc/sdhci-cadence.txt | 6 ++++--
> drivers/mmc/host/sdhci-cadence.c | 1 +
> 2 files changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
> index 750374f..c0f37cb 100644
> --- a/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
> +++ b/Documentation/devicetree/bindings/mmc/sdhci-cadence.txt
> @@ -1,7 +1,9 @@
> * Cadence SD/SDIO/eMMC Host Controller
>
> Required properties:
> -- compatible: should be "cdns,sd4hc".
> +- compatible: should be one of the following:
> + "cdns,sd4hc" - default of the IP
> + "socionext,uniphier-sd4hc" - for Socionext UniPhier SoCs
> - reg: offset and length of the register set for the device.
> - interrupts: a single interrupt specifier.
> - clocks: phandle to the input clock.
> @@ -19,7 +21,7 @@ if supported. See mmc.txt for details.
>
> Example:
> emmc: sdhci@5a000000 {
> - compatible = "cdns,sd4hc";
> + compatible = "socionext,uniphier-sd4hc", "cdns,sd4hc";
> reg = <0x5a000000 0x400>;
> interrupts = <0 78 4>;
> clocks = <&clk 4>;
> diff --git a/drivers/mmc/host/sdhci-cadence.c b/drivers/mmc/host/sdhci-cadence.c
> index 1501cfd..4b0ecb9 100644
> --- a/drivers/mmc/host/sdhci-cadence.c
> +++ b/drivers/mmc/host/sdhci-cadence.c
> @@ -262,6 +262,7 @@ static int sdhci_cdns_probe(struct platform_device *pdev)
> }
>
> static const struct of_device_id sdhci_cdns_match[] = {
> + { .compatible = "socionext,uniphier-sd4hc" },
> { .compatible = "cdns,sd4hc" },
> { /* sentinel */ }
> };
> --
> 2.7.4
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v5 2/2] mmc: sdhci-cadence: add Cadence SD4HC support
From: Ulf Hansson @ 2016-12-16 11:07 UTC (permalink / raw)
To: Masahiro Yamada
Cc: Rob Herring, linux-mmc, Adrian Hunter, Douglas Anderson,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Al Cooper,
Linux Kernel Mailing List, Stefan Wahren, Andrei Pistirica,
Wolfram Sang, Mark Rutland, Simon Horman
In-Reply-To: <CAK7LNASaKpGyE_qpQGyqxeVRSkqwj6Rm4WKNq2Zs4Woj4MY01A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[...]
>>
>> I think a better approach is to use the new sdhci-caps* bindings to
>> mask those caps that can't be trusted. And then use the generic mmc
>> bindings for speed modes instead.
>>
>> That should be a safer approach, right!?
>
> Right.
>
> But, if I know the caps register bits 63-32 are all zero,
> I need not explicitly specify sdhci-caps-mask, right?
Maybe not. I don't have a strong opinion here, so l am fine with
whatever you choose.
Kind regards
Uffe
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v7 1/2] mtd: nand: add tango NFC dt bindings doc
From: Marc Gonzalez @ 2016-12-16 10:56 UTC (permalink / raw)
To: Arnd Bergmann, Boris Brezillon
Cc: Mark Rutland, DT, Mason, Richard Weinberger, Rob Herring,
linux-mtd, Sebastian Frias
In-Reply-To: <1978954.SRtJpfOGp1@wuerfel>
On 07/11/2016 10:18, Arnd Bergmann wrote:
> On Tuesday, October 25, 2016 3:15:50 PM CET Marc Gonzalez wrote:
>> Add the tango NAND Flash Controller dt bindings documentation.
>>
>> Signed-off-by: Marc Gonzalez <marc_gonzalez@sigmadesigns.com>
>> ---
>> Documentation/devicetree/bindings/mtd/tango-nand.txt | 38 ++++++++++++++++++++++++++++++
>> 1 file changed, 38 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/mtd/tango-nand.txt b/Documentation/devicetree/bindings/mtd/tango-nand.txt
>> new file mode 100644
>> index 000000000000..3cbf95d6595a
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/mtd/tango-nand.txt
>> @@ -0,0 +1,38 @@
>> +Sigma Designs Tango4 NAND Flash Controller (NFC)
>> +
>> +Required properties:
>> +
>> +- compatible: "sigma,smp8758-nand"
>> +- reg: address/size of nfc_reg, nfc_mem, and pbus_reg
>> +- dmas: reference to the DMA channel used by the controller
>> +- dma-names: "nfc_sbox"
>
> Drop the "nfc_" prefix here, it seems redundant.
I took a closer look at Documentation/devicetree/bindings and
arch/arm/boot/dts and it seems the overwhelming nomenclature
is "rx", "tx" (with variants around these two).
Should I just use "rxtx" for my driver?
>> +- clocks: reference to the system clock
>> +- #address-cells: <1>
>> +- #size-cells: <0>
>> +
>> +Children nodes represent the available NAND chips.
>> +See Documentation/devicetree/bindings/mtd/nand.txt for generic bindings.
>> +
>> +Example:
>> +
>> + nand: nand@2c000 {
>> + compatible = "sigma,smp8758-nand";
>> + reg = <0x2c000 0x30 0x2d000 0x800 0x20000 0x1000>;
>
> It would be nicer to write this as
>
> reg = <0x2c000 0x30>, <0x2d000 0x800>, <0x20000 0x1000>;
>
> which is identical in binary format.
This is indeed a marked improvement. Boris, if I fix these two nits,
can I submit a single patch, or do you want them split?
Regards.
______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/
^ permalink raw reply
* Re: [PATCH v4] dt-bindings: power: supply: bq24735: reverse the polarity of ac-detect
From: Jon Hunter @ 2016-12-16 10:55 UTC (permalink / raw)
To: Peter Rosin, linux-kernel
Cc: Sebastian Reichel, Rob Herring, Mark Rutland, Stephen Warren,
linux-pm, devicetree
In-Reply-To: <1481881440-13464-1-git-send-email-peda@axentia.se>
On 16/12/16 09:44, Peter Rosin wrote:
> The ACOK pin on the bq24735 is active-high, of course meaning that when
> AC is OK the pin is high. However, all Tegra dts files have incorrectly
> specified active-high even though the signal is inverted on the Tegra
> boards. This has worked since the Linux driver has also inverted the
> meaning of the GPIO. Fix this situation by simply specifying in the
> bindings what everybody else agrees on; that the ti,ac-detect-gpios is
> active on AC adapter absence.
>
> Signed-off-by: Peter Rosin <peda@axentia.se>
> ---
> Documentation/devicetree/bindings/power/supply/ti,bq24735.txt | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> Hi!
>
> v3 -> v4 changes:
> - improve the bindings text in the spirit of remarks from Jon Hunter
>
> (There were no v1 nor v2, I made a mistake and started from v3 this time.)
>
> This patch is the result of this discussion:
> http://marc.info/?t=148152531800002
>
> I don't like how it changes the one thing that is seems correct, but
> what to do?
>
> Cheers,
> peda
>
> diff --git a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
> index 3bf55757ceec..c95e16e2dc56 100644
> --- a/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
> +++ b/Documentation/devicetree/bindings/power/supply/ti,bq24735.txt
> @@ -8,8 +8,10 @@ Optional properties :
> - interrupts : Specify the interrupt to be used to trigger when the AC
> adapter is either plugged in or removed.
> - ti,ac-detect-gpios : This GPIO is optionally used to read the AC adapter
> - presence. This is a Host GPIO that is configured as an input and
> - connected to the bq24735.
> + status. This is a Host GPIO that is configured as an input and connected
> + to the ACOK pin on the bq24735. Note: for backwards compatibility reasons,
> + the GPIO must be active on AC adapter absence despite ACOK being active
> + (high) on AC adapter presence.
> - ti,charge-current : Used to control and set the charging current. This value
> must be between 128mA and 8.128A with a 64mA step resolution. The POR value
> is 0x0000h. This number is in mA (e.g. 8192), see spec for more information
Thanks.
Acked-by: Jon Hunter <jonathanh@nvidia.com>
Cheers
Jon
--
nvpublic
^ permalink raw reply
* Re: [PATCH v4] i2c: designware: Cleaning and comment style fixes.
From: Jarkko Nikula @ 2016-12-16 10:48 UTC (permalink / raw)
To: Luis Oliveira, wsa-z923LK4zBo2bacvFa/9K2g,
robh+dt-DgEjT+Ai2ygdnm+yROfE0A, mark.rutland-5wv7dgnIgG8,
andriy.shevchenko-VuQAYsv1563Yd54FQh9/CA,
mika.westerberg-VuQAYsv1563Yd54FQh9/CA,
linux-i2c-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: Ramiro.Oliveira-HKixBCOQz3hWk0Htik3J/w,
Joao.Pinto-HKixBCOQz3hWk0Htik3J/w,
CARLOS.PALMINHA-HKixBCOQz3hWk0Htik3J/w
In-Reply-To: <928f25ad1fb39ec458758070d100852f7e47f1bb.1481881774.git.lolivei-HKixBCOQz3hWk0Htik3J/w@public.gmane.org>
On 12/16/2016 11:54 AM, Luis Oliveira wrote:
> The purpose of this commit is to fix some comments and styling issues in the
> existing code due to the need of reuse this code. What is being made here is:
>
./scripts/checkpatch.pl complains about this. Following in ~/.vimrc will
help if vim is your editor for git commit:
autocmd Filetype gitcommit set textwidth=72
Although not that big issue.
Acked-by: Jarkko Nikula <jarkko.nikula-VuQAYsv1563Yd54FQh9/CA@public.gmane.org>
--
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
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