Re: [v2, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI driver.
lash_pdata *f_pdata;
> struct spi_nor *nor;
> struct mtd_info *mtd;
> + enum read_mode mode;
> + enum read_mode dt_mode = SPI_NOR_QUAD;
> unsigned int cs;
> + unsigned int rev_reg;
> int i, ret;
>
> + /* Determine, whether or not octal transfer MAY be supported */
But you already know that from DT, no ?
> + rev_reg = readl(cqspi->iobase + CQSPI_REG_MODULEID);
> + dev_info(dev, "CQSPI Module id %x\n", rev_reg);
> +
> + switch (rev_reg & CQSPI_REG_MODULEID_CONF_ID_MASK) {
> + case CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY:
> + case CQSPI_REG_MODULEID_CONF_ID_OCTAL:
> + mode = SPI_NOR_OCTAL;
> + break;
> + case CQSPI_REG_MODULEID_CONF_ID_QUAD:
> + case CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY:
Does this work on all revisions of CQSPI ?
> + mode = SPI_NOR_QUAD;
> + break;
> + }
> +
> + if (cqspi->max_mode == CQSPI_MAX_MODE_OCTAL)
> + dt_mode = SPI_NOR_OCTAL;
> +
> + if (mode == SPI_NOR_QUAD && dt_mode == SPI_NOR_OCTAL)
> + dev_warn(dev, "Requested octal mode is not supported by the
> device.");
> + else if (mode == SPI_NOR_OCTAL && dt_mode == SPI_NOR_QUAD)
> + mode = SPI_NOR_QUAD;
> +
> /* Get flash device data */
> for_each_available_child_of_node(dev->of_node, np) {
> ret = of_property_read_u32(np, "reg", );
> @@ -1123,7 +1183,7 @@ static int cqspi_setup_flash(struct cqspi_st *cqspi,
> struct device_node *np)
> goto err;
> }
>
> - ret = spi_nor_scan(nor, NULL, SPI_NOR_QUAD);
> + ret = spi_nor_scan(nor, NULL, mode);
> if (ret)
> goto err;
>
> @@ -1277,13 +1337,6 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
> #define CQSPI_DEV_PM_OPS NULL
> #endif
>
> -static struct of_device_id const cqspi_dt_ids[] = {
> - {.compatible = "cdns,qspi-nor",},
> - { /* end of table */ }
> -};
> -
> -MODULE_DEVICE_TABLE(of, cqspi_dt_ids);
> -
> static struct platform_driver cqspi_platform_driver = {
> .probe = cqspi_probe,
> .remove = cqspi_remove,
>
--
Best regards,
Marek Vasut
Re: [v2, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI driver.
On 03/10/2017 01:00 PM, Artur Jedrysek wrote:
>
> From: Marek Vasut [mailto:marek.va...@gmail.com]
> Sent: 10 March 2017 04:37
>> On 03/08/2017 09:02 AM, Artur Jedrysek wrote:
>>> Recent versions of Cadence QSPI controller support Octal SPI transfers
>>> as well. This patch updates existing driver to support such feature.
>>>
>>> It is not possible to determine whether or not octal mode is supported
>>> just by looking at revision register alone. To solve that, an additional
>>> compatible in Device Tree is added to indicate such capability.
>>> Both (revision and compatible) are used to determine, which mode to
>>> pass to spi_nor_scan() call.
>>>
>>> Signed-off-by: Artur Jedrysek <jar...@cadence.com>
>>> ---
>>> Changelog:
>>> v2: Use new compatible in DT, instead of boolean property, to indicate
>>> Octal SPI support.
>>> Extracted Kconfig update to seperate patch.
>>> ---
>>> drivers/mtd/spi-nor/cadence-quadspi.c | 69
>>> +++
>>> 1 file changed, 61 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c
>>> b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> index 9f8102d..a96471d 100644
>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> @@ -76,6 +76,7 @@ struct cqspi_st {
>>> u32 fifo_depth;
>>> u32 fifo_width;
>>> u32 trigger_address;
>>> + u32 max_mode;
>>
>> I think it's time to introduce flags instead,
>> ie. #define CQSPI_FLAG_SUPPORTS_OCTAL BIT(0)
>>
>>> struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>>> };
>>>
>>> @@ -87,6 +88,10 @@ struct cqspi_st {
>>> #define CQSPI_INST_TYPE_SINGLE 0
>>> #define CQSPI_INST_TYPE_DUAL 1
>>> #define CQSPI_INST_TYPE_QUAD 2
>>> +#define CQSPI_INST_TYPE_OCTAL 3
>>> +
>>> +#define CQSPI_MAX_MODE_QUAD0
>>> +#define CQSPI_MAX_MODE_OCTAL 1
>>>
>>> #define CQSPI_DUMMY_CLKS_PER_BYTE 8
>>> #define CQSPI_DUMMY_BYTES_MAX 4
>>> @@ -204,6 +209,14 @@ struct cqspi_st {
>>> #define CQSPI_REG_CMDWRITEDATALOWER0xA8
>>> #define CQSPI_REG_CMDWRITEDATAUPPER0xAC
>>>
>>> +#define CQSPI_REG_MODULEID 0xFC
>>> +#define CQSPI_REG_MODULEID_CONF_ID_MASK0x3
>>> +#define CQSPI_REG_MODULEID_CONF_ID_LSB 0
>>> +#define CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY 0x0
>>> +#define CQSPI_REG_MODULEID_CONF_ID_OCTAL 0x1
>>> +#define CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY0x2
>>> +#define CQSPI_REG_MODULEID_CONF_ID_QUAD0x3
>>> +
>>> /* Interrupt status bits */
>>> #define CQSPI_REG_IRQ_MODE_ERR BIT(0)
>>> #define CQSPI_REG_IRQ_UNDERFLOWBIT(1)
>>> @@ -866,6 +879,9 @@ static int cqspi_set_protocol(struct spi_nor *nor,
>>> const int read)
>>> case SPI_NOR_QUAD:
>>> f_pdata->data_width = CQSPI_INST_TYPE_QUAD;
>>> break;
>>> + case SPI_NOR_OCTAL:
>>> + f_pdata->data_width = CQSPI_INST_TYPE_OCTAL;
>>> + break;
>>> default:
>>> return -EINVAL;
>>> }
>>> @@ -977,6 +993,17 @@ static int cqspi_write_reg(struct spi_nor *nor, u8
>>> opcode, u8 *buf, int len)
>>> return ret;
>>> }
>>>
>>> +static const u32 cqspi_max_mode_quad = CQSPI_MAX_MODE_QUAD;
>>> +static const u32 cqspi_max_mode_octal = CQSPI_MAX_MODE_OCTAL;
>>> +
>>> +static struct of_device_id const cqspi_dt_ids[] = {
>>> + { .compatible = "cdns,qspi-nor", .data = _max_mode_quad },
>>> + { .compatible = "cdns,ospi-nor", .data = _max_mode_octal },
>>
>> .data = (void *)CQSPI_FLAG_SUPPORTS_OCTAL , then you don't need that
>> static const stuff ...
>>
>
> I had some doubts regarding that approach, as it may be dependent on the
> CPU architecture (32-bit, 64-bit) and endianness. .data needs to be first
> casted to unsigned long for reading, to ensure correct access and to
>
Re: [PATCH v2] mtd: Fix mtdblock for >4GB MTD devices
On 03/01/2017 05:14 AM, lepton wrote: > If checking some calling side, the len is from cache_size of struct > mtdblk_dev, it's defined as unsigned int now. So it's not 64bit yet. Ummm ... since you're top-posting, I have no clue which part do you refer to , sorry. > BTW, seems it's just block size (512) at some other calling side. > > (Sorry for previous same content email, just found out it's html > format and rejected by mail list) > > On Mon, Feb 27, 2017 at 1:31 AM, Marek Vasut <marek.va...@gmail.com> wrote: >> On 02/22/2017 03:15 AM, Lepton Wu wrote: >>> Change to use loff_t instead of unsigned long in some functions >>> to make sure mtdblock can handle offset bigger than 4G in 32 bits mode. >>> >>> Signed-off-by: Lepton Wu <ytht@gmail.com> >>> --- >>> Changes in v2: >>> - Make the commit message more clearer and fix some format issues. >>> >>> drivers/mtd/mtdblock.c| 35 ++- >>> drivers/mtd/mtdblock_ro.c | 4 ++-- >>> 2 files changed, 20 insertions(+), 19 deletions(-) >>> >>> diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c >>> index bb4c14f83c75..373c0edca803 100644 >>> --- a/drivers/mtd/mtdblock.c >>> +++ b/drivers/mtd/mtdblock.c >>> @@ -61,8 +61,8 @@ static void erase_callback(struct erase_info *done) >>> wake_up(wait_q); >>> } >>> >>> -static int erase_write (struct mtd_info *mtd, unsigned long pos, >>> - int len, const char *buf) >>> +static int erase_write(struct mtd_info *mtd, loff_t pos, int len, >>> +const char *buf) >> >> Can the length be 64bit too now ? >> >> [...] >> >> -- >> Best regards, >> Marek Vasut -- Best regards, Marek Vasut
Re: [v2, 4/4] dt-bindings: mtd: Add Octal SPI support to Cadence QSPI.
On 03/10/2017 01:03 PM, Artur Jedrysek wrote:
>
>> From: Marek Vasut [mailto:marek.va...@gmail.com]
>> Sent: 10 March 2017 04:39
>> On 03/08/2017 09:05 AM, Artur Jedrysek wrote:
>>> This patch updates Cadence QSPI Device Tree documentation to include
>>> information about new compatible used to indicate, whether or not
>>> Octal SPI transfers are supported by the device.
>>>
>>> Signed-off-by: Artur Jedrysek <jar...@cadence.com>
>>> ---
>>> Changelog:
>>> v2: Use new compatible, instead of boolean property, to indicate
>>> Octal SPI support.
>>> ---
>>> Documentation/devicetree/bindings/mtd/cadence-quadspi.txt | 4 +++-
>>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>> b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>>> index f248056..41d1e98 100644
>>> --- a/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>>> +++ b/Documentation/devicetree/bindings/mtd/cadence-quadspi.txt
>>> @@ -1,7 +1,9 @@
>>> * Cadence Quad SPI controller
>>>
>>> Required properties:
>>> -- compatible : Should be "cdns,qspi-nor".
>>> +- compatible : Should be "cdns,{qspi|ospi}-nor".
>>
>> Please explicitly list all compatibles , ie.
>> Should be "cdns,qspi-nor" or "cdns,ospi-nor".
>>
>
> Two possible options are also explicitly listed right below. I did that
> in the way it was done in other driver for Cadence IP:
> /Documentation/devicetree/bindings/net/macb.txt
> Should I remove all of that and replace it with both options, separated
> by an "or" word?
Wait for Rob to confirm.
>> But I think the ospi is backward compatible with qspi, right ? So the
>> binding for ospi should list both, ie.
>> compatible = "cdns,ospi-nor", "cdns,qspi-nor";
>>
>
> Yes, the ospi is backwards compatible with qspi. However, the sole point
> of introducing new compatible was to differentiate between them (which
> was previously done using a boolean flag, but that was criticized).
> Listing both bindings for ospi would only help for kernels not
> containing driver update being subject of this patch series, as both
> ospi and qspi are handled by the same driver - just with a slight
> difference. I apologize if I got something wrong here.
You got it right, I was just curious about the compatibility.
>>> + Use "cdns,qspi-nor" for Quad SPI controller.
>>> + Use "cdns,ospi-nor" for Octal SPI controller.
>>> - reg : Contains two entries, each of which is a tuple consisting of a
>>> physical address and length. The first entry is the address and
>>> length of the controller register set. The second entry is the
>>>
>>
>>
>> --
>> Best regards,
>> Marek Vasut
--
Best regards,
Marek Vasut
Re: [v2, 2/4] mtd: spi-nor: Add Octal SPI support to Cadence QSPI driver.
On 03/10/2017 03:09 PM, Artur Jedrysek wrote:
CCing Dinh.
[...]
>>>>> + /* Determine, whether or not octal transfer MAY be supported */
>>>>
>>>> But you already know that from DT, no ?
>>>>
>>>
>>> Cadence Octal (formerly Quad) SPI Controller is sold as an IP, and is
>>> configurable. This includes max SPI mode. It is possible to detect, that
>>> Octal SPI controller is configured (during hardware compilation) to support
>>> up to Quad mode, using revision register.
>>
>> So the octal-spi controller always has this config register, but the
>> quad-spi controller may or may not have this register ?
>>
>
> This register was always present. In Quad-SPI, however, it didn't contain
> information about maximum possible mode, as only quad was possible, and
> meaning of bits checked here was different.
OK
>>>>> + rev_reg = readl(cqspi->iobase + CQSPI_REG_MODULEID);
>>>>> + dev_info(dev, "CQSPI Module id %x\n", rev_reg);
>>>>> +
>>>>> + switch (rev_reg & CQSPI_REG_MODULEID_CONF_ID_MASK) {
>>>>> + case CQSPI_REG_MODULEID_CONF_ID_OCTAL_PHY:
>>>>> + case CQSPI_REG_MODULEID_CONF_ID_OCTAL:
>>>>> + mode = SPI_NOR_OCTAL;
>>>>> + break;
>>>>> + case CQSPI_REG_MODULEID_CONF_ID_QUAD:
>>>>> + case CQSPI_REG_MODULEID_CONF_ID_QUAD_PHY:
>>>>
>>>> Does this work on all revisions of CQSPI ?
>>>>
>>>
>>> After having a more thorough look at specification of older IP version
>>> (quad only) it seems, that revision register format has indeed changed.
>>> This will be fixed in the next version of the patch.
>>
>> Can the quad-spi controller be configured only as dual or single ?
>> What about the octal one ? These cases should probably be handled
>> somehow too, right ?
>>
>
> Quad-SPI controller can always support single, dual and quad. There was
> no option to configure max mode. Octal-SPI controller can be configured
> to support either octal or quad mode. No controller could be configured
> (during hardware compilation/synthesis) to support only single/dual
> SPI mode. To put it shortly: single, dual and quad is always supported.
So basically the whole check you need to perform here is
mode = quad;
if (controller->flags & CAN_DO_OCTAL) {
if (readl(ID_REGISTER) & IS_CONFIGURED_AS_OCTAL)
mode = octal;
}
--
Best regards,
Marek Vasut
Re: [PATCH v5 1/6] mtd: spi-nor: introduce more SPI protocols and the Dual Transfer Mode
T(best_match));
> + if (cmd < 0)
> + return -EINVAL;
> +
> + pp = >page_programs[cmd];
> + nor->program_opcode = pp->opcode;
> + nor->write_proto = pp->proto;
> + return 0;
> +}
> +
> +static int spi_nor_select_erase(struct spi_nor *nor,
> + const struct flash_info *info)
> +{
> + struct mtd_info *mtd = >mtd;
> +
> +#ifdef CONFIG_MTD_SPI_NOR_USE_4K_SECTORS
> + /* prefer "small sector" erase if possible */
> + if (info->flags & SECT_4K) {
> + nor->erase_opcode = SPINOR_OP_BE_4K;
> + mtd->erasesize = 4096;
> + } else if (info->flags & SECT_4K_PMC) {
> + nor->erase_opcode = SPINOR_OP_BE_4K_PMC;
> + mtd->erasesize = 4096;
> + } else
> +#endif
> + {
> + nor->erase_opcode = SPINOR_OP_SE;
> + mtd->erasesize = info->sector_size;
> + }
> + return 0;
> +}
> +
> +static int spi_nor_setup(struct spi_nor *nor, const struct flash_info *info,
> + const struct spi_nor_flash_parameter *params,
> + const struct spi_nor_hwcaps *hwcaps)
> +{
> + u32 ignored_mask, shared_mask;
> + bool enable_quad_io;
> + int err;
> +
> + /*
> + * Keep only the hardware capabilities supported by both the SPI
> + * controller and the SPI flash memory.
> + */
> + shared_mask = hwcaps->mask & params->hwcaps.mask;
> +
> + /* SPI protocol classes N-N-N are not supported yet. */
> + ignored_mask = (SNOR_HWCAPS_READ_2_2_2 |
> + SNOR_HWCAPS_READ_4_4_4 |
> + SNOR_HWCAPS_READ_8_8_8 |
> + SNOR_HWCAPS_PP_4_4_4 |
> + SNOR_HWCAPS_PP_8_8_8);
> + if (shared_mask & ignored_mask) {
> + dev_dbg(nor->dev,
> + "SPI protocol classes N-N-N are not supported yet.\n");
> + shared_mask &= ~ignored_mask;
> + }
> +
> + /* Select the (Fast) Read command. */
> + err = spi_nor_select_read(nor, params, shared_mask);
> + if (err) {
> + dev_err(nor->dev, "invalid (fast) read\n");
What does this information tell me, as an end user ? If I see this error
message, what sort of conclusion can I derive from it ? I have
no idea ...
> + return err;
> + }
> +
> + /* Select the Page Program command. */
> + err = spi_nor_select_pp(nor, params, shared_mask);
> + if (err) {
> + dev_err(nor->dev, "invalid page program\n");
> + return err;
> + }
> +
> + /* Select the Sector Erase command. */
> + err = spi_nor_select_erase(nor, info);
> + if (err) {
> + dev_err(nor->dev, "invalid sector/block erase\n");
> + return err;
> + }
> +
> + /* Enable Quad I/O if needed. */
> + enable_quad_io = (spi_nor_get_protocol_width(nor->read_proto) == 4 ||
> + spi_nor_get_protocol_width(nor->write_proto) == 4);
What if read_proto != write_proto ? Also, this is awful code ... fix it.
> + if (enable_quad_io && params->quad_enable)
> + nor->flash_quad_enable = params->quad_enable;
> + else
> + nor->flash_quad_enable = NULL;
> +
> + return 0;
> +}
> +
> +int spi_nor_scan(struct spi_nor *nor, const char *name,
> + const struct spi_nor_hwcaps *hwcaps)
> +{
> + struct spi_nor_flash_parameter params;
> const struct flash_info *info = NULL;
> struct device *dev = nor->dev;
> struct mtd_info *mtd = >mtd;
> @@ -1548,6 +1824,11 @@ int spi_nor_scan(struct spi_nor *nor, const char
> *name, enum read_mode mode)
> if (ret)
> return ret;
>
> + /* Reset SPI protocol for all commands */
> + nor->reg_proto = SNOR_PROTO_1_1_1;
> + nor->read_proto = SNOR_PROTO_1_1_1;
> + nor->write_proto = SNOR_PROTO_1_1_1;
> +
> if (name)
> info = spi_nor_match_id(name);
> /* Try to auto-detect if chip name wasn't specified or not found */
[...]
--
Best regards,
Marek Vasut
Re: [PATCH v5 2/6] mtd: m25p80: add support of SPI 1-2-2 and 1-4-4 protocols
gt; * Read an address range from the nor chip. The address range
> * may be any size provided it is within the physical boundaries.
> @@ -130,13 +151,19 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t
> from, size_t len,
> {
> struct m25p *flash = nor->priv;
> struct spi_device *spi = flash->spi;
> - struct spi_transfer t[2];
> + unsigned int inst_nbits, addr_nbits, data_nbits, data_idx;
> + struct spi_transfer t[3];
> struct spi_message m;
> unsigned int dummy = nor->read_dummy;
> ssize_t ret;
> + int cmd_sz;
> +
> + /* get transfer protocols. */
> + m25p80_proto2nbits(nor->read_proto, _nbits,
> +_nbits, _nbits);
>
> /* convert the dummy cycles to the number of bytes */
> - dummy /= 8;
> + dummy = (dummy * addr_nbits) / 8;
>
> if (spi_flash_read_supported(spi)) {
> struct spi_flash_read_message msg;
> @@ -149,10 +176,9 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t
> from, size_t len,
> msg.read_opcode = nor->read_opcode;
> msg.addr_width = nor->addr_width;
> msg.dummy_bytes = dummy;
> - /* TODO: Support other combinations */
> - msg.opcode_nbits = SPI_NBITS_SINGLE;
> - msg.addr_nbits = SPI_NBITS_SINGLE;
> - msg.data_nbits = m25p80_rx_nbits(nor);
> + msg.opcode_nbits = inst_nbits;
> + msg.addr_nbits = addr_nbits;
> + msg.data_nbits = data_nbits;
>
> ret = spi_flash_read(spi, );
> if (ret < 0)
> @@ -167,20 +193,45 @@ static ssize_t m25p80_read(struct spi_nor *nor, loff_t
> from, size_t len,
> m25p_addr2cmd(nor, from, flash->command);
>
> t[0].tx_buf = flash->command;
> + t[0].tx_nbits = inst_nbits;
> t[0].len = m25p_cmdsz(nor) + dummy;
> spi_message_add_tail([0], );
>
> - t[1].rx_buf = buf;
> - t[1].rx_nbits = m25p80_rx_nbits(nor);
> - t[1].len = min3(len, spi_max_transfer_size(spi),
> - spi_max_message_size(spi) - t[0].len);
> - spi_message_add_tail([1], );
> + /*
> + * Set all dummy/mode cycle bits to avoid sending some manufacturer
> + * specific pattern, which might make the memory enter its Continuous
> + * Read mode by mistake.
> + * Based on the different mode cycle bit patterns listed and described
> + * in the JESD216B speficication, the 0xff value works for all memories
^
specification, typo
> + * and all manufacturers.
> + */
> + cmd_sz = t[0].len;
> + memset(flash->command + cmd_sz - dummy, 0xff, dummy);
> +
> + /* split the op code and address bytes into two transfers if needed. */
> + data_idx = 1;
> + if (addr_nbits != inst_nbits) {
> + t[0].len = 1;
> +
> + t[1].tx_buf = >command[1];
> + t[1].tx_nbits = addr_nbits;
> + t[1].len = cmd_sz - 1;
> + spi_message_add_tail([1], );
> +
> + data_idx = 2;
> + }
> +
> + t[data_idx].rx_buf = buf;
> + t[data_idx].rx_nbits = data_nbits;
> + t[data_idx].len = min3(len, spi_max_transfer_size(spi),
> +spi_max_message_size(spi) - cmd_sz);
> + spi_message_add_tail([data_idx], );
>
> ret = spi_sync(spi, );
> if (ret)
> return ret;
>
> - ret = m.actual_length - m25p_cmdsz(nor) - dummy;
> + ret = m.actual_length - cmd_sz;
> if (ret < 0)
> return -EIO;
> return ret;
> @@ -223,11 +274,20 @@ static int m25p_probe(struct spi_device *spi)
> spi_set_drvdata(spi, flash);
> flash->spi = spi;
>
> - if (spi->mode & SPI_RX_QUAD)
> + if (spi->mode & SPI_RX_QUAD) {
> hwcaps.mask |= SNOR_HWCAPS_READ_1_1_4;
> - else if (spi->mode & SPI_RX_DUAL)
> +
> + if (spi->mode & SPI_TX_QUAD)
> + hwcaps.mask |= (SNOR_HWCAPS_READ_1_4_4 |
> + SNOR_HWCAPS_PP_1_1_4 |
> + SNOR_HWCAPS_PP_1_4_4);
> + } else if (spi->mode & SPI_RX_DUAL) {
> hwcaps.mask |= SNOR_HWCAPS_READ_1_1_2;
>
> + if (spi->mode & SPI_TX_DUAL)
> + hwcaps.mask |= SNOR_HWCAPS_READ_1_2_2;
> + }
> +
> if (data && data->name)
> nor->mtd.name = data->name;
>
>
--
Best regards,
Marek Vasut
Re: [PATCH v2 2/2] mtd: spi-nor: add driver for STM32 quad spi flash controller
> + flash->cs = cs_num;
> + flash->presc = presc;
> +
> + flash->nor.dev = qspi->dev;
> + spi_nor_set_flash_node(>nor, np);
> + flash->nor.priv = flash;
> + mtd = >nor.mtd;
> + mtd->priv = >nor;
> +
> + flash->nor.read = stm32_qspi_read;
> + flash->nor.write = stm32_qspi_write;
> + flash->nor.erase = stm32_qspi_erase;
> + flash->nor.read_reg = stm32_qspi_read_reg;
> + flash->nor.write_reg = stm32_qspi_write_reg;
> + flash->nor.prepare = stm32_qspi_prep;
> + flash->nor.unprepare = stm32_qspi_unprep;
> +
> + writel_relaxed(LPTR_DFT_TIMEOUT, qspi->io_base + QUADSPI_LPTR);
> +
> + writel_relaxed(CR_PRESC(presc) | CR_FTHRES(3) | CR_TCEN | CR_SSHIFT
> +| CR_EN, qspi->io_base + QUADSPI_CR);
> +
> + /*
> + * in stm32 qspi controller, QUADSPI_DCR register has a fsize field
> + * which define the size of nor flash.
> + * if fsize is NULL, the controller can't sent spi-nor command.
> + * set a temporary value just to discover the nor flash with
> + * "spi_nor_scan". After, the right value (mtd->size) can be set.
> + */
Is 25 the smallest value ? Use a macro for this ...
> + flash->fsize = 25;
> +
> + ret = spi_nor_scan(>nor, NULL, flash_read);
> + if (ret) {
> + dev_err(qspi->dev, "device scan failed\n");
> + return ret;
> + }
> +
> + /* number of bytes in Flash memory = 2^[FSIZE+1] */
> + flash->fsize = __fls(mtd->size) - 1;
> +
> + writel_relaxed(DCR_CSHT(1), qspi->io_base + QUADSPI_DCR);
> +
> + ret = mtd_device_register(mtd, NULL, 0);
> + if (ret) {
> + dev_err(qspi->dev, "mtd device parse failed\n");
> + return ret;
> + }
> +
> + dev_dbg(qspi->dev, "read mm:%s cs:%d bus:%d\n",
> + qspi->read_mode == CCR_FMODE_MM ? "yes" : "no", cs_num, width);
> +
> + return 0;
> +}
[...]
--
Best regards,
Marek Vasut
Re: [PATCH v2 3/3] mtd: dataflash: Make use of "extened device information"
On 04/18/2017 04:21 PM, Andrey Smirnov wrote:
> In anticipation of supporting chips that need it, extend the size of
> struct flash_info's 'jedec_id' field to make room 2 byte of extended
> device information as well as add code to fetch this data during
> jedec_probe().
>
> Cc: cphe...@gmail.com
> Cc: David Woodhouse <dw...@infradead.org>
> Cc: Brian Norris <computersforpe...@gmail.com>
> Cc: Boris Brezillon <boris.brezil...@free-electrons.com>
> Cc: Marek Vasut <marek.va...@gmail.com>
> Cc: Richard Weinberger <rich...@nod.at>
> Cc: Cyrille Pitchen <cyrille.pitc...@atmel.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smir...@gmail.com>
> ---
>
> Changes since [v1]:
>
> - Formatting
>
> [v1] http://lkml.kernel.org/r/20170411161722.11164-1-andrew.smir...@gmail.com
>
>
> drivers/mtd/devices/mtd_dataflash.c | 113
> +---
> 1 file changed, 65 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/mtd/devices/mtd_dataflash.c
> b/drivers/mtd/devices/mtd_dataflash.c
> index 5b7a8c3..5d694a4 100644
> --- a/drivers/mtd/devices/mtd_dataflash.c
> +++ b/drivers/mtd/devices/mtd_dataflash.c
> @@ -690,7 +690,7 @@ struct flash_info {
> /* JEDEC id has a high byte of zero plus three data bytes:
>* the manufacturer id, then a two byte device id.
>*/
> - u32 jedec_id;
> + u64 jedec_id;
>
> /* The size listed here is what works with OP_ERASE_PAGE. */
> unsignednr_pages;
> @@ -713,63 +713,34 @@ static struct flash_info dataflash_data[] = {
>* These newer chips also support 128-byte security registers (with
>* 64 bytes one-time-programmable) and software write-protection.
>*/
> - { "AT45DB011B", 0x1f2200, 512, 264, 9, SUP_POW2PS},
> - { "at45db011d", 0x1f2200, 512, 256, 8, SUP_POW2PS | IS_POW2PS},
> + { "AT45DB011B", 0x1f2200, 512, 264, 9, SUP_POW2PS},
> + { "at45db011d", 0x1f2200, 512, 256, 8, SUP_POW2PS | IS_POW2PS},
>
> - { "AT45DB021B", 0x1f2300, 1024, 264, 9, SUP_POW2PS},
> - { "at45db021d", 0x1f2300, 1024, 256, 8, SUP_POW2PS | IS_POW2PS},
> + { "AT45DB021B", 0x1f2300, 1024, 264, 9, SUP_POW2PS},
> + { "at45db021d", 0x1f2300, 1024, 256, 8, SUP_POW2PS | IS_POW2PS},
>
> - { "AT45DB041x", 0x1f2400, 2048, 264, 9, SUP_POW2PS},
> - { "at45db041d", 0x1f2400, 2048, 256, 8, SUP_POW2PS | IS_POW2PS},
> + { "AT45DB041x", 0x1f2400, 2048, 264, 9, SUP_POW2PS},
> + { "at45db041d", 0x1f2400, 2048, 256, 8, SUP_POW2PS | IS_POW2PS},
>
> - { "AT45DB081B", 0x1f2500, 4096, 264, 9, SUP_POW2PS},
> - { "at45db081d", 0x1f2500, 4096, 256, 8, SUP_POW2PS | IS_POW2PS},
> + { "AT45DB081B", 0x1f2500, 4096, 264, 9, SUP_POW2PS},
> + { "at45db081d", 0x1f2500, 4096, 256, 8, SUP_POW2PS | IS_POW2PS},
>
> - { "AT45DB161x", 0x1f2600, 4096, 528, 10, SUP_POW2PS},
> - { "at45db161d", 0x1f2600, 4096, 512, 9, SUP_POW2PS | IS_POW2PS},
> + { "AT45DB161x", 0x1f2600, 4096, 528, 10, SUP_POW2PS},
> + { "at45db161d", 0x1f2600, 4096, 512, 9, SUP_POW2PS | IS_POW2PS},
>
> - { "AT45DB321x", 0x1f2700, 8192, 528, 10, 0}, /* rev C */
> + { "AT45DB321x", 0x1f2700, 8192, 528, 10, 0}, /* rev C */
>
> - { "AT45DB321x", 0x1f2701, 8192, 528, 10, SUP_POW2PS},
> - { "at45db321d", 0x1f2701, 8192, 512, 9, SUP_POW2PS | IS_POW2PS},
> + { "AT45DB321x", 0x1f2701, 8192, 528, 10, SUP_POW2PS},
> + { "at45db321d", 0x1f2701, 8192, 512, 9, SUP_POW2PS | IS_POW2PS},
>
> - { "AT45DB642x", 0x1f2800, 8192, 1056, 11, SUP_POW2PS},
> - { "at45db642d", 0x1f2800, 8192, 1024, 10, SUP_POW2PS | IS_POW2PS},
> + { "AT45DB642x", 0x1f2800, 8192, 1056, 11, SUP_POW2PS},
> + { "at45db642d", 0x1f2800, 8192, 1024, 10, SUP_POW2PS | IS_POW2PS},
> };
>
> -static struct flash_info *jedec_probe(struct spi_device *spi)
> +static struct flash_info *jedec_lookup(struct spi_device *spi, u64 jedec)
> {
> - int ret, i;
> - u8 code = OP_READ_ID;
> - u8 id[3];
> - u32 jedec;
> + int status, i;
> struct flash_info *info;
> - int status;
> -
> - /*
> - * JEDEC also defines an optional "extended device information"
> - * string for after ven
Re: [PATCH v2 1/3] mtd: dataflash: Replace C99 type with their kernel counterparts
On 04/18/2017 04:21 PM, Andrey Smirnov wrote: > Replace C99 type with their kernel counterparts as per request from > Marek Vasut. > > No functional change intended. > > Cc: cphe...@gmail.com > Cc: David Woodhouse <dw...@infradead.org> > Cc: Brian Norris <computersforpe...@gmail.com> > Cc: Boris Brezillon <boris.brezil...@free-electrons.com> > Cc: Marek Vasut <marek.va...@gmail.com> > Cc: Richard Weinberger <rich...@nod.at> > Cc: Cyrille Pitchen <cyrille.pitc...@atmel.com> > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Andrey Smirnov <andrew.smir...@gmail.com> Nice, thanks a lot ! Acked-by: Marek Vasut <marek.va...@gmail.com> -- Best regards, Marek Vasut
Re: [PATCH v2 2/3] mtd: dataflash: Improve coding style in jedec_probe()
On 04/18/2017 04:21 PM, Andrey Smirnov wrote:
> As per request from Marek Vasut, change the following:
Does that really have to be in the commit message ? ^_^'
>- Replace indentation between type and name of local variable from
> tabs to spaces
>
>- Replace magic number 0x1F with CFI_MFR_ATMEL macro
>
>- Replace variable 'tmp' with 'ret' and 'i' where appropriate
>
>- Reformat multi-line comments and add newlines where appropriate
>
> No functional change intended.
Appreciated, thanks!
Acked-by: Marek Vasut <marek.va...@gmail.com>
> Cc: cphe...@gmail.com
> Cc: David Woodhouse <dw...@infradead.org>
> Cc: Brian Norris <computersforpe...@gmail.com>
> Cc: Boris Brezillon <boris.brezil...@free-electrons.com>
> Cc: Marek Vasut <marek.va...@gmail.com>
> Cc: Richard Weinberger <rich...@nod.at>
> Cc: Cyrille Pitchen <cyrille.pitc...@atmel.com>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Andrey Smirnov <andrew.smir...@gmail.com>
> ---
> drivers/mtd/devices/mtd_dataflash.c | 31 +--
> 1 file changed, 17 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/mtd/devices/mtd_dataflash.c
> b/drivers/mtd/devices/mtd_dataflash.c
> index a566231..5b7a8c3 100644
> --- a/drivers/mtd/devices/mtd_dataflash.c
> +++ b/drivers/mtd/devices/mtd_dataflash.c
> @@ -82,6 +82,7 @@
> #define OP_WRITE_SECURITY_REVC 0x9A
> #define OP_WRITE_SECURITY0x9B/* revision D */
>
> +#define CFI_MFR_ATMEL0x1F
>
> struct dataflash {
> u8 command[4];
> @@ -738,14 +739,15 @@ static struct flash_info dataflash_data[] = {
>
> static struct flash_info *jedec_probe(struct spi_device *spi)
> {
> - int tmp;
> - u8 code = OP_READ_ID;
> - u8 id[3];
> - u32 jedec;
> - struct flash_info *info;
> + int ret, i;
> + u8 code = OP_READ_ID;
> + u8 id[3];
> + u32 jedec;
> + struct flash_info *info;
> int status;
>
> - /* JEDEC also defines an optional "extended device information"
> + /*
> + * JEDEC also defines an optional "extended device information"
>* string for after vendor-specific data, after the three bytes
>* we use here. Supporting some chips might require using it.
>*
> @@ -753,13 +755,14 @@ static struct flash_info *jedec_probe(struct spi_device
> *spi)
>* That's not an error; only rev C and newer chips handle it, and
>* only Atmel sells these chips.
>*/
> - tmp = spi_write_then_read(spi, , 1, id, 3);
> - if (tmp < 0) {
> + ret = spi_write_then_read(spi, , 1, id, 3);
> + if (ret < 0) {
> pr_debug("%s: error %d reading JEDEC ID\n",
> - dev_name(>dev), tmp);
> - return ERR_PTR(tmp);
> + dev_name(>dev), ret);
> + return ERR_PTR(ret);
> }
> - if (id[0] != 0x1f)
> +
> + if (id[0] != CFI_MFR_ATMEL)
> return NULL;
>
> jedec = id[0];
> @@ -768,9 +771,9 @@ static struct flash_info *jedec_probe(struct spi_device
> *spi)
> jedec = jedec << 8;
> jedec |= id[2];
>
> - for (tmp = 0, info = dataflash_data;
> - tmp < ARRAY_SIZE(dataflash_data);
> - tmp++, info++) {
> + for (i = 0, info = dataflash_data;
> + i < ARRAY_SIZE(dataflash_data);
> + i++, info++) {
> if (info->jedec_id == jedec) {
> pr_debug("%s: OTP, sector protect%s\n",
> dev_name(>dev),
>
--
Best regards,
Marek Vasut
Re: [PATCH 4/4] regulator: Add ROHM BD9571MWV-M PMIC regulator driver
On 04/18/2017 07:57 PM, Mark Brown wrote:
> On Sun, Apr 16, 2017 at 08:08:01PM +0200, Marek Vasut wrote:
>
> This looks good, a couple of minor things though:
>
>> +static int bd9571mwv_regulator_is_enabled(struct regulator_dev *reg)
>> +{
>> +/* Always enabled. */
>> +return 1;
>> +}
>
> This should be the default implementation, no need to open code it.
Fixed.
>> +int bd9571mwv_set_voltage_sel_regmap(struct regulator_dev *rdev,
>> + unsigned int sel)
>> +{
>> +return regmap_write_bits(rdev->regmap, BD9571MWV_DVFS_SETVID,
>> + rdev->desc->vsel_mask, sel);
>> +}
>
> regulator_set_voltage_sel_regmap().
I'm using different register for reading and setting the voltage, so
this doesn't work in this case. Plus these two registers have different
format to make it more fun.
--
Best regards,
Marek Vasut
Re: [PATCH v2 2/3] mtd: dataflash: Improve coding style in jedec_probe()
On 04/19/2017 12:36 AM, Joe Perches wrote:
> On Tue, 2017-04-18 at 20:25 +0200, Marek Vasut wrote:
>> On 04/18/2017 04:21 PM, Andrey Smirnov wrote:
>>> As per request from Marek Vasut, change the following:
>>
>> Does that really have to be in the commit message ? ^_^'
>>
>>>- Replace indentation between type and name of local variable from
>>> tabs to spaces
>>>
>>>- Replace magic number 0x1F with CFI_MFR_ATMEL macro
>>>
>>>- Replace variable 'tmp' with 'ret' and 'i' where appropriate
>>>
>>>- Reformat multi-line comments and add newlines where appropriate
>>>
>>> No functional change intended.
>>
>> Appreciated, thanks!
>
> trivia:
>
>>> diff --git a/drivers/mtd/devices/mtd_dataflash.c
>>> b/drivers/mtd/devices/mtd_dataflash.c
> []
>>> @@ -768,9 +771,9 @@ static struct flash_info *jedec_probe(struct spi_device
>>> *spi)
>>> jedec = jedec << 8;
>>> jedec |= id[2];
>>>
>>> - for (tmp = 0, info = dataflash_data;
>>> - tmp < ARRAY_SIZE(dataflash_data);
>>> - tmp++, info++) {
>>> + for (i = 0, info = dataflash_data;
>>> + i < ARRAY_SIZE(dataflash_data);
>>> + i++, info++) {
>>> if (info->jedec_id == jedec) {
>>> pr_debug("%s: OTP, sector protect%s\n",
>>> dev_name(>dev),
>
> This loop could be written without the i variable.
>
> for (info = dataflash_data;
>info < dataflash_data + ARRAY_SIZE(dataflash_data);
>info++) {
>
But in a separate patch please, so it's bisectable. I don't want to slap
such change into this patch.
--
Best regards,
Marek Vasut
Re: [PATCH v7 3/4] mtd: spi-nor: introduce Double Transfer Rate (DTR) SPI protocols
On 04/19/2017 12:51 AM, Cyrille Pitchen wrote:
> This patch introduces support to Double Transfer Rate (DTR) SPI protocols.
> DTR is used only for Fast Read operations.
>
> According to manufacturer datasheets, whatever the number of I/O lines
> used during instruction (x) and address/mode/dummy (y) clock cycles, DTR
> is used only during data (z) clock cycles of SPI x-y-z protocols.
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitc...@atmel.com>
[...]
> @@ -282,19 +305,22 @@ struct spi_nor_hwcaps {
> * As a matter of performances, it is relevant to use Quad SPI protocols
> first,
> * then Dual SPI protocols before Fast Read and lastly (Slow) Read.
> */
> -#define SNOR_HWCAPS_READ_MASKGENMASK(7, 0)
> +#define SNOR_HWCAPS_READ_MASKGENMASK(10, 0)
> #define SNOR_HWCAPS_READ BIT(0)
> #define SNOR_HWCAPS_READ_FASTBIT(1)
> -
> -#define SNOR_HWCAPS_READ_DUALGENMASK(4, 2)
> -#define SNOR_HWCAPS_READ_1_1_2 BIT(2)
> -#define SNOR_HWCAPS_READ_1_2_2 BIT(3)
> -#define SNOR_HWCAPS_READ_2_2_2 BIT(4)
> -
> -#define SNOR_HWCAPS_READ_QUADGENMASK(7, 5)
> -#define SNOR_HWCAPS_READ_1_1_4 BIT(5)
> -#define SNOR_HWCAPS_READ_1_4_4 BIT(6)
> -#define SNOR_HWCAPS_READ_4_4_4 BIT(7)
> +#define SNOR_HWCAPS_READ_1_1_1_DTR BIT(2)
> +
> +#define SNOR_HWCAPS_READ_DUALGENMASK(6, 3)
> +#define SNOR_HWCAPS_READ_1_1_2 BIT(3)
> +#define SNOR_HWCAPS_READ_1_2_2 BIT(4)
> +#define SNOR_HWCAPS_READ_2_2_2 BIT(5)
> +#define SNOR_HWCAPS_READ_1_2_2_DTR BIT(6)
> +
> +#define SNOR_HWCAPS_READ_QUADGENMASK(10, 7)
> +#define SNOR_HWCAPS_READ_1_1_4 BIT(7)
> +#define SNOR_HWCAPS_READ_1_4_4 BIT(8)
> +#define SNOR_HWCAPS_READ_4_4_4 BIT(9)
> +#define SNOR_HWCAPS_READ_1_4_4_DTR BIT(10)
I can't say I'm a big fan on this re-numeration when you add a new
entry. But unless you have a better idea, we'll have to live with this ...
--
Best regards,
Marek Vasut
Re: [PATCH v7 4/4] mtd: spi-nor: introduce Octo SPI protocols
On 04/19/2017 12:51 AM, Cyrille Pitchen wrote: > This patch starts adding support to Octo SPI protocols (SPI x-y-8). > > Op codes for Fast Read and/or Page Program operations using Octo SPI > protocols are not known yet (no JEDEC specification has defined them yet) > but we'd rather introduce the Octo SPI protocols now so it's done as it > should be. > > Signed-off-by: Cyrille Pitchen <cyrille.pitc...@atmel.com> Reviewed-by: Marek Vasut <marek.va...@gmail.com> -- Best regards, Marek Vasut
Re: [PATCH v7 1/4] mtd: spi-nor: introduce SPI 1-2-2 and SPI 1-4-4 protocols
On 04/19/2017 12:51 AM, Cyrille Pitchen wrote:
> This patch changes the prototype of spi_nor_scan(): its 3rd parameter
> is replaced by a 'struct spi_nor_hwcaps' pointer, which tells the spi-nor
> framework about the actual hardware capabilities supported by the SPI
> controller and its driver.
>
> Besides, this patch also introduces a new 'struct spi_nor_flash_parameter'
> telling the spi-nor framework about the hardware capabilities supported by
> the SPI flash memory and the associated settings required to use those
> hardware caps.
>
> Then, to improve the readability of spi_nor_scan(), the discovery of the
> memory settings and the memory initialization are now split into two
> dedicated functions.
>
> 1 - spi_nor_init_params()
>
> The spi_nor_init_params() function is responsible for initializing the
> 'struct spi_nor_flash_parameter'. Currently this structure is filled with
> legacy values but further patches will allow to override some parameter
> values dynamically, for instance by reading the JESD216 Serial Flash
> Discoverable Parameter (SFDP) tables from the SPI memory.
> The spi_nor_init_params() function only deals with the hardware
> capabilities of the SPI flash memory: especially it doesn't care about
> the hardware capabilities supported by the SPI controller.
>
> 2 - spi_nor_setup()
>
> The second function is called once the 'struct spi_nor_flash_parameter'
> has been initialized by spi_nor_init_params().
> With both 'struct spi_nor_flash_parameter' and 'struct spi_nor_hwcaps',
> the new argument of spi_nor_scan(), spi_nor_setup() computes the best
> match between hardware caps supported by both the (Q)SPI memory and
> controller hence selecting the relevant settings for (Fast) Read and Page
> Program operations.
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitc...@atmel.com>
[...]
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -119,13 +119,57 @@
> /* Configuration Register bits. */
> #define CR_QUAD_EN_SPAN BIT(1) /* Spansion Quad I/O */
>
> -enum read_mode {
> - SPI_NOR_NORMAL = 0,
> - SPI_NOR_FAST,
> - SPI_NOR_DUAL,
> - SPI_NOR_QUAD,
> +/* Supported SPI protocols */
> +#define SNOR_PROTO_INST_MASK GENMASK(23, 16)
> +#define SNOR_PROTO_INST_SHIFT16
> +#define SNOR_PROTO_INST(_nbits) \
> + u32)(_nbits)) << SNOR_PROTO_INST_SHIFT) & SNOR_PROTO_INST_MASK)
Is the u32 cast needed ?
> +#define SNOR_PROTO_ADDR_MASK GENMASK(15, 8)
> +#define SNOR_PROTO_ADDR_SHIFT8
> +#define SNOR_PROTO_ADDR(_nbits) \
> + u32)(_nbits)) << SNOR_PROTO_ADDR_SHIFT) & SNOR_PROTO_ADDR_MASK)
> +
> +#define SNOR_PROTO_DATA_MASK GENMASK(7, 0)
> +#define SNOR_PROTO_DATA_SHIFT0
> +#define SNOR_PROTO_DATA(_nbits) \
> + u32)(_nbits)) << SNOR_PROTO_DATA_SHIFT) & SNOR_PROTO_DATA_MASK)
[...]
> +static inline u8 spi_nor_get_protocol_inst_nbits(enum spi_nor_protocol proto)
> +{
> + return ((u32)(proto & SNOR_PROTO_INST_MASK)) >> SNOR_PROTO_INST_SHIFT;
DTTO, is the cast needed ?
> +}
> +
> +static inline u8 spi_nor_get_protocol_addr_nbits(enum spi_nor_protocol proto)
> +{
> + return ((u32)(proto & SNOR_PROTO_ADDR_MASK)) >> SNOR_PROTO_ADDR_SHIFT;
> +}
> +
> +static inline u8 spi_nor_get_protocol_data_nbits(enum spi_nor_protocol proto)
> +{
> + return ((u32)(proto & SNOR_PROTO_DATA_MASK)) >> SNOR_PROTO_DATA_SHIFT;
> +}
> +
> +static inline u8 spi_nor_get_protocol_width(enum spi_nor_protocol proto)
> +{
> + return spi_nor_get_protocol_data_nbits(proto);
> +}
[...]
Looks good otherwise.
--
Best regards,
Marek Vasut
Re: [PATCH v2 3/3] mtd: dataflash: Make use of "extened device information"
On 04/19/2017 04:58 AM, Andrey Smirnov wrote:
> On Tue, Apr 18, 2017 at 11:31 AM, Marek Vasut <marek.va...@gmail.com> wrote:
>> On 04/18/2017 04:21 PM, Andrey Smirnov wrote:
>>> In anticipation of supporting chips that need it, extend the size of
>>> struct flash_info's 'jedec_id' field to make room 2 byte of extended
>>> device information as well as add code to fetch this data during
>>> jedec_probe().
>>>
>>> Cc: cphe...@gmail.com
>>> Cc: David Woodhouse <dw...@infradead.org>
>>> Cc: Brian Norris <computersforpe...@gmail.com>
>>> Cc: Boris Brezillon <boris.brezil...@free-electrons.com>
>>> Cc: Marek Vasut <marek.va...@gmail.com>
>>> Cc: Richard Weinberger <rich...@nod.at>
>>> Cc: Cyrille Pitchen <cyrille.pitc...@atmel.com>
>>> Cc: linux-kernel@vger.kernel.org
>>> Signed-off-by: Andrey Smirnov <andrew.smir...@gmail.com>
>>> ---
>>>
>>> Changes since [v1]:
>>>
>>> - Formatting
>>>
>>> [v1]
>>> http://lkml.kernel.org/r/20170411161722.11164-1-andrew.smir...@gmail.com
>>>
>>>
>>> drivers/mtd/devices/mtd_dataflash.c | 113
>>> +---
>>> 1 file changed, 65 insertions(+), 48 deletions(-)
>>>
>>> diff --git a/drivers/mtd/devices/mtd_dataflash.c
>>> b/drivers/mtd/devices/mtd_dataflash.c
>>> index 5b7a8c3..5d694a4 100644
>>> --- a/drivers/mtd/devices/mtd_dataflash.c
>>> +++ b/drivers/mtd/devices/mtd_dataflash.c
>>> @@ -690,7 +690,7 @@ struct flash_info {
>>> /* JEDEC id has a high byte of zero plus three data bytes:
>>>* the manufacturer id, then a two byte device id.
>>>*/
>>> - u32 jedec_id;
>>> + u64 jedec_id;
>>>
>>> /* The size listed here is what works with OP_ERASE_PAGE. */
>>> unsignednr_pages;
>>> @@ -713,63 +713,34 @@ static struct flash_info dataflash_data[] = {
>>>* These newer chips also support 128-byte security registers (with
>>>* 64 bytes one-time-programmable) and software write-protection.
>>>*/
>>> - { "AT45DB011B", 0x1f2200, 512, 264, 9, SUP_POW2PS},
>>> - { "at45db011d", 0x1f2200, 512, 256, 8, SUP_POW2PS | IS_POW2PS},
>>> + { "AT45DB011B", 0x1f2200, 512, 264, 9, SUP_POW2PS},
>>> + { "at45db011d", 0x1f2200, 512, 256, 8, SUP_POW2PS | IS_POW2PS},
>>>
>>> - { "AT45DB021B", 0x1f2300, 1024, 264, 9, SUP_POW2PS},
>>> - { "at45db021d", 0x1f2300, 1024, 256, 8, SUP_POW2PS | IS_POW2PS},
>>> + { "AT45DB021B", 0x1f2300, 1024, 264, 9, SUP_POW2PS},
>>> + { "at45db021d", 0x1f2300, 1024, 256, 8, SUP_POW2PS | IS_POW2PS},
>>>
>>> - { "AT45DB041x", 0x1f2400, 2048, 264, 9, SUP_POW2PS},
>>> - { "at45db041d", 0x1f2400, 2048, 256, 8, SUP_POW2PS | IS_POW2PS},
>>> + { "AT45DB041x", 0x1f2400, 2048, 264, 9, SUP_POW2PS},
>>> + { "at45db041d", 0x1f2400, 2048, 256, 8, SUP_POW2PS | IS_POW2PS},
>>>
>>> - { "AT45DB081B", 0x1f2500, 4096, 264, 9, SUP_POW2PS},
>>> - { "at45db081d", 0x1f2500, 4096, 256, 8, SUP_POW2PS | IS_POW2PS},
>>> + { "AT45DB081B", 0x1f2500, 4096, 264, 9, SUP_POW2PS},
>>> + { "at45db081d", 0x1f2500, 4096, 256, 8, SUP_POW2PS | IS_POW2PS},
>>>
>>> - { "AT45DB161x", 0x1f2600, 4096, 528, 10, SUP_POW2PS},
>>> - { "at45db161d", 0x1f2600, 4096, 512, 9, SUP_POW2PS | IS_POW2PS},
>>> + { "AT45DB161x", 0x1f2600, 4096, 528, 10, SUP_POW2PS},
>>> + { "at45db161d", 0x1f2600, 4096, 512, 9, SUP_POW2PS | IS_POW2PS},
>>>
>>> - { "AT45DB321x", 0x1f2700, 8192, 528, 10, 0}, /* rev C */
>>> + { "AT45DB321x", 0x1f2700, 8192, 528, 10, 0}, /* rev C */
>>>
>>> - { "AT45DB321x", 0x1f2701, 8192, 528, 10, SUP_POW2PS},
>>> - { "at45db321d", 0x1f2701, 8192, 512, 9, SUP_POW2PS | IS_POW2PS},
>>> + { "AT45DB321x", 0x1f2701, 8192, 528, 10, SUP_POW2PS},
>>> + { "at45db321d", 0x1f2701, 8192, 512, 9, SUP_POW2PS | IS_POW2PS},
>>>
>>> - { "AT45DB642x", 0x1f2800, 8192,
Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller
gt;dev = aspi->dev;
> + nor->read = anarion_spi_nor_read;
> + nor->write = anarion_spi_nor_write;
> + nor->read_reg = anarion_spi_read_nor_reg;
> + nor->write_reg = anarion_spi_write_nor_reg;
> +
> + spi_nor_set_flash_node(nor, flash_node);
> +
> + ret = spi_nor_scan(>nor, NULL, SPI_NOR_DUAL);
> + if (ret)
> + return ret;
> +
> + switch (nor->flash_read) {
> + default:/* Fall through */
This will break once we add OSPI support ...
> + case SPI_NOR_NORMAL:
> + aspi->num_hi_z_clocks = nor->read_dummy;
> + aspi->xfer_mode_cmd = MODE_IO_X1;
> + aspi->xfer_mode_addr = MODE_IO_X1;
> + aspi->xfer_mode_data = MODE_IO_X1;
> + break;
> + case SPI_NOR_FAST:
> + aspi->num_hi_z_clocks = nor->read_dummy;
> + aspi->xfer_mode_cmd = MODE_IO_X1;
> + aspi->xfer_mode_addr = MODE_IO_X1;
> + aspi->xfer_mode_data = MODE_IO_X1;
> + break;
> + case SPI_NOR_DUAL:
> + aspi->num_hi_z_clocks = nor->read_dummy;
> + aspi->xfer_mode_cmd = MODE_IO_X1;
> + aspi->xfer_mode_addr = MODE_IO_X1;
> + aspi->xfer_mode_data = MODE_IO_X2;
> + break;
> + case SPI_NOR_QUAD:
> + aspi->num_hi_z_clocks = nor->read_dummy;
> + aspi->xfer_mode_cmd = MODE_IO_X1;
> + aspi->xfer_mode_addr = MODE_IO_X1;
> + aspi->xfer_mode_data = MODE_IO_X4;
> + break;
> + }
> +
> + aspi_setup_xip_read_chain(aspi, nor);
> +
> + mtd_device_register(>nor.mtd, NULL, 0);
> +
> + return 0;
> +}
> +
> +static int anarion_qspi_drv_remove(struct platform_device *pdev)
> +{
> + struct anarion_qspi *aspi = platform_get_drvdata(pdev);
> +
> + mtd_device_unregister(>nor.mtd);
> + return 0;
> +}
> +
> +static const struct of_device_id anarion_qspi_of_match[] = {
> + { .compatible = "adaptrum,anarion-qspi" },
> + { }
> +};
> +MODULE_DEVICE_TABLE(of, anarion_qspi_of_match);
> +
> +static struct platform_driver anarion_qspi_driver = {
> + .driver = {
> + .name = "anarion-qspi",
> + .of_match_table = anarion_qspi_of_match,
> + },
> + .probe = anarion_qspi_drv_probe,
> + .remove = anarion_qspi_drv_remove,
> +};
> +module_platform_driver(anarion_qspi_driver);
> +
> +MODULE_DESCRIPTION("Adaptrum Anarion Quad SPI Controller Driver");
> +MODULE_AUTHOR("Alexandru Gagniuc <mr.nuke...@gmail.com>");
> +MODULE_LICENSE("GPL v2");
>
--
Best regards,
Marek Vasut
Re: [PATCH V4 2/2] mfd: Add ROHM BD9571MWV-M MFD PMIC driver
On 07/18/2017 11:23 AM, Lee Jones wrote: > On Mon, 17 Jul 2017, Marek Vasut wrote: > >> Add the MFD part of the ROHM BD9571MWV-M PMIC driver and MAINTAINERS >> entry. The MFD part only specifies the regmap bits for the PMIC and >> binds the subdevs together. >> >> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com> >> Cc: linux-kernel@vger.kernel.org >> Cc: Geert Uytterhoeven <geert+rene...@glider.be> >> Cc: Lee Jones <lee.jo...@linaro.org> >> --- >> V2: - Change BD9571MWV_AVS_VD09_VID0,1,2,3 to BD9571MWV_AVS_VD09_VID(n) >> - Change BD9571MWV_AVS_DVFS_VID0,1,2,3 to BD9571MWV_AVS_DVFS_VID(n) >> - Make the AVS_VD09 range RW, so it can be used by the regulator >> driver for the VD09 regulator >> - Report the regmap read return values when attempting to read ID >> registers fails >> V3: No change >> V4: select REGMAP_IRQ >> --- >> MAINTAINERS | 11 ++ >> drivers/mfd/Kconfig | 14 +++ >> drivers/mfd/Makefile | 1 + >> drivers/mfd/bd9571mwv.c | 230 >> ++ >> include/linux/mfd/bd9571mwv.h | 115 + >> 5 files changed, 371 insertions(+) >> create mode 100644 drivers/mfd/bd9571mwv.c >> create mode 100644 include/linux/mfd/bd9571mwv.h > > Applied, thanks. I still don't see these patches in next, why ? -- Best regards, Marek Vasut
Re: [PATCH V3 2/2] mfd: Add ROHM BD9571MWV-M MFD PMIC driver
On 07/10/2017 05:33 PM, Geert Uytterhoeven wrote: > Hi Marek, > > On Tue, May 2, 2017 at 2:18 PM, Marek Vasut <marek.va...@gmail.com> wrote: >> Add the MFD part of the ROHM BD9571MWV-M PMIC driver and MAINTAINERS >> entry. The MFD part only specifies the regmap bits for the PMIC and >> binds the subdevs together. >> >> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com> >> Cc: linux-kernel@vger.kernel.org >> Cc: Geert Uytterhoeven <geert+rene...@glider.be> >> Cc: Lee Jones <lee.jo...@linaro.org> > >> --- a/drivers/mfd/Kconfig >> +++ b/drivers/mfd/Kconfig >> @@ -133,6 +133,19 @@ config MFD_BCM590XX >> help >> Support for the BCM590xx PMUs from Broadcom >> >> +config MFD_BD9571MWV >> + tristate "ROHM BD9571MWV PMIC" >> + select MFD_CORE >> + select REGMAP_I2C > > Missing "select REGMAP_IRQ". > > Else it fails with: Added -- Best regards, Marek Vasut
[PATCH V4 2/2] mfd: Add ROHM BD9571MWV-M MFD PMIC driver
Add the MFD part of the ROHM BD9571MWV-M PMIC driver and MAINTAINERS
entry. The MFD part only specifies the regmap bits for the PMIC and
binds the subdevs together.
Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
Cc: linux-kernel@vger.kernel.org
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: Lee Jones <lee.jo...@linaro.org>
---
V2: - Change BD9571MWV_AVS_VD09_VID0,1,2,3 to BD9571MWV_AVS_VD09_VID(n)
- Change BD9571MWV_AVS_DVFS_VID0,1,2,3 to BD9571MWV_AVS_DVFS_VID(n)
- Make the AVS_VD09 range RW, so it can be used by the regulator
driver for the VD09 regulator
- Report the regmap read return values when attempting to read ID
registers fails
V3: No change
V4: select REGMAP_IRQ
---
MAINTAINERS | 11 ++
drivers/mfd/Kconfig | 14 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/bd9571mwv.c | 230 ++
include/linux/mfd/bd9571mwv.h | 115 +
5 files changed, 371 insertions(+)
create mode 100644 drivers/mfd/bd9571mwv.c
create mode 100644 include/linux/mfd/bd9571mwv.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 0b6ed8eb58b0..f369c1cb287e 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11291,6 +11291,17 @@ L: linux-ser...@vger.kernel.org
S: Odd Fixes
F: drivers/tty/serial/rp2.*
+ROHM MULTIFUNCTION BD9571MWV-M PMIC DEVICE DRIVERS
+M: Marek Vasut <marek.vasut+rene...@gmail.com>
+L: linux-kernel@vger.kernel.org
+L: linux-renesas-...@vger.kernel.org
+S: Supported
+F: drivers/mfd/bd9571mwv.c
+F: drivers/regulator/bd9571mwv-regulator.c
+F: drivers/gpio/gpio-bd9571mwv.c
+F: include/linux/mfd/bd9571mwv.h
+F: Documentation/devicetree/bindings/mfd/bd9571mwv.txt
+
ROSE NETWORK LAYER
M: Ralf Baechle <r...@linux-mips.org>
L: linux-h...@vger.kernel.org
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 94ad2c1c3d90..667665926fb3 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -133,6 +133,20 @@ config MFD_BCM590XX
help
Support for the BCM590xx PMUs from Broadcom
+config MFD_BD9571MWV
+ tristate "ROHM BD9571MWV PMIC"
+ select MFD_CORE
+ select REGMAP_I2C
+ select REGMAP_IRQ
+ depends on I2C
+ help
+ Support for the ROHM BD9571MWV PMIC, which contains single
+ voltage regulator, voltage sampling units, GPIO block and
+ watchdog block.
+
+ This driver can also be built as a module. If so, the module
+ will be called bd9571mwv.
+
config MFD_AC100
tristate "X-Powers AC100"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 080793b3fd0e..db15df3d9e8d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_MFD_ACT8945A)+= act8945a.o
obj-$(CONFIG_MFD_SM501)+= sm501.o
obj-$(CONFIG_MFD_ASIC3)+= asic3.o tmio_core.o
obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
+obj-$(CONFIG_MFD_BD9571MWV)+= bd9571mwv.o
cros_ec_core-objs := cros_ec.o
cros_ec_core-$(CONFIG_ACPI)+= cros_ec_acpi_gpe.o
obj-$(CONFIG_MFD_CROS_EC) += cros_ec_core.o
diff --git a/drivers/mfd/bd9571mwv.c b/drivers/mfd/bd9571mwv.c
new file mode 100644
index ..64e088dfe7b0
--- /dev/null
+++ b/drivers/mfd/bd9571mwv.c
@@ -0,0 +1,230 @@
+/*
+ * ROHM BD9571MWV-M MFD driver
+ *
+ * Copyright (C) 2017 Marek Vasut <marek.vasut+rene...@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether expressed or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License version 2 for more details.
+ *
+ * Based on the TPS65086 driver
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+static const struct mfd_cell bd9571mwv_cells[] = {
+ { .name = "bd9571mwv-regulator", },
+ { .name = "bd9571mwv-gpio", },
+};
+
+static const struct regmap_range bd9571mwv_readable_yes_ranges[] = {
+ regmap_reg_range(BD9571MWV_VENDOR_CODE, BD9571MWV_PRODUCT_REVISION),
+ regmap_reg_range(BD9571MWV_AVS_SET_MONI, BD9571MWV_AVS_DVFS_VID(3)),
+ regmap_reg_range(BD9571MWV_VD18_VID, BD9571MWV_VD33_VID),
+ regmap_reg_range(BD9571MWV_DVFS_VINIT, BD9571MWV_DVFS_VINIT),
+ regmap_reg_range(BD9571MWV_DVFS_SETVMAX, BD9571MWV_DVFS_MONIVDAC),
+ regmap_reg_range(BD9571MWV_GPIO_IN, BD9571MWV_GPIO_IN),
+ regmap_reg_range(BD9571MWV_GPIO_INT, BD9571MWV_GPIO_INTMASK),
+ regmap_reg_range(BD9571MWV_INT_INTREQ, BD9571MWV_INT_INTMASK),
+};
+
+static const struct regmap_access_tabl
Re: [PATCH] mtd: nand: vf610_nfc: add NULL check on of_match_device() return value
On 07/17/2017 10:46 PM, Boris Brezillon wrote:
> Le Fri, 7 Jul 2017 01:59:26 -0500,
> "Gustavo A. R. Silva" <garsi...@embeddedor.com> a écrit :
>
>> Check return value from call to of_match_device()
>> in order to prevent a NULL pointer dereference.
>>
>> In case of NULL print error message and return -ENODEV
>>
>> Signed-off-by: Gustavo A. R. Silva <garsi...@embeddedor.com>
>> ---
>> drivers/mtd/nand/vf610_nfc.c | 5 +
>> 1 file changed, 5 insertions(+)
>>
>> diff --git a/drivers/mtd/nand/vf610_nfc.c b/drivers/mtd/nand/vf610_nfc.c
>> index 744ab10..ca0ab96 100644
>> --- a/drivers/mtd/nand/vf610_nfc.c
>> +++ b/drivers/mtd/nand/vf610_nfc.c
>> @@ -674,6 +674,11 @@ static int vf610_nfc_probe(struct platform_device *pdev)
>> }
>>
>> of_id = of_match_device(vf610_nfc_dt_ids, >dev);
>> +if (!of_id) {
>> +dev_err(>dev, "Failed to match device!\n");
>> +return -ENODEV;
>> +}
>> +
>
> While this check is functionally correct, this case cannot happen,
> because this is DT-only driver, and without a valid match in
> vf610_nfc_dt_ids the dev wouldn't have been probed in the first place.
>
> I'll let Stefan decide whether he wants it or not, but I see no real
> reason for this extra check.
So how did you trigger the issue in the first place ?
--
Best regards,
Marek Vasut
Re: [PATCH 1/2] clk: vc5: Add support for IDT VersaClock 5P49V5925
On 07/07/2017 03:57 AM, Vladimir Barinov wrote:
> From: Vladimir Barinov <vladimir.barinov+rene...@cogentembedded.com>
>
> Update IDT VersaClock 5 driver to support 5P49V5925. This chip has only
> external clock input, four fractional dividers (FODs) and five clock
> outputs (four universal clock outputs and one reference clock output at
> OUT0_SELB_I2C).
>
> Signed-off-by: Vladimir Barinov <vladimir.barinov+rene...@cogentembedded.com>
Reviewed-by: Marek Vasut <marek.va...@gmail.com>
> ---
> drivers/clk/clk-versaclock5.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/clk/clk-versaclock5.c b/drivers/clk/clk-versaclock5.c
> index ea7d552..88b749d 100644
> --- a/drivers/clk/clk-versaclock5.c
> +++ b/drivers/clk/clk-versaclock5.c
> @@ -126,6 +126,7 @@
> /* Supported IDT VC5 models. */
> enum vc5_model {
> IDT_VC5_5P49V5923,
> + IDT_VC5_5P49V5925,
> IDT_VC5_5P49V5933,
> IDT_VC5_5P49V5935,
> };
> @@ -595,6 +596,7 @@ static int vc5_map_index_to_output(const enum vc5_model
> model,
> case IDT_VC5_5P49V5933:
> return (n == 0) ? 0 : 3;
> case IDT_VC5_5P49V5923:
> + case IDT_VC5_5P49V5925:
> case IDT_VC5_5P49V5935:
> default:
> return n;
> @@ -785,6 +787,13 @@ static int vc5_remove(struct i2c_client *client)
> .flags = 0,
> };
>
> +static const struct vc5_chip_info idt_5p49v5925_info = {
> + .model = IDT_VC5_5P49V5925,
> + .clk_fod_cnt = 4,
> + .clk_out_cnt = 5,
> + .flags = 0,
> +};
> +
> static const struct vc5_chip_info idt_5p49v5933_info = {
> .model = IDT_VC5_5P49V5933,
> .clk_fod_cnt = 2,
> @@ -801,6 +810,7 @@ static int vc5_remove(struct i2c_client *client)
>
> static const struct i2c_device_id vc5_id[] = {
> { "5p49v5923", .driver_data = IDT_VC5_5P49V5923 },
> + { "5p49v5925", .driver_data = IDT_VC5_5P49V5925 },
> { "5p49v5933", .driver_data = IDT_VC5_5P49V5933 },
> { "5p49v5935", .driver_data = IDT_VC5_5P49V5935 },
> { }
> @@ -809,6 +819,7 @@ static int vc5_remove(struct i2c_client *client)
>
> static const struct of_device_id clk_vc5_of_match[] = {
> { .compatible = "idt,5p49v5923", .data = _5p49v5923_info },
> + { .compatible = "idt,5p49v5925", .data = _5p49v5925_info },
> { .compatible = "idt,5p49v5933", .data = _5p49v5933_info },
> { .compatible = "idt,5p49v5935", .data = _5p49v5935_info },
> { },
>
--
Best regards,
Marek Vasut
Re: [PATCH 2/2] dt: Add bindings for IDT VersaClock 5P49V5925
On 07/07/2017 03:57 AM, Vladimir Barinov wrote: > From: Vladimir Barinov <vladimir.barinov+rene...@cogentembedded.com> > > IDT VersaClock 5 5P49V5925 has 4 clock outputs In 1/2 you said it has 4 FODs and 5 outputs (and it does have 5 outputs, 4 from FODs and 1 I2C_OUTB). >, 4 fractional dividers. > Input clock source can be taken only from external reference clock. > > Signed-off-by: Vladimir Barinov <vladimir.barinov+rene...@cogentembedded.com> You might want to rebase this on top of and retest against [1], there are 8 patches in total fixing some stuff and adding VC6 support. [1] https://patchwork.kernel.org/patch/9831797/ > --- > Documentation/devicetree/bindings/clock/idt,versaclock5.txt | 11 +++ > 1 file changed, 7 insertions(+), 4 deletions(-) > > diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.txt > b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt > index 53d7e50..a1ad9e0 100644 > --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.txt > +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt > @@ -6,19 +6,21 @@ from 3 to 12 output clocks. > ==I2C device node== > > Required properties: > -- compatible:shall be one of "idt,5p49v5923" , "idt,5p49v5933" , > - "idt,5p49v5935". > +- compatible:shall be one of "idt,5p49v5923" , "idt,5p49v5925" , > + "idt,5p49v5933", "idt,5p49v5935". > - reg: i2c device address, shall be 0x68 or 0x6a. > - #clock-cells: from common clock binding; shall be set to 1. > - clocks:from common clock binding; list of parent clock handles, > - - 5p49v5923: (required) either or both of XTAL or CLKIN > + - 5p49v5923 and > + - 5p49v5925: (required) either or both of XTAL or CLKIN > reference clock. > - 5p49v5933 and > - 5p49v5935: (optional) property not present (internal > Xtal used) or CLKIN reference > clock. > - clock-names: from common clock binding; clock input names, can be > - - 5p49v5923: (required) either or both of "xin", "clkin". > + - 5p49v5923 and > + - 5p49v5925: (required) either or both of "xin", "clkin". > - 5p49v5933 and > - 5p49v5935: (optional) property not present or "clkin". > > @@ -37,6 +39,7 @@ clock specifier, the following mapping applies: > 1 -- OUT1 > 2 -- OUT4 > > +5P49V5925 and > 5P49V5935: > 0 -- OUT0_SEL_I2CB > 1 -- OUT1 > -- Best regards, Marek Vasut
Re: [PATCH 2/2] dt: Add bindings for IDT VersaClock 5P49V5925
On 07/09/2017 07:38 PM, Vladimir Barinov wrote: > On 09.07.2017 16:31, Marek Vasut wrote: >> On 07/07/2017 03:57 AM, Vladimir Barinov wrote: >>> From: Vladimir Barinov <vladimir.barinov+rene...@cogentembedded.com> >>> >>> IDT VersaClock 5 5P49V5925 has 4 clock outputs >> In 1/2 you said it has 4 FODs and 5 outputs (and it does have 5 outputs, >> 4 from FODs and 1 I2C_OUTB). > That was a copy typo. Thank you for pointing to this. > >> >>> , 4 fractional dividers. >>> Input clock source can be taken only from external reference clock. >>> >>> Signed-off-by: Vladimir Barinov >>> <vladimir.barinov+rene...@cogentembedded.com> >> You might want to rebase this on top of and retest against [1], there >> are 8 patches in total fixing some stuff and adding VC6 support. >> >> [1] https://patchwork.kernel.org/patch/9831797/ > > I've tested your patch series with ULCB board and HDMI display. > I will rebase 5P49V5925 patches on top of them. Thanks! That makes things easy :) -- Best regards, Marek Vasut
Re: [PATCH v2 2/2] dt: Add bindings for IDT VersaClock 5P49V5925
On 07/09/2017 07:40 PM, Vladimir Barinov wrote: > From: Vladimir Barinov <vladimir.barinov+rene...@cogentembedded.com> > > IDT VersaClock 5 5P49V5925 has 5 clock outputs, 4 fractional dividers. > Input clock source can be taken only from external reference clock. > > Signed-off-by: Vladimir Barinov <vladimir.barinov+rene...@cogentembedded.com> Reviewed-by: Marek Vasut <marek.va...@gmail.com> > --- > Changes in version 2: > - fixed typo in patch header: VC5 has 5 clock outputs > - rebased against patch: > [V3,7/8] clk: vc5: Add bindings for IDT VersaClock 5P49V6901 > > Documentation/devicetree/bindings/clock/idt,versaclock5.txt | 4 > 1 file changed, 4 insertions(+) > > diff --git a/Documentation/devicetree/bindings/clock/idt,versaclock5.txt > b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt > index 66ef0a0..05a245c 100644 > --- a/Documentation/devicetree/bindings/clock/idt,versaclock5.txt > +++ b/Documentation/devicetree/bindings/clock/idt,versaclock5.txt > @@ -8,6 +8,7 @@ generators providing from 3 to 12 output clocks. > Required properties: > - compatible:shall be one of > "idt,5p49v5923" > + "idt,5p49v5925" > "idt,5p49v5933" > "idt,5p49v5935" > "idt,5p49v6901" > @@ -15,6 +16,7 @@ Required properties: > - #clock-cells: from common clock binding; shall be set to 1. > - clocks:from common clock binding; list of parent clock handles, > - 5p49v5923 and > + 5p49v5925 and > 5p49v6901: (required) either or both of XTAL or CLKIN > reference clock. > - 5p49v5933 and > @@ -23,6 +25,7 @@ Required properties: > clock. > - clock-names: from common clock binding; clock input names, can be > - 5p49v5923 and > + 5p49v5925 and > 5p49v6901: (required) either or both of "xin", "clkin". > - 5p49v5933 and > - 5p49v5935: (optional) property not present or "clkin". > @@ -42,6 +45,7 @@ clock specifier, the following mapping applies: > 1 -- OUT1 > 2 -- OUT4 > > +5P49V5925 and > 5P49V5935: > 0 -- OUT0_SEL_I2CB > 1 -- OUT1 > -- Best regards, Marek Vasut
Re: [PATCH 1/3] ARM: dts: Bindings for Altera Quadspi Controller Version 2
On 06/29/2017 05:03 PM, matthew.gerl...@linux.intel.com wrote: > > > On Thu, 29 Jun 2017, Marek Vasut wrote: > >> On 06/29/2017 01:09 AM, Rob Herring wrote: >>> On Tue, Jun 27, 2017 at 08:57:14AM -0700, >>> matthew.gerl...@linux.intel.com wrote: >>>> >>>> >>>> On Tue, 27 Jun 2017, Marek Vasut wrote: >>>> >>>>> On 06/27/2017 04:32 PM, matthew.gerl...@linux.intel.com wrote: >>>>>> >>>>>> >>>>>> On Tue, 27 Jun 2017, Marek Vasut wrote: >>>>>> >>>>>> Hi Marek, >>>>>> >>>>>> Thanks for the feedback. See my comments below. >>>>>> >>>>>> Matthew Gerlach >>>>>> >>>>>>> On 06/26/2017 06:13 PM, matthew.gerl...@linux.intel.com wrote: >>>>>>>> From: Matthew Gerlach <matthew.gerl...@linux.intel.com> >>>>>>>> >>>>>>>> Device Tree bindings for Version 2 of the Altera Quadspi Controller >>>>>>>> that can be optionally paired with a windowed bridge. >>>>>>>> >>>>>>>> Signed-off-by: Matthew Gerlach <matthew.gerl...@linux.intel.com> >>>>>>>> --- >>>>>>>> .../devicetree/bindings/mtd/altera-quadspi-v2.txt | 37 >>>>>>>> ++ >>>>>>>> 1 file changed, 37 insertions(+) >>>>>>>> create mode 100644 >>>>>>>> Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>>>>> >>>>>>>> diff --git >>>>>>>> a/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>>>>> b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>>>>> new file mode 100644 >>>>>>>> index 000..8ba63d7 >>>>>>>> --- /dev/null >>>>>>>> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>>>>> @@ -0,0 +1,37 @@ >>>>>>>> +* Altera Quad SPI Controller Version 2 >>>>>>>> + >>>>>>>> +Required properties: >>>>>>>> +- compatible : Should be "altr,quadspi-v2". >>>>>>>> +- reg : Contains at least two entries, and possibly three entries, >>>>>>>> each of >>>>>>>> +which is a tuple consisting of a physical address and length. >>>>>>>> +- reg-names : Should contain the names "avl_csr" and "avl_mem" >>>>>>>> corresponding >>>>>>>> + to the control and status registers and qspi memory, >>>>>>>> respectively. >>>>>>>> + >>>>>>>> + >>>>>>>> +The Altera Quad SPI Controller Version 2 can be paired with a >>>>>>>> windowed bridge >>>>>>>> +in order to reduce the footprint of the memory interface. When a >>>>>>>> windowed >>>>>>>> +bridge is used, reads and writes of data must be 32 bits wide. >>>>>>>> + >>>>>>>> +Optional properties: >>>>>>>> +- reg-names : Should contain the name "avl_window", if the >>>>>>>> windowed >>>>>>>> bridge >>>>>>>> + is used. This name corresponds to the register space >>>>>>>> that >>>>>>>> + controls the window. >>>>>>>> +- window-size : The size of the window which must be an even power >>>>>>>> of 2. >>>>>>>> +- read-bit-reverse : A boolean indicating the data read from the >>>>>>>> flash should >>>>>>>> + be bit reversed on a byte by byte basis before being >>>>>>>> + delivered to the MTD layer. >>>>>>>> +- write-bit-reverse : A boolean indicating the data written to the >>>>>>>> flash should >>>>>>>> + be bit reversed on a byte by byte basis. >>>>>>> >>>>>>> Is there ever a usecase where you need to set just one of these >>>>>>> props ? >>>>>>> Also, they
Re: [PATCH V3 2/2] mfd: Add ROHM BD9571MWV-M MFD PMIC driver
On 07/03/2017 01:55 PM, Lee Jones wrote: > On Tue, 27 Jun 2017, Marek Vasut wrote: > >> On 05/02/2017 02:18 PM, Marek Vasut wrote: >>> Add the MFD part of the ROHM BD9571MWV-M PMIC driver and MAINTAINERS >>> entry. The MFD part only specifies the regmap bits for the PMIC and >>> binds the subdevs together. >>> >>> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com> >>> Cc: linux-kernel@vger.kernel.org >>> Cc: Geert Uytterhoeven <geert+rene...@glider.be> >>> Cc: Lee Jones <lee.jo...@linaro.org> >> >> Lee, bump, do you plan to apply these patches anytime soon ? > > Looks like these were missed for some reason. Yes, they missed the previous merge window for some reason too, while the rest of the patches from the set made it in. It'd be real nice if they made it into the 4.13 ... > I'll put them back on the pile for review. > > Please note that the merge window is currently open, so you are likely > to experience a period of inactivity until -rc1 is released. I'd really appreciate it if these patches made it into the 4.13 release, see above. >>> --- >>> V2: - Change BD9571MWV_AVS_VD09_VID0,1,2,3 to BD9571MWV_AVS_VD09_VID(n) >>> - Change BD9571MWV_AVS_DVFS_VID0,1,2,3 to BD9571MWV_AVS_DVFS_VID(n) >>> - Make the AVS_VD09 range RW, so it can be used by the regulator >>> driver for the VD09 regulator >>> - Report the regmap read return values when attempting to read ID >>> registers fails >>> V3: No change >>> --- >>> MAINTAINERS | 11 ++ >>> drivers/mfd/Kconfig | 13 +++ >>> drivers/mfd/Makefile | 1 + >>> drivers/mfd/bd9571mwv.c | 230 >>> ++ >>> include/linux/mfd/bd9571mwv.h | 115 + >>> 5 files changed, 370 insertions(+) >>> create mode 100644 drivers/mfd/bd9571mwv.c >>> create mode 100644 include/linux/mfd/bd9571mwv.h >>> >>> diff --git a/MAINTAINERS b/MAINTAINERS >>> index 2a9aa4be5846..9c30e6b00358 100644 >>> --- a/MAINTAINERS >>> +++ b/MAINTAINERS >>> @@ -10942,6 +10942,17 @@ L: linux-ser...@vger.kernel.org >>> S: Odd Fixes >>> F: drivers/tty/serial/rp2.* >>> >>> +ROHM MULTIFUNCTION BD9571MWV-M PMIC DEVICE DRIVERS >>> +M: Marek Vasut <marek.vasut+rene...@gmail.com> >>> +L: linux-kernel@vger.kernel.org >>> +L: linux-renesas-...@vger.kernel.org >>> +S: Supported >>> +F: drivers/mfd/bd9571mwv.c >>> +F: drivers/regulator/bd9571mwv-regulator.c >>> +F: drivers/gpio/gpio-bd9571mwv.c >>> +F: include/linux/mfd/bd9571mwv.h >>> +F: Documentation/devicetree/bindings/mfd/bd9571mwv.txt >>> + >>> ROSE NETWORK LAYER >>> M: Ralf Baechle <r...@linux-mips.org> >>> L: linux-h...@vger.kernel.org >>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >>> index 3eb5c93595f6..6f4668ae28cc 100644 >>> --- a/drivers/mfd/Kconfig >>> +++ b/drivers/mfd/Kconfig >>> @@ -133,6 +133,19 @@ config MFD_BCM590XX >>> help >>> Support for the BCM590xx PMUs from Broadcom >>> >>> +config MFD_BD9571MWV >>> + tristate "ROHM BD9571MWV PMIC" >>> + select MFD_CORE >>> + select REGMAP_I2C >>> + depends on I2C >>> + help >>> + Support for the ROHM BD9571MWV PMIC, which contains single >>> + voltage regulator, voltage sampling units, GPIO block and >>> + watchdog block. >>> + >>> + This driver can also be built as a module. If so, the module >>> + will be called bd9571mwv. >>> + >>> config MFD_AC100 >>> tristate "X-Powers AC100" >>> select MFD_CORE >>> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile >>> index c16bf1ea0ea9..3cacc9748e13 100644 >>> --- a/drivers/mfd/Makefile >>> +++ b/drivers/mfd/Makefile >>> @@ -10,6 +10,7 @@ obj-$(CONFIG_MFD_ACT8945A)+= act8945a.o >>> obj-$(CONFIG_MFD_SM501)+= sm501.o >>> obj-$(CONFIG_MFD_ASIC3)+= asic3.o tmio_core.o >>> obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o >>> +obj-$(CONFIG_MFD_BD9571MWV)+= bd9571mwv.o >>> cros_ec_core-objs := cros_ec.o >>> cros_ec_core-$(CONFIG_ACPI)+= cros_ec_acpi_gpe.o >>> obj-$(CONFIG_MFD_CROS_EC) += cros_ec_core.o >>> diff --git a/drivers/mfd/bd9571mwv.c b/drivers/m
Re: [PATCH V3 2/2] mfd: Add ROHM BD9571MWV-M MFD PMIC driver
On 07/03/2017 03:48 PM, Lee Jones wrote: > On Mon, 03 Jul 2017, Marek Vasut wrote: > >> On 07/03/2017 01:55 PM, Lee Jones wrote: >>> On Tue, 27 Jun 2017, Marek Vasut wrote: >>> >>>> On 05/02/2017 02:18 PM, Marek Vasut wrote: >>>>> Add the MFD part of the ROHM BD9571MWV-M PMIC driver and MAINTAINERS >>>>> entry. The MFD part only specifies the regmap bits for the PMIC and >>>>> binds the subdevs together. >>>>> >>>>> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com> >>>>> Cc: linux-kernel@vger.kernel.org >>>>> Cc: Geert Uytterhoeven <geert+rene...@glider.be> >>>>> Cc: Lee Jones <lee.jo...@linaro.org> >>>> >>>> Lee, bump, do you plan to apply these patches anytime soon ? >>> >>> Looks like these were missed for some reason. >> >> Yes, they missed the previous merge window for some reason too, while >> the rest of the patches from the set made it in. It'd be real nice if >> they made it into the 4.13 ... > > They won't. The merge window is already open. > > You're looking at v4.14 now. So the rest of the patches made it into 4.12 and the MFD bit will make it into 4.14 ... why precisely ? >>> I'll put them back on the pile for review. >>> >>> Please note that the merge window is currently open, so you are likely >>> to experience a period of inactivity until -rc1 is released. >> >> I'd really appreciate it if these patches made it into the 4.13 release, >> see above. >> >>>>> --- >>>>> V2: - Change BD9571MWV_AVS_VD09_VID0,1,2,3 to BD9571MWV_AVS_VD09_VID(n) >>>>> - Change BD9571MWV_AVS_DVFS_VID0,1,2,3 to BD9571MWV_AVS_DVFS_VID(n) >>>>> - Make the AVS_VD09 range RW, so it can be used by the regulator >>>>> driver for the VD09 regulator >>>>> - Report the regmap read return values when attempting to read ID >>>>> registers fails >>>>> V3: No change >>>>> --- >>>>> MAINTAINERS | 11 ++ >>>>> drivers/mfd/Kconfig | 13 +++ >>>>> drivers/mfd/Makefile | 1 + >>>>> drivers/mfd/bd9571mwv.c | 230 >>>>> ++ >>>>> include/linux/mfd/bd9571mwv.h | 115 + >>>>> 5 files changed, 370 insertions(+) >>>>> create mode 100644 drivers/mfd/bd9571mwv.c >>>>> create mode 100644 include/linux/mfd/bd9571mwv.h >>>>> >>>>> diff --git a/MAINTAINERS b/MAINTAINERS >>>>> index 2a9aa4be5846..9c30e6b00358 100644 >>>>> --- a/MAINTAINERS >>>>> +++ b/MAINTAINERS >>>>> @@ -10942,6 +10942,17 @@ L: linux-ser...@vger.kernel.org >>>>> S: Odd Fixes >>>>> F: drivers/tty/serial/rp2.* >>>>> >>>>> +ROHM MULTIFUNCTION BD9571MWV-M PMIC DEVICE DRIVERS >>>>> +M: Marek Vasut <marek.vasut+rene...@gmail.com> >>>>> +L: linux-kernel@vger.kernel.org >>>>> +L: linux-renesas-...@vger.kernel.org >>>>> +S: Supported >>>>> +F: drivers/mfd/bd9571mwv.c >>>>> +F: drivers/regulator/bd9571mwv-regulator.c >>>>> +F: drivers/gpio/gpio-bd9571mwv.c >>>>> +F: include/linux/mfd/bd9571mwv.h >>>>> +F: Documentation/devicetree/bindings/mfd/bd9571mwv.txt >>>>> + >>>>> ROSE NETWORK LAYER >>>>> M: Ralf Baechle <r...@linux-mips.org> >>>>> L: linux-h...@vger.kernel.org >>>>> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig >>>>> index 3eb5c93595f6..6f4668ae28cc 100644 >>>>> --- a/drivers/mfd/Kconfig >>>>> +++ b/drivers/mfd/Kconfig >>>>> @@ -133,6 +133,19 @@ config MFD_BCM590XX >>>>> help >>>>> Support for the BCM590xx PMUs from Broadcom >>>>> >>>>> +config MFD_BD9571MWV >>>>> + tristate "ROHM BD9571MWV PMIC" >>>>> + select MFD_CORE >>>>> + select REGMAP_I2C >>>>> + depends on I2C >>>>> + help >>>>> + Support for the ROHM BD9571MWV PMIC, which contains single >>>>> + voltage regulator, voltage sampling units, GPIO block and >>>>>
Re: [PATCH] mtd: spi-nor: cqspi: remove duplicate const
On 06/27/2017 09:48 PM, Cyrille Pitchen wrote:
> Le 27/06/2017 à 18:11, Marek Vasut a écrit :
>> On 06/27/2017 05:34 PM, Arnd Bergmann wrote:
>>> The variable was already marked 'const' before the previous
>>> patch, but the qualifier was in an unusual place, and now the
>>> extra 'const' causes a harmless warning:
>>>
>>> drivers/mtd/spi-nor/cadence-quadspi.c:1286:34: error: duplicate 'const'
>>> declaration specifier [-Werror=duplicate-decl-specifier]
>>>
>>> This removes the other 'const' instead.
>>
>> Isn't that const array const elements , thus two consts ?
>> IMO the original code is correct.
>
> Indeed the 2 'const' don't have the same meaning so the original code
> may be correct but with an array like cqspi_dt_ids[] we can't write
> "cqspi_dt_ids = (const struct of_device_id *);" anyway.
Not sure I understand what you're trying to say here.
> So I think the 2nd 'const' is useless here then if this patch removes a
> warning, let's apply it, right?
>
>>
>>> Fixes: f993c123b461 ("mtd: spi-nor: cqspi: make of_device_ids const")
>>> Signed-off-by: Arnd Bergmann <a...@arndb.de>
>>> ---
>>> drivers/mtd/spi-nor/cadence-quadspi.c | 2 +-
>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c
>>> b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> index d315c326e72f..53c7d8e0327a 100644
>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> @@ -1283,7 +1283,7 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>> #define CQSPI_DEV_PM_OPS NULL
>>> #endif
>>>
>>> -static const struct of_device_id const cqspi_dt_ids[] = {
>>> +static const struct of_device_id cqspi_dt_ids[] = {
>>> {.compatible = "cdns,qspi-nor",},
>>> { /* end of table */ }
>>> };
>>>
>>
>>
>
--
Best regards,
Marek Vasut
Re: [PATCH 01/14] qcom: mtd: nand: Add driver data for QPIC DMA
+ u32 ecc_modes;
> + bool dma_bam_enabled;
> +};
> +
> static inline struct qcom_nand_host *to_qcom_nand_host(struct nand_chip
> *chip)
> {
> return container_of(chip, struct qcom_nand_host, chip);
> @@ -2073,6 +2087,7 @@ static int qcom_nandc_probe(struct platform_device
> *pdev)
> struct device_node *dn = dev->of_node, *child;
> struct resource *res;
> int ret;
> + const struct qcom_nand_driver_data *driver_data;
>
> nandc = devm_kzalloc(>dev, sizeof(*nandc), GFP_KERNEL);
> if (!nandc)
> @@ -2087,7 +2102,10 @@ static int qcom_nandc_probe(struct platform_device
> *pdev)
> return -ENODEV;
> }
>
> - nandc->ecc_modes = (unsigned long)dev_data;
> + driver_data = (const struct qcom_nand_driver_data *)dev_data;
> +
> + nandc->ecc_modes = driver_data->ecc_modes;
> + nandc->dma_bam_enabled = driver_data->dma_bam_enabled;
>
> res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> nandc->base = devm_ioremap_resource(dev, res);
> @@ -2179,15 +2197,26 @@ static int qcom_nandc_remove(struct platform_device
> *pdev)
> return 0;
> }
>
> -#define EBI2_NANDC_ECC_MODES (ECC_RS_4BIT | ECC_BCH_8BIT)
>
> +static const struct qcom_nand_driver_data ebi2_nandc_data = {
> + .ecc_modes = (ECC_RS_4BIT | ECC_BCH_8BIT),
> + .dma_bam_enabled = false,
> +};
> +
> +static const struct qcom_nand_driver_data qpic_nandc_v1_4_0_data = {
> + .ecc_modes = (ECC_BCH_4BIT | ECC_BCH_8BIT),
> + .dma_bam_enabled = true,
> +};
> /*
> * data will hold a struct pointer containing more differences once we
> support
> * more controller variants
> */
> static const struct of_device_id qcom_nandc_of_match[] = {
> { .compatible = "qcom,ipq806x-nand",
> - .data = (void *)EBI2_NANDC_ECC_MODES,
> + .data = (void *)_nandc_data,
> + },
> + { .compatible = "qcom,qpic-nandc-v1.4.0",
> + .data = (void *)_nandc_v1_4_0_data,
> },
> {}
> };
>
--
Best regards,
Marek Vasut
Re: [PATCH 1/3] ARM: dts: Bindings for Altera Quadspi Controller Version 2
On 06/29/2017 01:09 AM, Rob Herring wrote: > On Tue, Jun 27, 2017 at 08:57:14AM -0700, matthew.gerl...@linux.intel.com > wrote: >> >> >> On Tue, 27 Jun 2017, Marek Vasut wrote: >> >>> On 06/27/2017 04:32 PM, matthew.gerl...@linux.intel.com wrote: >>>> >>>> >>>> On Tue, 27 Jun 2017, Marek Vasut wrote: >>>> >>>> Hi Marek, >>>> >>>> Thanks for the feedback. See my comments below. >>>> >>>> Matthew Gerlach >>>> >>>>> On 06/26/2017 06:13 PM, matthew.gerl...@linux.intel.com wrote: >>>>>> From: Matthew Gerlach <matthew.gerl...@linux.intel.com> >>>>>> >>>>>> Device Tree bindings for Version 2 of the Altera Quadspi Controller >>>>>> that can be optionally paired with a windowed bridge. >>>>>> >>>>>> Signed-off-by: Matthew Gerlach <matthew.gerl...@linux.intel.com> >>>>>> --- >>>>>> .../devicetree/bindings/mtd/altera-quadspi-v2.txt | 37 >>>>>> ++ >>>>>> 1 file changed, 37 insertions(+) >>>>>> create mode 100644 >>>>>> Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>>> >>>>>> diff --git >>>>>> a/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>>> b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>>> new file mode 100644 >>>>>> index 000..8ba63d7 >>>>>> --- /dev/null >>>>>> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>>> @@ -0,0 +1,37 @@ >>>>>> +* Altera Quad SPI Controller Version 2 >>>>>> + >>>>>> +Required properties: >>>>>> +- compatible : Should be "altr,quadspi-v2". >>>>>> +- reg : Contains at least two entries, and possibly three entries, >>>>>> each of >>>>>> +which is a tuple consisting of a physical address and length. >>>>>> +- reg-names : Should contain the names "avl_csr" and "avl_mem" >>>>>> corresponding >>>>>> + to the control and status registers and qspi memory, >>>>>> respectively. >>>>>> + >>>>>> + >>>>>> +The Altera Quad SPI Controller Version 2 can be paired with a >>>>>> windowed bridge >>>>>> +in order to reduce the footprint of the memory interface. When a >>>>>> windowed >>>>>> +bridge is used, reads and writes of data must be 32 bits wide. >>>>>> + >>>>>> +Optional properties: >>>>>> +- reg-names : Should contain the name "avl_window", if the windowed >>>>>> bridge >>>>>> + is used. This name corresponds to the register space that >>>>>> + controls the window. >>>>>> +- window-size : The size of the window which must be an even power >>>>>> of 2. >>>>>> +- read-bit-reverse : A boolean indicating the data read from the >>>>>> flash should >>>>>> + be bit reversed on a byte by byte basis before being >>>>>> + delivered to the MTD layer. >>>>>> +- write-bit-reverse : A boolean indicating the data written to the >>>>>> flash should >>>>>> + be bit reversed on a byte by byte basis. >>>>> >>>>> Is there ever a usecase where you need to set just one of these props ? >>>>> Also, they're altera specific, so altr, prefix should be added. >>>> >>>> In general, I think if bit reversal is required, it would be required in >>>> both directions. However, anything is possible when using FPGAs. So >>>> I thought separate booleans would be future proofing the bindings. >>> >>> Maybe we should drop this whole thing and add it when this is actually >>> required. >>> >>> Are there any users of this in the wild currently ? >>> >>> What is the purpose of doing this per-byte bit reverse instead of >>> storing th bits in the original order ? >> >> Hi Marek, >> >> Yes, there is hardware that has been in the wild for years that needs this >> bit reversal. The specific use case is
Re: [PATCH 03/14] qcom: mtd: nand: Fixed config error for BCH
On 06/29/2017 09:15 AM, Abhishek Sahu wrote: > The configuration for BCH is not correct in the current > driver so this patch fixed the same. Fix the commit message, I have no idea what this patch does or fixes. > Signed-off-by: Abhishek Sahu <abs...@codeaurora.org> > --- > drivers/mtd/nand/qcom_nandc.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/mtd/nand/qcom_nandc.c b/drivers/mtd/nand/qcom_nandc.c > index 520add9..410ee53 100644 > --- a/drivers/mtd/nand/qcom_nandc.c > +++ b/drivers/mtd/nand/qcom_nandc.c > @@ -1919,7 +1919,7 @@ static int qcom_nand_host_setup(struct qcom_nand_host > *host) > | wide_bus << WIDE_FLASH > | 1 << DEV0_CFG1_ECC_DISABLE; > > - host->ecc_bch_cfg = host->bch_enabled << ECC_CFG_ECC_DISABLE > + host->ecc_bch_cfg = !host->bch_enabled << ECC_CFG_ECC_DISABLE > | 0 << ECC_SW_RESET > | host->cw_data << ECC_NUM_DATA_BYTES > | 1 << ECC_FORCE_CLK_OPEN > -- Best regards, Marek Vasut
Re: [PATCH 07/14] qcom: mtd: nand: support for passing flags in transfer functions
oob_buf, oob_size); > + oob_buf, oob_size, 0); > } > > if (data_buf) > @@ -1402,7 +1416,7 @@ static int copy_last_cw(struct qcom_nand_host *host, > int page) > > config_cw_read(nandc); > > - read_data_dma(nandc, FLASH_BUF_ACC, nandc->data_buffer, size); > + read_data_dma(nandc, FLASH_BUF_ACC, nandc->data_buffer, size, 0); > > ret = submit_descs(nandc); > if (ret) > @@ -1470,19 +1484,19 @@ static int qcom_nandc_read_page_raw(struct mtd_info > *mtd, > > config_cw_read(nandc); > > - read_data_dma(nandc, reg_off, data_buf, data_size1); > + read_data_dma(nandc, reg_off, data_buf, data_size1, 0); > reg_off += data_size1; > data_buf += data_size1; > > - read_data_dma(nandc, reg_off, oob_buf, oob_size1); > + read_data_dma(nandc, reg_off, oob_buf, oob_size1, 0); > reg_off += oob_size1; > oob_buf += oob_size1; > > - read_data_dma(nandc, reg_off, data_buf, data_size2); > + read_data_dma(nandc, reg_off, data_buf, data_size2, 0); > reg_off += data_size2; > data_buf += data_size2; > > - read_data_dma(nandc, reg_off, oob_buf, oob_size2); > + read_data_dma(nandc, reg_off, oob_buf, oob_size2, 0); > oob_buf += oob_size2; > } > > @@ -1549,7 +1563,8 @@ static int qcom_nandc_write_page(struct mtd_info *mtd, > struct nand_chip *chip, > > config_cw_write_pre(nandc); > > - write_data_dma(nandc, FLASH_BUF_ACC, data_buf, data_size); > + write_data_dma(nandc, FLASH_BUF_ACC, data_buf, data_size, > +i == (ecc->steps - 1) ? NAND_BAM_NO_EOT : 0); > > /* >* when ECC is enabled, we don't really need to write anything > @@ -1562,7 +1577,7 @@ static int qcom_nandc_write_page(struct mtd_info *mtd, > struct nand_chip *chip, > oob_buf += host->bbm_size; > > write_data_dma(nandc, FLASH_BUF_ACC + data_size, > -oob_buf, oob_size); > +oob_buf, oob_size, 0); > } > > config_cw_write_post(nandc); > @@ -1618,19 +1633,19 @@ static int qcom_nandc_write_page_raw(struct mtd_info > *mtd, > > config_cw_write_pre(nandc); > > - write_data_dma(nandc, reg_off, data_buf, data_size1); > + write_data_dma(nandc, reg_off, data_buf, data_size1, 0); > reg_off += data_size1; > data_buf += data_size1; > > - write_data_dma(nandc, reg_off, oob_buf, oob_size1); > + write_data_dma(nandc, reg_off, oob_buf, oob_size1, 0); > reg_off += oob_size1; > oob_buf += oob_size1; > > - write_data_dma(nandc, reg_off, data_buf, data_size2); > + write_data_dma(nandc, reg_off, data_buf, data_size2, 0); > reg_off += data_size2; > data_buf += data_size2; > > - write_data_dma(nandc, reg_off, oob_buf, oob_size2); > + write_data_dma(nandc, reg_off, oob_buf, oob_size2, 0); > oob_buf += oob_size2; > > config_cw_write_post(nandc); > @@ -1685,7 +1700,7 @@ static int qcom_nandc_write_oob(struct mtd_info *mtd, > struct nand_chip *chip, > > config_cw_write_pre(nandc); > write_data_dma(nandc, FLASH_BUF_ACC, nandc->data_buffer, > -data_size + oob_size); > +data_size + oob_size, 0); > config_cw_write_post(nandc); > > ret = submit_descs(nandc); > @@ -1769,7 +1784,8 @@ static int qcom_nandc_block_markbad(struct mtd_info > *mtd, loff_t ofs) > update_rw_regs(host, 1, false); > > config_cw_write_pre(nandc); > - write_data_dma(nandc, FLASH_BUF_ACC, nandc->data_buffer, host->cw_size); > + write_data_dma(nandc, FLASH_BUF_ACC, nandc->data_buffer, > +host->cw_size, 0); > config_cw_write_post(nandc); > > ret = submit_descs(nandc); > -- Best regards, Marek Vasut
Re: [PATCH 05/14] qcom: mtd: nand: allocate bam transaction
nsaction */
> +static struct bam_transaction *
> +alloc_bam_transaction(struct qcom_nand_controller *nandc, unsigned int
> num_cw)
> +{
> + struct bam_transaction *bam_txn;
> +
> + bam_txn = devm_kzalloc(nandc->dev, sizeof(*bam_txn), GFP_KERNEL);
Can you make these four allocations into a single allocation ?
> + if (!bam_txn)
> + return NULL;
> +
> + bam_txn->bam_ce =
> + devm_kzalloc(nandc->dev, sizeof(*bam_txn->bam_ce) *
> + num_cw * QPIC_PER_CW_MAX_CMD_ELEMENTS, GFP_KERNEL);
> + if (!bam_txn->bam_ce)
> + return NULL;
> +
> + bam_txn->cmd_sgl =
> + devm_kzalloc(nandc->dev, sizeof(*bam_txn->cmd_sgl) * num_cw *
> + QPIC_PER_CW_MAX_CMD_SGL, GFP_KERNEL);
> + if (!bam_txn->cmd_sgl)
> + return NULL;
> +
> + bam_txn->data_sg =
> + devm_kzalloc(nandc->dev, sizeof(*bam_txn->data_sg) *
> + num_cw * QPIC_PER_CW_MAX_DATA_SGL, GFP_KERNEL);
> + if (!bam_txn->data_sg)
> + return NULL;
> +
> + nandc->max_cwperpage = num_cw;
> +
> + return bam_txn;
> +}
> +
> static inline struct qcom_nand_host *to_qcom_nand_host(struct nand_chip
> *chip)
> {
> return container_of(chip, struct qcom_nand_host, chip);
> @@ -1868,6 +1958,8 @@ static int qcom_nand_host_setup(struct qcom_nand_host
> *host)
> mtd_set_ooblayout(mtd, _nand_ooblayout_ops);
>
> cwperpage = mtd->writesize / ecc->size;
> + nandc->max_cwperpage = max_t(unsigned int, nandc->max_cwperpage,
> + cwperpage);
>
> /*
>* DATA_UD_BYTES varies based on whether the read/write command protects
> @@ -2010,6 +2102,19 @@ static int qcom_nandc_alloc(struct
> qcom_nand_controller *nandc)
> dev_err(nandc->dev, "failed to request cmd channel\n");
> return -ENODEV;
> }
> +
> + /*
> + * Initially allocate BAM transaction to read ONFI param page.
> + * After detecting all the devices, this BAM transaction will
> + * be freed and the next BAM tranasction will be allocated with
> + * maximum codeword size
> + */
> + nandc->bam_txn = alloc_bam_transaction(nandc, 1);
> + if (!nandc->bam_txn) {
> + dev_err(nandc->dev,
> + "failed to allocate bam transaction\n");
> + return -ENOMEM;
> + }
> }
>
> INIT_LIST_HEAD(>desc_list);
> @@ -2153,6 +2258,17 @@ static int qcom_probe_nand_devices(struct
> qcom_nand_controller *nandc)
> if (list_empty(>host_list))
> return -ENODEV;
>
> + if (nandc->dma_bam_enabled) {
> + free_bam_transaction(nandc);
> + nandc->bam_txn = alloc_bam_transaction(nandc,
> +nandc->max_cwperpage);
> + if (!nandc->bam_txn) {
> + dev_err(nandc->dev,
> + "failed to allocate bam transaction\n");
> + return -ENOMEM;
> + }
> + }
> +
> list_for_each_entry_safe(host, tmp, >host_list, node) {
> ret = qcom_nand_mtd_register(nandc, host, child);
> if (ret) {
>
--
Best regards,
Marek Vasut
Re: [PATCH 02/14] qcom: mtd: nand: add and initialize QPIC DMA resources
com_nand_controller {
> int buf_start;
>
> __le32 *reg_read_buf;
> + dma_addr_t reg_read_buf_phys;
> int reg_read_pos;
>
> struct nandc_regs *regs;
> @@ -1956,16 +1968,48 @@ static int qcom_nandc_alloc(struct
> qcom_nand_controller *nandc)
> if (!nandc->regs)
> return -ENOMEM;
>
> - nandc->reg_read_buf = devm_kzalloc(nandc->dev,
> - MAX_REG_RD * sizeof(*nandc->reg_read_buf),
> - GFP_KERNEL);
> - if (!nandc->reg_read_buf)
> - return -ENOMEM;
>
> - nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx");
> - if (!nandc->chan) {
> - dev_err(nandc->dev, "failed to request slave channel\n");
> - return -ENODEV;
> + if (!nandc->dma_bam_enabled) {
> + nandc->reg_read_buf =
> + devm_kzalloc(nandc->dev, MAX_REG_RD *
> + sizeof(*nandc->reg_read_buf), GFP_KERNEL);
> +
> + if (!nandc->reg_read_buf)
> + return -ENOMEM;
> +
> + nandc->chan = dma_request_slave_channel(nandc->dev, "rxtx");
> + if (!nandc->chan) {
> + dev_err(nandc->dev,
> + "failed to request slave channel\n");
> + return -ENODEV;
> + }
> + } else {
> + nandc->reg_read_buf =
> + dmam_alloc_coherent(nandc->dev, MAX_REG_RD *
> + sizeof(*nandc->reg_read_buf),
> + >reg_read_buf_phys,
> + GFP_KERNEL);
> +
> + if (!nandc->reg_read_buf)
> + return -ENOMEM;
> +
> + nandc->tx_chan = dma_request_slave_channel(nandc->dev, "tx");
> + if (!nandc->tx_chan) {
> + dev_err(nandc->dev, "failed to request tx channel\n");
> + return -ENODEV;
> + }
> +
> + nandc->rx_chan = dma_request_slave_channel(nandc->dev, "rx");
> + if (!nandc->rx_chan) {
> + dev_err(nandc->dev, "failed to request rx channel\n");
> + return -ENODEV;
> + }
> +
> + nandc->cmd_chan = dma_request_slave_channel(nandc->dev, "cmd");
> + if (!nandc->cmd_chan) {
> + dev_err(nandc->dev, "failed to request cmd channel\n");
> + return -ENODEV;
> + }
> }
>
> INIT_LIST_HEAD(>desc_list);
> @@ -1978,7 +2022,19 @@ static int qcom_nandc_alloc(struct
> qcom_nand_controller *nandc)
>
> static void qcom_nandc_unalloc(struct qcom_nand_controller *nandc)
> {
> - dma_release_channel(nandc->chan);
> + if (nandc->dma_bam_enabled) {
> + if (nandc->tx_chan)
> + dma_release_channel(nandc->tx_chan);
> +
> + if (nandc->rx_chan)
> + dma_release_channel(nandc->rx_chan);
> +
> + if (nandc->cmd_chan)
> + dma_release_channel(nandc->cmd_chan);
> + } else {
> + if (nandc->chan)
> + dma_release_channel(nandc->chan);
> + }
> }
>
> /* one time setup of a few nand controller registers */
> @@ -2063,16 +2119,20 @@ static int qcom_nandc_parse_dt(struct platform_device
> *pdev)
> struct device_node *np = nandc->dev->of_node;
> int ret;
>
> - ret = of_property_read_u32(np, "qcom,cmd-crci", >cmd_crci);
> - if (ret) {
> - dev_err(nandc->dev, "command CRCI unspecified\n");
> - return ret;
> - }
> + if (!nandc->dma_bam_enabled) {
> + ret = of_property_read_u32(np, "qcom,cmd-crci",
> +>cmd_crci);
> + if (ret) {
> + dev_err(nandc->dev, "command CRCI unspecified\n");
> + return ret;
> + }
>
> - ret = of_property_read_u32(np, "qcom,data-crci", >data_crci);
> - if (ret) {
> - dev_err(nandc->dev, "data CRCI unspecified\n");
> - return ret;
> + ret = of_property_read_u32(np, "qcom,data-crci",
> +>data_crci);
> + if (ret) {
> + dev_err(nandc->dev, "data CRCI unspecified\n");
> + return ret;
> + }
> }
>
> return 0;
> @@ -2128,7 +2188,7 @@ static int qcom_nandc_probe(struct platform_device
> *pdev)
>
> ret = qcom_nandc_alloc(nandc);
> if (ret)
> - return ret;
> + goto err_core_clk;
>
> ret = clk_prepare_enable(nandc->core_clk);
> if (ret)
>
Can you please fix your mailer to stop adding "QUALCOMM INDIA, on behalf
of Qualcomm Innovation Center"... stuff at the bottom of the patches ?
--
Best regards,
Marek Vasut
Re: [PATCH] mtd: spi-nor: cqspi: remove duplicate const
On 06/27/2017 10:21 PM, Cyrille Pitchen wrote:
> Le 27/06/2017 à 21:51, Marek Vasut a écrit :
>> On 06/27/2017 09:48 PM, Cyrille Pitchen wrote:
>>> Le 27/06/2017 à 18:11, Marek Vasut a écrit :
>>>> On 06/27/2017 05:34 PM, Arnd Bergmann wrote:
>>>>> The variable was already marked 'const' before the previous
>>>>> patch, but the qualifier was in an unusual place, and now the
>>>>> extra 'const' causes a harmless warning:
>>>>>
>>>>> drivers/mtd/spi-nor/cadence-quadspi.c:1286:34: error: duplicate 'const'
>>>>> declaration specifier [-Werror=duplicate-decl-specifier]
>>>>>
>>>>> This removes the other 'const' instead.
>>>>
>>>> Isn't that const array const elements , thus two consts ?
>>>> IMO the original code is correct.
>>>
>>> Indeed the 2 'const' don't have the same meaning so the original code
>>> may be correct but with an array like cqspi_dt_ids[] we can't write
>>> "cqspi_dt_ids = (const struct of_device_id *);" anyway.
>>
>> Not sure I understand what you're trying to say here.
>>
>
> Just that once an array like cqspi_dt_ids[] has been defined, you can
> never assign it a new value later:
>
> cqspi_dt_ids = ; /* that doesn't work */
>
> For this specific point, an array behaves like a constant pointer (but
> otherwise I agree that arrays and constant pointers are not the same
> things). So I guess the 2nd 'const' could be considered as implicit :)
I had to think about this a little and I think what you're getting at is
something slightly different.
IIUC if this was array of pointers (which it is not), you'd need this
const Type *const name[] , but since this is array of Type , you can do
with const Type name[] and the whole thing is implicitly const.
But in that case:
Acked-by: Marek Vasut <marek.va...@gmail.com>
>>> So I think the 2nd 'const' is useless here then if this patch removes a
>>> warning, let's apply it, right?
>>>
>>>>
>>>>> Fixes: f993c123b461 ("mtd: spi-nor: cqspi: make of_device_ids const")
>>>>> Signed-off-by: Arnd Bergmann <a...@arndb.de>
>>>>> ---
>>>>> drivers/mtd/spi-nor/cadence-quadspi.c | 2 +-
>>>>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c
>>>>> b/drivers/mtd/spi-nor/cadence-quadspi.c
>>>>> index d315c326e72f..53c7d8e0327a 100644
>>>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>>>> @@ -1283,7 +1283,7 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>>>> #define CQSPI_DEV_PM_OPS NULL
>>>>> #endif
>>>>>
>>>>> -static const struct of_device_id const cqspi_dt_ids[] = {
>>>>> +static const struct of_device_id cqspi_dt_ids[] = {
>>>>> {.compatible = "cdns,qspi-nor",},
>>>>> { /* end of table */ }
>>>>> };
>>>>>
>>>>
>>>>
>>>
>>
>>
>
--
Best regards,
Marek Vasut
Re: [PATCH 1/3] ARM: dts: Bindings for Altera Quadspi Controller Version 2
On 06/27/2017 09:32 PM, matthew.gerl...@linux.intel.com wrote: [...] >>> Hi Marek, >>> >>> I am trying to write an MTD/spi-nor driver for version 2 of the >>> Altera Quadspi contoller. This controller is soft IP that is deployed >>> in a FPGA. As such, this component/driver can be used in wide range of >>> use cases. The controller could be used to update EPCQ/EPCS flash >>> stores containing bit streams, but this component could be used for >>> flash for filesystems or any non-volatile data store. My hope is that >>> all possible use cases should be covered by this driver. >> >> How does this particular case where you have to reverse the bits look >> like ? > > The use case for reversing the bits involves a processor updating > EPCQ/EPCS flash whose contents are read by the FPGA on power up. The > processor and Altera Quadspi component, inside the configured FPGA, > access the bits in one way serially, but the hardware that accesses the > flash during power accesses the bits in the opposite way serially. So it's the same crap Xilinx does, they have some tool to do this with their bitstream before they write it into flash. But then, you only have to do it with your bitstream. If you put ie. UBI after the bitstream part, the UBI can be in normal bit ordering. So instead of polluting the DT bindings with this, make a similar tool to what Xilinx has, do the bit shuffling using that tool and then write the bitstream into the flash in the correct order. >>>>>>> Thinking about this binding more, I wonder if the binding name(s) >>>>>>> should be (read|write)-bit8-reverse to indicate reversings the bits >>>>>>> in a byte as opposed to reversing the bits in a 32 bit word? >>>>>>> >>>>>>> I don't think bit reversal is specific to Altera/Intel components. >>>>>>> I see >>>>>>> a nand driver performing bit reversal, and I think I've recently >>>>>>> seen >>>>>>> other FPGA based drivers requiring bit reversal. >>>>>> >>>>>> $ git grep bit.reverse Documentation/devicetree/ | wc -l >>>>>> 0 >>>>>> >>>>>> So we don't have such a generic binding . It's up to Rob (I guess) to >>>>>> decide whether this is SoC specific and should've altr, prefix or >>>>>> not. >>>>>> IMO it is. >>>>> >>>>> I agree there is no generic binding at this time, and I look forward >>>>> to any input from Rob and anyone else on this issue. I think it is >>>>> worth pointing out that this really isn't an issue of an SoC, but >>>>> rather >>>>> it is an >>>>> issue of how data in the flash chip is accessed.I think what makes >>>>> this issue >>>>> "weird" is that we have different hardware accessing the data in the >>>>> flash with a different perspective. The FPGA looks at the data >>>>> from one >>>>> perspective on power up, and a processor trying to update the flash >>>>> has >>>>> a different perspective. >>>> >>>> Another thing I'd ask here is, is that bit-reverse a hardware property >>>> or is that some software configuration thing ? >>> >>> I would say the bit reversal is a property of the FPGA that is reading >>> the flash at power up. >> >> So it's not a property of the block, but rather of the bus somewhere ? > > You are correct, it is not a property of the Altera Quadspi component, but > a property of the fpga and external hardware that access the flash on > power up. So yes, it's a property of that small thing which loads the bitstream from the EPCS/EPCQ and programs the cells in the FPGA. This shouldn't be in this driver nor it's bindings, see above. -- Best regards, Marek Vasut
Re: [RESEND][PATCH V2 4/4] regulator: Add ROHM BD9571MWV-M PMIC regulator driver
On 04/26/2017 12:31 AM, Mark Brown wrote: > On Tue, Apr 25, 2017 at 08:32:10PM +0200, Marek Vasut wrote: >> Add driver for the regulator block in the ROHM BD9571MWV-W MFD PMIC. >> This block supports three voltage monitors, VD18, VD25, VD33 for the >> 1V8, 2V5, 3V3 voltage rails and a single voltage regulator for the >> DVFS rail. > > Please do not submit new versions of already applied patches, please > submit incremental updates to the existing code. Modifying existing > commits creates problems for other users building on top of those > commits so it's best practice to only change pubished git commits if > absolutely essential. I resubmitted this on Lee's request [1]. [1] https://www.mail-archive.com/linux-renesas-soc@vger.kernel.org/msg13599.html -- Best regards, Marek Vasut
Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
On 04/25/2017 07:23 PM, Leonard Crestez wrote:
> On Tue, 2017-04-25 at 14:02 -0300, Fabio Estevam wrote:
>> On Tue, Apr 25, 2017 at 2:02 PM, Fabio Estevam <feste...@gmail.com> wrote:
>>>
>>> Hi Leonard,
>>>
>>> On Tue, Apr 25, 2017 at 1:57 PM, Leonard Crestez
>>> <leonard.cres...@nxp.com> wrote:
>>>>
>>>> The board file for imx6sx-dbg overrides cpufreq operating points to use
>>>> higher voltages. This is done because the board has a shared rail for
>>>> VDD_ARM_IN and VDD_SOC_IN and when using LDO bypass the shared voltage
>>>> needs to be a value suitable for both ARM and SOC.
>>>>
>>>> This was introduced in:
>>>>
>>>> commit 54183bd7f766 ("ARM: imx6sx-sdb: add revb board and make it default")
>>>>
>>>> This only only applies to LDO bypass mode, a feature not present in
>>>> upstream. When LDOs are enabled the effect is to use higher voltages than
>>>> necesarry for no good reason.
>>>>
>>>> Setting these higher voltages can make some boards fail to boot with ugly
>>>> semi-random crashes, reminiscent of memory corruption. These failures
>>>> happen the first time the lowest idle state is used. Remove the OPP
>>>> override in order to fix those crashes.
>>>>
>>>> Signed-off-by: Leonard Crestez <leonard.cres...@nxp.com>
>>>>
>>>> ---
>>>> It's not clear exactly why the crashes happen. Perhaps waking up from idle
>>>> draws more power than is available? Removing this override is a correct
>>>> change anyway so maybe there is no need to investigate deeper.
>
>>> Marek just sent a similar one a few minutes ago:
>>> http://lists.infradead.org/pipermail/linux-arm-kernel/2017-April/503230.html
>
>> Forgot to add Marek.
>
> Wow, that was literally 15 minutes before my patch. In my defense I did
> search the archives before starting to format the patch but it had not
> arrived yet.
Hehehe :-)
> Anyway, that version also sets the supply for reg_arm and reg_soc. It
> is not necessary for fixing the crash I'm seeing but is good because it
> will result in the minimum voltage on VDD_ARM_SOC_IN rather than a fix
> 1375mv. I tested Marek's patch and it works fine on my rev B board
> (which otherwise fails to boot upstream).
Oh that's nice , thanks ! I don't have SDB and I hacked it up after a
brief discussion with Fabio without even compile-testing it, thus RFC.
Glad to hear it works and thanks for testing it ! Can you add a formal
Tested-by please ?
--
Best regards,
Marek Vasut
[RESEND][PATCH V2 2/4] mfd: Add ROHM BD9571MWV-M MFD PMIC driver
Add the MFD part of the ROHM BD9571MWV-M PMIC driver and MAINTAINERS
entry. The MFD part only specifies the regmap bits for the PMIC and
binds the subdevs together.
Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
Cc: linux-kernel@vger.kernel.org
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: Lee Jones <lee.jo...@linaro.org>
---
V2: - Change BD9571MWV_AVS_VD09_VID0,1,2,3 to BD9571MWV_AVS_VD09_VID(n)
- Change BD9571MWV_AVS_DVFS_VID0,1,2,3 to BD9571MWV_AVS_DVFS_VID(n)
- Make the AVS_VD09 range RW, so it can be used by the regulator
driver for the VD09 regulator
- Report the regmap read return values when attempting to read ID
registers fails
---
MAINTAINERS | 11 ++
drivers/mfd/Kconfig | 13 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/bd9571mwv.c | 230 ++
include/linux/mfd/bd9571mwv.h | 115 +
5 files changed, 370 insertions(+)
create mode 100644 drivers/mfd/bd9571mwv.c
create mode 100644 include/linux/mfd/bd9571mwv.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 67280051e30a..6f19aa08375d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10917,6 +10917,17 @@ L: linux-ser...@vger.kernel.org
S: Odd Fixes
F: drivers/tty/serial/rp2.*
+ROHM MULTIFUNCTION BD9571MWV-M PMIC DEVICE DRIVERS
+M: Marek Vasut <marek.vasut+rene...@gmail.com>
+L: linux-kernel@vger.kernel.org
+L: linux-renesas-...@vger.kernel.org
+S: Supported
+F: drivers/mfd/bd9571mwv.c
+F: drivers/regulator/bd9571mwv-regulator.c
+F: drivers/gpio/gpio-bd9571mwv.c
+F: include/linux/mfd/bd9571mwv.h
+F: Documentation/devicetree/bindings/mfd/bd9571mwv.txt
+
ROSE NETWORK LAYER
M: Ralf Baechle <r...@linux-mips.org>
L: linux-h...@vger.kernel.org
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de68b5ba8741..fa1f41ef5332 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -133,6 +133,19 @@ config MFD_BCM590XX
help
Support for the BCM590xx PMUs from Broadcom
+config MFD_BD9571MWV
+ tristate "ROHM BD9571MWV PMIC"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C
+ help
+ Support for the ROHM BD9571MWV PMIC, which contains single
+ voltage regulator, voltage sampling units, GPIO block and
+ watchdog block.
+
+ This driver can also be built as a module. If so, the module
+ will be called bd9571mwv.
+
config MFD_AC100
tristate "X-Powers AC100"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index fa86dbe65e52..e2c82d2b108d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_MFD_ACT8945A)+= act8945a.o
obj-$(CONFIG_MFD_SM501)+= sm501.o
obj-$(CONFIG_MFD_ASIC3)+= asic3.o tmio_core.o
obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
+obj-$(CONFIG_MFD_BD9571MWV)+= bd9571mwv.o
cros_ec_core-objs := cros_ec.o
cros_ec_core-$(CONFIG_ACPI)+= cros_ec_acpi_gpe.o
obj-$(CONFIG_MFD_CROS_EC) += cros_ec_core.o
diff --git a/drivers/mfd/bd9571mwv.c b/drivers/mfd/bd9571mwv.c
new file mode 100644
index ..64e088dfe7b0
--- /dev/null
+++ b/drivers/mfd/bd9571mwv.c
@@ -0,0 +1,230 @@
+/*
+ * ROHM BD9571MWV-M MFD driver
+ *
+ * Copyright (C) 2017 Marek Vasut <marek.vasut+rene...@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether expressed or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License version 2 for more details.
+ *
+ * Based on the TPS65086 driver
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+static const struct mfd_cell bd9571mwv_cells[] = {
+ { .name = "bd9571mwv-regulator", },
+ { .name = "bd9571mwv-gpio", },
+};
+
+static const struct regmap_range bd9571mwv_readable_yes_ranges[] = {
+ regmap_reg_range(BD9571MWV_VENDOR_CODE, BD9571MWV_PRODUCT_REVISION),
+ regmap_reg_range(BD9571MWV_AVS_SET_MONI, BD9571MWV_AVS_DVFS_VID(3)),
+ regmap_reg_range(BD9571MWV_VD18_VID, BD9571MWV_VD33_VID),
+ regmap_reg_range(BD9571MWV_DVFS_VINIT, BD9571MWV_DVFS_VINIT),
+ regmap_reg_range(BD9571MWV_DVFS_SETVMAX, BD9571MWV_DVFS_MONIVDAC),
+ regmap_reg_range(BD9571MWV_GPIO_IN, BD9571MWV_GPIO_IN),
+ regmap_reg_range(BD9571MWV_GPIO_INT, BD9571MWV_GPIO_INTMASK),
+ regmap_reg_range(BD9571MWV_INT_INTREQ, BD9571MWV_INT_INTMASK),
+};
+
+static const struct regmap_access_table bd9571mwv_readable_table = {
+ .yes_ranges = bd9571mwv_r
[RESEND][PATCH V2 4/4] regulator: Add ROHM BD9571MWV-M PMIC regulator driver
Add driver for the regulator block in the ROHM BD9571MWV-W MFD PMIC.
This block supports three voltage monitors, VD18, VD25, VD33 for the
1V8, 2V5, 3V3 voltage rails and a single voltage regulator for the
DVFS rail.
Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
Cc: linux-kernel@vger.kernel.org
Cc: Liam Girdwood <lgirdw...@gmail.com>
Cc: Mark Brown <broo...@kernel.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
---
V2: - Do not open-code the .is_enabled as the regulators are always enabled
- Add the VD09 regulator
---
drivers/regulator/Kconfig | 11 ++
drivers/regulator/Makefile | 1 +
drivers/regulator/bd9571mwv-regulator.c | 178
3 files changed, 190 insertions(+)
create mode 100644 drivers/regulator/bd9571mwv-regulator.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 290eeb88832c..b6e512df6df4 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -163,6 +163,17 @@ config REGULATOR_BCM590XX
BCM590xx PMUs. This will enable support for the software
controllable LDO/Switching regulators.
+config REGULATOR_BD9571MWV
+ tristate "ROHM BD9571MWV Regulators"
+ depends on MFD_BD9571MWV
+ help
+ This driver provides support for the voltage regulators on the
+ ROHM BD9571MWV PMIC. This will enable support for the software
+ controllable regulator and voltage sampling units.
+
+ This driver can also be built as a module. If so, the module
+ will be called bd9571mwv-regulator.
+
config REGULATOR_CPCAP
tristate "Motorola CPCAP regulator"
depends on MFD_CPCAP
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index bf5ed7a9df99..c777c8493bf7 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_REGULATOR_AS3711) += as3711-regulator.o
obj-$(CONFIG_REGULATOR_AS3722) += as3722-regulator.o
obj-$(CONFIG_REGULATOR_AXP20X) += axp20x-regulator.o
obj-$(CONFIG_REGULATOR_BCM590XX) += bcm590xx-regulator.o
+obj-$(CONFIG_REGULATOR_BD9571MWV) += bd9571mwv-regulator.o
obj-$(CONFIG_REGULATOR_DA903X) += da903x.o
obj-$(CONFIG_REGULATOR_DA9052) += da9052-regulator.o
obj-$(CONFIG_REGULATOR_DA9055) += da9055-regulator.o
diff --git a/drivers/regulator/bd9571mwv-regulator.c
b/drivers/regulator/bd9571mwv-regulator.c
new file mode 100644
index ..8ba206fec31e
--- /dev/null
+++ b/drivers/regulator/bd9571mwv-regulator.c
@@ -0,0 +1,178 @@
+/*
+ * ROHM BD9571MWV-M regulator driver
+ *
+ * Copyright (C) 2017 Marek Vasut <marek.vasut+rene...@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether expressed or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License version 2 for more details.
+ *
+ * Based on the TPS65086 driver
+ *
+ * NOTE: VD09 is missing
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+enum bd9571mwv_regulators { VD09, VD18, VD25, VD33, DVFS };
+
+#define BD9571MWV_REG(_name, _of, _id, _ops, _vr, _vm, _nv, _min, _step,
_lmin)\
+ { \
+ .name = _name,\
+ .of_match = of_match_ptr(_of),\
+ .regulators_node= "regulators", \
+ .id = _id, \
+ .ops= &_ops,\
+ .n_voltages = _nv, \
+ .type = REGULATOR_VOLTAGE,\
+ .owner = THIS_MODULE, \
+ .vsel_reg = _vr, \
+ .vsel_mask = _vm, \
+ .min_uV = _min, \
+ .uV_step= _step,\
+ .linear_min_sel = _lmin,\
+ }
+
+int bd9571mwv_avs_get_moni_state(struct regulator_dev *rdev)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(rdev->regmap, BD9571MWV_AVS_SET_MONI, );
+ if (ret != 0)
+ return ret;
+
+ return val & BD9571MWV_AVS_SET_MONI_MASK;
+}
+
+int bd9571mwv_avs_set_voltage_sel_regmap(struct regulator_dev *rdev,
+unsigned int sel)
+{
+ int ret;
+
+ ret = bd9571mwv_avs_get_moni_state(rdev);
+ if (ret < 0)
+ ret
Re: [PATCH v8 1/4] mtd: spi-nor: introduce SPI 1-2-2 and SPI 1-4-4 protocols
On 04/25/2017 10:08 PM, Cyrille Pitchen wrote: > From: Cyrille Pitchen <cyrille.pitc...@atmel.com> > > This patch changes the prototype of spi_nor_scan(): its 3rd parameter > is replaced by a 'struct spi_nor_hwcaps' pointer, which tells the spi-nor > framework about the actual hardware capabilities supported by the SPI > controller and its driver. > > Besides, this patch also introduces a new 'struct spi_nor_flash_parameter' > telling the spi-nor framework about the hardware capabilities supported by > the SPI flash memory and the associated settings required to use those > hardware caps. > > Then, to improve the readability of spi_nor_scan(), the discovery of the > memory settings and the memory initialization are now split into two > dedicated functions. > > 1 - spi_nor_init_params() > > The spi_nor_init_params() function is responsible for initializing the > 'struct spi_nor_flash_parameter'. Currently this structure is filled with > legacy values but further patches will allow to override some parameter > values dynamically, for instance by reading the JESD216 Serial Flash > Discoverable Parameter (SFDP) tables from the SPI memory. > The spi_nor_init_params() function only deals with the hardware > capabilities of the SPI flash memory: especially it doesn't care about > the hardware capabilities supported by the SPI controller. > > 2 - spi_nor_setup() > > The second function is called once the 'struct spi_nor_flash_parameter' > has been initialized by spi_nor_init_params(). > With both 'struct spi_nor_flash_parameter' and 'struct spi_nor_hwcaps', > the new argument of spi_nor_scan(), spi_nor_setup() computes the best > match between hardware caps supported by both the (Q)SPI memory and > controller hence selecting the relevant settings for (Fast) Read and Page > Program operations. > > Signed-off-by: Cyrille Pitchen <cyrille.pitc...@atmel.com> Reviewed-by: Marek Vasut <marek.va...@gmail.com> [...] -- Best regards, Marek Vasut
Re: [PATCH v8 3/4] mtd: spi-nor: introduce Double Transfer Rate (DTR) SPI protocols
On 04/25/2017 10:08 PM, Cyrille Pitchen wrote: > From: Cyrille Pitchen <cyrille.pitc...@atmel.com> > > This patch introduces support to Double Transfer Rate (DTR) SPI protocols. > DTR is used only for Fast Read operations. > > According to manufacturer datasheets, whatever the number of I/O lines > used during instruction (x) and address/mode/dummy (y) clock cycles, DTR > is used only during data (z) clock cycles of SPI x-y-z protocols. > > Signed-off-by: Cyrille Pitchen <cyrille.pitc...@atmel.com> Reviewed-by: Marek Vasut <marek.va...@gmail.com> -- Best regards, Marek Vasut
Re: [PATCH 2/4] mfd: Add ROHM BD9571MWV-M MFD PMIC driver
On 04/24/2017 01:38 PM, Lee Jones wrote:
> On Sun, 16 Apr 2017, Marek Vasut wrote:
[...]
>> +static int bd9571mwv_identify(struct bd9571mwv *bd)
>> +{
>> +struct device *dev = bd->dev;
>> +unsigned int value;
>> +int ret;
>> +
>> +ret = regmap_read(bd->regmap, BD9571MWV_VENDOR_CODE, );
>> +if (ret) {
>> +dev_err(dev, "Failed to read vendor code register\n");
>
> Nit: Don't you care what the return value is?
Not really, but I added the reporting. I wonder whether it shouldn't be
the regmap which reports those kinds of failures, just like ie. when
k*alloc() fails.
>> +return ret;
>> +}
>> +
>> +if (value != BD9571MWV_VENDOR_CODE_VAL) {
>> +dev_err(dev, "Invalid vendor code ID %02x (expected %02x)\n",
>> +value, BD9571MWV_VENDOR_CODE_VAL);
>> +return -EINVAL;
>> +}
>> +
>> +ret = regmap_read(bd->regmap, BD9571MWV_PRODUCT_CODE, );
>> +if (ret) {
>> +dev_err(dev, "Failed to read product code register\n");
>
> Same.
And fixed globally ...
--
Best regards,
Marek Vasut
[PATCH V2 2/4] mfd: Add ROHM BD9571MWV-M MFD PMIC driver
Add the MFD part of the ROHM BD9571MWV-M PMIC driver and MAINTAINERS
entry. The MFD part only specifies the regmap bits for the PMIC and
binds the subdevs together.
Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
Cc: linux-kernel@vger.kernel.org
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: Lee Jones <lee.jo...@linaro.org>
---
V2: - Change BD9571MWV_AVS_VD09_VID0,1,2,3 to BD9571MWV_AVS_VD09_VID(n)
- Change BD9571MWV_AVS_DVFS_VID0,1,2,3 to BD9571MWV_AVS_DVFS_VID(n)
- Make the AVS_VD09 range RW, so it can be used by the regulator
driver for the VD09 regulator
- Report the regmap read return values when attempting to read ID
registers fails
---
MAINTAINERS | 11 ++
drivers/mfd/Kconfig | 13 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/bd9571mwv.c | 230 ++
include/linux/mfd/bd9571mwv.h | 115 +
5 files changed, 370 insertions(+)
create mode 100644 drivers/mfd/bd9571mwv.c
create mode 100644 include/linux/mfd/bd9571mwv.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 67280051e30a..6f19aa08375d 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10917,6 +10917,17 @@ L: linux-ser...@vger.kernel.org
S: Odd Fixes
F: drivers/tty/serial/rp2.*
+ROHM MULTIFUNCTION BD9571MWV-M PMIC DEVICE DRIVERS
+M: Marek Vasut <marek.vasut+rene...@gmail.com>
+L: linux-kernel@vger.kernel.org
+L: linux-renesas-...@vger.kernel.org
+S: Supported
+F: drivers/mfd/bd9571mwv.c
+F: drivers/regulator/bd9571mwv-regulator.c
+F: drivers/gpio/gpio-bd9571mwv.c
+F: include/linux/mfd/bd9571mwv.h
+F: Documentation/devicetree/bindings/mfd/bd9571mwv.txt
+
ROSE NETWORK LAYER
M: Ralf Baechle <r...@linux-mips.org>
L: linux-h...@vger.kernel.org
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index de68b5ba8741..fa1f41ef5332 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -133,6 +133,19 @@ config MFD_BCM590XX
help
Support for the BCM590xx PMUs from Broadcom
+config MFD_BD9571MWV
+ tristate "ROHM BD9571MWV PMIC"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C
+ help
+ Support for the ROHM BD9571MWV PMIC, which contains single
+ voltage regulator, voltage sampling units, GPIO block and
+ watchdog block.
+
+ This driver can also be built as a module. If so, the module
+ will be called bd9571mwv.
+
config MFD_AC100
tristate "X-Powers AC100"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index fa86dbe65e52..e2c82d2b108d 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_MFD_ACT8945A)+= act8945a.o
obj-$(CONFIG_MFD_SM501)+= sm501.o
obj-$(CONFIG_MFD_ASIC3)+= asic3.o tmio_core.o
obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
+obj-$(CONFIG_MFD_BD9571MWV)+= bd9571mwv.o
cros_ec_core-objs := cros_ec.o
cros_ec_core-$(CONFIG_ACPI)+= cros_ec_acpi_gpe.o
obj-$(CONFIG_MFD_CROS_EC) += cros_ec_core.o
diff --git a/drivers/mfd/bd9571mwv.c b/drivers/mfd/bd9571mwv.c
new file mode 100644
index ..64e088dfe7b0
--- /dev/null
+++ b/drivers/mfd/bd9571mwv.c
@@ -0,0 +1,230 @@
+/*
+ * ROHM BD9571MWV-M MFD driver
+ *
+ * Copyright (C) 2017 Marek Vasut <marek.vasut+rene...@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether expressed or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License version 2 for more details.
+ *
+ * Based on the TPS65086 driver
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+static const struct mfd_cell bd9571mwv_cells[] = {
+ { .name = "bd9571mwv-regulator", },
+ { .name = "bd9571mwv-gpio", },
+};
+
+static const struct regmap_range bd9571mwv_readable_yes_ranges[] = {
+ regmap_reg_range(BD9571MWV_VENDOR_CODE, BD9571MWV_PRODUCT_REVISION),
+ regmap_reg_range(BD9571MWV_AVS_SET_MONI, BD9571MWV_AVS_DVFS_VID(3)),
+ regmap_reg_range(BD9571MWV_VD18_VID, BD9571MWV_VD33_VID),
+ regmap_reg_range(BD9571MWV_DVFS_VINIT, BD9571MWV_DVFS_VINIT),
+ regmap_reg_range(BD9571MWV_DVFS_SETVMAX, BD9571MWV_DVFS_MONIVDAC),
+ regmap_reg_range(BD9571MWV_GPIO_IN, BD9571MWV_GPIO_IN),
+ regmap_reg_range(BD9571MWV_GPIO_INT, BD9571MWV_GPIO_INTMASK),
+ regmap_reg_range(BD9571MWV_INT_INTREQ, BD9571MWV_INT_INTMASK),
+};
+
+static const struct regmap_access_table bd9571mwv_readable_table = {
+ .yes_ranges = bd9571mwv_r
[PATCH V2 4/4] regulator: Add ROHM BD9571MWV-M PMIC regulator driver
Add driver for the regulator block in the ROHM BD9571MWV-W MFD PMIC.
This block supports three voltage monitors, VD18, VD25, VD33 for the
1V8, 2V5, 3V3 voltage rails and a single voltage regulator for the
DVFS rail.
Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
Cc: linux-kernel@vger.kernel.org
Cc: Liam Girdwood <lgirdw...@gmail.com>
Cc: Mark Brown <broo...@kernel.org>
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
---
V2: - Do not open-code the .is_enabled as the regulators are always enabled
- Add the VD09 regulator
---
drivers/regulator/Kconfig | 11 ++
drivers/regulator/Makefile | 1 +
drivers/regulator/bd9571mwv-regulator.c | 178
3 files changed, 190 insertions(+)
create mode 100644 drivers/regulator/bd9571mwv-regulator.c
diff --git a/drivers/regulator/Kconfig b/drivers/regulator/Kconfig
index 290eeb88832c..b6e512df6df4 100644
--- a/drivers/regulator/Kconfig
+++ b/drivers/regulator/Kconfig
@@ -163,6 +163,17 @@ config REGULATOR_BCM590XX
BCM590xx PMUs. This will enable support for the software
controllable LDO/Switching regulators.
+config REGULATOR_BD9571MWV
+ tristate "ROHM BD9571MWV Regulators"
+ depends on MFD_BD9571MWV
+ help
+ This driver provides support for the voltage regulators on the
+ ROHM BD9571MWV PMIC. This will enable support for the software
+ controllable regulator and voltage sampling units.
+
+ This driver can also be built as a module. If so, the module
+ will be called bd9571mwv-regulator.
+
config REGULATOR_CPCAP
tristate "Motorola CPCAP regulator"
depends on MFD_CPCAP
diff --git a/drivers/regulator/Makefile b/drivers/regulator/Makefile
index bf5ed7a9df99..c777c8493bf7 100644
--- a/drivers/regulator/Makefile
+++ b/drivers/regulator/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_REGULATOR_AS3711) += as3711-regulator.o
obj-$(CONFIG_REGULATOR_AS3722) += as3722-regulator.o
obj-$(CONFIG_REGULATOR_AXP20X) += axp20x-regulator.o
obj-$(CONFIG_REGULATOR_BCM590XX) += bcm590xx-regulator.o
+obj-$(CONFIG_REGULATOR_BD9571MWV) += bd9571mwv-regulator.o
obj-$(CONFIG_REGULATOR_DA903X) += da903x.o
obj-$(CONFIG_REGULATOR_DA9052) += da9052-regulator.o
obj-$(CONFIG_REGULATOR_DA9055) += da9055-regulator.o
diff --git a/drivers/regulator/bd9571mwv-regulator.c
b/drivers/regulator/bd9571mwv-regulator.c
new file mode 100644
index ..8ba206fec31e
--- /dev/null
+++ b/drivers/regulator/bd9571mwv-regulator.c
@@ -0,0 +1,178 @@
+/*
+ * ROHM BD9571MWV-M regulator driver
+ *
+ * Copyright (C) 2017 Marek Vasut <marek.vasut+rene...@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether expressed or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License version 2 for more details.
+ *
+ * Based on the TPS65086 driver
+ *
+ * NOTE: VD09 is missing
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+enum bd9571mwv_regulators { VD09, VD18, VD25, VD33, DVFS };
+
+#define BD9571MWV_REG(_name, _of, _id, _ops, _vr, _vm, _nv, _min, _step,
_lmin)\
+ { \
+ .name = _name,\
+ .of_match = of_match_ptr(_of),\
+ .regulators_node= "regulators", \
+ .id = _id, \
+ .ops= &_ops,\
+ .n_voltages = _nv, \
+ .type = REGULATOR_VOLTAGE,\
+ .owner = THIS_MODULE, \
+ .vsel_reg = _vr, \
+ .vsel_mask = _vm, \
+ .min_uV = _min, \
+ .uV_step= _step,\
+ .linear_min_sel = _lmin,\
+ }
+
+int bd9571mwv_avs_get_moni_state(struct regulator_dev *rdev)
+{
+ unsigned int val;
+ int ret;
+
+ ret = regmap_read(rdev->regmap, BD9571MWV_AVS_SET_MONI, );
+ if (ret != 0)
+ return ret;
+
+ return val & BD9571MWV_AVS_SET_MONI_MASK;
+}
+
+int bd9571mwv_avs_set_voltage_sel_regmap(struct regulator_dev *rdev,
+unsigned int sel)
+{
+ int ret;
+
+ ret = bd9571mwv_avs_get_moni_state(rdev);
+ if (ret < 0)
+ ret
Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller
lined above, but changing this to _rep() seems innocent enough.
What reason ?
>>> +if (len) {
>>> +aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, len);
>>> +data = aspi_read_reg(aspi, ASPI_REG_DATA1);
>>> +memcpy(buf, , len);
>>> +}
>>> +}
[...]
>>> +switch (nor->flash_read) {
>>> +default:/* Fall through */
>>
>> This will break once we add OSPI support ...
>
> Ooh, I see the API here has changed significantly from the 4.9 LTS
> branch where we originally developed the driver.I will add and test
> normal and FAST_READ support, but I won't have the bandwidth to test
> other modes yet. Those will have to remain as a TODO.
Sigh, please be so kind and use -next for your development next time ...
>>> +case SPI_NOR_NORMAL:
>>> +aspi->num_hi_z_clocks = nor->read_dummy;
>>> +aspi->xfer_mode_cmd = MODE_IO_X1;
>>> +aspi->xfer_mode_addr = MODE_IO_X1;
>>> +aspi->xfer_mode_data = MODE_IO_X1;
>>> +break;
>>> +case SPI_NOR_FAST:
>>> +aspi->num_hi_z_clocks = nor->read_dummy;
>>> +aspi->xfer_mode_cmd = MODE_IO_X1;
>>> +aspi->xfer_mode_addr = MODE_IO_X1;
>>> +aspi->xfer_mode_data = MODE_IO_X1;
>>> +break;
>>> +case SPI_NOR_DUAL:
>>> +aspi->num_hi_z_clocks = nor->read_dummy;
>>> +aspi->xfer_mode_cmd = MODE_IO_X1;
>>> +aspi->xfer_mode_addr = MODE_IO_X1;
>>> +aspi->xfer_mode_data = MODE_IO_X2;
>>> +break;
>>> +case SPI_NOR_QUAD:
>>> +aspi->num_hi_z_clocks = nor->read_dummy;
>>> +aspi->xfer_mode_cmd = MODE_IO_X1;
>>> +aspi->xfer_mode_addr = MODE_IO_X1;
>>> +aspi->xfer_mode_data = MODE_IO_X4;
>>> +break;
>>> +}
>>> +
>>> +aspi_setup_xip_read_chain(aspi, nor);
>>> +
>>> +mtd_device_register(>nor.mtd, NULL, 0);
>>> +
>>> +return 0;
>>> +}
>
> [snip]
--
Best regards,
Marek Vasut
Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller
On 07/31/2017 10:54 PM, Alexandru Gagniuc wrote:
> Hi Marek,
>
> Me again!
>
> On 07/29/2017 02:34 AM, Marek Vasut wrote:
>> On 07/29/2017 12:07 AM, Alexandru Gagniuc wrote:
>>> +static void aspi_drain_fifo(struct anarion_qspi *aspi, uint8_t *buf,
>>> size_t len)
>>> +{
>>> +uint32_t data;
>>
>> Is this stuff below something like ioread32_rep() ?
>>
> [snip]
>>> +aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
>>> +while (len >= 4) {
>>> +memcpy(, buf, sizeof(data));
>>> +aspi_write_reg(spi, ASPI_REG_DATA1, data);
>>
>> iowrite32_rep ?
>>
>>> +buf += 4;
>>> +len -= 4;
>>> +}
>
> I looked at using io(read|write)32_rep in these two places, and I've run
> into some issues.
>
> First, I'm seeing unaligned MMIO accesses, which are not supported on
> ARC. Note that 'buf' has an alignment of 1, while the register requires
> an alignment of 4. The memcpy() in-between takes care of that, which was
> the original intent.
>
> Other than that, we still need to break off the tail because we need to
> update ASPI_REG_BYTE_COUNT before writing/reading any more data from the
> FIFO. We have to keep track of the remainder, so we're not really saving
> any SLOC.
>
> I'd like to keep the original version as I find it to be much more
> symmetrical and readable.
Look at the other drivers, AFAIR the io*_rep() issue was solved over and
over again, maybe you can factor that code out.
> Thanks,
> Alex
>
>>> +
>>> +if (len) {
>>> +aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, len);
>>> +memcpy(, buf, len);
>>> +aspi_write_reg(spi, ASPI_REG_DATA1, data);
>>> +}
>>> +}
>
>
> Exhibit A: aspi_seed_fifo with writesl()
>
> static void aspi_seed_fifo(struct anarion_qspi *spi,
>const uint8_t *buf, size_t len)
> {
> uint32_t data;
> void __iomem *data_reg = (void *)(spi->regbase + ASPI_REG_DATA1);
>
> aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
> writesl(data_reg, buf, len / 4);
> buf += len & ~0x03;
> len &= 0x03;
>
> if (len) {
> aspi_write_reg(spi, ASPI_REG_BYTE_COUNT, len);
> memcpy(, buf, len);
> aspi_write_reg(spi, ASPI_REG_DATA1, data);
> }
> }
--
Best regards,
Marek Vasut
Re: [PATCH 4/5] mtd: spi-nor: Add driver for Adaptrum Anarion QSPI controller
On 08/01/2017 12:20 AM, Alexandru Gagniuc wrote:
> On 07/31/2017 02:33 PM, Marek Vasut wrote:
>> On 07/31/2017 07:17 PM, Alexandru Gagniuc wrote:
>
> Hi Marek,
>
> Thank you again for your feedback. I've applied a majority of your
> suggestions, and I am very happy with the result. I should have v2
> posted within a day or so.
No. You should have v2 out in about a week or so after people have time
to review v1 some more.
> [snip]
>>>>> +/*
>>>>> + * This mask does not match reality. Get over it:
>>>>
>>>> What is this about ?
>>>
>>> Each stage of the QSPI chain has two registers. The second register has
>>> a bitfield which takes in the length of the stage. For example, for
>>> DATA2, we can set the length up to 0x4000, but for ADDR2, we can only
>>> set a max of 4 bytes. I wrote this comment as a reminder to myself to be
>>> careful about using this mask. I'll rephrase the comment for [v2]
>>
>> Please do.
>>
> Staged for [PATCH v2]
>
>>>>> + * DATA2:0x3fff
>>>>> + * CMD2:0x0003
>>>>> + * ADDR2:0x0007
>>>>> + * PERF2:0x
>>>>> + * HI_Z:0x003f
>>>>> + * BCNT:0x0007
>>>>> + */
>>>>> +#define CHAIN_LEN(x)((x - 1) & ASPI_DATA_LEN_MASK)
btw parenthesis around (x) missing, although this is like GEN_MASK() or
something here ...
>>>>> +struct anarion_qspi {
>>>>> +structspi_nor nor;
>>>>> +structdevice *dev;
>>>>> +uintptr_tregbase;
>>>>
>>>> Should be void __iomem * I guess ?
>>>
>>> I chose uintptr_t as opposed to void *, because arithmetic on void * is
>>> not valid in C. What is the right answer hen, without risking undefined
>>> behavior?
>>
>> What sort of arithmetic ? It's perfectly valid in general ...
>
> ISO/IEC 9899:201x, Section 6.5.6, constraint(2) is not met when the one
> of the operands to addition is a void pointer.
> Section 6.2.5 (19) defines void to be an incomplete type.
Is that something new in C 201x draft ? Anyway, this would mean half of
the drivers are broken, so I'm not convinced.
> [snip]
>
>>>> Is this stuff below something like ioread32_rep() ?
>>>>
>>>>> +aspi_write_reg(aspi, ASPI_REG_BYTE_COUNT, sizeof(uint32_t));
>>>>> +while (len >= 4) {
>>>>> +data = aspi_read_reg(aspi, ASPI_REG_DATA1);
>>>>> +memcpy(buf, , sizeof(data));
>>>>> +buf += 4;
>>>>> +len -= 4;
>>>>> +}
>>>
>>> That is very similar to ioread32_rep, yes. I kept this as for the
>>> reasons outlined above, but changing this to _rep() seems innocent
>>> enough.
>>
>> What reason ?
>
> Being able to share the code between the different codebases where it is
> used.
Yes, that argument isn't gonna work, it'd make things impossible to
maintain in the kernel.
--
Best regards,
Marek Vasut
Re: [PATCH] mtd: spi-nor: cqspi: fix error return code in cqspi_probe()
On 08/09/2017 06:22 PM, Gustavo A. R. Silva wrote:
> platform_get_irq() returns an error code, but the cadence-quadspi
> driver ignores it and always returns -ENXIO. This is not correct
> and, prevents -EPROBE_DEFER from being propagated properly.
>
> Print and propagate the return value of platform_get_irq on failure.
>
> This issue was detected with the help of Coccinelle.
>
> Signed-off-by: Gustavo A. R. Silva <gust...@embeddedor.com>
> ---
> drivers/mtd/spi-nor/cadence-quadspi.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c
> b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 53c7d8e..15dacf5 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -1202,8 +1202,8 @@ static int cqspi_probe(struct platform_device *pdev)
> /* Obtain IRQ line. */
> irq = platform_get_irq(pdev, 0);
> if (irq < 0) {
> - dev_err(dev, "Cannot obtain IRQ.\n");
> - return -ENXIO;
> + dev_err(dev, "Cannot obtain IRQ: %d\n", irq);
"Cannot obtain IRQ, ret=%i\n" please . It's inobvious what the value
means otherwise.
> + return irq;
> }
>
> ret = clk_prepare_enable(cqspi->clk);
>
--
Best regards,
Marek Vasut
Re: [PATCH 2/2] mtd: spi-nor: Altera ASMI Parallel II IP Core
+ struct altera_asmip2_plat_data *qdata)
> +{
> + struct device *dev = >dev;
> + int ret, i;
> +
> + ret = altera_asmip2_create(dev, qdata->csr_base);
> +
> + if (ret) {
> + dev_err(dev, "failed to create qspi device %d\n", ret);
> + return ret;
> + }
> +
> + for (i = 0; i < qdata->num_chip_sel; i++) {
> + ret = altera_qspi_add_bank(dev, i, NULL);
> + if (ret) {
> + dev_err(dev, "failed to add qspi bank %d\n", ret);
> + break;
> + }
> + }
> +
> + return ret;
> +}
> +
> +static int altera_asmip2_probe(struct platform_device *pdev)
> +{
> + struct device_node *np = pdev->dev.of_node;
> + struct device *dev = >dev;
> + struct altera_asmip2_plat_data *qdata;
> + struct resource *res;
> + void __iomem *csr_base;
> + u32 bank;
> + int ret;
> + struct device_node *pp;
> +
> + qdata = dev_get_platdata(dev);
> +
> + if (qdata)
> + return __probe_with_data(pdev, qdata);
Avoid function names with __ prefix.
> + if (!np) {
> + dev_err(dev, "no device tree found %p\n", pdev);
> + return -ENODEV;
> + }
You might as well introduce a function to probe with of to be consistent
... or convert between pdata and ofdata and have one function for both.
> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + csr_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(csr_base)) {
> + dev_err(dev, "%s: ERROR: failed to map csr base\n", __func__);
> + return PTR_ERR(csr_base);
> + }
> +
> + ret = altera_asmip2_create(dev, csr_base);
> +
> + if (ret) {
> + dev_err(dev, "failed to create qspi device\n");
> + return ret;
> + }
> +
> + for_each_available_child_of_node(np, pp) {
> + of_property_read_u32(pp, "reg", );
> + if (bank >= ALTERA_ASMIP2_MAX_NUM_FLASH_CHIP) {
> + dev_err(dev, "bad reg value %u >= %u\n", bank,
> + ALTERA_ASMIP2_MAX_NUM_FLASH_CHIP);
> + goto error;
> + }
> +
> + if (altera_qspi_add_bank(dev, bank, pp)) {
> + dev_err(dev, "failed to add bank %u\n", bank);
> + goto error;
> + }
> + }
> +
> + return 0;
> +error:
> + altera_asmip2_remove_banks(dev);
> + return -EIO;
> +}
> +
> +static int altera_asmip2_remove(struct platform_device *pdev)
> +{
> + if (!pdev) {
> + dev_err(>dev, "%s NULL\n", __func__);
> + return -EINVAL;
Can this really happen ?
> + } else {
> + return altera_asmip2_remove_banks(>dev);
> + }
> +}
> +
> +static const struct of_device_id altera_asmip2_id_table[] = {
> +
> + { .compatible = "altr,asmi_parallel2",},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, altera_asmip2_id_table);
> +
> +static struct platform_driver altera_asmip2_driver = {
> + .driver = {
> + .name = ALTERA_ASMIP2_DRV_NAME,
> + .of_match_table = altera_asmip2_id_table,
> + },
> + .probe = altera_asmip2_probe,
> + .remove = altera_asmip2_remove,
> +};
> +module_platform_driver(altera_asmip2_driver);
> +
> +MODULE_AUTHOR("Matthew Gerlach <matthew.gerl...@linux.intel.com>");
> +MODULE_DESCRIPTION("Altera ASMI Parallel II");
> +MODULE_LICENSE("GPL v2");
> +MODULE_ALIAS("platform:" ALTERA_ASMIP2_DRV_NAME);
> diff --git a/include/linux/mtd/altera-asmip2.h
> b/include/linux/mtd/altera-asmip2.h
> new file mode 100644
> index 000..580c43c
> --- /dev/null
> +++ b/include/linux/mtd/altera-asmip2.h
> @@ -0,0 +1,24 @@
> +/*
> + *
> + * Copyright 2017 Intel Corporation, Inc.
> + *
> + * 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.
> + */
> +#ifndef __ALTERA_QUADSPI_H
> +#define __ALTERA_QUADSPI_H
> +
> +#include
> +
> +#define ALTERA_ASMIP2_DRV_NAME "altr-asmip2"
> +#define ALTERA_ASMIP2_MAX_NUM_FLASH_CHIP 3
> +#define ALTERA_ASMIP2_RESOURCE_SIZE 0x10
> +
> +struct altera_asmip2_plat_data {
> + void __iomem *csr_base;
> + u32 num_chip_sel;
> +};
> +
> +#endif
>
--
Best regards,
Marek Vasut
Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
On 05/03/2017 04:58 PM, Leonard Crestez wrote: > On Wed, 2017-05-03 at 16:26 +0200, Marek Vasut wrote: > >> On 05/03/2017 03:57 PM, Shawn Guo wrote: >>> >>> On Tue, Apr 25, 2017 at 07:28:06PM +0200, Marek Vasut wrote: >>>> >>>> On 04/25/2017 07:23 PM, Leonard Crestez wrote: >>>>> >>>>> Anyway, that version also sets the supply for reg_arm and reg_soc. It >>>>> is not necessary for fixing the crash I'm seeing but is good because it >>>>> will result in the minimum voltage on VDD_ARM_SOC_IN rather than a fix >>>>> 1375mv. I tested Marek's patch and it works fine on my rev B board >>>>> (which otherwise fails to boot upstream). >>>> Oh that's nice , thanks ! I don't have SDB and I hacked it up after a >>>> brief discussion with Fabio without even compile-testing it, thus RFC. >>>> Glad to hear it works and thanks for testing it ! Can you add a formal >>>> Tested-by please ? >>> Hi Marek, >> Hi Shawn, >> >>> Thanks for your patch. But I prefer Leonard's version because: 1) it >>> has a better commit log; 2) it sticks to one-patch-does-one-thing >>> policy. > >> 2) It actually fixes a problem with the voltage rails such that the DVFS >>works without leaving the system in unstable or dead state. You do >>need the second part of my patch if you drop the OPP hackery, without >>it the power framework cannot correctly configure the core voltages, >>so the patch from Leonard makes things worse. > > No, I think there is a misunderstanding here. The second part of your > patch will cause cpufreq poking at LDOs to indirectly adjust the input > from the PMIC to the minimum required (this is LDO target + > min_dropout_uv). Without it by default VDD_ARM_SOC_IN will remain fixed > as 1375mV from boot. Who sets / guarantees that default value for ARM and SOC rails ? With the OPP override in place, there's at least the guarantee that both rails will have the same voltage requirement. If you remove the OPP override without modeling the actual regulator wiring, the guarantee is gone. > That default value should be high enough for all cpufreq settings. > Setting the LDO parent will cause this voltage to be dynamically > reduced when possible (at low frequencies). This is not required for > proper operation, it is just an optimization to do more of the > regulation in the PMIC instead. It should work just fine without the > second part. > > That OPP override exists for "LDO bypass" mode, a feature not present > in upstream. In that mode the internal regulators are set to bypass > mode and voltage is controlled directly from the PMIC. Since VDD_ARM > and VDD_SOC have different minimum requirements but are joined on the > board the OPP voltages are adjusted to max(VDD_ARM, VDD_SOC). If LDOs > are enabled there is no good reason to do this. > > I don't care which patch goes in but the effect of the patch should be > clarified. > > -- > Regards, > Leonard > -- Best regards, Marek Vasut
Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
On 05/03/2017 04:41 PM, Shawn Guo wrote: > On Wed, May 03, 2017 at 04:32:06PM +0200, Marek Vasut wrote: >> On 05/03/2017 04:26 PM, Marek Vasut wrote: >>> On 05/03/2017 03:57 PM, Shawn Guo wrote: >>>> On Tue, Apr 25, 2017 at 07:28:06PM +0200, Marek Vasut wrote: >>>>> On 04/25/2017 07:23 PM, Leonard Crestez wrote: >>>>>> Anyway, that version also sets the supply for reg_arm and reg_soc. It >>>>>> is not necessary for fixing the crash I'm seeing but is good because it >>>>>> will result in the minimum voltage on VDD_ARM_SOC_IN rather than a fix >>>>>> 1375mv. I tested Marek's patch and it works fine on my rev B board >>>>>> (which otherwise fails to boot upstream). >>>>> >>>>> Oh that's nice , thanks ! I don't have SDB and I hacked it up after a >>>>> brief discussion with Fabio without even compile-testing it, thus RFC. >>>>> Glad to hear it works and thanks for testing it ! Can you add a formal >>>>> Tested-by please ? >>>> >>>> Hi Marek, >>> >>> Hi Shawn, >>> >>>> Thanks for your patch. But I prefer Leonard's version because: 1) it >>>> has a better commit log; 2) it sticks to one-patch-does-one-thing >>>> policy. >>> >>> Well I'd prefer this patch because >>> 1) It has T-B >> >> Correction, two TBs [1] >> >> [1] https://patchwork.kernel.org/patch/9698749/ > > That doesn't mean Leonard's patch hasn't been tested by anyone. That's what the T-B tags are for ... >>> 2) It actually fixes a problem with the voltage rails such that the DVFS >>>works without leaving the system in unstable or dead state. You do >>>need the second part of my patch if you drop the OPP hackery, without >>>it the power framework cannot correctly configure the core voltages, >>>so the patch from Leonard makes things worse. > > If that's true, I will change my mind. Well you can check the schematic ... the ARM and SOC rails share the same upstream regulator. -- Best regards, Marek Vasut
Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
On 05/03/2017 04:26 PM, Marek Vasut wrote: > On 05/03/2017 03:57 PM, Shawn Guo wrote: >> On Tue, Apr 25, 2017 at 07:28:06PM +0200, Marek Vasut wrote: >>> On 04/25/2017 07:23 PM, Leonard Crestez wrote: >>>> Anyway, that version also sets the supply for reg_arm and reg_soc. It >>>> is not necessary for fixing the crash I'm seeing but is good because it >>>> will result in the minimum voltage on VDD_ARM_SOC_IN rather than a fix >>>> 1375mv. I tested Marek's patch and it works fine on my rev B board >>>> (which otherwise fails to boot upstream). >>> >>> Oh that's nice , thanks ! I don't have SDB and I hacked it up after a >>> brief discussion with Fabio without even compile-testing it, thus RFC. >>> Glad to hear it works and thanks for testing it ! Can you add a formal >>> Tested-by please ? >> >> Hi Marek, > > Hi Shawn, > >> Thanks for your patch. But I prefer Leonard's version because: 1) it >> has a better commit log; 2) it sticks to one-patch-does-one-thing >> policy. > > Well I'd prefer this patch because > 1) It has T-B Correction, two TBs [1] [1] https://patchwork.kernel.org/patch/9698749/ > 2) It actually fixes a problem with the voltage rails such that the DVFS >works without leaving the system in unstable or dead state. You do >need the second part of my patch if you drop the OPP hackery, without >it the power framework cannot correctly configure the core voltages, >so the patch from Leonard makes things worse. > >> But I'm going to wait for a while to get Peter's comment discussed, >> before I actually apply Leonard's patch. >> >> Shawn >> > > -- Best regards, Marek Vasut
Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
On 05/03/2017 03:57 PM, Shawn Guo wrote: > On Tue, Apr 25, 2017 at 07:28:06PM +0200, Marek Vasut wrote: >> On 04/25/2017 07:23 PM, Leonard Crestez wrote: >>> Anyway, that version also sets the supply for reg_arm and reg_soc. It >>> is not necessary for fixing the crash I'm seeing but is good because it >>> will result in the minimum voltage on VDD_ARM_SOC_IN rather than a fix >>> 1375mv. I tested Marek's patch and it works fine on my rev B board >>> (which otherwise fails to boot upstream). >> >> Oh that's nice , thanks ! I don't have SDB and I hacked it up after a >> brief discussion with Fabio without even compile-testing it, thus RFC. >> Glad to hear it works and thanks for testing it ! Can you add a formal >> Tested-by please ? > > Hi Marek, Hi Shawn, > Thanks for your patch. But I prefer Leonard's version because: 1) it > has a better commit log; 2) it sticks to one-patch-does-one-thing > policy. Well I'd prefer this patch because 1) It has T-B 2) It actually fixes a problem with the voltage rails such that the DVFS works without leaving the system in unstable or dead state. You do need the second part of my patch if you drop the OPP hackery, without it the power framework cannot correctly configure the core voltages, so the patch from Leonard makes things worse. > But I'm going to wait for a while to get Peter's comment discussed, > before I actually apply Leonard's patch. > > Shawn > -- Best regards, Marek Vasut
Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
On 05/03/2017 07:58 PM, Leonard Crestez wrote: > On Wed, 2017-05-03 at 17:59 +0200, Marek Vasut wrote: >> On 05/03/2017 04:58 PM, Leonard Crestez wrote: >>> On Wed, 2017-05-03 at 16:26 +0200, Marek Vasut wrote: >>>> 2) It actually fixes a problem with the voltage rails such that the DVFS >>>>works without leaving the system in unstable or dead state. You do >>>>need the second part of my patch if you drop the OPP hackery, without >>>>it the power framework cannot correctly configure the core voltages, >>>>so the patch from Leonard makes things worse. >>> No, I think there is a misunderstanding here. The second part of your >>> patch will cause cpufreq poking at LDOs to indirectly adjust the input >>> from the PMIC to the minimum required (this is LDO target + >>> min_dropout_uv). Without it by default VDD_ARM_SOC_IN will remain fixed >>> as 1375mV from boot. > >> Who sets / guarantees that default value for ARM and SOC rails ? > > I think it's from the PMIC hardware itself (but maybe uboot plays with > it). VDD_ARM_SOC_IN on this board is tied to SW1AB from MMPF0200: > > http://www.nxp.com/assets/documents/data/en/data-sheets/MMPF0200.pdf > > It seems reasonable to rely on such voltages set externally. Isn't it an established rule that Linux should not depend on bootloader settings ? Or did that change ? >> With the OPP override in place, there's at least the guarantee that both >> rails will have the same voltage requirement. If you remove the OPP >> override without modeling the actual regulator wiring, the guarantee is >> gone. > > The imx6sx chip has internal LDO_ARM and LDO_SOC regulators which can > generate separate voltages for arm/soc. The fact that these regulators > share the same supply is only an issue when they are set in bypass > mode. > > However the boot issues happen on REV C but apparently not REV B of the > board. I don't have a good explanation for this so maybe I am missing > something. Well the regulator(s) cannot be correctly configured if the kernel doesn't have the correct power distribution described in the DT . -- Best regards, Marek Vasut
Re: [PATCH] mtd: spi-nor: cqspi: remove duplicate const
On 06/27/2017 05:34 PM, Arnd Bergmann wrote:
> The variable was already marked 'const' before the previous
> patch, but the qualifier was in an unusual place, and now the
> extra 'const' causes a harmless warning:
>
> drivers/mtd/spi-nor/cadence-quadspi.c:1286:34: error: duplicate 'const'
> declaration specifier [-Werror=duplicate-decl-specifier]
>
> This removes the other 'const' instead.
Isn't that const array const elements , thus two consts ?
IMO the original code is correct.
> Fixes: f993c123b461 ("mtd: spi-nor: cqspi: make of_device_ids const")
> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> ---
> drivers/mtd/spi-nor/cadence-quadspi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c
> b/drivers/mtd/spi-nor/cadence-quadspi.c
> index d315c326e72f..53c7d8e0327a 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -1283,7 +1283,7 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
> #define CQSPI_DEV_PM_OPS NULL
> #endif
>
> -static const struct of_device_id const cqspi_dt_ids[] = {
> +static const struct of_device_id cqspi_dt_ids[] = {
> {.compatible = "cdns,qspi-nor",},
> { /* end of table */ }
> };
>
--
Best regards,
Marek Vasut
Re: [PATCH 1/3] ARM: dts: Bindings for Altera Quadspi Controller Version 2
On 06/27/2017 04:32 PM, matthew.gerl...@linux.intel.com wrote: > > > On Tue, 27 Jun 2017, Marek Vasut wrote: > > Hi Marek, > > Thanks for the feedback. See my comments below. > > Matthew Gerlach > >> On 06/26/2017 06:13 PM, matthew.gerl...@linux.intel.com wrote: >>> From: Matthew Gerlach <matthew.gerl...@linux.intel.com> >>> >>> Device Tree bindings for Version 2 of the Altera Quadspi Controller >>> that can be optionally paired with a windowed bridge. >>> >>> Signed-off-by: Matthew Gerlach <matthew.gerl...@linux.intel.com> >>> --- >>> .../devicetree/bindings/mtd/altera-quadspi-v2.txt | 37 >>> ++ >>> 1 file changed, 37 insertions(+) >>> create mode 100644 >>> Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>> >>> diff --git >>> a/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>> b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>> new file mode 100644 >>> index 000..8ba63d7 >>> --- /dev/null >>> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>> @@ -0,0 +1,37 @@ >>> +* Altera Quad SPI Controller Version 2 >>> + >>> +Required properties: >>> +- compatible : Should be "altr,quadspi-v2". >>> +- reg : Contains at least two entries, and possibly three entries, >>> each of >>> +which is a tuple consisting of a physical address and length. >>> +- reg-names : Should contain the names "avl_csr" and "avl_mem" >>> corresponding >>> + to the control and status registers and qspi memory, >>> respectively. >>> + >>> + >>> +The Altera Quad SPI Controller Version 2 can be paired with a >>> windowed bridge >>> +in order to reduce the footprint of the memory interface. When a >>> windowed >>> +bridge is used, reads and writes of data must be 32 bits wide. >>> + >>> +Optional properties: >>> +- reg-names : Should contain the name "avl_window", if the windowed >>> bridge >>> + is used. This name corresponds to the register space that >>> + controls the window. >>> +- window-size : The size of the window which must be an even power >>> of 2. >>> +- read-bit-reverse : A boolean indicating the data read from the >>> flash should >>> + be bit reversed on a byte by byte basis before being >>> + delivered to the MTD layer. >>> +- write-bit-reverse : A boolean indicating the data written to the >>> flash should >>> + be bit reversed on a byte by byte basis. >> >> Is there ever a usecase where you need to set just one of these props ? >> Also, they're altera specific, so altr, prefix should be added. > > In general, I think if bit reversal is required, it would be required in > both directions. However, anything is possible when using FPGAs. So > I thought separate booleans would be future proofing the bindings. Maybe we should drop this whole thing and add it when this is actually required. Are there any users of this in the wild currently ? What is the purpose of doing this per-byte bit reverse instead of storing th bits in the original order ? > Thinking about this binding more, I wonder if the binding name(s) > should be (read|write)-bit8-reverse to indicate reversings the bits > in a byte as opposed to reversing the bits in a 32 bit word? > > I don't think bit reversal is specific to Altera/Intel components. I see > a nand driver performing bit reversal, and I think I've recently seen > other FPGA based drivers requiring bit reversal. $ git grep bit.reverse Documentation/devicetree/ | wc -l 0 So we don't have such a generic binding . It's up to Rob (I guess) to decide whether this is SoC specific and should've altr, prefix or not. IMO it is. -- Best regards, Marek Vasut
Re: [PATCH 2/3] mtd: spi-nor: core code for the Altera Quadspi Flash Controller v2
On 06/27/2017 04:57 PM, matthew.gerl...@linux.intel.com wrote:
[...]
>>> +static int altera_quadspi_read_reg(struct spi_nor *nor, u8 opcode,
>>> u8 *val,
>>> + int len)
>>> +{
>>> +struct altera_quadspi_flash *flash = nor->priv;
>>> +struct altera_quadspi *q = flash->q;
>>> +u32 data = 0;
>>> +
>>> +memset(val, 0, len);
>>> +
>>> +altera_quadspi_chip_select(q, flash->bank);
>>> +
>>> +switch (opcode) {
>>> +case SPINOR_OP_RDSR:
>>> +data = alt_qspi_readl(q->csr_base, QUADSPI_SR_REG);
>>> +dev_dbg(q->dev, "%s RDSR 0x%x\n", __func__, data);
>>> +*val = (u8)data & QUADSPI_SR_MASK;
>>> +break;
>>> +case SPINOR_OP_RDID:
>>> +if (q->opcode_id == EPCS_OPCODE_ID)
>>> +data = alt_qspi_readl(q->csr_base, QUADSPI_SID_REG);
>>> +else
>>> +data = alt_qspi_readl(q->csr_base, QUADSPI_RDID_REG);
>>> +
>>> +*((u32 *)val) = data;
>>
>> What are these awful casts ?
>
> This component requires reading the registers as 32 bit quantities. So it
> seemed the right thing to do to me.
Does this handle endianness well ?
>>> +break;
>>> +case SPINOR_OP_RDFSR:
>>> +data = alt_qspi_readl(q->csr_base, QUADSPI_FLAG_STATUS_REG);
>>> +dev_dbg(q->dev, "%s RDFSR 0x%x\n", __func__, data);
>>> +*val = (u8)(data & 0xff);
>>> +break;
>>> +default:
>>> +dev_dbg(q->dev, "%s UNHANDLED read_reg 0x%x\n",
>>> +__func__, opcode);
>>> +*val = 0;
>>> +break;
>>> +}
>>> +return 0;
>>> +}
[...]
>>> +#define WINDOW_ALIGN 4
>>> +#define WINDOW_MASK (WINDOW_ALIGN - 1)
>>
>> What are these undocumented macros in the middle of the code ?
>
> The bindings document states that when a windowed bridge
> is used, all accesses must be 32 bit. I can comment/rename
> and put at the top.
Yes please.
[...]
>>> diff --git a/include/linux/mtd/altera-quadspi.h
>>> b/include/linux/mtd/altera-quadspi.h
>>> new file mode 100644
>>> index 000..58f31ee
>>> --- /dev/null
>>> +++ b/include/linux/mtd/altera-quadspi.h
>>> @@ -0,0 +1,28 @@
>>> +/*
>>> + *
>>> + * Copyright 2017 Intel Corporation, Inc.
>>> + *
>>> + * 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.
>>> + */
>>> +#ifndef __ALTERA_QUADSPI_H
>>> +#define __ALTERA_QUADSPI_H
>>> +
>>> +#include
>>> +
>>> +#define ALTERA_QUADSPI_FL_BITREV_READ BIT(0)
>>> +#define ALTERA_QUADSPI_FL_BITREV_WRITE BIT(1)
>>> +
>>> +#define ALTERA_QUADSPI_MAX_NUM_FLASH_CHIP 3
>>> +
>>> +int altera_quadspi_create(struct device *dev, void __iomem *csr_base,
>>> + void __iomem *data_base, void __iomem *window_reg,
>>> + size_t window_size, u32 flags);
>>> +
>>> +int altera_qspi_add_bank(struct device *dev,
>>> + u32 bank, struct device_node *np);
>>> +
>>> +int altera_quadspi_remove_banks(struct device *dev);
>>
>> Why is this header needed at all ?
>
> This header is needed because of the very different ways
> FPGAs can be used with a processor running Linux. In the case of a
> soft processor in the FPGA or an ARM connected to a FPGA, this header
> is not necessary because device trees are used to probe the driver.
> However, if the FPGA is on a PCIe card connected to an x86, device trees
> are not generally used, and the pcie driver must enumerate the
> "sub-driver".
But we don't support that later part, do we ?
--
Best regards,
Marek Vasut
Re: [PATCH 1/3] ARM: dts: Bindings for Altera Quadspi Controller Version 2
On 06/27/2017 05:57 PM, matthew.gerl...@linux.intel.com wrote: > > > On Tue, 27 Jun 2017, Marek Vasut wrote: > >> On 06/27/2017 04:32 PM, matthew.gerl...@linux.intel.com wrote: >>> >>> >>> On Tue, 27 Jun 2017, Marek Vasut wrote: >>> >>> Hi Marek, >>> >>> Thanks for the feedback. See my comments below. >>> >>> Matthew Gerlach >>> >>>> On 06/26/2017 06:13 PM, matthew.gerl...@linux.intel.com wrote: >>>>> From: Matthew Gerlach <matthew.gerl...@linux.intel.com> >>>>> >>>>> Device Tree bindings for Version 2 of the Altera Quadspi Controller >>>>> that can be optionally paired with a windowed bridge. >>>>> >>>>> Signed-off-by: Matthew Gerlach <matthew.gerl...@linux.intel.com> >>>>> --- >>>>> .../devicetree/bindings/mtd/altera-quadspi-v2.txt | 37 >>>>> ++ >>>>> 1 file changed, 37 insertions(+) >>>>> create mode 100644 >>>>> Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>> >>>>> diff --git >>>>> a/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>> b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>> new file mode 100644 >>>>> index 000..8ba63d7 >>>>> --- /dev/null >>>>> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>> @@ -0,0 +1,37 @@ >>>>> +* Altera Quad SPI Controller Version 2 >>>>> + >>>>> +Required properties: >>>>> +- compatible : Should be "altr,quadspi-v2". >>>>> +- reg : Contains at least two entries, and possibly three entries, >>>>> each of >>>>> +which is a tuple consisting of a physical address and length. >>>>> +- reg-names : Should contain the names "avl_csr" and "avl_mem" >>>>> corresponding >>>>> + to the control and status registers and qspi memory, >>>>> respectively. >>>>> + >>>>> + >>>>> +The Altera Quad SPI Controller Version 2 can be paired with a >>>>> windowed bridge >>>>> +in order to reduce the footprint of the memory interface. When a >>>>> windowed >>>>> +bridge is used, reads and writes of data must be 32 bits wide. >>>>> + >>>>> +Optional properties: >>>>> +- reg-names : Should contain the name "avl_window", if the windowed >>>>> bridge >>>>> + is used. This name corresponds to the register space that >>>>> + controls the window. >>>>> +- window-size : The size of the window which must be an even power >>>>> of 2. >>>>> +- read-bit-reverse : A boolean indicating the data read from the >>>>> flash should >>>>> + be bit reversed on a byte by byte basis before being >>>>> + delivered to the MTD layer. >>>>> +- write-bit-reverse : A boolean indicating the data written to the >>>>> flash should >>>>> + be bit reversed on a byte by byte basis. >>>> >>>> Is there ever a usecase where you need to set just one of these props ? >>>> Also, they're altera specific, so altr, prefix should be added. >>> >>> In general, I think if bit reversal is required, it would be required in >>> both directions. However, anything is possible when using FPGAs. So >>> I thought separate booleans would be future proofing the bindings. >> >> Maybe we should drop this whole thing and add it when this is actually >> required. >> >> Are there any users of this in the wild currently ? >> >> What is the purpose of doing this per-byte bit reverse instead of >> storing th bits in the original order ? > > Hi Marek, > > Yes, there is hardware that has been in the wild for years that needs > this bit reversal. The specific use case is when a flash chip is > connected to > a FPGA, and the contents of the flash is used to configure the FPGA on > power up. In this use case, there is no processor involved with > configuring the FPGA. I am most familiar with this feature/bug with > Altera FPGAs, but I believe this issue exists with other programmable > devices. So the EPCQ/EPCS flash stores the bitstream in reverse or something ? What
Re: [PATCH 3/3] mtd: spi-nor: Altera Quadspi Flash Controller v2 Platform driver
On 06/27/2017 05:15 PM, matthew.gerl...@linux.intel.com wrote: > > > On Tue, 27 Jun 2017, Marek Vasut wrote: > >> On 06/26/2017 06:13 PM, matthew.gerl...@linux.intel.com wrote: >>> From: Matthew Gerlach <matthew.gerl...@linux.intel.com> >> >> Just wrap it into the Altera QSPI driver , no need for separate platform >> driver IMO. > > Hi Marek, > > I answered this question when you asked why the header file was > necessary, but I think further discussion could be helpful, since this > problem is becoming more prevelent. The Altera Quadspi component is a > soft IP in a > FPGA, and the processor using the component may or may not have device > tree support compiled into the Linux kernel. Since device tree support > may or may not be available, the device tree specific code must be > separated > from the core driver code. I see, that's fine, although there is no PCIe or other support in this submission. Is that planned ? > One can certainly make the case, that device tree support could/should > be available everywhere, but the current reality is most x86 Linux > kernel configurations do not include device tree support. > > For the record, I believe device trees, and more specifically device > tree overlays, are the best way for Linux to use FPGAs, but I have to > deal with the current realities. > > Thanks again for all the great feedback. > > Matthew Gerlach [...] -- Best regards, Marek Vasut
Re: [PATCH 1/3] ARM: dts: Bindings for Altera Quadspi Controller Version 2
On 06/27/2017 07:18 PM, matthew.gerl...@linux.intel.com wrote: > > > On Tue, 27 Jun 2017, Marek Vasut wrote: > >> On 06/27/2017 05:57 PM, matthew.gerl...@linux.intel.com wrote: >>> >>> >>> On Tue, 27 Jun 2017, Marek Vasut wrote: >>> >>>> On 06/27/2017 04:32 PM, matthew.gerl...@linux.intel.com wrote: >>>>> >>>>> >>>>> On Tue, 27 Jun 2017, Marek Vasut wrote: >>>>> >>>>> Hi Marek, >>>>> >>>>> Thanks for the feedback. See my comments below. >>>>> >>>>> Matthew Gerlach >>>>> >>>>>> On 06/26/2017 06:13 PM, matthew.gerl...@linux.intel.com wrote: >>>>>>> From: Matthew Gerlach <matthew.gerl...@linux.intel.com> >>>>>>> >>>>>>> Device Tree bindings for Version 2 of the Altera Quadspi Controller >>>>>>> that can be optionally paired with a windowed bridge. >>>>>>> >>>>>>> Signed-off-by: Matthew Gerlach <matthew.gerl...@linux.intel.com> >>>>>>> --- >>>>>>> .../devicetree/bindings/mtd/altera-quadspi-v2.txt | 37 >>>>>>> ++ >>>>>>> 1 file changed, 37 insertions(+) >>>>>>> create mode 100644 >>>>>>> Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>>>> >>>>>>> diff --git >>>>>>> a/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>>>> b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>>>> new file mode 100644 >>>>>>> index 000..8ba63d7 >>>>>>> --- /dev/null >>>>>>> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt >>>>>>> @@ -0,0 +1,37 @@ >>>>>>> +* Altera Quad SPI Controller Version 2 >>>>>>> + >>>>>>> +Required properties: >>>>>>> +- compatible : Should be "altr,quadspi-v2". >>>>>>> +- reg : Contains at least two entries, and possibly three entries, >>>>>>> each of >>>>>>> +which is a tuple consisting of a physical address and length. >>>>>>> +- reg-names : Should contain the names "avl_csr" and "avl_mem" >>>>>>> corresponding >>>>>>> + to the control and status registers and qspi memory, >>>>>>> respectively. >>>>>>> + >>>>>>> + >>>>>>> +The Altera Quad SPI Controller Version 2 can be paired with a >>>>>>> windowed bridge >>>>>>> +in order to reduce the footprint of the memory interface. When a >>>>>>> windowed >>>>>>> +bridge is used, reads and writes of data must be 32 bits wide. >>>>>>> + >>>>>>> +Optional properties: >>>>>>> +- reg-names : Should contain the name "avl_window", if the windowed >>>>>>> bridge >>>>>>> + is used. This name corresponds to the register space >>>>>>> that >>>>>>> + controls the window. >>>>>>> +- window-size : The size of the window which must be an even power >>>>>>> of 2. >>>>>>> +- read-bit-reverse : A boolean indicating the data read from the >>>>>>> flash should >>>>>>> + be bit reversed on a byte by byte basis before being >>>>>>> + delivered to the MTD layer. >>>>>>> +- write-bit-reverse : A boolean indicating the data written to the >>>>>>> flash should >>>>>>> + be bit reversed on a byte by byte basis. >>>>>> >>>>>> Is there ever a usecase where you need to set just one of these >>>>>> props ? >>>>>> Also, they're altera specific, so altr, prefix should be added. >>>>> >>>>> In general, I think if bit reversal is required, it would be >>>>> required in >>>>> both directions. However, anything is possible when using FPGAs. So >>>>> I thought separate booleans would be future proofing the bindings. >>>> >>>> Maybe we should drop th
Re: [PATCH 2/3] mtd: spi-nor: core code for the Altera Quadspi Flash Controller v2
On 06/27/2017 07:26 PM, matthew.gerl...@linux.intel.com wrote: [...] >>>>> +#ifndef __ALTERA_QUADSPI_H >>>>> +#define __ALTERA_QUADSPI_H >>>>> + >>>>> +#include >>>>> + >>>>> +#define ALTERA_QUADSPI_FL_BITREV_READ BIT(0) >>>>> +#define ALTERA_QUADSPI_FL_BITREV_WRITE BIT(1) >>>>> + >>>>> +#define ALTERA_QUADSPI_MAX_NUM_FLASH_CHIP 3 >>>>> + >>>>> +int altera_quadspi_create(struct device *dev, void __iomem *csr_base, >>>>> + void __iomem *data_base, void __iomem *window_reg, >>>>> + size_t window_size, u32 flags); >>>>> + >>>>> +int altera_qspi_add_bank(struct device *dev, >>>>> + u32 bank, struct device_node *np); >>>>> + >>>>> +int altera_quadspi_remove_banks(struct device *dev); >>>> >>>> Why is this header needed at all ? >>> >>> This header is needed because of the very different ways >>> FPGAs can be used with a processor running Linux. In the case of a >>> soft processor in the FPGA or an ARM connected to a FPGA, this header >>> is not necessary because device trees are used to probe the driver. >>> However, if the FPGA is on a PCIe card connected to an x86, device trees >>> are not generally used, and the pcie driver must enumerate the >>> "sub-driver". >> >> But we don't support that later part, do we ? > > There is currently v2 patch set for the intel-fpga PCIe driver being > reviewed where I am adding support for version 2 of the Altera Quadspi > controller. It'd be real nice to mention that in the cover letter with a link to that patchset , otherwise it's real hard to understand why you did this. > This technique of separating core driver code from platform/device tree > code has been reviewed and accepted for the Altera Partial > Reconfiguration IP, Altera Freeze Bridge, and the fpga region. -- Best regards, Marek Vasut
Re: [PATCH] regulator: bd9571mwv: Statize local symbols
On 05/28/2017 08:02 AM, Axel Lin wrote:
> These functions are only used by this driver, make them static.
>
> Signed-off-by: Axel Lin <axel@ingics.com>
Obviously correct,
Acked-by: Marek Vasut <marek.va...@gmail.com>
> ---
> drivers/regulator/bd9571mwv-regulator.c | 12 ++--
> 1 file changed, 6 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/regulator/bd9571mwv-regulator.c
> b/drivers/regulator/bd9571mwv-regulator.c
> index 8ba206f..c67a83d 100644
> --- a/drivers/regulator/bd9571mwv-regulator.c
> +++ b/drivers/regulator/bd9571mwv-regulator.c
> @@ -43,7 +43,7 @@ enum bd9571mwv_regulators { VD09, VD18, VD25, VD33, DVFS };
> .linear_min_sel = _lmin,\
> }
>
> -int bd9571mwv_avs_get_moni_state(struct regulator_dev *rdev)
> +static int bd9571mwv_avs_get_moni_state(struct regulator_dev *rdev)
> {
> unsigned int val;
> int ret;
> @@ -55,8 +55,8 @@ int bd9571mwv_avs_get_moni_state(struct regulator_dev *rdev)
> return val & BD9571MWV_AVS_SET_MONI_MASK;
> }
>
> -int bd9571mwv_avs_set_voltage_sel_regmap(struct regulator_dev *rdev,
> - unsigned int sel)
> +static int bd9571mwv_avs_set_voltage_sel_regmap(struct regulator_dev *rdev,
> + unsigned int sel)
> {
> int ret;
>
> @@ -68,7 +68,7 @@ int bd9571mwv_avs_set_voltage_sel_regmap(struct
> regulator_dev *rdev,
>rdev->desc->vsel_mask, sel);
> }
>
> -int bd9571mwv_avs_get_voltage_sel_regmap(struct regulator_dev *rdev)
> +static int bd9571mwv_avs_get_voltage_sel_regmap(struct regulator_dev *rdev)
> {
> unsigned int val;
> int ret;
> @@ -87,8 +87,8 @@ int bd9571mwv_avs_get_voltage_sel_regmap(struct
> regulator_dev *rdev)
> return val;
> }
>
> -int bd9571mwv_reg_set_voltage_sel_regmap(struct regulator_dev *rdev,
> - unsigned int sel)
> +static int bd9571mwv_reg_set_voltage_sel_regmap(struct regulator_dev *rdev,
> + unsigned int sel)
> {
> return regmap_write_bits(rdev->regmap, BD9571MWV_DVFS_SETVID,
>rdev->desc->vsel_mask, sel);
>
--
Best regards,
Marek Vasut
Re: [PATCH v2 1/1] MAINTAINERS: Update SPI NOR subsystem git repositories
On 06/13/2017 09:46 PM, Cyrille Pitchen wrote: > SPI NOR branches are now hosted on MTD repos, spi-nor/next is on l2-mtd > and spi-nor/fixes is on linux-mtd. > > Signed-off-by: Cyrille Pitchen <cyrille.pitc...@wedev4u.fr> Excellent. Acked-by: Marek Vasut <marek.va...@gmail.com> > --- > > ChangeLog > > v1 -> v2 > - add branch names 'spi-nor/fixes' and 'spi-nor/next' after url. > > MAINTAINERS | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/MAINTAINERS b/MAINTAINERS > index 3bf9897f8df4..46177688a4e8 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -12282,7 +12282,8 @@ M:Marek Vasut <marek.va...@gmail.com> > L: linux-...@lists.infradead.org > W: http://www.linux-mtd.infradead.org/ > Q: http://patchwork.ozlabs.org/project/linux-mtd/list/ > -T: git git://github.com/spi-nor/linux.git > +T: git git://git.infradead.org/linux-mtd.git spi-nor/fixes > +T: git git://git.infradead.org/l2-mtd.git spi-nor/next > S: Maintained > F: drivers/mtd/spi-nor/ > F: include/linux/mtd/spi-nor.h > -- Best regards, Marek Vasut
Re: [PATCH 1/1] mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables
KERNEL);
devm_() ?
> + if (!param_headers)
> + return -ENOMEM;
> +
> + err = spi_nor_read_sfdp(nor, sizeof(header),
> + psize, param_headers);
> + if (err) {
> + dev_err(nor->dev,
> + "failed to read SFDP parameter headers\n");
> + goto exit;
> + }
> + }
> +
> + /*
> + * Check other parameter headers to get the latest revision of
> + * the basic flash parameter table.
> + */
> + for (i = 0; i < header.nph; i++) {
> + param_header = _headers[i];
> +
> + if (SFDP_PARAM_HEADER_ID(param_header) == SFDP_BFPT_ID &&
> + param_header->major == SFDP_JESD216_MAJOR &&
> + (param_header->minor > bfpt_header->minor ||
> + (param_header->minor == bfpt_header->minor &&
> + param_header->length > bfpt_header->length)))
I'm not comfortable with this indent ...
> + bfpt_header = param_header;
> + }
Newline.
> + err = spi_nor_parse_bfpt(nor, bfpt_header, params);
> + if (err)
> + goto exit;
> +
> + /* Parse other parameter headers. */
> + for (i = 0; i < header.nph; i++) {
> + param_header = _headers[i];
> +
> + switch (SFDP_PARAM_HEADER_ID(param_header)) {
> + case SFDP_SECTOR_MAP_ID:
> + dev_info(nor->dev,
> + "non-uniform erase sector maps are not
> supported yet.\n");
> + break;
> +
> + default:
> + break;
> + }
> +
> + if (err)
> + goto exit;
> + }
> +
> +exit:
> + kfree(param_headers);
> + return err;
> +}
> +
> static int spi_nor_init_params(struct spi_nor *nor,
> const struct flash_info *info,
> struct spi_nor_flash_parameter *params)
> @@ -1646,6 +2210,22 @@ static int spi_nor_init_params(struct spi_nor *nor,
> }
> }
>
> + /* Override the parameters with data read from SFDP tables. */
> + nor->addr_width = 0;
> + nor->mtd.erasesize = 0;
> + if ((info->flags & (SPI_NOR_DUAL_READ | SPI_NOR_QUAD_READ)) &&
> + !(info->flags & SPI_NOR_SKIP_SFDP)) {
> + struct spi_nor_flash_parameter sfdp_params;
> +
> + memcpy(_params, params, sizeof(sfdp_params));
> + if (spi_nor_parse_sfdp(nor, _params)) {
> + nor->addr_width = 0;
> + nor->mtd.erasesize = 0;
> + } else {
> + memcpy(params, _params, sizeof(*params));
> + }
> + }
> +
> return 0;
> }
>
> @@ -1757,6 +2337,10 @@ static int spi_nor_select_erase(struct spi_nor *nor,
> {
> struct mtd_info *mtd = >mtd;
>
> + /* Do nothing if already configured from SFDP. */
> + if (mtd->erasesize)
> + return 0;
> +
> #ifdef CONFIG_MTD_SPI_NOR_USE_4K_SECTORS
> /* prefer "small sector" erase if possible */
> if (info->flags & SECT_4K) {
> @@ -1989,9 +2573,11 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> if (ret)
> return ret;
>
> - if (info->addr_width)
> + if (nor->addr_width) {
> + /* already configured by spi_nor_setup() */
> + } else if (info->addr_width) {
> nor->addr_width = info->addr_width;
> - else if (mtd->size > 0x100) {
> + } else if (mtd->size > 0x100) {
> /* enable 4-byte addressing if the device exceeds 16MiB */
> nor->addr_width = 4;
> if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> diff --git a/include/linux/mtd/spi-nor.h b/include/linux/mtd/spi-nor.h
> index 55faa2f07cca..0df3638ff0b8 100644
> --- a/include/linux/mtd/spi-nor.h
> +++ b/include/linux/mtd/spi-nor.h
> @@ -41,6 +41,8 @@
> #define SPINOR_OP_WREN 0x06/* Write enable */
> #define SPINOR_OP_RDSR 0x05/* Read status register */
> #define SPINOR_OP_WRSR 0x01/* Write status register 1 byte
> */
> +#define SPINOR_OP_RDSR2 0x3f/* Read status register 2 */
> +#define SPINOR_OP_WRSR2 0x3e/* Write status register 2 */
> #define SPINOR_OP_READ 0x03/* Read data bytes (low
> frequency) */
> #define SPINOR_OP_READ_FAST 0x0b/* Read data bytes (high frequency) */
> #define SPINOR_OP_READ_1_1_2 0x3b/* Read data bytes (Dual Output SPI) */
> @@ -56,6 +58,7 @@
> #define SPINOR_OP_CHIP_ERASE 0xc7/* Erase whole flash chip */
> #define SPINOR_OP_SE 0xd8/* Sector erase (usually 64KiB) */
> #define SPINOR_OP_RDID 0x9f/* Read JEDEC ID */
> +#define SPINOR_OP_RDSFDP 0x5a/* Read SFDP */
> #define SPINOR_OP_RDCR 0x35/* Read configuration register
> */
> #define SPINOR_OP_RDFSR 0x70/* Read flag status register */
>
> @@ -128,6 +131,9 @@
> /* Configuration Register bits. */
> #define CR_QUAD_EN_SPAN BIT(1) /* Spansion Quad I/O */
>
> +/* Status Register 2 bits. */
> +#define SR2_QUAD_EN_BIT7 BIT(7)
> +
> /* Supported SPI protocols */
> #define SNOR_PROTO_INST_MASK GENMASK(23, 16)
> #define SNOR_PROTO_INST_SHIFT16
>
--
Best regards,
Marek Vasut
Re: [PATCH] mtd: spi-nor: cqspi: make of_device_ids const
On 06/16/2017 11:35 AM, Arvind Yadav wrote:
> of_device_ids are not supposed to change at runtime. All functions
> working with of_device_ids provided by work with const
> of_device_ids. So mark the non-const structs as const.
>
> Signed-off-by: Arvind Yadav <arvind.yadav...@gmail.com>
Works for me
Reviewed-by: Marek Vasut <marek.va...@gmail.com>
> ---
> drivers/mtd/spi-nor/cadence-quadspi.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c
> b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 9f8102d..1a25637 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -1277,7 +1277,7 @@ static int cqspi_resume(struct device *dev)
> #define CQSPI_DEV_PM_OPS NULL
> #endif
>
> -static struct of_device_id const cqspi_dt_ids[] = {
> +static const struct of_device_id const cqspi_dt_ids[] = {
> {.compatible = "cdns,qspi-nor",},
> { /* end of table */ }
> };
>
--
Best regards,
Marek Vasut
Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
On 05/04/2017 11:42 AM, Leonard Crestez wrote: > On Wed, 2017-05-03 at 21:33 +0200, Marek Vasut wrote: >> On 05/03/2017 07:58 PM, Leonard Crestez wrote: >>> On Wed, 2017-05-03 at 17:59 +0200, Marek Vasut wrote: >>>> On 05/03/2017 04:58 PM, Leonard Crestez wrote: >>>>> On Wed, 2017-05-03 at 16:26 +0200, Marek Vasut wrote: > >>>>>> 2) It actually fixes a problem with the voltage rails such that the DVFS >>>>>>works without leaving the system in unstable or dead state. You do >>>>>>need the second part of my patch if you drop the OPP hackery, without >>>>>>it the power framework cannot correctly configure the core voltages, >>>>>>so the patch from Leonard makes things worse. > >>>>> No, I think there is a misunderstanding here. The second part of your >>>>> patch will cause cpufreq poking at LDOs to indirectly adjust the input >>>>> from the PMIC to the minimum required (this is LDO target + >>>>> min_dropout_uv). Without it by default VDD_ARM_SOC_IN will remain fixed >>>>> as 1375mV from boot. > >>>> Who sets / guarantees that default value for ARM and SOC rails ? > >>> I think it's from the PMIC hardware itself (but maybe uboot plays with >>> it). VDD_ARM_SOC_IN on this board is tied to SW1AB from MMPF0200: >>> >>> http://www.nxp.com/assets/documents/data/en/data-sheets/MMPF0200.pdf >>> >>> It seems reasonable to rely on such voltages set externally. > >> Isn't it an established rule that Linux should not depend on bootloader >> settings ? Or did that change ? > > I don't actually know. Is there a hard and fast rule about this, even when it > comes to voltages? Unless something changed recently, you are not supposed to depend on bootloader behavior. > In theory it is possible for a bootloader to set a low cpu frequency and low > voltage and then have the chip fail when the cpufreq driver attempts to go > higher. Setting vin-supply on reg_arm/reg_soc would fix that. > >> Well the regulator(s) cannot be correctly configured if the kernel >> doesn't have the correct power distribution described in the DT . > > It depends on your definition of "correctness". It it certainly > possible to get a functional system while only partially describing > regulator relationships. Then your description of the hardware in DT is not correct. > I think there is a further misunderstanding here. I have a problem > where imx6sx-sdb rev C boards crash on boot with upstream (but are > reported to work fine with rev B). Removing the OPP overrides fixes > this specific issue. > > I don't object to the second part of your patch, setting correct supply links > is a good thing for various reasons. It is just not necessary for fixing the > concrete crash mentioned above (and I tested this). It should probably go in > a separate patch. Mind you, my patch is not fixing any crash, it's correcting the regulator binding and removing the OPP override which is at that point useless. > It might seem a pedantic difference but it's good to accurately describe the > effect of patches in commit messages. For example it might help somebody > looking to backport various fixes. Which part of the following commit message is unclear? " The imx6sx-sdb has one power supply that drives both VDDARM_IN and VDDSOC_IN, which is the sw1a regulator on the PFUZE PMIC. Connect both inputs to the sw1a regulator on the PMIC and drop the OPP hackery which is no longer needed as the power framework will take care of the regulator configuration as needed. " btw if sending obvious bugfix upstream is met with this sort of resistance and pointless discussion, it is extremely demotivating. Waiting for a maintainer reply for 2-4 weeks, only to get a kurt reply like "I don't like the commit message" doesn't help either. -- Best regards, Marek Vasut
Re: [PATCH 2/5] mtd: nand: gpmi: add i.MX 7 SoC support
On 05/02/2017 11:17 AM, Boris Brezillon wrote: > Hi Han, > > On Fri, 21 Apr 2017 13:29:16 -0500 > Han Xu <xhnj...@gmail.com> wrote: > >>>> >>>>>>> But then, adding the type would only require 2-3 lines of change if I >>>>>>> add it to the GPMI_IS_MX6 macro... >>>>>> >>>>>> Then at least add a comment because using type = IMX6SX right under >>>>>> gpmi_data_mx7d can trigger some head-scratching. And put my R-B on V2. >>>>> >>>>> FWIW, I mentioned it in the commit message. >>>>> >>>>> I think rather then adding a comment it is cleaner to just add IS_IMX7D >>>>> and add it to the GPMI_IS_MX6 macro. That does not need a comment since >>>>> it implicitly says we have a i.MX 7 but treat it like i.MX 6 and it is a >>>>> rather small change. Does that sound acceptable? >>>> >>>> Sure, that's even better, thanks. >>>> >>>> btw isn't there some single-core mx7 (mx7s ?) , maybe we should just go >>>> with mx7 (without the d suffix). I dunno if it has GPMI NAND though, so >>>> maybe mx7d is the right thing to do here ... >>>> >>> >>> There is a Solo version yes, and it has GPMI NAND too. However, almost >>> all i.MX 7 IPs have been named imx7d by NXP for some reason (including >>> compatible strings, see grep -r -e imx7 Documentation/), so I thought I >>> stay consistent here... I missed the DT bit, sorry. the DT bindings say: - compatible : should be "fsl,-gpmi-nand" so if FSL invented their own buggy bindings, they need to get them through Rob :) IMO for MX7, this should be "imx7-gpmi-nand" , unless there's some incentive to discern the solo/dual chips and/or there is a future imx7 coming up with different GPMI NAND block version. >> Hi Guys, >> >> Yes, there should be a i.MX7 Solo version with one core fused out. IMO, can >> we use QUIRK to distinguish them rather than SoC name. I know I also sent >> some patch set with SoC Name but I prefer to use QUIRK now. > > Not sure what this means. Are you okay with Stefan's v2? IMO the GPMI controller in solo and dual should be the same, so there's no need to have quirks for it. -- Best regards, Marek Vasut
[PATCH V3 2/2] mfd: Add ROHM BD9571MWV-M MFD PMIC driver
Add the MFD part of the ROHM BD9571MWV-M PMIC driver and MAINTAINERS
entry. The MFD part only specifies the regmap bits for the PMIC and
binds the subdevs together.
Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
Cc: linux-kernel@vger.kernel.org
Cc: Geert Uytterhoeven <geert+rene...@glider.be>
Cc: Lee Jones <lee.jo...@linaro.org>
---
V2: - Change BD9571MWV_AVS_VD09_VID0,1,2,3 to BD9571MWV_AVS_VD09_VID(n)
- Change BD9571MWV_AVS_DVFS_VID0,1,2,3 to BD9571MWV_AVS_DVFS_VID(n)
- Make the AVS_VD09 range RW, so it can be used by the regulator
driver for the VD09 regulator
- Report the regmap read return values when attempting to read ID
registers fails
V3: No change
---
MAINTAINERS | 11 ++
drivers/mfd/Kconfig | 13 +++
drivers/mfd/Makefile | 1 +
drivers/mfd/bd9571mwv.c | 230 ++
include/linux/mfd/bd9571mwv.h | 115 +
5 files changed, 370 insertions(+)
create mode 100644 drivers/mfd/bd9571mwv.c
create mode 100644 include/linux/mfd/bd9571mwv.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 2a9aa4be5846..9c30e6b00358 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10942,6 +10942,17 @@ L: linux-ser...@vger.kernel.org
S: Odd Fixes
F: drivers/tty/serial/rp2.*
+ROHM MULTIFUNCTION BD9571MWV-M PMIC DEVICE DRIVERS
+M: Marek Vasut <marek.vasut+rene...@gmail.com>
+L: linux-kernel@vger.kernel.org
+L: linux-renesas-...@vger.kernel.org
+S: Supported
+F: drivers/mfd/bd9571mwv.c
+F: drivers/regulator/bd9571mwv-regulator.c
+F: drivers/gpio/gpio-bd9571mwv.c
+F: include/linux/mfd/bd9571mwv.h
+F: Documentation/devicetree/bindings/mfd/bd9571mwv.txt
+
ROSE NETWORK LAYER
M: Ralf Baechle <r...@linux-mips.org>
L: linux-h...@vger.kernel.org
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 3eb5c93595f6..6f4668ae28cc 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -133,6 +133,19 @@ config MFD_BCM590XX
help
Support for the BCM590xx PMUs from Broadcom
+config MFD_BD9571MWV
+ tristate "ROHM BD9571MWV PMIC"
+ select MFD_CORE
+ select REGMAP_I2C
+ depends on I2C
+ help
+ Support for the ROHM BD9571MWV PMIC, which contains single
+ voltage regulator, voltage sampling units, GPIO block and
+ watchdog block.
+
+ This driver can also be built as a module. If so, the module
+ will be called bd9571mwv.
+
config MFD_AC100
tristate "X-Powers AC100"
select MFD_CORE
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index c16bf1ea0ea9..3cacc9748e13 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -10,6 +10,7 @@ obj-$(CONFIG_MFD_ACT8945A)+= act8945a.o
obj-$(CONFIG_MFD_SM501)+= sm501.o
obj-$(CONFIG_MFD_ASIC3)+= asic3.o tmio_core.o
obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
+obj-$(CONFIG_MFD_BD9571MWV)+= bd9571mwv.o
cros_ec_core-objs := cros_ec.o
cros_ec_core-$(CONFIG_ACPI)+= cros_ec_acpi_gpe.o
obj-$(CONFIG_MFD_CROS_EC) += cros_ec_core.o
diff --git a/drivers/mfd/bd9571mwv.c b/drivers/mfd/bd9571mwv.c
new file mode 100644
index ..64e088dfe7b0
--- /dev/null
+++ b/drivers/mfd/bd9571mwv.c
@@ -0,0 +1,230 @@
+/*
+ * ROHM BD9571MWV-M MFD driver
+ *
+ * Copyright (C) 2017 Marek Vasut <marek.vasut+rene...@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed "as is" WITHOUT ANY WARRANTY of any
+ * kind, whether expressed or implied; without even the implied warranty
+ * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License version 2 for more details.
+ *
+ * Based on the TPS65086 driver
+ */
+
+#include
+#include
+#include
+#include
+
+#include
+
+static const struct mfd_cell bd9571mwv_cells[] = {
+ { .name = "bd9571mwv-regulator", },
+ { .name = "bd9571mwv-gpio", },
+};
+
+static const struct regmap_range bd9571mwv_readable_yes_ranges[] = {
+ regmap_reg_range(BD9571MWV_VENDOR_CODE, BD9571MWV_PRODUCT_REVISION),
+ regmap_reg_range(BD9571MWV_AVS_SET_MONI, BD9571MWV_AVS_DVFS_VID(3)),
+ regmap_reg_range(BD9571MWV_VD18_VID, BD9571MWV_VD33_VID),
+ regmap_reg_range(BD9571MWV_DVFS_VINIT, BD9571MWV_DVFS_VINIT),
+ regmap_reg_range(BD9571MWV_DVFS_SETVMAX, BD9571MWV_DVFS_MONIVDAC),
+ regmap_reg_range(BD9571MWV_GPIO_IN, BD9571MWV_GPIO_IN),
+ regmap_reg_range(BD9571MWV_GPIO_INT, BD9571MWV_GPIO_INTMASK),
+ regmap_reg_range(BD9571MWV_INT_INTREQ, BD9571MWV_INT_INTMASK),
+};
+
+static const struct regmap_access_table bd9571mwv_readable_table = {
+
Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
On 05/04/2017 02:44 PM, Shawn Guo wrote: > On Thu, May 04, 2017 at 12:06:11PM +0200, Marek Vasut wrote: >> On 05/04/2017 11:42 AM, Leonard Crestez wrote: >>> I think there is a further misunderstanding here. I have a problem >>> where imx6sx-sdb rev C boards crash on boot with upstream (but are >>> reported to work fine with rev B). Removing the OPP overrides fixes >>> this specific issue. >>> >>> I don't object to the second part of your patch, setting correct supply >>> links is a good thing for various reasons. It is just not necessary for >>> fixing the concrete crash mentioned above (and I tested this). It should >>> probably go in a separate patch. >> >> Mind you, my patch is not fixing any crash, it's correcting the >> regulator binding and removing the OPP override which is at that >> point useless. > > Heh, that's the primary reason why I prefer Leonard's patch, as his > patch is fixing a critical crash issue, while yours is just removing > some useless stuff and correcting something that doesn't show any real > problem. Maybe you want to compare those two patches again, his patch fixing the critical crash is removing the same "some useless stuff" as mine. Plus mine is actually making sure the regulators are configured correctly, not just removing "some useless stuff" and hoping for the best that bootloader will do the right thing. > So Leonard's patch will need to be applied for v4.12-rc and copy stable > kernel, while yours will only be applied for next merge window as a > cleanup/improving thing. Well mine is bugfix, but I'm clearly unable to get that information across. > Patches are similar, but they can be handled very differently, because > commit log tells completely different stories. Do you see how commit > log matters now? I understand how commit log matters and I still believe I described the change there just fine. >>> It might seem a pedantic difference but it's good to accurately describe >>> the effect of patches in commit messages. For example it might help >>> somebody looking to backport various fixes. > > @Leonard, no, it's not pedantic at all. I really appreciate your commit > message and all the comments you added in the discussion, which is > extremely helpful for us to understand the changes. > >> Which part of the following commit message is unclear? >> >> " >> The imx6sx-sdb has one power supply that drives both VDDARM_IN >> and VDDSOC_IN, which is the sw1a regulator on the PFUZE PMIC. >> Connect both inputs to the sw1a regulator on the PMIC and drop >> the OPP hackery which is no longer needed as the power framework >> will take care of the regulator configuration as needed. >> " > > Something unclear in my opinion: > > - The OPP hackery was never needed, as it's only needed for LDO bypass >mode which hasn't been supported by mainline kernel. It's not 'no >longer needed as the power framework ...'. I don't know what this is about, it's not from my patch either ... > - What are the related changes in power framework? It will be more >clear if we can have the particular commit mentioned here. Uh, I don't understand your question, there are no new changes in the power framework. The DT for the SDB was always wrong, my patch fixes it. >> btw if sending obvious bugfix upstream is met with this sort of > > Leonard's patch is an obvious bugfix, but yours is not. Please consider mine one. I marked it RFC because I don't even have the board and I needed someone to test it. >> resistance and pointless discussion, it is extremely demotivating. > > As I said to Leonard above, the discussion is extremely helpful, IMO. > >> Waiting for a maintainer reply for 2-4 weeks, only to get a kurt > > This is not a paid job for maintainer. It's expected that he doesn't > always reply in a timely manner, especially when the tree is kinda > 'frozen' for merge window. > >> reply like "I don't like the commit message" doesn't help either. > > What really annoys me is your attitude to commit message. We had this discussion before ... -- Best regards, Marek Vasut
Re: [PATCH] ARM: dts: imx6sx-sdb: Remove cpufreq OPP override
On 05/04/2017 03:41 PM, Shawn Guo wrote: > On Thu, May 04, 2017 at 03:08:41PM +0200, Marek Vasut wrote: >> On 05/04/2017 02:44 PM, Shawn Guo wrote: >>> On Thu, May 04, 2017 at 12:06:11PM +0200, Marek Vasut wrote: > > > >>>> Mind you, my patch is not fixing any crash, it's correcting the >>>> regulator binding and removing the OPP override which is at that >>>> point useless. >>> >>> Heh, that's the primary reason why I prefer Leonard's patch, as his >>> patch is fixing a critical crash issue, while yours is just removing >>> some useless stuff and correcting something that doesn't show any real >>> problem. >> >> Maybe you want to compare those two patches again, his patch fixing the >> critical crash is removing the same "some useless stuff" as mine. > > The difference is in commit message. From your commit message, people > do not see what real world user noticeable issue your patch is fixing. > >> Plus >> mine is actually making sure the regulators are configured correctly, >> not just removing "some useless stuff" and hoping for the best that >> bootloader will do the right thing. > > Without this part, we do not get anything worse. That said, it can be > an improvement in another patch. > >>>> Which part of the following commit message is unclear? >>>> >>>> " >>>> The imx6sx-sdb has one power supply that drives both VDDARM_IN >>>> and VDDSOC_IN, which is the sw1a regulator on the PFUZE PMIC. >>>> Connect both inputs to the sw1a regulator on the PMIC and drop >>>> the OPP hackery which is no longer needed as the power framework >>>> will take care of the regulator configuration as needed. >>>> " >>> >>> Something unclear in my opinion: >>> >>> - The OPP hackery was never needed, as it's only needed for LDO bypass >>>mode which hasn't been supported by mainline kernel. It's not 'no >>>longer needed as the power framework ...'. >> >> I don't know what this is about, it's not from my patch either ... > > So I guess you do not understand how the OPP hackery was born and why it > shouldn't be there for mainline kernel at all. The OPP hackery is there to keep both regulators configured the same way, since they share the same input voltage rail IMO. If you model the power distribution correctly, the OPP hackery can be removed. >>> - What are the related changes in power framework? It will be more >>>clear if we can have the particular commit mentioned here. >> >> Uh, I don't understand your question, there are no new changes in the >> power framework. The DT for the SDB was always wrong, my patch fixes it. > > Then 'no longer needed' in your commit message is really confusing. > > Shawn > -- Best regards, Marek Vasut
Re: [PATCH 2/3] mtd: spi-nor: core code for the Altera Quadspi Flash Controller v2
sk = SNOR_HWCAPS_READ |
> + SNOR_HWCAPS_READ_FAST |
> + SNOR_HWCAPS_READ_1_1_2 |
> + SNOR_HWCAPS_READ_1_1_4 |
> + SNOR_HWCAPS_PP,
> + };
> +
> + if (bank > q->num_flashes - 1)
> + return -EINVAL;
> +
> + altera_quadspi_chip_select(q, bank);
> +
> + flash = devm_kzalloc(q->dev, sizeof(*flash), GFP_KERNEL);
> + if (!flash)
> + return -ENOMEM;
> +
> + q->flash[bank] = flash;
> + nor = >nor;
> + nor->dev = dev;
> + nor->priv = flash;
> + nor->mtd.priv = nor;
> + flash->q = q;
> + flash->bank = bank;
> + spi_nor_set_flash_node(nor, np);
> +
> + /* spi nor framework*/
> + nor->read_reg = altera_quadspi_read_reg;
> + nor->write_reg = altera_quadspi_write_reg;
> + nor->read = altera_quadspi_read;
> + nor->write = altera_quadspi_write;
> + nor->erase = altera_quadspi_erase;
> + nor->flash_lock = altera_quadspi_lock;
> + nor->flash_unlock = altera_quadspi_unlock;
> +
> + /* scanning flash and checking dev id */
> +#ifdef CONFIG_OF
> + if (np && (of_modalias_node(np, modalias, sizeof(modalias)) < 0))
> + return -EINVAL;
> +#endif
> +
> + ret = spi_nor_scan(nor, modalias, );
> + if (ret) {
> + dev_err(nor->dev, "flash not found\n");
> + return ret;
> + }
> +
> + ret = mtd_device_register(>mtd, NULL, 0);
> +
> + altera_quadspi_unlock(nor, 0, 0);
> +
> + return ret;
> +}
> +
> +int altera_quadspi_create(struct device *dev, void __iomem *csr_base,
> + void __iomem *data_base, void __iomem *window_base,
> + size_t window_size, u32 flags)
> +{
> + struct altera_quadspi *q;
> +
> + q = devm_kzalloc(dev, sizeof(*q), GFP_KERNEL);
> + if (!q)
> + return -ENOMEM;
> +
> + q->dev = dev;
> + q->csr_base = csr_base;
> + q->data_base = data_base;
> + q->window_base = window_base;
> + q->window_size = window_size;
> +
> + q->flags = flags;
> +
> + dev_set_drvdata(dev, q);
> +
> + dev_dbg(dev, "%s SR=0x%x FSR=0x%x\n", __func__,
> + alt_qspi_readl(q->csr_base, QUADSPI_SR_REG),
> + alt_qspi_readl(q->csr_base, QUADSPI_FLAG_STATUS_REG));
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(altera_quadspi_create);
> +
> +int altera_qspi_add_bank(struct device *dev,
> + u32 bank, struct device_node *np)
> +{
> + struct altera_quadspi *q = dev_get_drvdata(dev);
> +
> + if (q->num_flashes >= ALTERA_QUADSPI_MAX_NUM_FLASH_CHIP)
> + return -ENOMEM;
> +
> + q->num_flashes++;
> +
> + return altera_quadspi_setup_banks(dev, bank, np);
> +}
> +EXPORT_SYMBOL_GPL(altera_qspi_add_bank);
> +
> +int altera_quadspi_remove_banks(struct device *dev)
> +{
> + struct altera_quadspi *q = dev_get_drvdata(dev);
> + struct altera_quadspi_flash *flash;
> + int i;
> + int ret = 0;
> +
> + /* clean up for all nor flash */
> + for (i = 0; i < q->num_flashes; i++) {
> + flash = q->flash[i];
> + if (!flash)
> + continue;
> +
> + /* clean up mtd stuff */
> + ret = mtd_device_unregister(>nor.mtd);
> + if (ret) {
> + dev_err(dev, "error removing mtd\n");
> + return ret;
> + }
> + }
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(altera_quadspi_remove_banks);
> +
> +MODULE_AUTHOR("Viet Nga Dao <vn...@altera.com>");
> +MODULE_AUTHOR("Yong Sern Lau <lau.yong.s...@intel.com>");
> +MODULE_AUTHOR("Matthew Gerlach <matthew.gerl...@linux.intel.com>");
> +MODULE_DESCRIPTION("Altera QuadSPI Version 2 Driver");
> +MODULE_LICENSE("GPL v2");
> diff --git a/include/linux/mtd/altera-quadspi.h
> b/include/linux/mtd/altera-quadspi.h
> new file mode 100644
> index 000..58f31ee
> --- /dev/null
> +++ b/include/linux/mtd/altera-quadspi.h
> @@ -0,0 +1,28 @@
> +/*
> + *
> + * Copyright 2017 Intel Corporation, Inc.
> + *
> + * 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.
> + */
> +#ifndef __ALTERA_QUADSPI_H
> +#define __ALTERA_QUADSPI_H
> +
> +#include
> +
> +#define ALTERA_QUADSPI_FL_BITREV_READ BIT(0)
> +#define ALTERA_QUADSPI_FL_BITREV_WRITE BIT(1)
> +
> +#define ALTERA_QUADSPI_MAX_NUM_FLASH_CHIP 3
> +
> +int altera_quadspi_create(struct device *dev, void __iomem *csr_base,
> + void __iomem *data_base, void __iomem *window_reg,
> + size_t window_size, u32 flags);
> +
> +int altera_qspi_add_bank(struct device *dev,
> + u32 bank, struct device_node *np);
> +
> +int altera_quadspi_remove_banks(struct device *dev);
Why is this header needed at all ?
> +#endif
>
--
Best regards,
Marek Vasut
Re: [PATCH v3 1/2] mtd: st_spi_fsm: remove SPINOR_OP_RDSR2 and use SPINOR_OP_RDCR instead
On 06/26/2017 03:09 PM, Cyrille Pitchen wrote:
> The 35h instruction op code has two aliases/macro definitions:
> - SPINOR_OP_RDCR from include/linux/mtd/spi-nor.h
> - SPINOR_OP_RDSR2 from drivers/mtd/devices/serial_flash_cmds.h
>
> Actually, some manufacturers name the associated internal register Status
> Register 2 whereas other manufacturers name it Configuration Register
> hence the two different macros for the very same instruction op code.
>
> Since the spi-nor.h file is the reference file for all SPI NOR instruction
> op codes, this patch removes the definition of the SPINOR_OP_RDSR2 macro.
>
> Also the SPINOR_OP_RDSR2 macro will be associated to another instruction
> op code in a further patch so we need to avoid a conflict defining this
> macro twice. Indeed the JESD216 rev B specification, defining the SFDP
> tables, also refers to the 3Eh and 3Fh instruction op codes to write/read
> the Status Register 2 on some SPI NOR flash memories, the 35h op code
> still being used to read the Configuration Register/Status Register 2 on
> other memories.
>
> Signed-off-by: Cyrille Pitchen <cyrille.pitc...@microchip.com>
Acked-by: Marek Vasut <marek.va...@gmail.com>
> ---
> drivers/mtd/devices/serial_flash_cmds.h | 1 -
> drivers/mtd/devices/st_spi_fsm.c| 4 ++--
> 2 files changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/mtd/devices/serial_flash_cmds.h
> b/drivers/mtd/devices/serial_flash_cmds.h
> index 8b81e15105dd..eba125c9f23f 100644
> --- a/drivers/mtd/devices/serial_flash_cmds.h
> +++ b/drivers/mtd/devices/serial_flash_cmds.h
> @@ -13,7 +13,6 @@
> #define _MTD_SERIAL_FLASH_CMDS_H
>
> /* Generic Flash Commands/OPCODEs */
> -#define SPINOR_OP_RDSR2 0x35
> #define SPINOR_OP_WRVCR 0x81
> #define SPINOR_OP_RDVCR 0x85
>
> diff --git a/drivers/mtd/devices/st_spi_fsm.c
> b/drivers/mtd/devices/st_spi_fsm.c
> index 804313a33f2b..21afd94cd904 100644
> --- a/drivers/mtd/devices/st_spi_fsm.c
> +++ b/drivers/mtd/devices/st_spi_fsm.c
> @@ -1445,7 +1445,7 @@ static int stfsm_s25fl_config(struct stfsm *fsm)
> }
>
> /* Check status of 'QE' bit, update if required. */
> - stfsm_read_status(fsm, SPINOR_OP_RDSR2, , 1);
> + stfsm_read_status(fsm, SPINOR_OP_RDCR, , 1);
> data_pads = ((fsm->stfsm_seq_read.seq_cfg >> 16) & 0x3) + 1;
> if (data_pads == 4) {
> if (!(cr1 & STFSM_S25FL_CONFIG_QE)) {
> @@ -1490,7 +1490,7 @@ static int stfsm_w25q_config(struct stfsm *fsm)
> return ret;
>
> /* Check status of 'QE' bit, update if required. */
> - stfsm_read_status(fsm, SPINOR_OP_RDSR2, , 1);
> + stfsm_read_status(fsm, SPINOR_OP_RDCR, , 1);
> data_pads = ((fsm->stfsm_seq_read.seq_cfg >> 16) & 0x3) + 1;
> if (data_pads == 4) {
> if (!(sr2 & W25Q_STATUS_QE)) {
>
--
Best regards,
Marek Vasut
Re: [PATCH 1/3] ARM: dts: Bindings for Altera Quadspi Controller Version 2
On 06/26/2017 06:13 PM, matthew.gerl...@linux.intel.com wrote:
> From: Matthew Gerlach <matthew.gerl...@linux.intel.com>
>
> Device Tree bindings for Version 2 of the Altera Quadspi Controller
> that can be optionally paired with a windowed bridge.
>
> Signed-off-by: Matthew Gerlach <matthew.gerl...@linux.intel.com>
> ---
> .../devicetree/bindings/mtd/altera-quadspi-v2.txt | 37
> ++
> 1 file changed, 37 insertions(+)
> create mode 100644
> Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt
>
> diff --git a/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt
> b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt
> new file mode 100644
> index 000..8ba63d7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/mtd/altera-quadspi-v2.txt
> @@ -0,0 +1,37 @@
> +* Altera Quad SPI Controller Version 2
> +
> +Required properties:
> +- compatible : Should be "altr,quadspi-v2".
> +- reg : Contains at least two entries, and possibly three entries, each of
> + which is a tuple consisting of a physical address and length.
> +- reg-names : Should contain the names "avl_csr" and "avl_mem" corresponding
> + to the control and status registers and qspi memory, respectively.
> +
> +
> +The Altera Quad SPI Controller Version 2 can be paired with a windowed bridge
> +in order to reduce the footprint of the memory interface. When a windowed
> +bridge is used, reads and writes of data must be 32 bits wide.
> +
> +Optional properties:
> +- reg-names : Should contain the name "avl_window", if the windowed bridge
> + is used. This name corresponds to the register space that
> + controls the window.
> +- window-size : The size of the window which must be an even power of 2.
> +- read-bit-reverse : A boolean indicating the data read from the flash should
> + be bit reversed on a byte by byte basis before being
> + delivered to the MTD layer.
> +- write-bit-reverse : A boolean indicating the data written to the flash
> should
> + be bit reversed on a byte by byte basis.
Is there ever a usecase where you need to set just one of these props ?
Also, they're altera specific, so altr, prefix should be added.
> +Example:
> +
> +qspi: spi@a0001000 {
> + compatible = "altr,quadspi-v2";
> + reg = <0xa0001000 0x40>, <0xb000 0x400>;
> + reg-names = "avl_csr", "avl_mem";
> +
> + flash@0 {
> + reg = <0>;
> + label = "FPGA Image";
> + };
> +};
>
--
Best regards,
Marek Vasut
Re: [PATCH V3 2/2] mfd: Add ROHM BD9571MWV-M MFD PMIC driver
On 05/02/2017 02:18 PM, Marek Vasut wrote:
> Add the MFD part of the ROHM BD9571MWV-M PMIC driver and MAINTAINERS
> entry. The MFD part only specifies the regmap bits for the PMIC and
> binds the subdevs together.
>
> Signed-off-by: Marek Vasut <marek.vasut+rene...@gmail.com>
> Cc: linux-kernel@vger.kernel.org
> Cc: Geert Uytterhoeven <geert+rene...@glider.be>
> Cc: Lee Jones <lee.jo...@linaro.org>
Lee, bump, do you plan to apply these patches anytime soon ?
> ---
> V2: - Change BD9571MWV_AVS_VD09_VID0,1,2,3 to BD9571MWV_AVS_VD09_VID(n)
> - Change BD9571MWV_AVS_DVFS_VID0,1,2,3 to BD9571MWV_AVS_DVFS_VID(n)
> - Make the AVS_VD09 range RW, so it can be used by the regulator
> driver for the VD09 regulator
> - Report the regmap read return values when attempting to read ID
> registers fails
> V3: No change
> ---
> MAINTAINERS | 11 ++
> drivers/mfd/Kconfig | 13 +++
> drivers/mfd/Makefile | 1 +
> drivers/mfd/bd9571mwv.c | 230
> ++
> include/linux/mfd/bd9571mwv.h | 115 +
> 5 files changed, 370 insertions(+)
> create mode 100644 drivers/mfd/bd9571mwv.c
> create mode 100644 include/linux/mfd/bd9571mwv.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 2a9aa4be5846..9c30e6b00358 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -10942,6 +10942,17 @@ L: linux-ser...@vger.kernel.org
> S: Odd Fixes
> F: drivers/tty/serial/rp2.*
>
> +ROHM MULTIFUNCTION BD9571MWV-M PMIC DEVICE DRIVERS
> +M: Marek Vasut <marek.vasut+rene...@gmail.com>
> +L: linux-kernel@vger.kernel.org
> +L: linux-renesas-...@vger.kernel.org
> +S: Supported
> +F: drivers/mfd/bd9571mwv.c
> +F: drivers/regulator/bd9571mwv-regulator.c
> +F: drivers/gpio/gpio-bd9571mwv.c
> +F: include/linux/mfd/bd9571mwv.h
> +F: Documentation/devicetree/bindings/mfd/bd9571mwv.txt
> +
> ROSE NETWORK LAYER
> M: Ralf Baechle <r...@linux-mips.org>
> L: linux-h...@vger.kernel.org
> diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
> index 3eb5c93595f6..6f4668ae28cc 100644
> --- a/drivers/mfd/Kconfig
> +++ b/drivers/mfd/Kconfig
> @@ -133,6 +133,19 @@ config MFD_BCM590XX
> help
> Support for the BCM590xx PMUs from Broadcom
>
> +config MFD_BD9571MWV
> + tristate "ROHM BD9571MWV PMIC"
> + select MFD_CORE
> + select REGMAP_I2C
> + depends on I2C
> + help
> + Support for the ROHM BD9571MWV PMIC, which contains single
> + voltage regulator, voltage sampling units, GPIO block and
> + watchdog block.
> +
> + This driver can also be built as a module. If so, the module
> + will be called bd9571mwv.
> +
> config MFD_AC100
> tristate "X-Powers AC100"
> select MFD_CORE
> diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
> index c16bf1ea0ea9..3cacc9748e13 100644
> --- a/drivers/mfd/Makefile
> +++ b/drivers/mfd/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_MFD_ACT8945A) += act8945a.o
> obj-$(CONFIG_MFD_SM501) += sm501.o
> obj-$(CONFIG_MFD_ASIC3) += asic3.o tmio_core.o
> obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
> +obj-$(CONFIG_MFD_BD9571MWV) += bd9571mwv.o
> cros_ec_core-objs:= cros_ec.o
> cros_ec_core-$(CONFIG_ACPI) += cros_ec_acpi_gpe.o
> obj-$(CONFIG_MFD_CROS_EC)+= cros_ec_core.o
> diff --git a/drivers/mfd/bd9571mwv.c b/drivers/mfd/bd9571mwv.c
> new file mode 100644
> index ..64e088dfe7b0
> --- /dev/null
> +++ b/drivers/mfd/bd9571mwv.c
> @@ -0,0 +1,230 @@
> +/*
> + * ROHM BD9571MWV-M MFD driver
> + *
> + * Copyright (C) 2017 Marek Vasut <marek.vasut+rene...@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether expressed or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License version 2 for more details.
> + *
> + * Based on the TPS65086 driver
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +
> +#include
> +
> +static const struct mfd_cell bd9571mwv_cells[] = {
> + { .name = "bd9571mwv-regulator", },
> + { .name = "bd9571mwv-gpio", },
> +};
> +
> +static const struct regmap_range bd9571mwv_readable_yes_ranges[] = {
> + regmap_reg_r
Re: [PATCH 3/3] mtd: spi-nor: Altera Quadspi Flash Controller v2 Platform driver
L;
> + window_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(window_base)) {
> + dev_err(dev, "%s: ERROR: failed to map window base\n",
> + __func__);
> + return PTR_ERR(data_base);
> + }
> +
> + of_property_read_u32(dev->of_node, "window-size", _size);
> +
> + if (!window_size) {
> + dev_err(dev,
> + "alv_window defined, %s",
> + "but no window-size defined\n");
> + return -EINVAL;
> + }
> + }
> +
> + if (of_property_read_bool(np, "read-bit-reverse"))
> + flags |= ALTERA_QUADSPI_FL_BITREV_READ;
> +
> + if (of_property_read_bool(np, "write-bit-reverse"))
> + flags |= ALTERA_QUADSPI_FL_BITREV_WRITE;
> +
> + ret = altera_quadspi_create(dev, csr_base, data_base,
> + window_base, (size_t)window_size, flags);
> +
> + if (ret) {
> + dev_err(dev, "failed to create qspi device\n");
> + return ret;
> + }
> +
> + for_each_available_child_of_node(np, pp) {
> + of_property_read_u32(pp, "reg", );
> + if (bank >= ALTERA_QUADSPI_MAX_NUM_FLASH_CHIP) {
> + dev_err(dev, "bad reg value %u >= %u\n", bank,
> + ALTERA_QUADSPI_MAX_NUM_FLASH_CHIP);
> + goto error;
> + }
> +
> + if (altera_qspi_add_bank(dev, bank, pp)) {
> + dev_err(dev, "failed to add bank %u\n", bank);
> + goto error;
> + }
> + }
> +
> + return 0;
> +error:
> + altera_quadspi_remove_banks(dev);
> + return -EIO;
> +}
> +
> +static int altera_quadspi_remove(struct platform_device *pdev)
> +{
> + return altera_quadspi_remove_banks(>dev);
> +}
> +
> +static const struct of_device_id altera_quadspi_id_table[] = {
> +
> + { .compatible = "altr,quadspi-v2",},
> + {}
> +};
> +MODULE_DEVICE_TABLE(of, altera_quadspi_id_table);
> +
> +static struct platform_driver altera_quadspi_driver = {
> + .driver = {
> + .name = "altera_quadspi_platform",
> + .of_match_table = altera_quadspi_id_table,
> + },
> + .probe = altera_quadspi_probe,
> + .remove = altera_quadspi_remove,
> +};
> +module_platform_driver(altera_quadspi_driver);
> +
> +MODULE_AUTHOR("Viet Nga Dao <vn...@altera.com>");
> +MODULE_AUTHOR("Yong Sern Lau <lau.yong.s...@intel.com>");
> +MODULE_AUTHOR("Matthew Gerlach <matthew.gerl...@linux.intel.com>");
> +MODULE_DESCRIPTION("Altera QuadSPI Version 2 Platform Driver");
> +MODULE_LICENSE("GPL v2");
>
--
Best regards,
Marek Vasut
Re: [PATCH v3 2/2] mtd: spi-nor: parse Serial Flash Discoverable Parameters (SFDP) tables
On 06/26/2017 03:10 PM, Cyrille Pitchen wrote: > This patch adds support to the JESD216 rev B standard and parses the SFDP > tables to dynamically initialize the 'struct spi_nor_flash_parameter'. > > Signed-off-by: Cyrille Pitchen <cyrille.pitc...@microchip.com> Changelog is missing. And for such a huge patch, the description is really tiny. > --- > drivers/mtd/spi-nor/spi-nor.c | 775 > +- > include/linux/mtd/spi-nor.h | 6 + > 2 files changed, 768 insertions(+), 13 deletions(-) Otherwise, Reviewed-by: Marek Vasut <marek.va...@gmail.com> -- Best regards, Marek Vasut
Re: [PATCH] Fix C++ kernel in include/linux/mtd/mtd.h
On 09/23/2017 10:13 PM, Pavel Machek wrote:
> C++ comments look wrong in kernel tree. Fix one.
AFAIR they are now (sadly) accepted, but this patch works for me.
Would be nice if trivial could pick it.
> Signed-off-by: Pavel Machek <pa...@ucw.cz>
>
> diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
> index 6cd0f6b..849543f1 100644
> --- a/include/linux/mtd/mtd.h
> +++ b/include/linux/mtd/mtd.h
> @@ -267,7 +267,7 @@ struct mtd_info {
>*/
> unsigned int bitflip_threshold;
>
> - // Kernel-only stuff starts here.
> + /* Kernel-only stuff starts here. */
> const char *name;
> int index;
>
>
--
Best regards,
Marek Vasut
Re: [PATCH] mtd: spi-nor: Allow Cadence QSPI support for ARM64
On 10/06/2017 04:56 PM, Thor Thayer wrote: > gentle ping... > > > On 09/29/2017 11:07 AM, thor.tha...@linux.intel.com wrote: >> From: Thor Thayer <thor.tha...@linux.intel.com> >> >> Allow ARM64 support for the Cadence QSPI interface by >> adding ARM64 as a dependency. >> >> Signed-off-by: Thor Thayer <thor.tha...@linux.intel.com> Reviewed-by: Marek Vasut <marek.va...@gmail.com> I presume it was tested on Stratix 10 ? >> --- >> drivers/mtd/spi-nor/Kconfig | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/drivers/mtd/spi-nor/Kconfig b/drivers/mtd/spi-nor/Kconfig >> index 69c638dd0484..f26aaa6d1430 100644 >> --- a/drivers/mtd/spi-nor/Kconfig >> +++ b/drivers/mtd/spi-nor/Kconfig >> @@ -50,7 +50,7 @@ config SPI_ATMEL_QUADSPI >> config SPI_CADENCE_QUADSPI >> tristate "Cadence Quad SPI controller" >> - depends on OF && (ARM || COMPILE_TEST) >> + depends on OF && (ARM || ARM64 || COMPILE_TEST) >> help >> Enable support for the Cadence Quad SPI Flash controller. >> > -- Best regards, Marek Vasut
Re: [PATCH v2 2/3] mtd: spi-nor: Altera ASMI Parallel II IP Core
On 09/20/2017 08:28 PM, matthew.gerl...@linux.intel.com wrote:
> From: Matthew Gerlach <matthew.gerl...@linux.intel.com>
>
> This patch adds support for a spi-nor, platform driver for the
> Altera ASMI Parallel II IP Core. The intended use case is to be able
> to update the flash used to load a FPGA at power up with mtd-utils.
>
> Signed-off-by: Matthew Gerlach <matthew.gerl...@linux.intel.com>
> ---
> v2:
> minor checkpatch fixing by Wu Hao <hao...@intel.com>
> Use read_dummy value as suggested by Cyrille Pitchen.
> Don't assume 4 byte addressing (Cryille Pichecn and Marek Vasut).
> Fixed #define indenting as suggested by Marek Vasut.
> Added units to timer values as suggested by Marek Vasut.
> Use io(read|write)8_rep() as suggested by Marek Vasut.
> Renamed function prefixed with __ as suggested by Marek Vasut.
[...]
> +#define QSPI_ACTION_REG 0
> +#define QSPI_ACTION_RST BIT(0)
> +#define QSPI_ACTION_EN BIT(1)
> +#define QSPI_ACTION_SC BIT(2)
> +#define QSPI_ACTION_CHIP_SEL_SFT 4
> +#define QSPI_ACTION_DUMMY_SFT8
> +#define QSPI_ACTION_READ_BACK_SFT16
> +
> +#define QSPI_FIFO_CNT_REG4
> +#define QSPI_FIFO_DEPTH 0x200
> +#define QSPI_FIFO_CNT_MSK0x3ff
> +#define QSPI_FIFO_CNT_RX_SFT 0
> +#define QSPI_FIFO_CNT_TX_SFT 12
> +
> +#define QSPI_DATA_REG0x8
> +
> +#define QSPI_POLL_TIMEOUT_US 1000
10 s poll timeout ? :)
> +#define QSPI_POLL_INTERVAL_US5
> +
> +struct altera_asmip2 {
[...]
Otherwise looks good
--
Best regards,
Marek Vasut
Re: [PATCH v4 0/6] K2G: Add QSPI support
On 10/16/2017 08:15 AM, Vignesh R wrote: > > > On Tuesday 03 October 2017 10:49 AM, Vignesh R wrote: >> >> >> This series adds support for Cadence QSPI IP present in TI's 66AK2G SoC. >> The patches enhance the existing cadence-quadspi driver to support >> loopback clock circuit, pm_runtime support and tweaks for 66AK2G SoC. > > Gentle ping on the series... It's IMO OK, Cyrille, can you pick it ? >> >> Change log: >> >> v4: >> * New patch to fix error handling sequence in probe. >> >> v3: >> * Fix build warnings reported by kbuild test bot. >> >> Resend: >> * Rebase to latest linux-next. >> * Collect Acked-bys >> >> v2: >> * Drop DT patches. Will be sent as separate series as requested by >> maintainer. >> * Split binding docs into separate patches. >> * Address comments by Rob Herring. >> >> Vignesh R (6): >> mtd: spi-nor: cadence-quadspi: Add TI 66AK2G SoC specific compatible >> mtd: spi-nor: cadence-quadspi: add a delay in write sequence >> mtd: spi-nor: cadence-quadspi: Add new binding to enable loop-back >> circuit >> mtd: spi-nor: cadence-quadspi: Add support to enable loop-back clock >> circuit >> mtd: spi-nor: cadence-quadspi: Fix error path in probe >> mtd: spi-nor: cadence-quadspi: Add runtime PM support >> >> .../devicetree/bindings/mtd/cadence-quadspi.txt| 7 ++- >> drivers/mtd/spi-nor/cadence-quadspi.c | 55 >> +++--- >> 2 files changed, 55 insertions(+), 7 deletions(-) >> > -- Best regards, Marek Vasut
Re: [PATCH] usb: dwc2: disable erroneous overcurrent condition
On 10/16/2017 03:57 PM, Dinh Nguyen wrote:
> For the case where an external VBUS is used, we should enable the external
> VBUS comparator in the driver. This would prevent an unnecessary
> overcurrent error which would then disable the host port.
>
> This patch uses the standard 'disable-over-current' binding to allow of the
> option of disabling the over-current condition.
>
> Signed-off-by: Dinh Nguyen <dingu...@kernel.org>
Reviewed-by: Marek Vasut <ma...@denx.de>
Similar patch was in U-Boot for two years now and it does the trick indeed.
> ---
> drivers/usb/dwc2/core.h | 4
> drivers/usb/dwc2/hcd.c| 5 +
> drivers/usb/dwc2/params.c | 3 +++
> 3 files changed, 12 insertions(+)
>
> diff --git a/drivers/usb/dwc2/core.h b/drivers/usb/dwc2/core.h
> index 8367d4f9..730d7eb 100644
> --- a/drivers/usb/dwc2/core.h
> +++ b/drivers/usb/dwc2/core.h
> @@ -395,6 +395,9 @@ enum dwc2_ep0_state {
> * (default when phy_type is UTMI+ or ULPI)
> * 1 - 6 MHz
> * (default when phy_type is Full Speed)
> + * @oc_disable: Flag to disable overcurrent condition.
> + * 0 - Allow overcurrent condition to get detected
> + * 1 - Disable overcurrent condtion to get detected
> * @ts_dline: Enable Term Select Dline pulsing
> * 0 - No (default)
> * 1 - Yes
> @@ -492,6 +495,7 @@ struct dwc2_core_params {
> bool dma_desc_fs_enable;
> bool host_support_fs_ls_low_power;
> bool host_ls_low_power_phy_clk;
> + bool oc_disable;
>
> u8 host_channels;
> u16 host_rx_fifo_size;
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index c263114..5e20336 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -213,6 +213,11 @@ static int dwc2_hs_phy_init(struct dwc2_hsotg *hsotg,
> bool select_phy)
> usbcfg &= ~(GUSBCFG_PHYIF16 | GUSBCFG_DDRSEL);
> if (hsotg->params.phy_ulpi_ddr)
> usbcfg |= GUSBCFG_DDRSEL;
> +
> + /* Set external VBUS indicator as needed. */
> + if (hsotg->params.oc_disable)
> + usbcfg |= (GUSBCFG_ULPI_INT_VBUS_IND |
> +GUSBCFG_INDICATORPASSTHROUGH);
> break;
> case DWC2_PHY_TYPE_PARAM_UTMI:
> /* UTMI+ interface */
> diff --git a/drivers/usb/dwc2/params.c b/drivers/usb/dwc2/params.c
> index a3ffe97..39e02cd 100644
> --- a/drivers/usb/dwc2/params.c
> +++ b/drivers/usb/dwc2/params.c
> @@ -335,6 +335,9 @@ static void dwc2_get_device_properties(struct dwc2_hsotg
> *hsotg)
> num);
> }
> }
> +
> + if (of_find_property(hsotg->dev->of_node, "disable-over-current", NULL))
> + p->oc_disable = true;
> }
>
> static void dwc2_check_param_otg_cap(struct dwc2_hsotg *hsotg)
>
--
Best regards,
Marek Vasut
Re: [PATCH v4 0/6] K2G: Add QSPI support
On 10/17/2017 12:29 AM, Cyrille Pitchen wrote: > Hi all, > > Le 16/10/2017 à 11:28, Marek Vasut a écrit : >> On 10/16/2017 08:15 AM, Vignesh R wrote: >>> >>> >>> On Tuesday 03 October 2017 10:49 AM, Vignesh R wrote: >>>> >>>> >>>> This series adds support for Cadence QSPI IP present in TI's 66AK2G SoC. >>>> The patches enhance the existing cadence-quadspi driver to support >>>> loopback clock circuit, pm_runtime support and tweaks for 66AK2G SoC. >>> >>> Gentle ping on the series... >> >> It's IMO OK, Cyrille, can you pick it ? >> > > The series looks good to me too. > > Marek, could you please add you "Acked-by" or "Reviewed-by" tag for the > record in patchwork ? It will be automatically added when I use > "pwclient git-am -s" to merge the patches into the spi-nor/next branch. Acked-by: Marek Vasut <marek.va...@gmail.com> -- Best regards, Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 09/26/2017 12:41 AM, matthew.gerl...@linux.intel.com wrote: > > > On Sun, 24 Sep 2017, Marek Vasut wrote: > >> On 09/24/2017 03:27 PM, Vignesh R wrote: >>> >>> >>> On 9/24/2017 6:42 PM, Marek Vasut wrote: >>>> On 09/24/2017 03:08 PM, Vignesh R wrote: >>>>> >>>>> >>>>> On 9/24/2017 5:31 PM, Marek Vasut wrote: >>>>>> On 09/24/2017 12:59 PM, Vignesh R wrote: >>>>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to >>>>>>> switch on QSPI power domain on TI 66AK2G SoC during probe. >>>>>>> >>>>>>> Signed-off-by: Vignesh R <vigne...@ti.com> >>>>>> >>>>>> Are you planning to add some more fine-grained PM control later? >>>>> >>>>> Yes, I will need to add fine-grained PM control at some point. But, >>>>> for >>>>> now SoC does not really support low power mode or runtime power saving >>>>> option. >>>>> The fact that driver still uses clk_prepare_*() calls to >>>>> enable/disable >>>>> clocks instead of pm_*() calls makes it a bit tricky though. >>>>> >>>>> Just figured out I forgot to add cleanup code in error handling >>>>> path of >>>>> probe(). Will fix that and send a v4. >>>> >>>> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM >>>> either, so it's fine for now. >>>> >>> >>> Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for >>> QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see >>> if its possible to get rid of clk_*() calls in favor of pm_*() calls. >> >> Not of the top of my head, sorry. +CC Matthew, he should know. > > I am not an expert at the clock framework nor the power management, but I > did ask around a bit. No one I asked was planning to change the clk_*() > calls to pm_*() call, but the feedback was that it would be a good idea. The question is, if we do the replacement, will it break on socfpga ? A quick test might be useful. -- Best regards, Marek Vasut
Re: [PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence
On 09/24/2017 12:59 PM, Vignesh R wrote:
> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
> Controller programming sequence, a delay equal to couple of QSPI master
> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
> to handle this and set this flag for TI 66AK2G SoC.
>
> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
>
> Signed-off-by: Vignesh R <vigne...@ti.com>
Is this TI specific or is this controller property ? I wouldn't be
surprised of the later ...
> ---
>
> v3:
> Fix build warnings reported by kbuild test bot.
>
> drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++-
> 1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c
> b/drivers/mtd/spi-nor/cadence-quadspi.c
> index 53c7d8e0327a..5cd5d6f7303f 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -38,6 +38,9 @@
> #define CQSPI_NAME "cadence-qspi"
> #define CQSPI_MAX_CHIPSELECT 16
>
> +/* Quirks */
> +#define CQSPI_NEEDS_WR_DELAY BIT(0)
> +
> struct cqspi_st;
>
> struct cqspi_flash_pdata {
> @@ -76,6 +79,7 @@ struct cqspi_st {
> u32 fifo_depth;
> u32 fifo_width;
> u32 trigger_address;
> + u32 wr_delay;
> struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
> };
>
> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor
> *nor,
> reinit_completion(>transfer_complete);
> writel(CQSPI_REG_INDIRECTWR_START_MASK,
> reg_base + CQSPI_REG_INDIRECTWR);
> + /*
> + * As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
> + * Controller programming sequence, couple of cycles of
> + * QSPI_REF_CLK delay is required for the above bit to
> + * be internally synchronized by the QSPI module. Provide 5
> + * cycles of delay.
> + */
> + if (cqspi->wr_delay)
> + ndelay(cqspi->wr_delay);
>
> while (remaining > 0) {
> write_bytes = remaining > page_size ? page_size : remaining;
> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
> struct cqspi_st *cqspi;
> struct resource *res;
> struct resource *res_ahb;
> + unsigned long data;
> int ret;
> int irq;
>
> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
> }
>
> cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
> + data = (unsigned long)of_device_get_match_data(dev);
> + if (data & CQSPI_NEEDS_WR_DELAY)
> + cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
> +cqspi->master_ref_clk_hz);
>
> ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
> pdev->name, cqspi);
> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
> #endif
>
> static const struct of_device_id cqspi_dt_ids[] = {
> - {.compatible = "cdns,qspi-nor",},
> + {
> + .compatible = "cdns,qspi-nor",
> + .data = (void *)0,
> + },
> + {
> + .compatible = "ti,k2g-qspi",
> + .data = (void *)CQSPI_NEEDS_WR_DELAY,
> + },
> { /* end of table */ }
> };
>
>
--
Best regards,
Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 09/24/2017 12:59 PM, Vignesh R wrote:
> Add pm_runtime* calls to cadence-quadspi driver. This is required to
> switch on QSPI power domain on TI 66AK2G SoC during probe.
>
> Signed-off-by: Vignesh R <vigne...@ti.com>
Are you planning to add some more fine-grained PM control later?
> ---
> drivers/mtd/spi-nor/cadence-quadspi.c | 11 +++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c
> b/drivers/mtd/spi-nor/cadence-quadspi.c
> index d9629e8f4798..2c8e6226d267 100644
> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
> @@ -31,6 +31,7 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -1224,6 +1225,13 @@ static int cqspi_probe(struct platform_device *pdev)
> return -ENXIO;
> }
>
> + pm_runtime_enable(>dev);
> + ret = pm_runtime_get_sync(>dev);
> + if (ret < 0) {
> + pm_runtime_put_noidle(>dev);
> + return ret;
> + }
> +
> ret = clk_prepare_enable(cqspi->clk);
> if (ret) {
> dev_err(dev, "Cannot enable QSPI clock.\n");
> @@ -1275,6 +1283,9 @@ static int cqspi_remove(struct platform_device *pdev)
>
> clk_disable_unprepare(cqspi->clk);
>
> + pm_runtime_put_sync(>dev);
> + pm_runtime_disable(>dev);
> +
> return 0;
> }
>
>
--
Best regards,
Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 09/24/2017 03:08 PM, Vignesh R wrote: > > > On 9/24/2017 5:31 PM, Marek Vasut wrote: >> On 09/24/2017 12:59 PM, Vignesh R wrote: >>> Add pm_runtime* calls to cadence-quadspi driver. This is required to >>> switch on QSPI power domain on TI 66AK2G SoC during probe. >>> >>> Signed-off-by: Vignesh R <vigne...@ti.com> >> >> Are you planning to add some more fine-grained PM control later? > > Yes, I will need to add fine-grained PM control at some point. But, for > now SoC does not really support low power mode or runtime power saving > option. > The fact that driver still uses clk_prepare_*() calls to enable/disable > clocks instead of pm_*() calls makes it a bit tricky though. > > Just figured out I forgot to add cleanup code in error handling path of > probe(). Will fix that and send a v4. OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM either, so it's fine for now. -- Best regards, Marek Vasut
Re: [PATCH v3 2/5] mtd: spi-nor: cadence-quadspi: add a delay in write sequence
On 09/24/2017 02:33 PM, Vignesh R wrote:
>
>
> On 9/24/2017 5:29 PM, Marek Vasut wrote:
>> On 09/24/2017 12:59 PM, Vignesh R wrote:
>>> As per 66AK2G02 TRM[1] SPRUHY8F section 11.15.5.3 Indirect Access
>>> Controller programming sequence, a delay equal to couple of QSPI master
>>> clock(~5ns) is required after setting CQSPI_REG_INDIRECTWR_START bit and
>>> writing data to the flash. Introduce a quirk flag CQSPI_NEEDS_WR_DELAY
>>> to handle this and set this flag for TI 66AK2G SoC.
>>>
>>> [1]http://www.ti.com/lit/ug/spruhy8f/spruhy8f.pdf
>>>
>>> Signed-off-by: Vignesh R <vigne...@ti.com>
>>
>> Is this TI specific or is this controller property ? I wouldn't be
>> surprised of the later ...
>
> I am not sure, there is no generic public documentation by cadence for
> this IP. TI TRM clearly states this delay is required and I have
> verified it practically that this delay is indeed needed.
> But current user of this IP, socfpga does not seem to mention anything
> about it. So, I guess its TI specific quirk.
OK, let's go with that then. I didn't observe any stability issues with
SoCFPGA, but I didn't run the flash at 100s of MHz either. At what kind
of frequencies does the quirk become relevant ?
>>
>>> ---
>>>
>>> v3:
>>> Fix build warnings reported by kbuild test bot.
>>>
>>> drivers/mtd/spi-nor/cadence-quadspi.c | 27 ++-
>>> 1 file changed, 26 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/mtd/spi-nor/cadence-quadspi.c
>>> b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> index 53c7d8e0327a..5cd5d6f7303f 100644
>>> --- a/drivers/mtd/spi-nor/cadence-quadspi.c
>>> +++ b/drivers/mtd/spi-nor/cadence-quadspi.c
>>> @@ -38,6 +38,9 @@
>>> #define CQSPI_NAME "cadence-qspi"
>>> #define CQSPI_MAX_CHIPSELECT 16
>>>
>>> +/* Quirks */
>>> +#define CQSPI_NEEDS_WR_DELAY BIT(0)
>>> +
>>> struct cqspi_st;
>>>
>>> struct cqspi_flash_pdata {
>>> @@ -76,6 +79,7 @@ struct cqspi_st {
>>> u32 fifo_depth;
>>> u32 fifo_width;
>>> u32 trigger_address;
>>> + u32 wr_delay;
>>> struct cqspi_flash_pdata f_pdata[CQSPI_MAX_CHIPSELECT];
>>> };
>>>
>>> @@ -608,6 +612,15 @@ static int cqspi_indirect_write_execute(struct spi_nor
>>> *nor,
>>> reinit_completion(>transfer_complete);
>>> writel(CQSPI_REG_INDIRECTWR_START_MASK,
>>>reg_base + CQSPI_REG_INDIRECTWR);
>>> + /*
>>> +* As per 66AK2G02 TRM SPRUHY8F section 11.15.5.3 Indirect Access
>>> +* Controller programming sequence, couple of cycles of
>>> +* QSPI_REF_CLK delay is required for the above bit to
>>> +* be internally synchronized by the QSPI module. Provide 5
>>> +* cycles of delay.
>>> +*/
>>> + if (cqspi->wr_delay)
>>> + ndelay(cqspi->wr_delay);
>>>
>>> while (remaining > 0) {
>>> write_bytes = remaining > page_size ? page_size : remaining;
>>> @@ -1156,6 +1169,7 @@ static int cqspi_probe(struct platform_device *pdev)
>>> struct cqspi_st *cqspi;
>>> struct resource *res;
>>> struct resource *res_ahb;
>>> + unsigned long data;
>>> int ret;
>>> int irq;
>>>
>>> @@ -1213,6 +1227,10 @@ static int cqspi_probe(struct platform_device *pdev)
>>> }
>>>
>>> cqspi->master_ref_clk_hz = clk_get_rate(cqspi->clk);
>>> + data = (unsigned long)of_device_get_match_data(dev);
>>> + if (data & CQSPI_NEEDS_WR_DELAY)
>>> + cqspi->wr_delay = 5 * DIV_ROUND_UP(NSEC_PER_SEC,
>>> + cqspi->master_ref_clk_hz);
>>>
>>> ret = devm_request_irq(dev, irq, cqspi_irq_handler, 0,
>>>pdev->name, cqspi);
>>> @@ -1284,7 +1302,14 @@ static const struct dev_pm_ops cqspi__dev_pm_ops = {
>>> #endif
>>>
>>> static const struct of_device_id cqspi_dt_ids[] = {
>>> - {.compatible = "cdns,qspi-nor",},
>>> + {
>>> + .compatible = "cdns,qspi-nor",
>>> + .data = (void *)0,
>>> + },
>>> + {
>>> + .compatible = "ti,k2g-qspi",
>>> + .data = (void *)CQSPI_NEEDS_WR_DELAY,
>>> + },
>>> { /* end of table */ }
>>> };
>>>
>>>
>>
>>
--
Best regards,
Marek Vasut
Re: [PATCH v3 5/5] mtd: spi-nor: cadence-quadspi: Add runtime PM support
On 09/24/2017 03:27 PM, Vignesh R wrote: > > > On 9/24/2017 6:42 PM, Marek Vasut wrote: >> On 09/24/2017 03:08 PM, Vignesh R wrote: >>> >>> >>> On 9/24/2017 5:31 PM, Marek Vasut wrote: >>>> On 09/24/2017 12:59 PM, Vignesh R wrote: >>>>> Add pm_runtime* calls to cadence-quadspi driver. This is required to >>>>> switch on QSPI power domain on TI 66AK2G SoC during probe. >>>>> >>>>> Signed-off-by: Vignesh R <vigne...@ti.com> >>>> >>>> Are you planning to add some more fine-grained PM control later? >>> >>> Yes, I will need to add fine-grained PM control at some point. But, for >>> now SoC does not really support low power mode or runtime power saving >>> option. >>> The fact that driver still uses clk_prepare_*() calls to enable/disable >>> clocks instead of pm_*() calls makes it a bit tricky though. >>> >>> Just figured out I forgot to add cleanup code in error handling path of >>> probe(). Will fix that and send a v4. >> >> OK, fine. Cleanups are welcome. The SoCFPGA doesn't do much runtime PM >> either, so it's fine for now. >> > > Ok thanks! Do you know if pm_runtime_get_sync() can enable clocks for > QSPI on SoCFPGA or if clk_prepare_enable() is needed? Just trying to see > if its possible to get rid of clk_*() calls in favor of pm_*() calls. Not of the top of my head, sorry. +CC Matthew, he should know. -- Best regards, Marek Vasut
Re: [PATCH 1/2] mtd: spi-nor: add an option to force 3byte adressing mode
On 08/28/2017 11:14 PM, Marcin Wojtas wrote:
> Hitherto code set 4B addressing mode for all SPI flashes whose
> size exceeds 16MB. However, changing the default 3B access
> in some cases may be harmful - it may happen that the Boot ROM
> is not capable of handling non-default state of the SPI NOR
> (e.g. after soft reset).
No, this happens when your hardware is broken and does NOT reset the
flash while reseting the CPU as well. If the flash is in some funny
state (ie. middle of page program or erase cycle, which takes long OR
4byte addressing mode) and the CPU resets , then if the SPI NOR is not
reset, your system will still get stuck . You need to fix your hardware
such that you use the reset output of your CPU to detect when the CPU
restarted and add logic to reset your storage as well.
This stuff below is a poor workaround and has it's known problems which
can only be solved by fixing the hardware. This problem keeps coming up
repeatedly, just skim through the Linux MTD and U-Boot lists for other
victims of bad HW design. I don't think I want to see it in the kernel
as that would motivate people to use it rather than fixing their HW .
> Some flash devices allow to access the
> memory above 128Mib without changing mode to 4byte thanks
> to special op codes (see SPI_NOR_4B_OPCODES flag). Unfortunately
> for those which don't support them, the problem persists.
>
> This patch adds optional property that can be added to the
> SPI flash node and which will force to use 3B addressing mode,
> limiting the accessible memory size to 16MiB.
> Binding documentation is updated accordingly.
>
> Signed-off-by: Marcin Wojtas <m...@semihalf.com>
> ---
> Documentation/devicetree/bindings/spi/spi-bus.txt | 2 ++
> drivers/mtd/spi-nor/spi-nor.c | 12 +++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/Documentation/devicetree/bindings/spi/spi-bus.txt
> b/Documentation/devicetree/bindings/spi/spi-bus.txt
> index 1f6e86f..f13b773 100644
> --- a/Documentation/devicetree/bindings/spi/spi-bus.txt
> +++ b/Documentation/devicetree/bindings/spi/spi-bus.txt
> @@ -77,6 +77,8 @@ All slave nodes can contain the following optional
> properties:
> Defaults to 1 if not present.
> - spi-rx-delay-us - Microsecond delay after a read transfer.
> - spi-tx-delay-us - Microsecond delay after a write transfer.
> +- spi-3byte-addressing - Empty property indicating device access to be done
> + only in 3byte addressing mode.
>
> Some SPI controllers and devices support Dual and Quad SPI transfer mode.
> It allows data in the SPI system to be transferred using 2 wires (DUAL) or 4
> diff --git a/drivers/mtd/spi-nor/spi-nor.c b/drivers/mtd/spi-nor/spi-nor.c
> index 1413828..029c87d 100644
> --- a/drivers/mtd/spi-nor/spi-nor.c
> +++ b/drivers/mtd/spi-nor/spi-nor.c
> @@ -2002,7 +2002,17 @@ int spi_nor_scan(struct spi_nor *nor, const char *name,
> if (JEDEC_MFR(info) == SNOR_MFR_SPANSION ||
> info->flags & SPI_NOR_4B_OPCODES)
> spi_nor_set_4byte_opcodes(nor, info);
> - else
> + else if (of_property_read_bool(np, "spi-3byte-addressing")) {
> + /*
> + * Do not enter 4byte mode in order to prevent
> + * the early bootloader to come up on non-default
> + * SPI NOR memory during boot. Limit accessible
> + * size to 16MiB.
> + */
> + nor->addr_width = 3;
> + mtd->size = 0x100;
> + dev_info(dev, "Force 3B addressing mode\n");
> + } else
> set_4byte(nor, info, 1);
> } else {
> nor->addr_width = 3;
>
--
Best regards,
Marek Vasut
Re: [PATCH 01/10] spi-nor: intel-spi: Fix number of protected range registers for BYT/LPT
On 09/01/2017 10:00 AM, Bin Meng wrote:
> The number of protected range registers is not the same on BYT/LPT/
> BXT. GPR0 only exists on Apollo Lake and its offset is reserved on
> other platforms.
>
> Signed-off-by: Bin Meng <bmeng...@gmail.com>
Can this use regmap ? What you're implementing here seems like regmap to me.
> ---
>
> drivers/mtd/spi-nor/intel-spi.c | 16 +++-
> 1 file changed, 11 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c
> index 8a596bf..e5b52e8 100644
> --- a/drivers/mtd/spi-nor/intel-spi.c
> +++ b/drivers/mtd/spi-nor/intel-spi.c
> @@ -67,8 +67,6 @@
> #define PR_LIMIT_MASK(0x3fff << PR_LIMIT_SHIFT)
> #define PR_RPE BIT(15)
> #define PR_BASE_MASK 0x3fff
> -/* Last PR is GPR0 */
> -#define PR_NUM (5 + 1)
>
> /* Offsets are from @ispi->sregs */
> #define SSFSTS_CTL 0x00
> @@ -96,14 +94,17 @@
> #define BYT_BCR 0xfc
> #define BYT_BCR_WPD BIT(0)
> #define BYT_FREG_NUM 5
> +#define BYT_PR_NUM 5
>
> #define LPT_PR 0x74
> #define LPT_SSFSTS_CTL 0x90
> #define LPT_FREG_NUM 5
> +#define LPT_PR_NUM 5
>
> #define BXT_PR 0x84
> #define BXT_SSFSTS_CTL 0xa0
> #define BXT_FREG_NUM 12
> +#define BXT_PR_NUM 6
>
> #define INTEL_SPI_TIMEOUT5000 /* ms */
> #define INTEL_SPI_FIFO_SZ64
> @@ -117,6 +118,7 @@
> * @pregs: Start of protection registers
> * @sregs: Start of software sequencer registers
> * @nregions: Maximum number of regions
> + * @pr_num: Maximum number of protected range registers
> * @writeable: Is the chip writeable
> * @swseq: Use SW sequencer in register reads/writes
> * @erase_64k: 64k erase supported
> @@ -132,6 +134,7 @@ struct intel_spi {
> void __iomem *pregs;
> void __iomem *sregs;
> size_t nregions;
> + size_t pr_num;
> bool writeable;
> bool swseq;
> bool erase_64k;
> @@ -167,7 +170,7 @@ static void intel_spi_dump_regs(struct intel_spi *ispi)
> for (i = 0; i < ispi->nregions; i++)
> dev_dbg(ispi->dev, "FREG(%d)=0x%08x\n", i,
> readl(ispi->base + FREG(i)));
> - for (i = 0; i < PR_NUM; i++)
> + for (i = 0; i < ispi->pr_num; i++)
> dev_dbg(ispi->dev, "PR(%d)=0x%08x\n", i,
> readl(ispi->pregs + PR(i)));
>
> @@ -182,7 +185,7 @@ static void intel_spi_dump_regs(struct intel_spi *ispi)
> dev_dbg(ispi->dev, "BCR=0x%08x\n", readl(ispi->base + BYT_BCR));
>
> dev_dbg(ispi->dev, "Protected regions:\n");
> - for (i = 0; i < PR_NUM; i++) {
> + for (i = 0; i < ispi->pr_num; i++) {
> u32 base, limit;
>
> value = readl(ispi->pregs + PR(i));
> @@ -286,6 +289,7 @@ static int intel_spi_init(struct intel_spi *ispi)
> ispi->sregs = ispi->base + BYT_SSFSTS_CTL;
> ispi->pregs = ispi->base + BYT_PR;
> ispi->nregions = BYT_FREG_NUM;
> + ispi->pr_num = BYT_PR_NUM;
>
> if (writeable) {
> /* Disable write protection */
> @@ -305,12 +309,14 @@ static int intel_spi_init(struct intel_spi *ispi)
> ispi->sregs = ispi->base + LPT_SSFSTS_CTL;
> ispi->pregs = ispi->base + LPT_PR;
> ispi->nregions = LPT_FREG_NUM;
> + ispi->pr_num = LPT_PR_NUM;
> break;
>
> case INTEL_SPI_BXT:
> ispi->sregs = ispi->base + BXT_SSFSTS_CTL;
> ispi->pregs = ispi->base + BXT_PR;
> ispi->nregions = BXT_FREG_NUM;
> + ispi->pr_num = BXT_PR_NUM;
> ispi->erase_64k = true;
> break;
>
> @@ -652,7 +658,7 @@ static bool intel_spi_is_protected(const struct intel_spi
> *ispi,
> {
> int i;
>
> - for (i = 0; i < PR_NUM; i++) {
> + for (i = 0; i < ispi->pr_num; i++) {
> u32 pr_base, pr_limit, pr_value;
>
> pr_value = readl(ispi->pregs + PR(i));
>
--
Best regards,
Marek Vasut
Re: [PATCH v2 2/3] mtd: spi-nor: Altera ASMI Parallel II IP Core
On 10/11/2017 07:00 PM, matthew.gerl...@linux.intel.com wrote: > > > On Tue, 10 Oct 2017, Marek Vasut wrote: > >> On 09/20/2017 08:28 PM, matthew.gerl...@linux.intel.com wrote: >>> From: Matthew Gerlach <matthew.gerl...@linux.intel.com> >>> >>> This patch adds support for a spi-nor, platform driver for the >>> Altera ASMI Parallel II IP Core. The intended use case is to be able >>> to update the flash used to load a FPGA at power up with mtd-utils. >>> >>> Signed-off-by: Matthew Gerlach <matthew.gerl...@linux.intel.com> >>> --- >>> v2: >>> minor checkpatch fixing by Wu Hao <hao...@intel.com> >>> Use read_dummy value as suggested by Cyrille Pitchen. >>> Don't assume 4 byte addressing (Cryille Pichecn and Marek Vasut). >>> Fixed #define indenting as suggested by Marek Vasut. >>> Added units to timer values as suggested by Marek Vasut. >>> Use io(read|write)8_rep() as suggested by Marek Vasut. >>> Renamed function prefixed with __ as suggested by Marek Vasut. >> >> [...] >> >>> +#define QSPI_ACTION_REG 0 >>> +#define QSPI_ACTION_RST BIT(0) >>> +#define QSPI_ACTION_EN BIT(1) >>> +#define QSPI_ACTION_SC BIT(2) >>> +#define QSPI_ACTION_CHIP_SEL_SFT 4 >>> +#define QSPI_ACTION_DUMMY_SFT 8 >>> +#define QSPI_ACTION_READ_BACK_SFT 16 >>> + >>> +#define QSPI_FIFO_CNT_REG 4 >>> +#define QSPI_FIFO_DEPTH 0x200 >>> +#define QSPI_FIFO_CNT_MSK 0x3ff >>> +#define QSPI_FIFO_CNT_RX_SFT 0 >>> +#define QSPI_FIFO_CNT_TX_SFT 12 >>> + >>> +#define QSPI_DATA_REG 0x8 >>> + >>> +#define QSPI_POLL_TIMEOUT_US 1000 >> >> 10 s poll timeout ? :) > > Hi Marek, > > The 10s timeout is fairly arbitrary. In other words, I pulled it out of > thin air. Can you suggest a better timeout? From a practical > standpoint 10s seemed to be much better than no timeout when I was > debugging bad FPGA images. Without a timeout I was hanging the system > when the FPGA image failed. With this timeout, we get a nice message > and Linux keeps running happily. AFAIK the SPI subsystem has a timeout which is adaptive to the bus clock, maybe that's what you want to use here ? -- Best regards, Marek Vasut
Re: [PATCH] mtd: cfi: convert inline functions to macros
On 10/11/2017 03:54 PM, Arnd Bergmann wrote: > The map_word_() functions, dating back to linux-2.6.8, try to perform > bitwise operations on a 'map_word' structure. This may have worked > with compilers that were current then (gcc-3.4 or earlier), but end > up being rather inefficient on any version I could try now (gcc-4.4 or > higher). Specifically we hit a problem analyzed in gcc PR81715 where we > fail to reuse the stack space for local variables. > > This can be seen immediately in the stack consumption for > cfi_staa_erase_varsize() and other functions that (with CONFIG_KASAN) > can be up to 2200 bytes. Changing the inline functions into macros brings > this down to 1280 bytes. Without KASAN, the same problem exists, but > the stack consumption is lower to start with, my patch shrinks it from > 920 to 496 bytes on with arm-linux-gnueabi-gcc-5.4, and saves around > 1KB in .text size for cfi_cmdset_0020.c, as it avoids copying map_word > structures for each call to one of these helpers. > > With the latest gcc-8 snapshot, the problem is fixed in upstream gcc, > but nobody uses that yet, so we should still work around it in mainline > kernels and probably backport the workaround to stable kernels as well. > We had a couple of other functions that suffered from the same gcc bug, > and all of those had a simpler workaround involving dummy variables > in the inline function. Unfortunately that did not work here, the > macro hack was the best I could come up with. > > It would also be helpful to have someone to a little performance testing > on the patch, to see how much it helps in terms of CPU utilitzation. > > Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715 > Cc: sta...@vger.kernel.org > Signed-off-by: Arnd Bergmann <a...@arndb.de> Don't you lose type-checking with this conversion to macros ? -- Best regards, Marek Vasut
Re: [PATCH] mtd: cfi: convert inline functions to macros
On 12/18/2017 10:16 AM, Arnd Bergmann wrote: > On Sun, Dec 17, 2017 at 9:34 PM, Richard Weinberger <rich...@nod.at> wrote: >> Am Mittwoch, 11. Oktober 2017, 15:54:10 CET schrieb Arnd Bergmann: >>> The map_word_() functions, dating back to linux-2.6.8, try to perform >>> bitwise operations on a 'map_word' structure. This may have worked >>> with compilers that were current then (gcc-3.4 or earlier), but end >>> up being rather inefficient on any version I could try now (gcc-4.4 or >>> higher). Specifically we hit a problem analyzed in gcc PR81715 where we >>> fail to reuse the stack space for local variables. >>> >>> This can be seen immediately in the stack consumption for >>> cfi_staa_erase_varsize() and other functions that (with CONFIG_KASAN) >>> can be up to 2200 bytes. Changing the inline functions into macros brings >>> this down to 1280 bytes. Without KASAN, the same problem exists, but >>> the stack consumption is lower to start with, my patch shrinks it from >>> 920 to 496 bytes on with arm-linux-gnueabi-gcc-5.4, and saves around >>> 1KB in .text size for cfi_cmdset_0020.c, as it avoids copying map_word >>> structures for each call to one of these helpers. >>> >>> With the latest gcc-8 snapshot, the problem is fixed in upstream gcc, >>> but nobody uses that yet, so we should still work around it in mainline >>> kernels and probably backport the workaround to stable kernels as well. >>> We had a couple of other functions that suffered from the same gcc bug, >>> and all of those had a simpler workaround involving dummy variables >>> in the inline function. Unfortunately that did not work here, the >>> macro hack was the best I could come up with. >>> >>> It would also be helpful to have someone to a little performance testing >>> on the patch, to see how much it helps in terms of CPU utilitzation. >>> >>> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715 >>> Cc: sta...@vger.kernel.org >>> Signed-off-by: Arnd Bergmann <a...@arndb.de> >> >> Acked-by: Richard Weinberger <rich...@nod.at> > > Thanks! > >> Marek, I know you are not super happy with this patch but IMHO this is the >> solution with the least hassle. >> While functions offer better type checking I think this functions are trivial >> enough to exist as macros too. >> Also forcing users to upgrade/fix their compilers is only possible in a >> perfect world. > > Right. To clarify, this is a potential security issue, as it might be used to > construct a stack overflow to cause privilege escalation when combined > with some other vulnerabilities. I'd definitely want this backported to > stable kernels as a precaution, and I'm preparing a patch to warn > about this kind of problem again in 'allmodconfig' kernels that > currently disable the warning on arm64 and x86. Wouldn't it make more sense to fix the compiler instead ? This still feels like we're fixing a bug at the wrong place ... -- Best regards, Marek Vasut
Re: [PATCH] mtd: cfi: convert inline functions to macros
On 12/18/2017 11:29 AM, Arnd Bergmann wrote: > On Mon, Dec 18, 2017 at 10:18 AM, Marek Vasut <marek.va...@gmail.com> wrote: >> On 12/18/2017 10:16 AM, Arnd Bergmann wrote: >>> On Sun, Dec 17, 2017 at 9:34 PM, Richard Weinberger <rich...@nod.at> wrote: >>>> Am Mittwoch, 11. Oktober 2017, 15:54:10 CET schrieb Arnd Bergmann: >>>>> The map_word_() functions, dating back to linux-2.6.8, try to perform >>>>> bitwise operations on a 'map_word' structure. This may have worked >>>>> with compilers that were current then (gcc-3.4 or earlier), but end >>>>> up being rather inefficient on any version I could try now (gcc-4.4 or >>>>> higher). Specifically we hit a problem analyzed in gcc PR81715 where we >>>>> fail to reuse the stack space for local variables. > ... >>>>> >>>>> With the latest gcc-8 snapshot, the problem is fixed in upstream gcc, >>>>> but nobody uses that yet, so we should still work around it in mainline >>>>> kernels and probably backport the workaround to stable kernels as well. >>>>> We had a couple of other functions that suffered from the same gcc bug, >>>>> and all of those had a simpler workaround involving dummy variables >>>>> in the inline function. Unfortunately that did not work here, the >>>>> macro hack was the best I could come up with. >>>>> >>>>> It would also be helpful to have someone to a little performance testing >>>>> on the patch, to see how much it helps in terms of CPU utilitzation. >>>>> >>>>> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715 >>>>> Cc: sta...@vger.kernel.org >>>>> Signed-off-by: Arnd Bergmann <a...@arndb.de> >>>> >>>> Acked-by: Richard Weinberger <rich...@nod.at> >>> >>> Thanks! >>> >>>> Marek, I know you are not super happy with this patch but IMHO this is the >>>> solution with the least hassle. >>>> While functions offer better type checking I think this functions are >>>> trivial >>>> enough to exist as macros too. >>>> Also forcing users to upgrade/fix their compilers is only possible in a >>>> perfect world. >>> >>> Right. To clarify, this is a potential security issue, as it might be used >>> to >>> construct a stack overflow to cause privilege escalation when combined >>> with some other vulnerabilities. I'd definitely want this backported to >>> stable kernels as a precaution, and I'm preparing a patch to warn >>> about this kind of problem again in 'allmodconfig' kernels that >>> currently disable the warning on arm64 and x86. >> >> Wouldn't it make more sense to fix the compiler instead ? >> This still feels like we're fixing a bug at the wrong place ... > > See above: the compiler is fixed in the gcc-8.x release branch, > which won't be out until next spring. People use all kinds of versions > as old as gcc-4.3, even if the fix was backported to older compilers > (which it is not), most users never rebuild their toolchains to get the > latest bugfix releases. > > For instance, the Android SDK comes with prebuilt binaries of > a gcc-4.9-prerelease version that has many known bugs that > were fixed either by the time the official 4.9 release happened, > or in one of the bugfix releases following it. But doesn't this mean we're taking the OpenSSL path (which didn't work out well for them IIRC) ? I don't have a better solution for this though ... -- Best regards, Marek Vasut
Re: [PATCH] mtd: cfi: convert inline functions to macros
On 12/17/2017 09:43 AM, Boris Brezillon wrote: > Hi Marek, > > On Wed, 11 Oct 2017 23:34:33 +0200 > Marek Vasut <marek.va...@gmail.com> wrote: > >> On 10/11/2017 03:54 PM, Arnd Bergmann wrote: >>> The map_word_() functions, dating back to linux-2.6.8, try to perform >>> bitwise operations on a 'map_word' structure. This may have worked >>> with compilers that were current then (gcc-3.4 or earlier), but end >>> up being rather inefficient on any version I could try now (gcc-4.4 or >>> higher). Specifically we hit a problem analyzed in gcc PR81715 where we >>> fail to reuse the stack space for local variables. >>> >>> This can be seen immediately in the stack consumption for >>> cfi_staa_erase_varsize() and other functions that (with CONFIG_KASAN) >>> can be up to 2200 bytes. Changing the inline functions into macros brings >>> this down to 1280 bytes. Without KASAN, the same problem exists, but >>> the stack consumption is lower to start with, my patch shrinks it from >>> 920 to 496 bytes on with arm-linux-gnueabi-gcc-5.4, and saves around >>> 1KB in .text size for cfi_cmdset_0020.c, as it avoids copying map_word >>> structures for each call to one of these helpers. >>> >>> With the latest gcc-8 snapshot, the problem is fixed in upstream gcc, >>> but nobody uses that yet, so we should still work around it in mainline >>> kernels and probably backport the workaround to stable kernels as well. >>> We had a couple of other functions that suffered from the same gcc bug, >>> and all of those had a simpler workaround involving dummy variables >>> in the inline function. Unfortunately that did not work here, the >>> macro hack was the best I could come up with. >>> >>> It would also be helpful to have someone to a little performance testing >>> on the patch, to see how much it helps in terms of CPU utilitzation. >>> >>> Link: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715 >>> Cc: sta...@vger.kernel.org >>> Signed-off-by: Arnd Bergmann <a...@arndb.de> >> >> Don't you lose type-checking with this conversion to macros ? >> > > Yes, we loose strict type checking, but if you look at the code, you'll > see that the macros do (valN).x[i], so, if valN is not a struct or > a union containing a field named x, the compiler will complain. That > should save us from devs passing random arguments to those macros. You could add some typeof() checking into those macros to make it even more secure ... or, you could keep them as functions. > Anyway, this code is not seeing a lot of changes lately, so I wouldn't > be so worried by the lack of strict type-checking implied by this > transition to macros. Well this transition makes the typechecking worse, not better, and I'm not super happy about that. -- Best regards, Marek Vasut
Re: [PATCH v2] dt-bindings: mtd: Add sst25vf016b to the list of supported chip names
On 11/17/2017 06:36 PM, Cyrille Pitchen wrote: > Hi Fabrizio, > > sorry but I won't apply this patch. ACK on that, if it can be detected with READID or SFDP, use that and don't add redundant stuff into the DT bindings. btw which renesas board has this SPI NOR on it ? > New values for the 'compatible' DT properties should only be added for > memory parts not supporting the JEDEC READ ID (0x9F) command. > > SST25 memories do support this command hence should use the "jedec,spi-nor" > value alone. For historical reasons, some DT nodes set their 'compatible' > string to something like ",", "jedec,spi-nor". > It should work as expected in most case however I discourage from doing so > in new device trees because it may have some side effects especially when > the m25p80.c driver is used between the spi-nor.c driver and the SPI > controller driver. > > It's all about setting the 2nd parameter of spi_nor_scan(), the 'name' > parameter, as NULL when possible. This parameter should be set to a non NULL > value only for memories not supporting the JEDEC READ ID op code (0x9F). > > Best regards, > > Cyrille > > Le 24/10/2017 à 15:50, Fabrizio Castro a écrit : >> There are a few DT files that make use of sst25vf016b in their >> compatible strings, and the driver supports this chip already. >> This patch improves the documentation and therefore the result >> of ./scripts/checkpatch.pl. >> >> Signed-off-by: Fabrizio Castro <fabrizio.cas...@bp.renesas.com> >> Signed-off-by: Chris Paterson <chris.paters...@renesas.com> >> Acked-by: Rob Herring <r...@kernel.org> >> Acked-by: Geert Uytterhoeven <geert+rene...@glider.be> >> --- >> Thank you Rob, thank you Geert, and sorry for the delay on this one. >> Here is v2. >> >> Changes in v2: >> * fixed subject prefix >> * added changelog >> >> Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt >> b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt >> index 4cab5d8..bf56d77 100644 >> --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt >> +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt >> @@ -31,6 +31,7 @@ Required properties: >> s25sl12801 >> s25fl008k >> s25fl064k >> + sst25vf016b >> sst25vf040b >> sst25wf040b >> m25p40 >> > -- Best regards, Marek Vasut
Re: [RFC] iio: light: acpi-als: Enable the light sensor on the Zenbook UX430UQ
On 11/19/2017 06:38 PM, Gabriele Mazzotta wrote:
> 2017-11-19 18:03 GMT+01:00 Jonathan Cameron <ji...@kernel.org>:
>> On Wed, 15 Nov 2017 20:27:54 -0700
>> Kiernan Hager <kah.listaddr...@gmail.com> wrote:
>>
>>> This makes acpi-als properly enable the light sensor on the Zenbook
>>> UX430UQ. I don't know if the checking that I do to make sure that the ACPI
>>> method exists is sufficient or if it should disable the sensor when the
>>> module is unloaded, so input is appreciated on those matters.
>>
>> Please wrap descriptions to around 72 characters (leaves room for indentation
>> in the email thread :)
>>
>> This seems very much like a board specific hack. I've cc'd people
>> who have worked on the driver recently and the acpi list.
>
> This is indeed board specific. I know many other laptops of the
> Zenbook line need something similar and if you look at
> drivers/platform/x86/asus-wmi.c or any other asus-* driver you'll
> find several quirks to enable various features.
Maybe that's where it should go . EC0 in the ACPI path seems like
Embedded Controller to me .
> Kiernan, check commit aca234f6378864d85514be558746c0ea6eabfa8e
> ("asus-wmi: provide access to ALS control") in particular.
>
>>>
>>> Signed-off-by: Kiernan Hager <kah.listaddr...@gmail.com>
>>> ---
>>> drivers/iio/light/acpi-als.c | 16
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/iio/light/acpi-als.c b/drivers/iio/light/acpi-als.c
>>> index c35e2f8df339..8fd66166f19f 100644
>>> --- a/drivers/iio/light/acpi-als.c
>>> +++ b/drivers/iio/light/acpi-als.c
>>> @@ -179,6 +179,10 @@ static int acpi_als_add(struct acpi_device *device)
>>> struct acpi_als *als;
>>> struct iio_dev *indio_dev;
>>> struct iio_buffer *buffer;
>>> + unsigned long long temp_val;
>>> + acpi_status status;
>>> + struct acpi_object_list arg_list;
>>> + union acpi_object arg;
>>>
>>> indio_dev = devm_iio_device_alloc(>dev, sizeof(*als));
>>> if (!indio_dev)
>>> @@ -203,6 +207,18 @@ static int acpi_als_add(struct acpi_device *device)
>>>
>>> iio_device_attach_buffer(indio_dev, buffer);
>>>
>>> + arg_list.count = 1;
>>> + arg_list.pointer =
>>> + arg.type = ACPI_TYPE_INTEGER;
>>> + arg.integer.value = 1;
>>> +
>>> + if (acpi_has_method(als->device->handle, "\\_SB.PCI0.LPCB.EC0.ALSC"))
>>> {
>> So this is poking something on
>> PCI bus 0
>> LPC bus B
>> Somethingbus 0
>> Ambient light controller.
>>
>> I assume there is a better way of establishing this needs to be set?
>>
>>> + status = acpi_evaluate_integer(als->device->handle,
>>> +"\\_SB.PCI0.LPCB.EC0.ALSC",
>>> +_list,
>>> +_val);
>>> + }
>>> +
>>> return devm_iio_device_register(>dev, indio_dev);
>>> }
>>>
>>
--
Best regards,
Marek Vasut
