Re: [PATCH v2 00/25] at24: remove

[Boris Brezillon]

On Mon, 19 Nov 2018 09:58:46 +0100
Bartosz Golaszewski  wrote:

> niedz., 18 lis 2018 o 17:03 Boris Brezillon
>  napisał(a):
> >
> > On Tue, 13 Nov 2018 15:01:08 +0100
> > Bartosz Golaszewski  wrote:
> >  
> > > As far as merging of this series goes: I'd like to avoid dragging it over
> > > four releases. The series is logically split into five groups:
> > >
> > > patches 1-2: nvmem and mtd changes
> > > patches 3-9: davinci arch-specific changes
> > > patches 10-13: networking changes
> > > patches 14-24: davinci specific again
> > > patch 25: final at24 change
> > >
> > > With that I believe we can do the following: Greg KH could pick up the
> > > first two patches into his char-misc tree.  
> >
> > The char-misc tree? Why not the MTD or NVMEM tree?
> >  
> 
> There is no NVMEM tree - Srinivas sends his patches to Greg to be
> applied to char-misc.

Okay, I didn't know that.

> 
> The second patch depends on the first one so in order to avoid more
> merging problems I suggested that the MTD patch go through char-misc
> as well. If you see a better solution, please let me know.

No that's fine, as long as I have an immutable tag/branch I can pull if
needed.

Re: [PATCH v2 06/29] mtd: Add support for reading MTD devices via the nvmem API

[Boris Brezillon]

On Mon, 20 Aug 2018 20:50:55 +0200
Bartosz Golaszewski  wrote:

> 2018-08-20 20:20 GMT+02:00 Boris Brezillon :
> > On Mon, 20 Aug 2018 11:43:34 +0100
> > Srinivas Kandagatla  wrote:
> >  
> >>
> >> Overall am still not able to clear visualize on how MTD bindings with
> >> nvmem cells would look in both partition and un-partition usecases?
> >> An example DT would be nice here!!  
> >
> > Something along those lines:
> >
> > mtdnode {
> > nvmem-cells {
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > cell@0 {
> > reg = <0x0 0x14>;
> > };
> > };
> >
> > partitions {
> > compatible = "fixed-partitions";
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > partition@0 {
> > reg = <0x0 0x2>;
> >
> > nvmem-cells {
> > #address-cells = <1>;
> > #size-cells = <1>;
> >
> > cell@0 {
> > reg = <0x0 0x10>;
> > };
> > };
> > };
> > };
> > };  
> 
> If there'll be an agreement on the bindings: will you be willing to
> merge Alban's patch even without support in the code for the above
> (with the assumption that it will be added later)?

No, because Alban's patch actually allows people to define and
reference nvmem cells in a DT, but without documenting it (see my first
reply).

> My use-case is on
> non-DT systems and creating nvmem devices corresponding to MTD
> partitions if fine by me.

What you propose is option #1 in my list of proposals, and it requires
some changes to avoid automatically assigning nvmem->dev.of_node to
parent->of_node (which will be != NULL when the MTD device has been
instantiated from a DT node).

> I also don't have the means to test the
> support for these bindings if I were to actually write them myself.

And that's the very reason I proposed #1. I don't want to block this
stuff, but in its current state, I'm not willing to accept it either.
Either we agree on the binding and patch the nvmem framework to support
this new binding, or we find a way to hide the fact that the mtd
device (the nvmem parent) has a DT node attached to it.

Re: [PATCH 00/15] ARM: pxa: switch to DMA slave maps

[Boris Brezillon]

On Wed, 04 Apr 2018 21:49:26 +0200
Robert Jarzmik  wrote:

> Ulf Hansson  writes:
> 
> > On 2 April 2018 at 16:26, Robert Jarzmik  wrote:  
> >> Hi,
> >>
> >> This serie is aimed at removing the dmaengine slave compat use, and 
> >> transfer
> >> knowledge of the DMA requestors into architecture code.
> >> As this looks like a patch bomb, each maintainer expressing for his tree 
> >> either
> >> an Ack or "I want to take through my tree" will be spared in the next 
> >> iterations
> >> of this serie.  
> >
> > Perhaps an option is to send this hole series as PR for 3.17 rc1, that
> > would removed some churns and make this faster/easier? Well, if you
> > receive the needed acks of course.  
> For 3.17-rc1 it looks a bit optimistic with the review time ... If I have all

Especially since 3.17-rc1 has been released more than 3 years ago :-),
but I guess you meant 4.17-rc1.

> acks, I'll queue it into my pxa tree. If at least one maintainer withholds his
> ack, the end of the serie (phase 3) won't be applied until it is sorted out.
> 
> Cheers.
> 
> --
> Robert

Re: [PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase()

[Boris Brezillon]

On Mon, 12 Feb 2018 22:03:07 +0100
Boris Brezillon <boris.brezil...@bootlin.com> wrote:

> mtd_erase() can return an error before ->fail_addr is initialized to
> MTD_FAIL_ADDR_UNKNOWN. Move this initialization at the very beginning
> of the function.

Applied the patchset after addressing Miquel's comments.

> 
> Signed-off-by: Boris Brezillon <boris.brezil...@bootlin.com>
> ---
>  drivers/mtd/mtdcore.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
> index a1c94526fb88..c87859ff338b 100644
> --- a/drivers/mtd/mtdcore.c
> +++ b/drivers/mtd/mtdcore.c
> @@ -953,6 +953,8 @@ EXPORT_SYMBOL_GPL(__put_mtd_device);
>   */
>  int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
>  {
> + instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
> +
>   if (!mtd->erasesize || !mtd->_erase)
>   return -ENOTSUPP;
>  
> @@ -961,7 +963,6 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info 
> *instr)
>   if (!(mtd->flags & MTD_WRITEABLE))
>   return -EROFS;
>  
> - instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
>   if (!instr->len) {
>   instr->state = MTD_ERASE_DONE;
>   mtd_erase_callback(instr);



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 00/47] arch-removal: device drivers

[Boris Brezillon]

Hi Arnd,

On Wed, 14 Mar 2018 16:35:13 +0100
Arnd Bergmann <a...@arndb.de> wrote:

> Hi driver maintainers,
> 
> I just posted one series with the removal of eight architectures,
> see https://lkml.org/lkml/2018/3/14/505 for details, or
> https://lwn.net/Articles/748074/ for more background.
> 
> These are the device drivers that go along with them. I have already
> picked up the drivers for arch/metag/ into my tree, they were reviewed
> earlier.
> 
> Please let me know if you have any concerns with the patch, or if you
> prefer to pick up the patches in your respective trees.  I created
> the patches with 'git format-patch -D', so they will not apply without
> manually removing those files.
> 
> For anything else, I'd keep the removal patches in my asm-generic tree
> and will send a pull request for 4.17 along with the actual arch removal.
> 
>Arnd
> 
> Arnd Bergmann
>   edac: remove tile driver
>   net: tile: remove ethernet drivers
>   net: adi: remove blackfin ethernet drivers
>   net: 8390: remove m32r specific bits
>   net: remove cris etrax ethernet driver
>   net: smsc: remove m32r specific smc91x configuration
>   raid: remove tile specific raid6 implementation
>   rtc: remove tile driver
>   rtc: remove bfin driver
>   char: remove obsolete ds1302 rtc driver
>   char: remove tile-srom.c
>   char: remove blackfin OTP driver
>   pcmcia: remove m32r drivers
>   pcmcia: remove blackfin driver
>   ASoC: remove blackfin drivers
>   video/logo: remove obsolete logo files
>   fbdev: remove blackfin drivers
>   fbdev: s1d13xxxfb: remove m32r specific hacks
>   crypto: remove blackfin CRC driver
>   media: platform: remove blackfin capture driver
>   media: platform: remove m32r specific arv driver
>   cpufreq: remove blackfin driver
>   cpufreq: remove cris specific drivers
>   gpio: remove etraxfs driver
>   pinctrl: remove adi2/blackfin drivers
>   ata: remove bf54x driver
>   input: keyboard: remove bf54x driver
>   input: misc: remove blackfin rotary driver
>   mmc: remove bfin_sdh driver
>   can: remove bfin_can driver
>   watchdog: remove bfin_wdt driver
>   mtd: maps: remove bfin-async-flash driver
>   mtd: nand: remove bf5xx_nand driver

If you don't mind, I'd like to take the mtd patches through the MTD
tree. As you've probably noticed, nand code has been moved around and
it's easier for me to carry those 2 simple changes in my tree than
creating an immutable branch.

Let me know if this is a problem.

Regards,

Boris

-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
https://bootlin.com

Re: [PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback()

[Boris Brezillon]

erase->len, NULL, NULL);  
> 
> Are you sure this is still needed? Maybe this should go away in your
> first patch?

Hm, indeed. This comment should be dropped.

> 
> > -
> > -   if (rc) {
> > +   if (rc)
> > erase->fail_addr = erase->addr;
> > -   erase->state = MTD_ERASE_FAILED;
> > -   } else {
> > -   erase->state = MTD_ERASE_DONE;
> > -   }
> > -   mtd_erase_callback(erase);
> > +
> > return rc;
> >  }
> >  
> > diff --git a/drivers/mtd/devices/slram.c b/drivers/mtd/devices/slram.c
> > index 0ec85f316d24..2f05e1801047 100644
> > --- a/drivers/mtd/devices/slram.c
> > +++ b/drivers/mtd/devices/slram.c
> > @@ -88,8 +88,6 @@ static int slram_erase(struct mtd_info *mtd, struct 
> > erase_info *instr)
> >  * I don't feel at all ashamed. This kind of thing is possible anyway
> >  * with flash, but unlikely.
> >  */  
> 
> Same with this comment.

Actually, I'm not sure I understand that comment, but I guess it's
talking about races between read/write and erase paths, so nothing
related to the changed I'm doing here.

> 
> > -   instr->state = MTD_ERASE_DONE;
> > -   mtd_erase_callback(instr);  
> 
> Space ?
> 
> > return(0);
> >  }
> >
> 
> 
> 
> 



-- 
Boris Brezillon, Bootlin (formerly Free Electrons)
Embedded Linux and Kernel engineering
http://bootlin.com

[PATCH 3/5] mtd: Stop assuming mtd_erase() is asynchronous

[Boris Brezillon]

None of the mtd->_erase() implementations work in an asynchronous manner,
so let's simplify MTD users that call mtd_erase(). All they need to do
is check the value returned by mtd_erase() and assume that != 0 means
failure.

Signed-off-by: Boris Brezillon <boris.brezil...@bootlin.com>
---
 drivers/mtd/devices/bcm47xxsflash.c |  3 --
 drivers/mtd/ftl.c   | 51 
 drivers/mtd/inftlmount.c|  5 +-
 drivers/mtd/mtdblock.c  | 20 
 drivers/mtd/mtdchar.c   | 33 +
 drivers/mtd/mtdconcat.c | 48 ++-
 drivers/mtd/mtdcore.c   |  8 ++--
 drivers/mtd/mtdoops.c   | 19 
 drivers/mtd/mtdpart.c   |  2 -
 drivers/mtd/mtdswap.c   | 32 -
 drivers/mtd/nftlmount.c |  4 +-
 drivers/mtd/rfd_ftl.c   | 92 +++--
 drivers/mtd/sm_ftl.c| 18 
 drivers/mtd/sm_ftl.h|  4 --
 drivers/mtd/tests/mtd_test.c|  4 --
 drivers/mtd/tests/speedtest.c   |  6 ---
 drivers/mtd/ubi/io.c| 35 --
 fs/jffs2/erase.c| 36 ++-
 include/linux/mtd/mtd.h |  2 -
 19 files changed, 52 insertions(+), 370 deletions(-)

diff --git a/drivers/mtd/devices/bcm47xxsflash.c 
b/drivers/mtd/devices/bcm47xxsflash.c
index e2bd81817df4..6b84947cfbea 100644
--- a/drivers/mtd/devices/bcm47xxsflash.c
+++ b/drivers/mtd/devices/bcm47xxsflash.c
@@ -95,9 +95,6 @@ static int bcm47xxsflash_erase(struct mtd_info *mtd, struct 
erase_info *erase)
else
erase->state = MTD_ERASE_DONE;
 
-   if (erase->callback)
-   erase->callback(erase);
-
return err;
 }
 
diff --git a/drivers/mtd/ftl.c b/drivers/mtd/ftl.c
index 664d206a4cbe..fcf9907e7987 100644
--- a/drivers/mtd/ftl.c
+++ b/drivers/mtd/ftl.c
@@ -140,12 +140,6 @@ typedef struct partition_t {
 #define XFER_PREPARED  0x03
 #define XFER_FAILED0x04
 
-/**/
-
-
-static void ftl_erase_callback(struct erase_info *done);
-
-
 /*==
 
 Scan_header() checks to see if a memory region contains an FTL
@@ -349,17 +343,19 @@ static int erase_xfer(partition_t *part,
 return -ENOMEM;
 
 erase->mtd = part->mbd.mtd;
-erase->callback = ftl_erase_callback;
 erase->addr = xfer->Offset;
 erase->len = 1 << part->header.EraseUnitSize;
-erase->priv = (u_long)part;
 
 ret = mtd_erase(part->mbd.mtd, erase);
+if (!ret) {
+   xfer->state = XFER_ERASED;
+   xfer->EraseCount++;
+} else {
+   xfer->state = XFER_FAILED;
+   pr_notice("ftl_cs: erase failed: err = %d\n", ret);
+}
 
-if (!ret)
-   xfer->EraseCount++;
-else
-   kfree(erase);
+kfree(erase);
 
 return ret;
 } /* erase_xfer */
@@ -371,37 +367,6 @@ static int erase_xfer(partition_t *part,
 
 ==*/
 
-static void ftl_erase_callback(struct erase_info *erase)
-{
-partition_t *part;
-struct xfer_info_t *xfer;
-int i;
-
-/* Look up the transfer unit */
-part = (partition_t *)(erase->priv);
-
-for (i = 0; i < part->header.NumTransferUnits; i++)
-   if (part->XferInfo[i].Offset == erase->addr) break;
-
-if (i == part->header.NumTransferUnits) {
-   printk(KERN_NOTICE "ftl_cs: internal error: "
-  "erase lookup failed!\n");
-   return;
-}
-
-xfer = >XferInfo[i];
-if (erase->state == MTD_ERASE_DONE)
-   xfer->state = XFER_ERASED;
-else {
-   xfer->state = XFER_FAILED;
-   printk(KERN_NOTICE "ftl_cs: erase failed: state = %d\n",
-  erase->state);
-}
-
-kfree(erase);
-
-} /* ftl_erase_callback */
-
 static int prepare_xfer(partition_t *part, int i)
 {
 erase_unit_header_t header;
diff --git a/drivers/mtd/inftlmount.c b/drivers/mtd/inftlmount.c
index 8d6bb189ea8e..0f47be4834d8 100644
--- a/drivers/mtd/inftlmount.c
+++ b/drivers/mtd/inftlmount.c
@@ -393,9 +393,10 @@ int INFTL_formatblock(struct INFTLrecord *inftl, int block)
   mark only the failed block in the bbt. */
for (physblock = 0; physblock < inftl->EraseSize;
 physblock += instr->len, instr->addr += instr->len) {
-   mtd_erase(inftl->mbd.mtd, instr);
+   int ret;
 
-   if (instr->state == MTD_ERASE_FAILED) {
+   ret = mtd_erase(inftl->mbd.mtd, instr);
+   if (ret) {
printk(KERN_WARNING "INFTL: error while formatting 
block %d\n",
block);

[PATCH 2/5] mtd: Get rid of unused fields in struct erase_info

[Boris Brezillon]

Some fields are not used by MTD drivers, users or core code. Moreover,
those fields are not documented, so get rid of them to avoid any
confusion.

Signed-off-by: Boris Brezillon <boris.brezil...@bootlin.com>
---
 include/linux/mtd/mtd.h | 5 -
 1 file changed, 5 deletions(-)

diff --git a/include/linux/mtd/mtd.h b/include/linux/mtd/mtd.h
index 205ededccc60..2a407dc9beaa 100644
--- a/include/linux/mtd/mtd.h
+++ b/include/linux/mtd/mtd.h
@@ -48,14 +48,9 @@ struct erase_info {
uint64_t addr;
uint64_t len;
uint64_t fail_addr;
-   u_long time;
-   u_long retries;
-   unsigned dev;
-   unsigned cell;
void (*callback) (struct erase_info *self);
u_long priv;
u_char state;
-   struct erase_info *next;
 };
 
 struct mtd_erase_region_info {
-- 
2.14.1

[PATCH 5/5] mtd: Stop updating erase_info->state and calling mtd_erase_callback()

[Boris Brezillon]

MTD users are no longer checking erase_info->state to determine if the
erase operation failed or succeeded. Moreover, mtd_erase_callback() is
now a NOP.

We can safely get rid of all mtd_erase_callback() calls and all
erase_info->state assignments. While at it, get rid of the
erase_info->state field, all MTD_ERASE_XXX definitions and the
mtd_erase_callback() function.

Signed-off-by: Boris Brezillon <boris.brezil...@bootlin.com>
---
 drivers/mtd/chips/cfi_cmdset_0001.c  | 16 ++--
 drivers/mtd/chips/cfi_cmdset_0002.c  | 26 +++---
 drivers/mtd/chips/cfi_cmdset_0020.c  |  3 ---
 drivers/mtd/chips/map_ram.c  |  2 --
 drivers/mtd/devices/bcm47xxsflash.c  |  9 +
 drivers/mtd/devices/block2mtd.c  |  7 +--
 drivers/mtd/devices/docg3.c  | 16 ++--
 drivers/mtd/devices/lart.c   |  4 
 drivers/mtd/devices/mtd_dataflash.c  |  4 
 drivers/mtd/devices/mtdram.c |  2 --
 drivers/mtd/devices/phram.c  |  2 --
 drivers/mtd/devices/pmc551.c |  2 --
 drivers/mtd/devices/powernv_flash.c  | 11 ++-
 drivers/mtd/devices/slram.c  |  2 --
 drivers/mtd/devices/spear_smi.c  |  3 ---
 drivers/mtd/devices/sst25l.c |  3 ---
 drivers/mtd/devices/st_spi_fsm.c |  4 
 drivers/mtd/lpddr/lpddr2_nvm.c   | 10 ++
 drivers/mtd/lpddr/lpddr_cmds.c   |  2 --
 drivers/mtd/mtdconcat.c  |  1 -
 drivers/mtd/mtdcore.c|  6 ++
 drivers/mtd/mtdpart.c|  5 -
 drivers/mtd/nand/nand_base.c | 15 +++
 drivers/mtd/onenand/onenand_base.c   | 17 -
 drivers/mtd/spi-nor/spi-nor.c|  3 ---
 drivers/mtd/ubi/gluebi.c |  3 ---
 drivers/net/ethernet/sfc/falcon/mtd.c| 11 +--
 drivers/net/ethernet/sfc/mtd.c   | 11 +--
 drivers/staging/goldfish/goldfish_nand.c |  3 ---
 include/linux/mtd/mtd.h  |  9 -
 30 files changed, 20 insertions(+), 192 deletions(-)

diff --git a/drivers/mtd/chips/cfi_cmdset_0001.c 
b/drivers/mtd/chips/cfi_cmdset_0001.c
index 5e1b68cbcd0a..d4c07b85f18e 100644
--- a/drivers/mtd/chips/cfi_cmdset_0001.c
+++ b/drivers/mtd/chips/cfi_cmdset_0001.c
@@ -1993,20 +1993,8 @@ static int __xipram do_erase_oneblock(struct map_info 
*map, struct flchip *chip,
 
 static int cfi_intelext_erase_varsize(struct mtd_info *mtd, struct erase_info 
*instr)
 {
-   unsigned long ofs, len;
-   int ret;
-
-   ofs = instr->addr;
-   len = instr->len;
-
-   ret = cfi_varsize_frob(mtd, do_erase_oneblock, ofs, len, NULL);
-   if (ret)
-   return ret;
-
-   instr->state = MTD_ERASE_DONE;
-   mtd_erase_callback(instr);
-
-   return 0;
+   return cfi_varsize_frob(mtd, do_erase_oneblock, instr->addr,
+   instr->len, NULL);
 }
 
 static void cfi_intelext_sync (struct mtd_info *mtd)
diff --git a/drivers/mtd/chips/cfi_cmdset_0002.c 
b/drivers/mtd/chips/cfi_cmdset_0002.c
index 56aa6b75213d..668e2cbc155b 100644
--- a/drivers/mtd/chips/cfi_cmdset_0002.c
+++ b/drivers/mtd/chips/cfi_cmdset_0002.c
@@ -2415,20 +2415,8 @@ static int __xipram do_erase_oneblock(struct map_info 
*map, struct flchip *chip,
 
 static int cfi_amdstd_erase_varsize(struct mtd_info *mtd, struct erase_info 
*instr)
 {
-   unsigned long ofs, len;
-   int ret;
-
-   ofs = instr->addr;
-   len = instr->len;
-
-   ret = cfi_varsize_frob(mtd, do_erase_oneblock, ofs, len, NULL);
-   if (ret)
-   return ret;
-
-   instr->state = MTD_ERASE_DONE;
-   mtd_erase_callback(instr);
-
-   return 0;
+   return cfi_varsize_frob(mtd, do_erase_oneblock, instr->addr,
+   instr->len, NULL);
 }
 
 
@@ -2436,7 +2424,6 @@ static int cfi_amdstd_erase_chip(struct mtd_info *mtd, 
struct erase_info *instr)
 {
struct map_info *map = mtd->priv;
struct cfi_private *cfi = map->fldrv_priv;
-   int ret = 0;
 
if (instr->addr != 0)
return -EINVAL;
@@ -2444,14 +2431,7 @@ static int cfi_amdstd_erase_chip(struct mtd_info *mtd, 
struct erase_info *instr)
if (instr->len != mtd->size)
return -EINVAL;
 
-   ret = do_erase_chip(map, >chips[0]);
-   if (ret)
-   return ret;
-
-   instr->state = MTD_ERASE_DONE;
-   mtd_erase_callback(instr);
-
-   return 0;
+   return do_erase_chip(map, >chips[0]);
 }
 
 static int do_atmel_lock(struct map_info *map, struct flchip *chip,
diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c 
b/drivers/mtd/chips/cfi_cmdset_0020.c
index 7d342965f392..7b7658a05036 100644
--- a/drivers/mtd/chips/cfi_cmdset_0020.c
+++ b/drivers/mtd/chips/cfi_cmdset_0020.c
@@ -9

[PATCH 4/5] mtd: Unconditionally update ->fail_addr and ->addr in part_erase()

[Boris Brezillon]

->fail_addr and ->addr can be updated no matter the result of
parent->_erase(), we just need to remove the code doing the same thing
in mtd_erase_callback() to avoid adjusting those fields twice.

Note that this can be done because all MTD users have been converted to
not pass an erase_info->callback() and are thus only taking the
->addr_fail and ->addr fields into account after part_erase() has
returned.

While we're at it, get rid of the erase_info->mtd field which was only
needed to let mtd_erase_callback() get the partition device back.

Signed-off-by: Boris Brezillon <boris.brezil...@bootlin.com>
---
 drivers/mtd/ftl.c |  1 -
 drivers/mtd/inftlmount.c  |  3 ---
 drivers/mtd/mtdblock.c|  1 -
 drivers/mtd/mtdchar.c |  1 -
 drivers/mtd/mtdconcat.c   |  1 -
 drivers/mtd/mtdoops.c |  1 -
 drivers/mtd/mtdpart.c | 16 
 drivers/mtd/mtdswap.c |  2 --
 drivers/mtd/nand/nand_base.c  |  1 -
 drivers/mtd/nand/nand_bbt.c   |  1 -
 drivers/mtd/nftlmount.c   |  1 -
 drivers/mtd/rfd_ftl.c |  1 -
 drivers/mtd/sm_ftl.c  |  1 -
 drivers/mtd/tests/mtd_test.c  |  1 -
 drivers/mtd/tests/speedtest.c |  1 -
 drivers/mtd/ubi/io.c  |  1 -
 fs/jffs2/erase.c  |  1 -
 include/linux/mtd/mtd.h   |  3 ++-
 18 files changed, 6 insertions(+), 32 deletions(-)

diff --git a/drivers/mtd/ftl.c b/drivers/mtd/ftl.c
index fcf9907e7987..0a6adfaec7b5 100644
--- a/drivers/mtd/ftl.c
+++ b/drivers/mtd/ftl.c
@@ -342,7 +342,6 @@ static int erase_xfer(partition_t *part,
 if (!erase)
 return -ENOMEM;
 
-erase->mtd = part->mbd.mtd;
 erase->addr = xfer->Offset;
 erase->len = 1 << part->header.EraseUnitSize;
 
diff --git a/drivers/mtd/inftlmount.c b/drivers/mtd/inftlmount.c
index 0f47be4834d8..aab4f68bd36f 100644
--- a/drivers/mtd/inftlmount.c
+++ b/drivers/mtd/inftlmount.c
@@ -208,8 +208,6 @@ static int find_boot_record(struct INFTLrecord *inftl)
if (ip->Reserved0 != ip->firstUnit) {
struct erase_info *instr = >instr;
 
-   instr->mtd = inftl->mbd.mtd;
-
/*
 *  Most likely this is using the
 *  undocumented qiuck mount feature.
@@ -385,7 +383,6 @@ int INFTL_formatblock(struct INFTLrecord *inftl, int block)
   _first_? */
 
/* Use async erase interface, test return code */
-   instr->mtd = inftl->mbd.mtd;
instr->addr = block * inftl->EraseSize;
instr->len = inftl->mbd.mtd->erasesize;
/* Erase one physical eraseblock at a time, even though the NAND api
diff --git a/drivers/mtd/mtdblock.c b/drivers/mtd/mtdblock.c
index 7b2b7f651181..a5b1933c0490 100644
--- a/drivers/mtd/mtdblock.c
+++ b/drivers/mtd/mtdblock.c
@@ -65,7 +65,6 @@ static int erase_write (struct mtd_info *mtd, unsigned long 
pos,
/*
 * First, let's erase the flash block.
 */
-   erase.mtd = mtd;
erase.addr = pos;
erase.len = len;
 
diff --git a/drivers/mtd/mtdchar.c b/drivers/mtd/mtdchar.c
index 2beb22dd6bbb..c06b33f80e75 100644
--- a/drivers/mtd/mtdchar.c
+++ b/drivers/mtd/mtdchar.c
@@ -726,7 +726,6 @@ static int mtdchar_ioctl(struct file *file, u_int cmd, 
u_long arg)
erase->addr = einfo32.start;
erase->len = einfo32.length;
}
-   erase->mtd = mtd;
 
ret = mtd_erase(mtd, erase);
kfree(erase);
diff --git a/drivers/mtd/mtdconcat.c b/drivers/mtd/mtdconcat.c
index caa09bf6e572..93c47e56d9d8 100644
--- a/drivers/mtd/mtdconcat.c
+++ b/drivers/mtd/mtdconcat.c
@@ -427,7 +427,6 @@ static int concat_erase(struct mtd_info *mtd, struct 
erase_info *instr)
erase->len = length;
 
length -= erase->len;
-   erase->mtd = subdev;
if ((err = mtd_erase(subdev, erase))) {
/* sanity check: should never happen since
 * block alignment has been checked above */
diff --git a/drivers/mtd/mtdoops.c b/drivers/mtd/mtdoops.c
index 028ded59297b..9f25111fd559 100644
--- a/drivers/mtd/mtdoops.c
+++ b/drivers/mtd/mtdoops.c
@@ -94,7 +94,6 @@ static int mtdoops_erase_block(struct mtdoops_context *cxt, 
int offset)
int ret;
int page;
 
-   erase.mtd = mtd;
erase.addr = offset;
erase.len = mtd->erasesize;
 
diff --git a/drivers/mtd/mtdpart.c b/drivers/mtd/mtdpart.c
index ae1206633d9d..1c07a6f0dfe5 100644
--- a/drivers/mtd/mtdpart.c
+++ b/drivers/mtd/mtdpart.c
@@ -205,23 +205,15 @@ static int part_erase(struct mtd_info *mtd, struct 
erase_info *instr)
 
instr->addr += part->offset

[PATCH 1/5] mtd: Initialize ->fail_addr early in mtd_erase()

[Boris Brezillon]

mtd_erase() can return an error before ->fail_addr is initialized to
MTD_FAIL_ADDR_UNKNOWN. Move this initialization at the very beginning
of the function.

Signed-off-by: Boris Brezillon <boris.brezil...@bootlin.com>
---
 drivers/mtd/mtdcore.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/mtd/mtdcore.c b/drivers/mtd/mtdcore.c
index a1c94526fb88..c87859ff338b 100644
--- a/drivers/mtd/mtdcore.c
+++ b/drivers/mtd/mtdcore.c
@@ -953,6 +953,8 @@ EXPORT_SYMBOL_GPL(__put_mtd_device);
  */
 int mtd_erase(struct mtd_info *mtd, struct erase_info *instr)
 {
+   instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
+
if (!mtd->erasesize || !mtd->_erase)
return -ENOTSUPP;
 
@@ -961,7 +963,6 @@ int mtd_erase(struct mtd_info *mtd, struct erase_info 
*instr)
if (!(mtd->flags & MTD_WRITEABLE))
return -EROFS;
 
-   instr->fail_addr = MTD_FAIL_ADDR_UNKNOWN;
if (!instr->len) {
instr->state = MTD_ERASE_DONE;
mtd_erase_callback(instr);
-- 
2.14.1

[PATCH 0/5] mtd: Simplify erase handling

[Boris Brezillon]

Hello,

This series aims at simplifying erase handling both in MTD drivers and
MTD users code.

Historically, the erase operation has been designed to be asynchronous,
which, in theory, is a good thing since erasing a block usually takes
longer that reading/writing to a flash. In practice, all drivers are
implementing ->_erase() in a synchronous manner. Moreover, both drivers
and users are inconsistently updating/checking the erase_info fields.

In order to simplify things, let's assume ->_erase() is and will always
be synchronous. This also make error code checking more consistent and
allows us to get rid of a few hundred lines of code.

Regards,

Boris

Boris Brezillon (5):
  mtd: Initialize ->fail_addr early in mtd_erase()
  mtd: Get rid of unused fields in struct erase_info
  mtd: Stop assuming mtd_erase() is asynchronous
  mtd: Unconditionally update ->fail_addr and ->addr in part_erase()
  mtd: Stop updating erase_info->state and calling mtd_erase_callback()

 drivers/mtd/chips/cfi_cmdset_0001.c  | 16 +-
 drivers/mtd/chips/cfi_cmdset_0002.c  | 26 ++---
 drivers/mtd/chips/cfi_cmdset_0020.c  |  3 --
 drivers/mtd/chips/map_ram.c  |  2 -
 drivers/mtd/devices/bcm47xxsflash.c  | 12 +
 drivers/mtd/devices/block2mtd.c  |  7 +--
 drivers/mtd/devices/docg3.c  | 16 +-
 drivers/mtd/devices/lart.c   |  4 --
 drivers/mtd/devices/mtd_dataflash.c  |  4 --
 drivers/mtd/devices/mtdram.c |  2 -
 drivers/mtd/devices/phram.c  |  2 -
 drivers/mtd/devices/pmc551.c |  2 -
 drivers/mtd/devices/powernv_flash.c  | 11 +---
 drivers/mtd/devices/slram.c  |  2 -
 drivers/mtd/devices/spear_smi.c  |  3 --
 drivers/mtd/devices/sst25l.c |  3 --
 drivers/mtd/devices/st_spi_fsm.c |  4 --
 drivers/mtd/ftl.c| 52 +++---
 drivers/mtd/inftlmount.c |  8 ++-
 drivers/mtd/lpddr/lpddr2_nvm.c   | 10 +---
 drivers/mtd/lpddr/lpddr_cmds.c   |  2 -
 drivers/mtd/mtdblock.c   | 21 
 drivers/mtd/mtdchar.c| 34 +---
 drivers/mtd/mtdconcat.c  | 48 +
 drivers/mtd/mtdcore.c| 17 +++---
 drivers/mtd/mtdoops.c| 20 ---
 drivers/mtd/mtdpart.c| 23 ++--
 drivers/mtd/mtdswap.c| 34 
 drivers/mtd/nand/nand_base.c | 16 ++
 drivers/mtd/nand/nand_bbt.c  |  1 -
 drivers/mtd/nftlmount.c  |  5 +-
 drivers/mtd/onenand/onenand_base.c   | 17 --
 drivers/mtd/rfd_ftl.c| 93 ++--
 drivers/mtd/sm_ftl.c | 19 ---
 drivers/mtd/sm_ftl.h |  4 --
 drivers/mtd/spi-nor/spi-nor.c|  3 --
 drivers/mtd/tests/mtd_test.c |  5 --
 drivers/mtd/tests/speedtest.c|  7 ---
 drivers/mtd/ubi/gluebi.c |  3 --
 drivers/mtd/ubi/io.c | 36 -
 drivers/net/ethernet/sfc/falcon/mtd.c| 11 +---
 drivers/net/ethernet/sfc/mtd.c   | 11 +---
 drivers/staging/goldfish/goldfish_nand.c |  3 --
 fs/jffs2/erase.c | 37 ++---
 include/linux/mtd/mtd.h  | 19 +--
 45 files changed, 79 insertions(+), 599 deletions(-)

-- 
2.14.1

Re: [PATCH v2 05/11] mtd: cfi: reduce stack size with KASAN

[Boris Brezillon]

On Wed, 14 Jun 2017 23:15:40 +0200
Arnd Bergmann <a...@arndb.de> wrote:

> When CONFIG_KASAN is used, we consume a lot of extra stack space:
> 
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'do_write_buffer':
> drivers/mtd/chips/cfi_cmdset_0020.c:603:1: error: the frame size of 2184 
> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_erase_varsize':
> drivers/mtd/chips/cfi_cmdset_0020.c:972:1: error: the frame size of 1936 
> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> drivers/mtd/chips/cfi_cmdset_0001.c: In function 'do_write_buffer':
> drivers/mtd/chips/cfi_cmdset_0001.c:1841:1: error: the frame size of 1776 
> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> 
> This marks some functions as noinline_if_stackbloat to keep reduce the
> overall stack size.
> 
> Signed-off-by: Arnd Bergmann <a...@arndb.de>

Acked-by: Boris Brezillon <boris.brezil...@free-electrons.com>

> ---
>  drivers/mtd/chips/cfi_cmdset_0020.c | 8 
>  include/linux/mtd/map.h | 8 
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c 
> b/drivers/mtd/chips/cfi_cmdset_0020.c
> index 7d342965f392..5eee5e883f55 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0020.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0020.c
> @@ -244,7 +244,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info 
> *map)
>  }
>  
>  
> -static inline int do_read_onechip(struct map_info *map, struct flchip *chip, 
> loff_t adr, size_t len, u_char *buf)
> +static noinline_if_stackbloat int do_read_onechip(struct map_info *map, 
> struct flchip *chip, loff_t adr, size_t len, u_char *buf)
>  {
>   map_word status, status_OK;
>   unsigned long timeo;
> @@ -728,7 +728,7 @@ cfi_staa_writev(struct mtd_info *mtd, const struct kvec 
> *vecs,
>  }
>  
>  
> -static inline int do_erase_oneblock(struct map_info *map, struct flchip 
> *chip, unsigned long adr)
> +static noinline_if_stackbloat int do_erase_oneblock(struct map_info *map, 
> struct flchip *chip, unsigned long adr)
>  {
>   struct cfi_private *cfi = map->fldrv_priv;
>   map_word status, status_OK;
> @@ -1029,7 +1029,7 @@ static void cfi_staa_sync (struct mtd_info *mtd)
>   }
>  }
>  
> -static inline int do_lock_oneblock(struct map_info *map, struct flchip 
> *chip, unsigned long adr)
> +static noinline_if_stackbloat int do_lock_oneblock(struct map_info *map, 
> struct flchip *chip, unsigned long adr)
>  {
>   struct cfi_private *cfi = map->fldrv_priv;
>   map_word status, status_OK;
> @@ -1175,7 +1175,7 @@ static int cfi_staa_lock(struct mtd_info *mtd, loff_t 
> ofs, uint64_t len)
>   }
>   return 0;
>  }
> -static inline int do_unlock_oneblock(struct map_info *map, struct flchip 
> *chip, unsigned long adr)
> +static noinline_if_stackbloat int do_unlock_oneblock(struct map_info *map, 
> struct flchip *chip, unsigned long adr)
>  {
>   struct cfi_private *cfi = map->fldrv_priv;
>   map_word status, status_OK;
> diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> index 3aa56e3104bb..29db74314db8 100644
> --- a/include/linux/mtd/map.h
> +++ b/include/linux/mtd/map.h
> @@ -316,7 +316,7 @@ static inline map_word map_word_or(struct map_info *map, 
> map_word val1, map_word
>   return r;
>  }
>  
> -static inline int map_word_andequal(struct map_info *map, map_word val1, 
> map_word val2, map_word val3)
> +static noinline_if_stackbloat int map_word_andequal(struct map_info *map, 
> map_word val1, map_word val2, map_word val3)
>  {
>   int i;
>  
> @@ -328,7 +328,7 @@ static inline int map_word_andequal(struct map_info *map, 
> map_word val1, map_wor
>   return 1;
>  }
>  
> -static inline int map_word_bitsset(struct map_info *map, map_word val1, 
> map_word val2)
> +static noinline_if_stackbloat int map_word_bitsset(struct map_info *map, 
> map_word val1, map_word val2)
>  {
>   int i;
>  
> @@ -362,7 +362,7 @@ static inline map_word map_word_load(struct map_info 
> *map, const void *ptr)
>   return r;
>  }
>  
> -static inline map_word map_word_load_partial(struct map_info *map, map_word 
> orig, const unsigned char *buf, int start, int len)
> +static noinline_if_stackbloat map_word map_word_load_partial(struct map_info 
> *map, map_word orig, const unsigned char *buf, int start, int len)
>  {
>   int i;
>  
> @@ -392,7 +392,7 @@ static inline map_word map_word_load_partial(struct 
> map_info *map, map_word orig
>  #define MAP_FF_LIMIT 8
>  #endif
>  
> -static inline map_word map_word_ff(struct map_info *map)
> +static noinline_if_stackbloat map_word map_word_ff(struct map_info *map)
>  {
>   map_word r;
>   int i;

Re: [PATCH v2 05/11] mtd: cfi: reduce stack size with KASAN

[Boris Brezillon]

On Fri, 4 Aug 2017 11:09:53 +0200
Arnd Bergmann <a...@arndb.de> wrote:

> On Fri, Aug 4, 2017 at 9:42 AM, Boris Brezillon
> <boris.brezil...@free-electrons.com> wrote:
> > On Wed, 14 Jun 2017 23:15:40 +0200
> > Arnd Bergmann <a...@arndb.de> wrote:
> >  
> >> When CONFIG_KASAN is used, we consume a lot of extra stack space:
> >>
> >> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'do_write_buffer':
> >> drivers/mtd/chips/cfi_cmdset_0020.c:603:1: error: the frame size of 2184 
> >> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> >> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_erase_varsize':
> >> drivers/mtd/chips/cfi_cmdset_0020.c:972:1: error: the frame size of 1936 
> >> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> >> drivers/mtd/chips/cfi_cmdset_0001.c: In function 'do_write_buffer':
> >> drivers/mtd/chips/cfi_cmdset_0001.c:1841:1: error: the frame size of 1776 
> >> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> >>
> >> This marks some functions as noinline_if_stackbloat to keep reduce the
> >> overall stack size.
> >>
> >> Signed-off-by: Arnd Bergmann <a...@arndb.de>
> >> ---
> >>  drivers/mtd/chips/cfi_cmdset_0020.c | 8 
> >>  include/linux/mtd/map.h | 8 
> >>  2 files changed, 8 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c 
> >> b/drivers/mtd/chips/cfi_cmdset_0020.c
> >> index 7d342965f392..5eee5e883f55 100644
> >> --- a/drivers/mtd/chips/cfi_cmdset_0020.c
> >> +++ b/drivers/mtd/chips/cfi_cmdset_0020.c
> >> @@ -244,7 +244,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info 
> >> *map)
> >>  }
> >>
> >>
> >> -static inline int do_read_onechip(struct map_info *map, struct flchip 
> >> *chip, loff_t adr, size_t len, u_char *buf)
> >> +static noinline_if_stackbloat int do_read_onechip(struct map_info *map, 
> >> struct flchip *chip, loff_t adr, size_t len, u_char *buf)  
> >
> > Why do we even need to mark those functions inline in the first place?
> > Isn't the compiler smart enough to decide when it should inline things?  
> 
> I'm pretty sure it doesn't need the 'inline' keywork. I see that the code was
> addedlike this in linux-2.4.0-test3pre3 along with the rest of the mtd layer,
> so it has always been 'inline' and nobody ever bothered to remove that
> during later cleanups.
> 
> We could probably just mark this function as 'noinline' here and never
> inline it,
> but I would rather not add more than one variant of noinline_if_stackbloat:
> almost all other users of noinline_if_stackbloat are for functions that have
> to be inline in normal builds, so it is defined as being either
> 'inline' or 'noinline'
> depending on whether KASAN is active.

Okay. Let's keep it like that then.

Re: [PATCH v2 05/11] mtd: cfi: reduce stack size with KASAN

[Boris Brezillon]

On Wed, 14 Jun 2017 23:15:40 +0200
Arnd Bergmann  wrote:

> When CONFIG_KASAN is used, we consume a lot of extra stack space:
> 
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'do_write_buffer':
> drivers/mtd/chips/cfi_cmdset_0020.c:603:1: error: the frame size of 2184 
> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> drivers/mtd/chips/cfi_cmdset_0020.c: In function 'cfi_staa_erase_varsize':
> drivers/mtd/chips/cfi_cmdset_0020.c:972:1: error: the frame size of 1936 
> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> drivers/mtd/chips/cfi_cmdset_0001.c: In function 'do_write_buffer':
> drivers/mtd/chips/cfi_cmdset_0001.c:1841:1: error: the frame size of 1776 
> bytes is larger than 1536 bytes [-Werror=frame-larger-than=]
> 
> This marks some functions as noinline_if_stackbloat to keep reduce the
> overall stack size.
> 
> Signed-off-by: Arnd Bergmann 
> ---
>  drivers/mtd/chips/cfi_cmdset_0020.c | 8 
>  include/linux/mtd/map.h | 8 
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/mtd/chips/cfi_cmdset_0020.c 
> b/drivers/mtd/chips/cfi_cmdset_0020.c
> index 7d342965f392..5eee5e883f55 100644
> --- a/drivers/mtd/chips/cfi_cmdset_0020.c
> +++ b/drivers/mtd/chips/cfi_cmdset_0020.c
> @@ -244,7 +244,7 @@ static struct mtd_info *cfi_staa_setup(struct map_info 
> *map)
>  }
>  
>  
> -static inline int do_read_onechip(struct map_info *map, struct flchip *chip, 
> loff_t adr, size_t len, u_char *buf)
> +static noinline_if_stackbloat int do_read_onechip(struct map_info *map, 
> struct flchip *chip, loff_t adr, size_t len, u_char *buf)

Why do we even need to mark those functions inline in the first place?
Isn't the compiler smart enough to decide when it should inline things?

>  {
>   map_word status, status_OK;
>   unsigned long timeo;
> @@ -728,7 +728,7 @@ cfi_staa_writev(struct mtd_info *mtd, const struct kvec 
> *vecs,
>  }
>  
>  
> -static inline int do_erase_oneblock(struct map_info *map, struct flchip 
> *chip, unsigned long adr)
> +static noinline_if_stackbloat int do_erase_oneblock(struct map_info *map, 
> struct flchip *chip, unsigned long adr)
>  {
>   struct cfi_private *cfi = map->fldrv_priv;
>   map_word status, status_OK;
> @@ -1029,7 +1029,7 @@ static void cfi_staa_sync (struct mtd_info *mtd)
>   }
>  }
>  
> -static inline int do_lock_oneblock(struct map_info *map, struct flchip 
> *chip, unsigned long adr)
> +static noinline_if_stackbloat int do_lock_oneblock(struct map_info *map, 
> struct flchip *chip, unsigned long adr)
>  {
>   struct cfi_private *cfi = map->fldrv_priv;
>   map_word status, status_OK;
> @@ -1175,7 +1175,7 @@ static int cfi_staa_lock(struct mtd_info *mtd, loff_t 
> ofs, uint64_t len)
>   }
>   return 0;
>  }
> -static inline int do_unlock_oneblock(struct map_info *map, struct flchip 
> *chip, unsigned long adr)
> +static noinline_if_stackbloat int do_unlock_oneblock(struct map_info *map, 
> struct flchip *chip, unsigned long adr)
>  {
>   struct cfi_private *cfi = map->fldrv_priv;
>   map_word status, status_OK;
> diff --git a/include/linux/mtd/map.h b/include/linux/mtd/map.h
> index 3aa56e3104bb..29db74314db8 100644
> --- a/include/linux/mtd/map.h
> +++ b/include/linux/mtd/map.h
> @@ -316,7 +316,7 @@ static inline map_word map_word_or(struct map_info *map, 
> map_word val1, map_word
>   return r;
>  }
>  
> -static inline int map_word_andequal(struct map_info *map, map_word val1, 
> map_word val2, map_word val3)
> +static noinline_if_stackbloat int map_word_andequal(struct map_info *map, 
> map_word val1, map_word val2, map_word val3)

It's indeed needed for those defined in a header.

>  {
>   int i;
>  
> @@ -328,7 +328,7 @@ static inline int map_word_andequal(struct map_info *map, 
> map_word val1, map_wor
>   return 1;
>  }
>  
> -static inline int map_word_bitsset(struct map_info *map, map_word val1, 
> map_word val2)
> +static noinline_if_stackbloat int map_word_bitsset(struct map_info *map, 
> map_word val1, map_word val2)
>  {
>   int i;
>  
> @@ -362,7 +362,7 @@ static inline map_word map_word_load(struct map_info 
> *map, const void *ptr)
>   return r;
>  }
>  
> -static inline map_word map_word_load_partial(struct map_info *map, map_word 
> orig, const unsigned char *buf, int start, int len)
> +static noinline_if_stackbloat map_word map_word_load_partial(struct map_info 
> *map, map_word orig, const unsigned char *buf, int start, int len)
>  {
>   int i;
>  
> @@ -392,7 +392,7 @@ static inline map_word map_word_load_partial(struct 
> map_info *map, map_word orig
>  #define MAP_FF_LIMIT 8
>  #endif
>  
> -static inline map_word map_word_ff(struct map_info *map)
> +static noinline_if_stackbloat map_word map_word_ff(struct map_info *map)
>  {
>   map_word r;
>   int i;

Re: [PATCH] dt-bindings: gpmc: Correct location of generic gpmc binding

[Boris Brezillon]

On Wed, 21 Jun 2017 08:14:54 -0400
Tom Rini  wrote:

> The binding bus/ti-gpmc.txt has been moved to
> memory-controllers/omap-gpmc.txt.  Update all references to this in
> order to make reading and understanding a given binding easier.
> 
> Cc: David Woodhouse 
> Cc: Brian Norris 
> Cc:Boris Brezillon 
> Cc: Marek Vasut 
> Cc: Richard Weinberger 
> Cc: Cyrille Pitchen 
> Cc: Rob Herring 
> Cc: Mark Rutland 
> Signed-off-by: Tom Rini 

Applied.

Thanks,

Boris

> ---
>  Documentation/devicetree/bindings/mtd/gpmc-nand.txt| 2 +-
>  Documentation/devicetree/bindings/mtd/gpmc-nor.txt | 4 ++--
>  Documentation/devicetree/bindings/mtd/gpmc-onenand.txt | 2 +-
>  Documentation/devicetree/bindings/net/gpmc-eth.txt | 4 ++--
>  4 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt 
> b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> index 174f68c26c1b..dd559045593d 100644
> --- a/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> +++ b/Documentation/devicetree/bindings/mtd/gpmc-nand.txt
> @@ -5,7 +5,7 @@ the GPMC controller with a name of "nand".
>  
>  All timing relevant properties as well as generic gpmc child properties are
>  explained in a separate documents - please refer to
> -Documentation/devicetree/bindings/bus/ti-gpmc.txt
> +Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
>  
>  For NAND specific properties such as ECC modes or bus width, please refer to
>  Documentation/devicetree/bindings/mtd/nand.txt
> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-nor.txt 
> b/Documentation/devicetree/bindings/mtd/gpmc-nor.txt
> index 4828c17bb784..131d3a74d0bd 100644
> --- a/Documentation/devicetree/bindings/mtd/gpmc-nor.txt
> +++ b/Documentation/devicetree/bindings/mtd/gpmc-nor.txt
> @@ -5,7 +5,7 @@ child nodes of the GPMC controller with a name of "nor".
>  
>  All timing relevant properties as well as generic GPMC child properties are
>  explained in a separate documents. Please refer to
> -Documentation/devicetree/bindings/bus/ti-gpmc.txt
> +Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
>  
>  Required properties:
>  - bank-width:Width of NOR flash in bytes. GPMC supports 
> 8-bit and
> @@ -28,7 +28,7 @@ Required properties:
>  
>  Optional properties:
>  - gpmc,XXX   Additional GPMC timings and settings parameters. See
> - Documentation/devicetree/bindings/bus/ti-gpmc.txt
> + 
> Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
>  
>  Optional properties for partition table parsing:
>  - #address-cells: should be set to 1
> diff --git a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt 
> b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> index 5d8fa527c496..b6e8bfd024f4 100644
> --- a/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> +++ b/Documentation/devicetree/bindings/mtd/gpmc-onenand.txt
> @@ -5,7 +5,7 @@ the GPMC controller with a name of "onenand".
>  
>  All timing relevant properties as well as generic gpmc child properties are
>  explained in a separate documents - please refer to
> -Documentation/devicetree/bindings/bus/ti-gpmc.txt
> +Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
>  
>  Required properties:
>  
> diff --git a/Documentation/devicetree/bindings/net/gpmc-eth.txt 
> b/Documentation/devicetree/bindings/net/gpmc-eth.txt
> index ace4a64b3695..f7da3d73ca1b 100644
> --- a/Documentation/devicetree/bindings/net/gpmc-eth.txt
> +++ b/Documentation/devicetree/bindings/net/gpmc-eth.txt
> @@ -9,7 +9,7 @@ the GPMC controller with an "ethernet" name.
>  
>  All timing relevant properties as well as generic GPMC child properties are
>  explained in a separate documents. Please refer to
> -Documentation/devicetree/bindings/bus/ti-gpmc.txt
> +Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
>  
>  For the properties relevant to the ethernet controller connected to the GPMC
>  refer to the binding documentation of the device. For example, the 
> documentation
> @@ -43,7 +43,7 @@ Required properties:
>  
>  Optional properties:
>  - gpmc,XXX   Additional GPMC timings and settings parameters. See
> - Documentation/devicetree/bindings/bus/ti-gpmc.txt
> + 
> Documentation/devicetree/bindings/memory-controllers/omap-gpmc.txt
>  
>  Example:
>  

Re: [RFC PATCH 3/3] devicetree: macb: Add optional property tsu-clk

[Boris Brezillon]

Hi Harini,

On Mon, 14 Sep 2015 09:39:05 +0530
Harini Katakam <harinikatakamli...@gmail.com> wrote:

> On Fri, Sep 11, 2015 at 10:22 PM, Sören Brinkmann
> <soren.brinkm...@xilinx.com> wrote:
> > Hi Harini,
> >
> > On Fri, 2015-09-11 at 01:27PM +0530, Harini Katakam wrote:
> >> Add TSU clock frequency to be used for 1588 support in macb driver.
> >>
> >> Signed-off-by: Harini Katakam <hari...@xilinx.com>
> >> ---
> >>  Documentation/devicetree/bindings/net/macb.txt |3 +++
> >>  1 file changed, 3 insertions(+)
> >>
> >> diff --git a/Documentation/devicetree/bindings/net/macb.txt 
> >> b/Documentation/devicetree/bindings/net/macb.txt
> >> index b5d7976..f7c0ea8 100644
> >> --- a/Documentation/devicetree/bindings/net/macb.txt
> >> +++ b/Documentation/devicetree/bindings/net/macb.txt
> >> @@ -19,6 +19,9 @@ Required properties:
> >>   Optional elements: 'tx_clk'
> >>  - clocks: Phandles to input clocks.
> >>
> >> +Optional properties:
> >> +- tsu-clk: Time stamp unit clock frequency used.
> >
> > Why are we not using the CCF and a clk_get_rate() in the driver?
> >
> 
> If the clock source was only internal, we could use this
> approach as usual. But TSU clock can be configured to
> come from an external clock source or internal.

How about declaring a fixed-rate clk [1] if it comes from an external
clk, and using a clk driver for the internal clk case?
This way you'll be able to use the clk API (including the
clk_get_rate() function) instead of introducing a new way to retrieve a
clk frequency.

Best Regards,

Boris

[1]http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/clock/fixed-clock.txt

-- 
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html