Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1213327 > unrolled thread

Re: [PATCH v10 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v10 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610,  MPC5125 and others Brian Norris <computersforpeace@gmail.com> - 2015-08-25 22:40 +0200
    Re: [PATCH v10 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610,  MPC5125 and others Stefan Agner <stefan@agner.ch> - 2015-08-27 03:20 +0200
      Re: [PATCH v10 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610,  MPC5125 and others Brian Norris <computersforpeace@gmail.com> - 2015-08-27 18:50 +0200

#1213327 — Re: [PATCH v10 1/5] mtd: nand: vf610_nfc: Freescale NFC for VF610, MPC5125 and others

One more thing...

On Mon, Aug 03, 2015 at 11:27:26AM +0200, Stefan Agner wrote:
> --- /dev/null
> +++ b/drivers/mtd/nand/vf610_nfc.c
> @@ -0,0 +1,645 @@
...
> +struct vf610_nfc {
> +	struct mtd_info mtd;
> +	struct nand_chip chip;
> +	struct device *dev;
> +	void __iomem *regs;
> +	struct completion cmd_done;
> +	uint buf_offset;
> +	int page_sz;

AFAICT (even with the 2nd patch), you never really use this field. You
just set it/increment it, but don't use it for anything. Kill it?

> +	/* Status and ID are in alternate locations. */
> +	int alt_buf;
> +#define ALT_BUF_ID   1
> +#define ALT_BUF_STAT 2
> +#define ALT_BUF_ONFI 3
> +	struct clk *clk;
> +};

Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1214263

On 2015-08-25 13:34, Brian Norris wrote:
> One more thing...
> 
> On Mon, Aug 03, 2015 at 11:27:26AM +0200, Stefan Agner wrote:
>> --- /dev/null
>> +++ b/drivers/mtd/nand/vf610_nfc.c
>> @@ -0,0 +1,645 @@
> ...
>> +struct vf610_nfc {
>> +	struct mtd_info mtd;
>> +	struct nand_chip chip;
>> +	struct device *dev;
>> +	void __iomem *regs;
>> +	struct completion cmd_done;
>> +	uint buf_offset;
>> +	int page_sz;
> 
> AFAICT (even with the 2nd patch), you never really use this field. You
> just set it/increment it, but don't use it for anything. Kill it?

It is used in the write path, I think I meant to use it for subpage
writes, when I thought it would just mean to transfer only parts of the
page to the controller.

However, as the subpage discussion basically concluded in not using it
for now on this controller, we can as well transfer the complete page
(page_sz). Or is there another case in which vf610_nfc_write_buf could
be called with less than page_sz?

--
Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1214697

On Wed, Aug 26, 2015 at 06:10:05PM -0700, Stefan Agner wrote:
> On 2015-08-25 13:34, Brian Norris wrote:
> > One more thing...
> > 
> > On Mon, Aug 03, 2015 at 11:27:26AM +0200, Stefan Agner wrote:
> >> --- /dev/null
> >> +++ b/drivers/mtd/nand/vf610_nfc.c
> >> @@ -0,0 +1,645 @@
> > ...
> >> +struct vf610_nfc {
> >> +	struct mtd_info mtd;
> >> +	struct nand_chip chip;
> >> +	struct device *dev;
> >> +	void __iomem *regs;
> >> +	struct completion cmd_done;
> >> +	uint buf_offset;
> >> +	int page_sz;
> > 
> > AFAICT (even with the 2nd patch), you never really use this field. You
> > just set it/increment it, but don't use it for anything. Kill it?
> 
> It is used in the write path, I think I meant to use it for subpage
> writes, when I thought it would just mean to transfer only parts of the
> page to the controller.

Ah, you're right. Sorry, I missed that. I got mixed up seeing most of
your uses of 'page_sz' were for a local variable of the same name, not
this field.

> However, as the subpage discussion basically concluded in not using it
> for now on this controller, we can as well transfer the complete page
> (page_sz). Or is there another case in which vf610_nfc_write_buf could
> be called with less than page_sz?

I'll leave that up to you. I'm perfectly fine leaving it in, now that I
see its proper use. Just in case things change in the future, I think it
does help to clarify the flow of information a little. Although, I might
recommend a change in naming, since it could get confused with the
actual page size -- which is normally constant -- whereas this field
changes dynamically depending on the command-in-flight.

Perhaps the struct could have 'write_len' (to help represent an action)
and the local variable in vf610_nfc_command() could be 'tfr_len' (to
distinguish how it isn't necessarily identical to 'write_len')? Just
throwing (likely bad) ideas out there.

Regards,
Brian
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web