All of lore.kernel.org
 help / color / mirror / Atom feed
From: Boris Brezillon <boris.brezillon@bootlin.com>
To: Liang Yang <liang.yang@amlogic.com>
Cc: Jianxin Pan <jianxin.pan@amlogic.com>,
	<linux-mtd@lists.infradead.org>, Rob Herring <robh@kernel.org>,
	Hanjie Lin <hanjie.lin@amlogic.com>,
	Victor Wan <victor.wan@amlogic.com>,
	Neil Armstrong <narmstrong@baylibre.com>,
	Martin Blumenstingl <martin.blumenstingl@googlemail.com>,
	Richard Weinberger <richard@nod.at>,
	Yixun Lan <yixun.lan@amlogic.com>, <linux-kernel@vger.kernel.org>,
	Marek Vasut <marek.vasut@gmail.com>,
	Jian Hu <jian.hu@amlogic.com>,
	Kevin Hilman <khilman@baylibre.com>,
	Carlo Caione <carlo@caione.org>,
	<linux-amlogic@lists.infradead.org>,
	Brian Norris <computersforpeace@gmail.com>,
	David Woodhouse <dwmw2@infradead.org>,
	<linux-arm-kernel@lists.infradead.org>,
	Jerome Brunet <jbrunet@baylibre.com>
Subject: Re: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
Date: Fri, 19 Oct 2018 10:10:33 +0200	[thread overview]
Message-ID: <20181019101033.0a160506@bbrezillon> (raw)
In-Reply-To: <09f76f17-05c4-d9e0-0167-f68e224f00c4@amlogic.com>

On Fri, 19 Oct 2018 15:29:05 +0800
Liang Yang <liang.yang@amlogic.com> wrote:

> > How about defining that the HW returns an array of __le64 instead and then
> > define the following macros which you can use after converting in the
> > CPU endianness
> > 
> > #define ECC_GET_PROTECTED_OOB_BYTE(x, y)	(((x) >> (8 * (1 + y)) & GENMASK(7, 0))
> > #define ECC_COMPLETE			BIT(31)
> > #define ECC_ERR_CNT(x)			(((x) >> 24) & GENMASK(5, 0))
> > 
> > (I'm not entirely sure the field positions are correct, but I'll let you
> > check that).
> >   
> ok. i think it should be:
> 
> #define ECC_GET_PROTECTED_OOB_BYTE(x, y)	(((x) >> (8 * y) &
> GENMASK(7, 0))
> 
> if x represents the u64 and y represents the index of the u64.

Absolutely.

> 
> 
> 
> >> +
> >> +#define PER_INFO_BYTE	(sizeof(struct meson_nfc_info_format))
> >> +
> >> +struct meson_nfc_nand_chip {
> >> +	struct list_head node;
> >> +	struct nand_chip nand;
> >> +	bool is_scramble;  
> > 
> > I think I already mentioned the NAND_NEED_SCRAMBLING flag []. Please
> > drop this field and test (chip->flags & NAND_NEED_SCRAMBLING) instead.
> >   
> em, i use NAND_NEED_SCRAMBLING and is_scramble is set:
> static int meson_nand_attach_chip(struct nand_chip *nand)
> {
> 	......
> 	meson_chip->is_scramble =
> 		(nand->options & NAND_NEED_SCRAMBLING) ? 1 : 0;
> 	......
> }

Why do you need to add a new field then? Just test
nand->options & NAND_NEED_SCRAMBLING directly or provide a helper
function that does that.

WARNING: multiple messages have this Message-ID (diff)
From: boris.brezillon@bootlin.com (Boris Brezillon)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
Date: Fri, 19 Oct 2018 10:10:33 +0200	[thread overview]
Message-ID: <20181019101033.0a160506@bbrezillon> (raw)
In-Reply-To: <09f76f17-05c4-d9e0-0167-f68e224f00c4@amlogic.com>

On Fri, 19 Oct 2018 15:29:05 +0800
Liang Yang <liang.yang@amlogic.com> wrote:

> > How about defining that the HW returns an array of __le64 instead and then
> > define the following macros which you can use after converting in the
> > CPU endianness
> > 
> > #define ECC_GET_PROTECTED_OOB_BYTE(x, y)	(((x) >> (8 * (1 + y)) & GENMASK(7, 0))
> > #define ECC_COMPLETE			BIT(31)
> > #define ECC_ERR_CNT(x)			(((x) >> 24) & GENMASK(5, 0))
> > 
> > (I'm not entirely sure the field positions are correct, but I'll let you
> > check that).
> >   
> ok. i think it should be:
> 
> #define ECC_GET_PROTECTED_OOB_BYTE(x, y)	(((x) >> (8 * y) &
> GENMASK(7, 0))
> 
> if x represents the u64 and y represents the index of the u64.

Absolutely.

> 
> 
> 
> >> +
> >> +#define PER_INFO_BYTE	(sizeof(struct meson_nfc_info_format))
> >> +
> >> +struct meson_nfc_nand_chip {
> >> +	struct list_head node;
> >> +	struct nand_chip nand;
> >> +	bool is_scramble;  
> > 
> > I think I already mentioned the NAND_NEED_SCRAMBLING flag []. Please
> > drop this field and test (chip->flags & NAND_NEED_SCRAMBLING) instead.
> >   
> em, i use NAND_NEED_SCRAMBLING and is_scramble is set:
> static int meson_nand_attach_chip(struct nand_chip *nand)
> {
> 	......
> 	meson_chip->is_scramble =
> 		(nand->options & NAND_NEED_SCRAMBLING) ? 1 : 0;
> 	......
> }

Why do you need to add a new field then? Just test
nand->options & NAND_NEED_SCRAMBLING directly or provide a helper
function that does that.

WARNING: multiple messages have this Message-ID (diff)
From: boris.brezillon@bootlin.com (Boris Brezillon)
To: linus-amlogic@lists.infradead.org
Subject: [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller
Date: Fri, 19 Oct 2018 10:10:33 +0200	[thread overview]
Message-ID: <20181019101033.0a160506@bbrezillon> (raw)
In-Reply-To: <09f76f17-05c4-d9e0-0167-f68e224f00c4@amlogic.com>

On Fri, 19 Oct 2018 15:29:05 +0800
Liang Yang <liang.yang@amlogic.com> wrote:

> > How about defining that the HW returns an array of __le64 instead and then
> > define the following macros which you can use after converting in the
> > CPU endianness
> > 
> > #define ECC_GET_PROTECTED_OOB_BYTE(x, y)	(((x) >> (8 * (1 + y)) & GENMASK(7, 0))
> > #define ECC_COMPLETE			BIT(31)
> > #define ECC_ERR_CNT(x)			(((x) >> 24) & GENMASK(5, 0))
> > 
> > (I'm not entirely sure the field positions are correct, but I'll let you
> > check that).
> >   
> ok. i think it should be:
> 
> #define ECC_GET_PROTECTED_OOB_BYTE(x, y)	(((x) >> (8 * y) &
> GENMASK(7, 0))
> 
> if x represents the u64 and y represents the index of the u64.

Absolutely.

> 
> 
> 
> >> +
> >> +#define PER_INFO_BYTE	(sizeof(struct meson_nfc_info_format))
> >> +
> >> +struct meson_nfc_nand_chip {
> >> +	struct list_head node;
> >> +	struct nand_chip nand;
> >> +	bool is_scramble;  
> > 
> > I think I already mentioned the NAND_NEED_SCRAMBLING flag []. Please
> > drop this field and test (chip->flags & NAND_NEED_SCRAMBLING) instead.
> >   
> em, i use NAND_NEED_SCRAMBLING and is_scramble is set:
> static int meson_nand_attach_chip(struct nand_chip *nand)
> {
> 	......
> 	meson_chip->is_scramble =
> 		(nand->options & NAND_NEED_SCRAMBLING) ? 1 : 0;
> 	......
> }

Why do you need to add a new field then? Just test
nand->options & NAND_NEED_SCRAMBLING directly or provide a helper
function that does that.

  reply	other threads:[~2018-10-19  8:10 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-18  5:09 [PATCH v5 0/2] mtd: rawnand: meson: add Amlogic NAND driver support Jianxin Pan
2018-10-18  5:09 ` Jianxin Pan
2018-10-18  5:09 ` Jianxin Pan
2018-10-18  5:09 ` Jianxin Pan
2018-10-18  5:09 ` [PATCH v5 1/2] dt-bindings: nand: meson: add Amlogic NAND controller driver Jianxin Pan
2018-10-18  5:09   ` Jianxin Pan
2018-10-18  5:09   ` Jianxin Pan
2018-10-18  5:09   ` Jianxin Pan
2018-10-18 16:48   ` Rob Herring
2018-10-18 16:48     ` Rob Herring
2018-10-18 16:48     ` Rob Herring
2018-10-18 16:48     ` Rob Herring
2018-10-18  5:09 ` [PATCH v5 2/2] mtd: rawnand: meson: add support for Amlogic NAND flash controller Jianxin Pan
2018-10-18  5:09   ` Jianxin Pan
2018-10-18  5:09   ` Jianxin Pan
2018-10-18 14:24   ` Boris Brezillon
2018-10-18 14:24     ` Boris Brezillon
2018-10-18 14:24     ` Boris Brezillon
2018-10-19  7:29     ` Liang Yang
2018-10-19  7:29       ` Liang Yang
2018-10-19  7:29       ` Liang Yang
2018-10-18 19:33   ` Boris Brezillon
2018-10-18 19:33     ` Boris Brezillon
2018-10-18 19:33     ` Boris Brezillon
2018-10-19  7:29     ` Liang Yang
2018-10-19  7:29       ` Liang Yang
2018-10-19  7:29       ` Liang Yang
2018-10-19  8:10       ` Boris Brezillon [this message]
2018-10-19  8:10         ` Boris Brezillon
2018-10-19  8:10         ` Boris Brezillon
2018-10-19  8:30         ` Liang Yang
2018-10-19  8:30           ` Liang Yang
2018-10-19  8:30           ` Liang Yang
2018-10-18 20:34   ` Boris Brezillon
2018-10-18 20:34     ` Boris Brezillon
2018-10-18 20:34     ` Boris Brezillon
2018-10-18 20:39   ` Boris Brezillon
2018-10-18 20:39     ` Boris Brezillon
2018-10-18 20:39     ` Boris Brezillon
2018-10-19  8:04     ` Liang Yang
2018-10-19  8:04       ` Liang Yang
2018-10-19  8:04       ` Liang Yang
2018-10-18 20:50   ` Boris Brezillon
2018-10-18 20:50     ` Boris Brezillon
2018-10-18 20:50     ` Boris Brezillon
2018-10-19  8:29     ` Liang Yang
2018-10-19  8:29       ` Liang Yang
2018-10-19  8:29       ` Liang Yang
2018-10-19  8:42       ` Boris Brezillon
2018-10-19  8:42         ` Boris Brezillon
2018-10-19  8:42         ` Boris Brezillon
2018-10-19  9:01         ` Liang Yang
2018-10-19  9:01           ` Liang Yang
2018-10-19  9:01           ` Liang Yang

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=20181019101033.0a160506@bbrezillon \
    --to=boris.brezillon@bootlin.com \
    --cc=carlo@caione.org \
    --cc=computersforpeace@gmail.com \
    --cc=dwmw2@infradead.org \
    --cc=hanjie.lin@amlogic.com \
    --cc=jbrunet@baylibre.com \
    --cc=jian.hu@amlogic.com \
    --cc=jianxin.pan@amlogic.com \
    --cc=khilman@baylibre.com \
    --cc=liang.yang@amlogic.com \
    --cc=linux-amlogic@lists.infradead.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mtd@lists.infradead.org \
    --cc=marek.vasut@gmail.com \
    --cc=martin.blumenstingl@googlemail.com \
    --cc=narmstrong@baylibre.com \
    --cc=richard@nod.at \
    --cc=robh@kernel.org \
    --cc=victor.wan@amlogic.com \
    --cc=yixun.lan@amlogic.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.