Message ID | 1355150521-3339-4-git-send-email-jagannadh.teki@gmail.com |
---|---|
State | Superseded |
Delegated to: | Mike Frysinger |
Headers | show |
Hi, On Mon, Dec 10, 2012 at 6:42 AM, Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> wrote: > This patch provides support to program a flash config register. > > Configuration register contains the control bits used to configure > the different configurations and security features of a device. > > User need to set these bits through spi_flash_cmd_write_config() > based on their usage. > > Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> > --- > drivers/mtd/spi/spi_flash.c | 27 +++++++++++++++++++++++++++ > drivers/mtd/spi/spi_flash_internal.h | 3 +++ > 2 files changed, 30 insertions(+), 0 deletions(-) > > diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c > index 800ed8b..a8f0af0 100644 > --- a/drivers/mtd/spi/spi_flash.c > +++ b/drivers/mtd/spi/spi_flash.c > @@ -274,6 +274,33 @@ int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr) > return 0; > } > > +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr) > +{ > + u8 cmd; > + int ret; > + > + ret = spi_flash_cmd_write_enable(flash); > + if (ret < 0) { > + debug("SF: enabling write failed\n"); > + return ret; > + } > + > + cmd = CMD_WRITE_STATUS; > + ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &cr, 2); Does this assume a particular endianness? Perhaps should put the values into a byte array instead? > + if (ret) { > + debug("SF: fail to write config register\n"); > + return ret; > + } > + > + ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); > + if (ret < 0) { > + debug("SF: write config register timed out\n"); > + return ret; > + } > + > + return 0; > +} > + > /* > * The following table holds all device probe functions > * > diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h > index 141cfa8..9287778 100644 > --- a/drivers/mtd/spi/spi_flash_internal.h > +++ b/drivers/mtd/spi/spi_flash_internal.h > @@ -77,6 +77,9 @@ static inline int spi_flash_cmd_write_disable(struct spi_flash *flash) > /* Program the status register. */ > int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr); > > +/* Program the config register. */ > +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr); > + > /* > * Same as spi_flash_cmd_read() except it also claims/releases the SPI > * bus. Used as common part of the ->read() operation. > -- > 1.7.0.4 > > _______________________________________________ > U-Boot mailing list > U-Boot@lists.denx.de > http://lists.denx.de/mailman/listinfo/u-boot Regards, Simon
Hi Simon, On Wed, Dec 12, 2012 at 12:05 PM, Simon Glass <sjg@chromium.org> wrote: > Hi, > > On Mon, Dec 10, 2012 at 6:42 AM, Jagannadha Sutradharudu Teki > <jagannadh.teki@gmail.com> wrote: >> This patch provides support to program a flash config register. >> >> Configuration register contains the control bits used to configure >> the different configurations and security features of a device. >> >> User need to set these bits through spi_flash_cmd_write_config() >> based on their usage. >> >> Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> >> --- >> drivers/mtd/spi/spi_flash.c | 27 +++++++++++++++++++++++++++ >> drivers/mtd/spi/spi_flash_internal.h | 3 +++ >> 2 files changed, 30 insertions(+), 0 deletions(-) >> >> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c >> index 800ed8b..a8f0af0 100644 >> --- a/drivers/mtd/spi/spi_flash.c >> +++ b/drivers/mtd/spi/spi_flash.c >> @@ -274,6 +274,33 @@ int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr) >> return 0; >> } >> >> +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr) >> +{ >> + u8 cmd; >> + int ret; >> + >> + ret = spi_flash_cmd_write_enable(flash); >> + if (ret < 0) { >> + debug("SF: enabling write failed\n"); >> + return ret; >> + } >> + >> + cmd = CMD_WRITE_STATUS; >> + ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &cr, 2); > > Does this assume a particular endianness? Perhaps should put the > values into a byte array instead? Yes it follows the endianness. On my QPP patch, ret = spi_flash_cmd_write_config(flash, 0x0200); where 02 is config and 00 is status register WRR have CMD+status+config => for CMD+16 data format. Let me know if you need any info. Thanks, Jagan. > >> + if (ret) { >> + debug("SF: fail to write config register\n"); >> + return ret; >> + } >> + >> + ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); >> + if (ret < 0) { >> + debug("SF: write config register timed out\n"); >> + return ret; >> + } >> + >> + return 0; >> +} >> + >> /* >> * The following table holds all device probe functions >> * >> diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h >> index 141cfa8..9287778 100644 >> --- a/drivers/mtd/spi/spi_flash_internal.h >> +++ b/drivers/mtd/spi/spi_flash_internal.h >> @@ -77,6 +77,9 @@ static inline int spi_flash_cmd_write_disable(struct spi_flash *flash) >> /* Program the status register. */ >> int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr); >> >> +/* Program the config register. */ >> +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr); >> + >> /* >> * Same as spi_flash_cmd_read() except it also claims/releases the SPI >> * bus. Used as common part of the ->read() operation. >> -- >> 1.7.0.4 >> >> _______________________________________________ >> U-Boot mailing list >> U-Boot@lists.denx.de >> http://lists.denx.de/mailman/listinfo/u-boot > > Regards, > Simon
Hi Jagan, On Wed, Dec 12, 2012 at 7:20 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: > Hi Simon, > > On Wed, Dec 12, 2012 at 12:05 PM, Simon Glass <sjg@chromium.org> wrote: >> Hi, >> >> On Mon, Dec 10, 2012 at 6:42 AM, Jagannadha Sutradharudu Teki >> <jagannadh.teki@gmail.com> wrote: >>> This patch provides support to program a flash config register. >>> >>> Configuration register contains the control bits used to configure >>> the different configurations and security features of a device. >>> >>> User need to set these bits through spi_flash_cmd_write_config() >>> based on their usage. >>> >>> Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> >>> --- >>> drivers/mtd/spi/spi_flash.c | 27 +++++++++++++++++++++++++++ >>> drivers/mtd/spi/spi_flash_internal.h | 3 +++ >>> 2 files changed, 30 insertions(+), 0 deletions(-) >>> >>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c >>> index 800ed8b..a8f0af0 100644 >>> --- a/drivers/mtd/spi/spi_flash.c >>> +++ b/drivers/mtd/spi/spi_flash.c >>> @@ -274,6 +274,33 @@ int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr) >>> return 0; >>> } >>> >>> +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr) >>> +{ >>> + u8 cmd; >>> + int ret; >>> + >>> + ret = spi_flash_cmd_write_enable(flash); >>> + if (ret < 0) { >>> + debug("SF: enabling write failed\n"); >>> + return ret; >>> + } >>> + >>> + cmd = CMD_WRITE_STATUS; >>> + ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &cr, 2); >> >> Does this assume a particular endianness? Perhaps should put the >> values into a byte array instead? > > Yes it follows the endianness. My concern was more that u16 is being used to send two bytes. How about: u8 data[2]; put_unaligned_le16(cr, data) > > On my QPP patch, > ret = spi_flash_cmd_write_config(flash, 0x0200); > > where 02 is config and 00 is status register > WRR have CMD+status+config => for CMD+16 data format. > > Let me know if you need any info. OK. Regards, Simon > > Thanks, > Jagan. > >> >>> + if (ret) { >>> + debug("SF: fail to write config register\n"); >>> + return ret; >>> + } >>> + >>> + ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); >>> + if (ret < 0) { >>> + debug("SF: write config register timed out\n"); >>> + return ret; >>> + } >>> + >>> + return 0; >>> +} >>> + >>> /* >>> * The following table holds all device probe functions >>> * >>> diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h >>> index 141cfa8..9287778 100644 >>> --- a/drivers/mtd/spi/spi_flash_internal.h >>> +++ b/drivers/mtd/spi/spi_flash_internal.h >>> @@ -77,6 +77,9 @@ static inline int spi_flash_cmd_write_disable(struct spi_flash *flash) >>> /* Program the status register. */ >>> int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr); >>> >>> +/* Program the config register. */ >>> +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr); >>> + >>> /* >>> * Same as spi_flash_cmd_read() except it also claims/releases the SPI >>> * bus. Used as common part of the ->read() operation. >>> -- >>> 1.7.0.4 >>> >>> _______________________________________________ >>> U-Boot mailing list >>> U-Boot@lists.denx.de >>> http://lists.denx.de/mailman/listinfo/u-boot >> >> Regards, >> Simon
Hi Simon, On Wed, Dec 12, 2012 at 8:53 PM, Simon Glass <sjg@chromium.org> wrote: > Hi Jagan, > > On Wed, Dec 12, 2012 at 7:20 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: >> Hi Simon, >> >> On Wed, Dec 12, 2012 at 12:05 PM, Simon Glass <sjg@chromium.org> wrote: >>> Hi, >>> >>> On Mon, Dec 10, 2012 at 6:42 AM, Jagannadha Sutradharudu Teki >>> <jagannadh.teki@gmail.com> wrote: >>>> This patch provides support to program a flash config register. >>>> >>>> Configuration register contains the control bits used to configure >>>> the different configurations and security features of a device. >>>> >>>> User need to set these bits through spi_flash_cmd_write_config() >>>> based on their usage. >>>> >>>> Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> >>>> --- >>>> drivers/mtd/spi/spi_flash.c | 27 +++++++++++++++++++++++++++ >>>> drivers/mtd/spi/spi_flash_internal.h | 3 +++ >>>> 2 files changed, 30 insertions(+), 0 deletions(-) >>>> >>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c >>>> index 800ed8b..a8f0af0 100644 >>>> --- a/drivers/mtd/spi/spi_flash.c >>>> +++ b/drivers/mtd/spi/spi_flash.c >>>> @@ -274,6 +274,33 @@ int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr) >>>> return 0; >>>> } >>>> >>>> +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr) >>>> +{ >>>> + u8 cmd; >>>> + int ret; >>>> + >>>> + ret = spi_flash_cmd_write_enable(flash); >>>> + if (ret < 0) { >>>> + debug("SF: enabling write failed\n"); >>>> + return ret; >>>> + } >>>> + >>>> + cmd = CMD_WRITE_STATUS; >>>> + ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &cr, 2); >>> >>> Does this assume a particular endianness? Perhaps should put the >>> values into a byte array instead? >> >> Yes it follows the endianness. > > My concern was more that u16 is being used to send two bytes. How about: > > u8 data[2]; Means I need to send status on data[1] and config on data[0].? > > put_unaligned_le16(cr, data) I couldn't understand this, may be for endianness.? Thanks, Jagan. > >> >> On my QPP patch, >> ret = spi_flash_cmd_write_config(flash, 0x0200); >> >> where 02 is config and 00 is status register >> WRR have CMD+status+config => for CMD+16 data format. >> >> Let me know if you need any info. > > OK. > > Regards, > Simon > >> >> Thanks, >> Jagan. >> >>> >>>> + if (ret) { >>>> + debug("SF: fail to write config register\n"); >>>> + return ret; >>>> + } >>>> + >>>> + ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); >>>> + if (ret < 0) { >>>> + debug("SF: write config register timed out\n"); >>>> + return ret; >>>> + } >>>> + >>>> + return 0; >>>> +} >>>> + >>>> /* >>>> * The following table holds all device probe functions >>>> * >>>> diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h >>>> index 141cfa8..9287778 100644 >>>> --- a/drivers/mtd/spi/spi_flash_internal.h >>>> +++ b/drivers/mtd/spi/spi_flash_internal.h >>>> @@ -77,6 +77,9 @@ static inline int spi_flash_cmd_write_disable(struct spi_flash *flash) >>>> /* Program the status register. */ >>>> int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr); >>>> >>>> +/* Program the config register. */ >>>> +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr); >>>> + >>>> /* >>>> * Same as spi_flash_cmd_read() except it also claims/releases the SPI >>>> * bus. Used as common part of the ->read() operation. >>>> -- >>>> 1.7.0.4 >>>> >>>> _______________________________________________ >>>> U-Boot mailing list >>>> U-Boot@lists.denx.de >>>> http://lists.denx.de/mailman/listinfo/u-boot >>> >>> Regards, >>> Simon
Hi Jagan, On Wed, Dec 12, 2012 at 8:21 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: > Hi Simon, > > On Wed, Dec 12, 2012 at 8:53 PM, Simon Glass <sjg@chromium.org> wrote: >> Hi Jagan, >> >> On Wed, Dec 12, 2012 at 7:20 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: >>> Hi Simon, >>> >>> On Wed, Dec 12, 2012 at 12:05 PM, Simon Glass <sjg@chromium.org> wrote: >>>> Hi, >>>> >>>> On Mon, Dec 10, 2012 at 6:42 AM, Jagannadha Sutradharudu Teki >>>> <jagannadh.teki@gmail.com> wrote: >>>>> This patch provides support to program a flash config register. >>>>> >>>>> Configuration register contains the control bits used to configure >>>>> the different configurations and security features of a device. >>>>> >>>>> User need to set these bits through spi_flash_cmd_write_config() >>>>> based on their usage. >>>>> >>>>> Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> >>>>> --- >>>>> drivers/mtd/spi/spi_flash.c | 27 +++++++++++++++++++++++++++ >>>>> drivers/mtd/spi/spi_flash_internal.h | 3 +++ >>>>> 2 files changed, 30 insertions(+), 0 deletions(-) >>>>> >>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c >>>>> index 800ed8b..a8f0af0 100644 >>>>> --- a/drivers/mtd/spi/spi_flash.c >>>>> +++ b/drivers/mtd/spi/spi_flash.c >>>>> @@ -274,6 +274,33 @@ int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr) >>>>> return 0; >>>>> } >>>>> >>>>> +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr) >>>>> +{ >>>>> + u8 cmd; >>>>> + int ret; >>>>> + >>>>> + ret = spi_flash_cmd_write_enable(flash); >>>>> + if (ret < 0) { >>>>> + debug("SF: enabling write failed\n"); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + cmd = CMD_WRITE_STATUS; >>>>> + ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &cr, 2); >>>> >>>> Does this assume a particular endianness? Perhaps should put the >>>> values into a byte array instead? >>> >>> Yes it follows the endianness. >> >> My concern was more that u16 is being used to send two bytes. How about: >> >> u8 data[2]; > > Means I need to send status on data[1] and config on data[0].? > >> >> put_unaligned_le16(cr, data) > > I couldn't understand this, may be for endianness.? Yes that's right. Just checking that your code will work correctly on a big-endian machine. Will it? It is normally not a good idea to cast a short into a char[2]. Regards, Simon > > Thanks, > Jagan. > >> >>> >>> On my QPP patch, >>> ret = spi_flash_cmd_write_config(flash, 0x0200); >>> >>> where 02 is config and 00 is status register >>> WRR have CMD+status+config => for CMD+16 data format. >>> >>> Let me know if you need any info. >> >> OK. >> >> Regards, >> Simon >> >>> >>> Thanks, >>> Jagan. >>> >>>> >>>>> + if (ret) { >>>>> + debug("SF: fail to write config register\n"); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); >>>>> + if (ret < 0) { >>>>> + debug("SF: write config register timed out\n"); >>>>> + return ret; >>>>> + } >>>>> + >>>>> + return 0; >>>>> +} >>>>> + >>>>> /* >>>>> * The following table holds all device probe functions >>>>> * >>>>> diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h >>>>> index 141cfa8..9287778 100644 >>>>> --- a/drivers/mtd/spi/spi_flash_internal.h >>>>> +++ b/drivers/mtd/spi/spi_flash_internal.h >>>>> @@ -77,6 +77,9 @@ static inline int spi_flash_cmd_write_disable(struct spi_flash *flash) >>>>> /* Program the status register. */ >>>>> int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr); >>>>> >>>>> +/* Program the config register. */ >>>>> +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr); >>>>> + >>>>> /* >>>>> * Same as spi_flash_cmd_read() except it also claims/releases the SPI >>>>> * bus. Used as common part of the ->read() operation. >>>>> -- >>>>> 1.7.0.4 >>>>> >>>>> _______________________________________________ >>>>> U-Boot mailing list >>>>> U-Boot@lists.denx.de >>>>> http://lists.denx.de/mailman/listinfo/u-boot >>>> >>>> Regards, >>>> Simon
Hi Simon, On Thu, Dec 13, 2012 at 4:03 AM, Simon Glass <sjg@chromium.org> wrote: > Hi Jagan, > > On Wed, Dec 12, 2012 at 8:21 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: >> Hi Simon, >> >> On Wed, Dec 12, 2012 at 8:53 PM, Simon Glass <sjg@chromium.org> wrote: >>> Hi Jagan, >>> >>> On Wed, Dec 12, 2012 at 7:20 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: >>>> Hi Simon, >>>> >>>> On Wed, Dec 12, 2012 at 12:05 PM, Simon Glass <sjg@chromium.org> wrote: >>>>> Hi, >>>>> >>>>> On Mon, Dec 10, 2012 at 6:42 AM, Jagannadha Sutradharudu Teki >>>>> <jagannadh.teki@gmail.com> wrote: >>>>>> This patch provides support to program a flash config register. >>>>>> >>>>>> Configuration register contains the control bits used to configure >>>>>> the different configurations and security features of a device. >>>>>> >>>>>> User need to set these bits through spi_flash_cmd_write_config() >>>>>> based on their usage. >>>>>> >>>>>> Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> >>>>>> --- >>>>>> drivers/mtd/spi/spi_flash.c | 27 +++++++++++++++++++++++++++ >>>>>> drivers/mtd/spi/spi_flash_internal.h | 3 +++ >>>>>> 2 files changed, 30 insertions(+), 0 deletions(-) >>>>>> >>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c >>>>>> index 800ed8b..a8f0af0 100644 >>>>>> --- a/drivers/mtd/spi/spi_flash.c >>>>>> +++ b/drivers/mtd/spi/spi_flash.c >>>>>> @@ -274,6 +274,33 @@ int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr) >>>>>> return 0; >>>>>> } >>>>>> >>>>>> +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr) >>>>>> +{ >>>>>> + u8 cmd; >>>>>> + int ret; >>>>>> + >>>>>> + ret = spi_flash_cmd_write_enable(flash); >>>>>> + if (ret < 0) { >>>>>> + debug("SF: enabling write failed\n"); >>>>>> + return ret; >>>>>> + } >>>>>> + >>>>>> + cmd = CMD_WRITE_STATUS; >>>>>> + ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &cr, 2); >>>>> >>>>> Does this assume a particular endianness? Perhaps should put the >>>>> values into a byte array instead? >>>> >>>> Yes it follows the endianness. >>> >>> My concern was more that u16 is being used to send two bytes. How about: >>> >>> u8 data[2]; >> >> Means I need to send status on data[1] and config on data[0].? >> >>> >>> put_unaligned_le16(cr, data) >> >> I couldn't understand this, may be for endianness.? > > Yes that's right. Just checking that your code will work correctly on > a big-endian machine. Will it? It is normally not a good idea to cast > a short into a char[2]. OK. Let me explain my senario here. in below function I am passing 0x0200 spi_flash_cmd_write_config(flash, 0x0200); out of 2-byte data 0x0200 first 00 will pass as a status register value and 0x02 will pass as a config value.it's took the bytes from LSB onwards. Can you please explain how put_unaligned_le16(cr, data will work for this case? Thanks, Jagan. > > Regards, > Simon > >> >> Thanks, >> Jagan. >> >>> >>>> >>>> On my QPP patch, >>>> ret = spi_flash_cmd_write_config(flash, 0x0200); >>>> >>>> where 02 is config and 00 is status register >>>> WRR have CMD+status+config => for CMD+16 data format. >>>> >>>> Let me know if you need any info. >>> >>> OK. >>> >>> Regards, >>> Simon >>> >>>> >>>> Thanks, >>>> Jagan. >>>> >>>>> >>>>>> + if (ret) { >>>>>> + debug("SF: fail to write config register\n"); >>>>>> + return ret; >>>>>> + } >>>>>> + >>>>>> + ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); >>>>>> + if (ret < 0) { >>>>>> + debug("SF: write config register timed out\n"); >>>>>> + return ret; >>>>>> + } >>>>>> + >>>>>> + return 0; >>>>>> +} >>>>>> + >>>>>> /* >>>>>> * The following table holds all device probe functions >>>>>> * >>>>>> diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h >>>>>> index 141cfa8..9287778 100644 >>>>>> --- a/drivers/mtd/spi/spi_flash_internal.h >>>>>> +++ b/drivers/mtd/spi/spi_flash_internal.h >>>>>> @@ -77,6 +77,9 @@ static inline int spi_flash_cmd_write_disable(struct spi_flash *flash) >>>>>> /* Program the status register. */ >>>>>> int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr); >>>>>> >>>>>> +/* Program the config register. */ >>>>>> +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr); >>>>>> + >>>>>> /* >>>>>> * Same as spi_flash_cmd_read() except it also claims/releases the SPI >>>>>> * bus. Used as common part of the ->read() operation. >>>>>> -- >>>>>> 1.7.0.4 >>>>>> >>>>>> _______________________________________________ >>>>>> U-Boot mailing list >>>>>> U-Boot@lists.denx.de >>>>>> http://lists.denx.de/mailman/listinfo/u-boot >>>>> >>>>> Regards, >>>>> Simon
Hi Jagan, On Thu, Dec 13, 2012 at 8:08 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: > Hi Simon, > > On Thu, Dec 13, 2012 at 4:03 AM, Simon Glass <sjg@chromium.org> wrote: >> Hi Jagan, >> >> On Wed, Dec 12, 2012 at 8:21 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: >>> Hi Simon, >>> >>> On Wed, Dec 12, 2012 at 8:53 PM, Simon Glass <sjg@chromium.org> wrote: >>>> Hi Jagan, >>>> >>>> On Wed, Dec 12, 2012 at 7:20 AM, Jagan Teki <jagannadh.teki@gmail.com> wrote: >>>>> Hi Simon, >>>>> >>>>> On Wed, Dec 12, 2012 at 12:05 PM, Simon Glass <sjg@chromium.org> wrote: >>>>>> Hi, >>>>>> >>>>>> On Mon, Dec 10, 2012 at 6:42 AM, Jagannadha Sutradharudu Teki >>>>>> <jagannadh.teki@gmail.com> wrote: >>>>>>> This patch provides support to program a flash config register. >>>>>>> >>>>>>> Configuration register contains the control bits used to configure >>>>>>> the different configurations and security features of a device. >>>>>>> >>>>>>> User need to set these bits through spi_flash_cmd_write_config() >>>>>>> based on their usage. >>>>>>> >>>>>>> Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> >>>>>>> --- >>>>>>> drivers/mtd/spi/spi_flash.c | 27 +++++++++++++++++++++++++++ >>>>>>> drivers/mtd/spi/spi_flash_internal.h | 3 +++ >>>>>>> 2 files changed, 30 insertions(+), 0 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c >>>>>>> index 800ed8b..a8f0af0 100644 >>>>>>> --- a/drivers/mtd/spi/spi_flash.c >>>>>>> +++ b/drivers/mtd/spi/spi_flash.c >>>>>>> @@ -274,6 +274,33 @@ int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr) >>>>>>> return 0; >>>>>>> } >>>>>>> >>>>>>> +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr) >>>>>>> +{ >>>>>>> + u8 cmd; >>>>>>> + int ret; >>>>>>> + >>>>>>> + ret = spi_flash_cmd_write_enable(flash); >>>>>>> + if (ret < 0) { >>>>>>> + debug("SF: enabling write failed\n"); >>>>>>> + return ret; >>>>>>> + } >>>>>>> + >>>>>>> + cmd = CMD_WRITE_STATUS; >>>>>>> + ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &cr, 2); >>>>>> >>>>>> Does this assume a particular endianness? Perhaps should put the >>>>>> values into a byte array instead? >>>>> >>>>> Yes it follows the endianness. >>>> >>>> My concern was more that u16 is being used to send two bytes. How about: >>>> >>>> u8 data[2]; >>> >>> Means I need to send status on data[1] and config on data[0].? >>> >>>> >>>> put_unaligned_le16(cr, data) >>> >>> I couldn't understand this, may be for endianness.? >> >> Yes that's right. Just checking that your code will work correctly on >> a big-endian machine. Will it? It is normally not a good idea to cast >> a short into a char[2]. > > OK. > > Let me explain my senario here. > in below function I am passing 0x0200 > spi_flash_cmd_write_config(flash, 0x0200); > > out of 2-byte data 0x0200 first 00 will pass as a status register value > and 0x02 will pass as a config value.it's took the bytes from LSB > onwards. > > Can you please explain how put_unaligned_le16(cr, data will work > for this case? Possibly... The prototype is: int spi_flash_cmd_write(struct spi_slave *spi, const u8 *cmd, size_t cmd_len, const void *data, size_t data_len); You are doing: u16 value; ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &cr, 2); Passing a u16 cast to a u8 * is probably not endian safe. Instead I think you should do: u8 cr[2]; (get bytes into cr) ret = spi_flash_cmd_write(flash->spi, &cmd, 1, cr, 2); Regards, Simon > > Thanks, > Jagan. > >> >> Regards, >> Simon >> >>> >>> Thanks, >>> Jagan. >>> >>>> >>>>> >>>>> On my QPP patch, >>>>> ret = spi_flash_cmd_write_config(flash, 0x0200); >>>>> >>>>> where 02 is config and 00 is status register >>>>> WRR have CMD+status+config => for CMD+16 data format. >>>>> >>>>> Let me know if you need any info. >>>> >>>> OK. >>>> >>>> Regards, >>>> Simon >>>> >>>>> >>>>> Thanks, >>>>> Jagan. >>>>> >>>>>> >>>>>>> + if (ret) { >>>>>>> + debug("SF: fail to write config register\n"); >>>>>>> + return ret; >>>>>>> + } >>>>>>> + >>>>>>> + ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); >>>>>>> + if (ret < 0) { >>>>>>> + debug("SF: write config register timed out\n"); >>>>>>> + return ret; >>>>>>> + } >>>>>>> + >>>>>>> + return 0; >>>>>>> +} >>>>>>> + >>>>>>> /* >>>>>>> * The following table holds all device probe functions >>>>>>> * >>>>>>> diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h >>>>>>> index 141cfa8..9287778 100644 >>>>>>> --- a/drivers/mtd/spi/spi_flash_internal.h >>>>>>> +++ b/drivers/mtd/spi/spi_flash_internal.h >>>>>>> @@ -77,6 +77,9 @@ static inline int spi_flash_cmd_write_disable(struct spi_flash *flash) >>>>>>> /* Program the status register. */ >>>>>>> int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr); >>>>>>> >>>>>>> +/* Program the config register. */ >>>>>>> +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr); >>>>>>> + >>>>>>> /* >>>>>>> * Same as spi_flash_cmd_read() except it also claims/releases the SPI >>>>>>> * bus. Used as common part of the ->read() operation. >>>>>>> -- >>>>>>> 1.7.0.4 >>>>>>> >>>>>>> _______________________________________________ >>>>>>> U-Boot mailing list >>>>>>> U-Boot@lists.denx.de >>>>>>> http://lists.denx.de/mailman/listinfo/u-boot >>>>>> >>>>>> Regards, >>>>>> Simon
diff --git a/drivers/mtd/spi/spi_flash.c b/drivers/mtd/spi/spi_flash.c index 800ed8b..a8f0af0 100644 --- a/drivers/mtd/spi/spi_flash.c +++ b/drivers/mtd/spi/spi_flash.c @@ -274,6 +274,33 @@ int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr) return 0; } +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr) +{ + u8 cmd; + int ret; + + ret = spi_flash_cmd_write_enable(flash); + if (ret < 0) { + debug("SF: enabling write failed\n"); + return ret; + } + + cmd = CMD_WRITE_STATUS; + ret = spi_flash_cmd_write(flash->spi, &cmd, 1, &cr, 2); + if (ret) { + debug("SF: fail to write config register\n"); + return ret; + } + + ret = spi_flash_cmd_wait_ready(flash, SPI_FLASH_PROG_TIMEOUT); + if (ret < 0) { + debug("SF: write config register timed out\n"); + return ret; + } + + return 0; +} + /* * The following table holds all device probe functions * diff --git a/drivers/mtd/spi/spi_flash_internal.h b/drivers/mtd/spi/spi_flash_internal.h index 141cfa8..9287778 100644 --- a/drivers/mtd/spi/spi_flash_internal.h +++ b/drivers/mtd/spi/spi_flash_internal.h @@ -77,6 +77,9 @@ static inline int spi_flash_cmd_write_disable(struct spi_flash *flash) /* Program the status register. */ int spi_flash_cmd_write_status(struct spi_flash *flash, u8 sr); +/* Program the config register. */ +int spi_flash_cmd_write_config(struct spi_flash *flash, u16 cr); + /* * Same as spi_flash_cmd_read() except it also claims/releases the SPI * bus. Used as common part of the ->read() operation.
This patch provides support to program a flash config register. Configuration register contains the control bits used to configure the different configurations and security features of a device. User need to set these bits through spi_flash_cmd_write_config() based on their usage. Signed-off-by: Jagannadha Sutradharudu Teki <jagannadh.teki@gmail.com> --- drivers/mtd/spi/spi_flash.c | 27 +++++++++++++++++++++++++++ drivers/mtd/spi/spi_flash_internal.h | 3 +++ 2 files changed, 30 insertions(+), 0 deletions(-)