All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <computersforpeace@gmail.com>
To: "Bean Huo 霍斌斌 (beanhuo)" <beanhuo@micron.com>
Cc: Cyrille Pitchen <cyrille.pitchen@atmel.com>,
	"linux-mtd@lists.infradead.org" <linux-mtd@lists.infradead.org>,
	"nicolas.ferre@atmel.com" <nicolas.ferre@atmel.com>,
	"marex@denx.de" <marex@denx.de>,
	"vigneshr@ti.com" <vigneshr@ti.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	"robh+dt@kernel.org" <robh+dt@kernel.org>,
	"pawel.moll@arm.com" <pawel.moll@arm.com>,
	"mark.rutland@arm.com" <mark.rutland@arm.com>,
	"ijc+devicetree@hellion.org.uk" <ijc+devicetree@hellion.org.uk>,
	"galak@codeaurora.org" <galak@codeaurora.org>
Subject: Re: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller
Date: Thu, 17 Dec 2015 16:41:29 -0800	[thread overview]
Message-ID: <20151218004129.GF10460@google.com> (raw)
In-Reply-To: <20151218002901.GE10460@google.com>

On Thu, Dec 17, 2015 at 04:29:01PM -0800, Brian Norris wrote:
> On Tue, Dec 08, 2015 at 06:21:00AM +0000, Bean Huo 霍斌斌 wrote:

> OK, so I think your patch is broken:
> 
> > >     commit 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for Micron
> > >     SPI NOR")
> 
> How did you test this? Specifically, this can't possibly have worked
> with a regular drivers/spi/ controllers, since:
> 
>  (a) you're enabling 4/4/4 (i.e., "Quad mode") on the flash but
>  (b) m25p80_read() only sets .rx_bits for the data; i.e., it's using
>  1/1/4 (i.e., "Extended mode")
> 
> I'm tempted to essentially revert that, as it looks essentially
> untested. It would be nice to have a cleaner baseline before trying to
> extend it with Cyrille's work.
> 
> Cyrille, what do you think? Is my analysis at all correct here? (Sorry
> if this is addressed elsewhere; there's a lot of text in this
> conversation, but I'm getting hung up very early.) And if so, does it
> hurt to just drop Micron "Quad mode" (4/4/4)?

It looks like you address some of this in patch 2, where you (as I do)
claim that Micron support is broken. It seems to me that it could be
better to kill it than to try to fix it. But maybe that's just the
frustrated maintainer in me speaking.

Brian

WARNING: multiple messages have this Message-ID (diff)
From: Brian Norris <computersforpeace-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
To: "Bean Huo 霍斌斌 (beanhuo)"
	<beanhuo-AL4WhLSQfzjQT0dZR+AlfA@public.gmane.org>
Cc: Cyrille Pitchen
	<cyrille.pitchen-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>,
	"linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-mtd-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	"nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org"
	<nicolas.ferre-AIFe0yeh4nAAvxtiuMwx3w@public.gmane.org>,
	"marex-ynQEQJNshbs@public.gmane.org"
	<marex-ynQEQJNshbs@public.gmane.org>,
	"vigneshr-l0cyMroinI0@public.gmane.org"
	<vigneshr-l0cyMroinI0@public.gmane.org>,
	"linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org"
	<linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org>,
	"devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	"robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org"
	<robh+dt-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>,
	"pawel.moll-5wv7dgnIgG8@public.gmane.org"
	<pawel.moll-5wv7dgnIgG8@public.gmane.org>,
	"mark.rutland-5wv7dgnIgG8@public.gmane.org"
	<mark.rutland-5wv7dgnIgG8@public.gmane.org>,
	"ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org"
	<ijc+devicetree-KcIKpvwj1kUDXYZnReoRVg@public.gmane.org>,
	"galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org"
	<galak-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Subject: Re: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller
Date: Thu, 17 Dec 2015 16:41:29 -0800	[thread overview]
Message-ID: <20151218004129.GF10460@google.com> (raw)
In-Reply-To: <20151218002901.GE10460-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>

On Thu, Dec 17, 2015 at 04:29:01PM -0800, Brian Norris wrote:
> On Tue, Dec 08, 2015 at 06:21:00AM +0000, Bean Huo 霍斌斌 wrote:

> OK, so I think your patch is broken:
> 
> > >     commit 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for Micron
> > >     SPI NOR")
> 
> How did you test this? Specifically, this can't possibly have worked
> with a regular drivers/spi/ controllers, since:
> 
>  (a) you're enabling 4/4/4 (i.e., "Quad mode") on the flash but
>  (b) m25p80_read() only sets .rx_bits for the data; i.e., it's using
>  1/1/4 (i.e., "Extended mode")
> 
> I'm tempted to essentially revert that, as it looks essentially
> untested. It would be nice to have a cleaner baseline before trying to
> extend it with Cyrille's work.
> 
> Cyrille, what do you think? Is my analysis at all correct here? (Sorry
> if this is addressed elsewhere; there's a lot of text in this
> conversation, but I'm getting hung up very early.) And if so, does it
> hurt to just drop Micron "Quad mode" (4/4/4)?

It looks like you address some of this in patch 2, where you (as I do)
claim that Micron support is broken. It seems to me that it could be
better to kill it than to try to fix it. But maybe that's just the
frustrated maintainer in me speaking.

Brian
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

WARNING: multiple messages have this Message-ID (diff)
From: computersforpeace@gmail.com (Brian Norris)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller
Date: Thu, 17 Dec 2015 16:41:29 -0800	[thread overview]
Message-ID: <20151218004129.GF10460@google.com> (raw)
In-Reply-To: <20151218002901.GE10460@google.com>

On Thu, Dec 17, 2015 at 04:29:01PM -0800, Brian Norris wrote:
> On Tue, Dec 08, 2015 at 06:21:00AM +0000, Bean Huo ??? wrote:

> OK, so I think your patch is broken:
> 
> > >     commit 548cd3ab54da ("mtd: spi-nor: Add quad I/O support for Micron
> > >     SPI NOR")
> 
> How did you test this? Specifically, this can't possibly have worked
> with a regular drivers/spi/ controllers, since:
> 
>  (a) you're enabling 4/4/4 (i.e., "Quad mode") on the flash but
>  (b) m25p80_read() only sets .rx_bits for the data; i.e., it's using
>  1/1/4 (i.e., "Extended mode")
> 
> I'm tempted to essentially revert that, as it looks essentially
> untested. It would be nice to have a cleaner baseline before trying to
> extend it with Cyrille's work.
> 
> Cyrille, what do you think? Is my analysis at all correct here? (Sorry
> if this is addressed elsewhere; there's a lot of text in this
> conversation, but I'm getting hung up very early.) And if so, does it
> hurt to just drop Micron "Quad mode" (4/4/4)?

It looks like you address some of this in patch 2, where you (as I do)
claim that Micron support is broken. It seems to me that it could be
better to kill it than to try to fix it. But maybe that's just the
frustrated maintainer in me speaking.

Brian

  reply	other threads:[~2015-12-18  0:41 UTC|newest]

Thread overview: 78+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-07 14:09 [PATCH linux-next 0/5] mtd: spi-nor: add driver for Atmel QSPI controller Cyrille Pitchen
2015-12-07 14:09 ` Cyrille Pitchen
2015-12-07 14:09 ` Cyrille Pitchen
2015-12-07 14:09 ` [PATCH linux-next 1/5] mtd: spi-nor: properly detect the memory when it boots in Quad or Dual mode Cyrille Pitchen
2015-12-07 14:09   ` Cyrille Pitchen
2015-12-07 14:09   ` Cyrille Pitchen
2015-12-18  1:55   ` Brian Norris
2015-12-18  1:55     ` Brian Norris
2015-12-18 11:19     ` Cyrille Pitchen
2015-12-18 11:19       ` Cyrille Pitchen
2015-12-18 11:19       ` Cyrille Pitchen
2016-01-04 16:50     ` Cyrille Pitchen
2016-01-04 16:50       ` Cyrille Pitchen
2016-01-04 16:50       ` Cyrille Pitchen
2015-12-18  2:08   ` Brian Norris
2015-12-18  2:08     ` Brian Norris
2015-12-18  2:08     ` Brian Norris
2015-12-07 14:09 ` [PATCH linux-next 2/5] mtd: spi-nor: fix Quad SPI mode support for Spansion, Micron and Macronix Cyrille Pitchen
2015-12-07 14:09   ` Cyrille Pitchen
2015-12-07 14:09   ` Cyrille Pitchen
2015-12-18  2:18   ` Brian Norris
2015-12-18  2:18     ` Brian Norris
2015-12-18  2:18     ` Brian Norris
2016-01-04 16:12     ` Cyrille Pitchen
2016-01-04 16:12       ` Cyrille Pitchen
2016-01-04 16:12       ` Cyrille Pitchen
2015-12-07 14:09 ` [PATCH linux-next 3/5] mtd: m25p80: add support of dual and quad spi protocols to all commands Cyrille Pitchen
2015-12-07 14:09   ` Cyrille Pitchen
2015-12-07 14:09   ` Cyrille Pitchen
2015-12-07 14:09 ` [PATCH linux-next 4/5] Documentation: atmel-quadspi: add binding file for Atmel QSPI driver Cyrille Pitchen
2015-12-07 14:09   ` Cyrille Pitchen
2015-12-07 14:09   ` Cyrille Pitchen
2015-12-09  3:16   ` Rob Herring
2015-12-09  3:16     ` Rob Herring
2015-12-11  9:26   ` Nicolas Ferre
2015-12-11  9:26     ` Nicolas Ferre
2015-12-11  9:26     ` Nicolas Ferre
2015-12-07 14:09 ` [PATCH linux-next 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller Cyrille Pitchen
2015-12-07 14:09   ` Cyrille Pitchen
2015-12-07 14:09   ` Cyrille Pitchen
2015-12-07 15:25   ` [PATCH] mtd: atmel-quadspi: fix compare_const_fl.cocci warnings kbuild test robot
2015-12-07 15:25     ` kbuild test robot
2015-12-07 15:25     ` kbuild test robot
2015-12-07 15:25   ` [PATCH linux-next 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller kbuild test robot
2015-12-07 15:25     ` kbuild test robot
2015-12-07 15:25     ` kbuild test robot
2015-12-07 15:25   ` [PATCH] mtd: atmel-quadspi: fix odd_ptr_err.cocci warnings kbuild test robot
2015-12-07 15:25     ` kbuild test robot
2015-12-07 15:25     ` kbuild test robot
2015-12-11 14:50   ` [PATCH linux-next 5/5] mtd: atmel-quadspi: add driver for Atmel QSPI controller Nicolas Ferre
2015-12-11 14:50     ` Nicolas Ferre
2015-12-11 14:50     ` Nicolas Ferre
2015-12-07 19:34 ` [PATCH linux-next 0/5] mtd: spi-nor: " Brian Norris
2015-12-07 19:34   ` Brian Norris
2015-12-07 19:34   ` Brian Norris
2015-12-08  6:21   ` Bean Huo 霍斌斌 (beanhuo)
2015-12-08  6:21     ` Bean Huo 霍斌斌 (beanhuo)
2015-12-08  6:21     ` Bean Huo 霍斌斌 (beanhuo)
2015-12-18  0:29     ` Brian Norris
2015-12-18  0:29       ` Brian Norris
2015-12-18  0:29       ` Brian Norris
2015-12-18  0:29       ` Brian Norris
2015-12-18  0:41       ` Brian Norris [this message]
2015-12-18  0:41         ` Brian Norris
2015-12-18  0:41         ` Brian Norris
2015-12-18  0:41         ` Brian Norris
2016-01-20  3:41       ` Bean Huo 霍斌斌 (beanhuo)
2016-01-20  3:41         ` Bean Huo 霍斌斌 (beanhuo)
2016-01-20  3:41         ` Bean Huo 霍斌斌 (beanhuo)
2016-01-20  3:41         ` Bean Huo 霍斌斌 (beanhuo)
2015-12-08  6:44   ` Bean Huo 霍斌斌 (beanhuo)
2015-12-08  6:44     ` Bean Huo 霍斌斌 (beanhuo)
2015-12-08  6:44     ` Bean Huo 霍斌斌 (beanhuo)
2015-12-08 10:25   ` Cyrille Pitchen
2015-12-08 10:25     ` Cyrille Pitchen
2015-12-08 14:32     ` Cyrille Pitchen
2015-12-08 14:32       ` Cyrille Pitchen
2015-12-08 14:32       ` Cyrille Pitchen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151218004129.GF10460@google.com \
    --to=computersforpeace@gmail.com \
    --cc=beanhuo@micron.com \
    --cc=cyrille.pitchen@atmel.com \
    --cc=devicetree@vger.kernel.org \
    --cc=galak@codeaurora.org \
    --cc=ijc+devicetree@hellion.org.uk \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marex@denx.de \
    --cc=mark.rutland@arm.com \
    --cc=nicolas.ferre@atmel.com \
    --cc=pawel.moll@arm.com \
    --cc=robh+dt@kernel.org \
    --cc=vigneshr@ti.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.