Re: [RESEND PATCH] board: ti: common: board_detect: Fix EEPROM offset read for 1-byte

2023-10-26 Thread Prasanth Mantena
On 13:05-20231020, Nishanth Menon wrote:
> On 20:52-20231020, Kumar, Udit wrote:
> > Thanks Prasanth
> > 
> > On 10/20/2023 12:31 PM, Prasanth Babu Mantena wrote:
> > > EEPROM detection logic in ti_i2c_eeprom_get() involves reading the total
> > > size followed by reading 1-byte size with an offset 1. This commit fixes
> > > the header matching issue in commit 9f393a2d7af8 ("board: ti: common:
> > > board_detect: Fix EEPROM read quirk for 2-byte").
> > You can fixes below as well. I think you can avoid this in commit message
> > > 
> > > In the previous commit, the value with one offset is being read into
> > > offset_test, but the pointer used to match was still ep. After reading
> > > with an offset 1, the second byte of the header is compared with the 
> > > 1-byte
> > > data read from EEPROM. This is taken care by comparing proper first byte
> > > value from the header.
> > Nice catch
> 
> Yes indeed.. here is a possible improvement of  commit message:
> 
> EEPROM detection logic in ti_i2c_eeprom_get() involves reading the
> total size and the 1-byte size with an offset 1.
> 
> The value with one offset is read into "offset_test," but the pointer
> used to match was still "ep". This results in comparing results
> against the previously read data while the intent is to ensure the
> detection of the bad 2-byte addressing eeproms that get stuck after
> responding initially to 1- byte accesses. This behavior was detected
> on BeagleBone-AI64.
> 
> > > 
> > > Signed-off-by: Prasanth Babu Mantena 
> > > Fixes: 9f393a2d7af8 (board: ti: common: board_detect: Fix EEPROM read 
> > > quirk for 2-byte)
> > Please consider Fixes line, first than Signed-off-by
> 
> Yup, fixes and SoB (checkpatch typically warns)
> > 
> > 
> > Please copy Nishanth in patch as well .
> > 
> > > ---
> > > Resending due to incorrect patch tag last time.
> > > 
> > >   board/ti/common/board_detect.c | 2 +-
> > >   1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/board/ti/common/board_detect.c 
> > > b/board/ti/common/board_detect.c
> > > index 9a53884c98..17fe8f8069 100644
> > > --- a/board/ti/common/board_detect.c
> > > +++ b/board/ti/common/board_detect.c
> > > @@ -128,7 +128,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int 
> > > bus_addr, int dev_addr,
> > >   rc = dm_i2c_read(dev, 0x1, &offset_test, sizeof(offset_test));
> > > - if (*((u32 *)ep) != (header & 0xFF))
> > > + if (offset_test != ((header >> 8) & 0xFF))
> > >   one_byte_addressing = false;
> > >   /* Corrupted data??? */
> 
> 
> You missed the else CONFIG_IS_ENABLED(DM_I2C)  case with the same
> bug?
> 
> Also please CC Matwey V. Kornilov  ,  Jason
> Kridner  and Robert Nelson 
> 
> So that we don't have regressions on their boards.
> 
> https://lore.kernel.org/all/CAJs94Ebdd4foOjhGFu9Bop0v=b1us9nedlxfhgcy23ukglz...@mail.gmail.com/

Thanks for the comments Nishanth.
Will send a v2 addressing them.

Regards,
Prasanth

> 
> -- 
> Regards,
> Nishanth Menon
> Key (0xDDB5849D1736249D) / Fingerprint: F8A2 8693 54EB 8232 17A3  1A34 DDB5 
> 849D 1736 249D


Re: [RESEND PATCH] board: ti: common: board_detect: Fix EEPROM offset read for 1-byte

2023-10-26 Thread Prasanth Mantena
On 20:52-20231020, Kumar, Udit wrote:
> Thanks Prasanth
> 
> On 10/20/2023 12:31 PM, Prasanth Babu Mantena wrote:
> > EEPROM detection logic in ti_i2c_eeprom_get() involves reading the total
> > size followed by reading 1-byte size with an offset 1. This commit fixes
> > the header matching issue in commit 9f393a2d7af8 ("board: ti: common:
> > board_detect: Fix EEPROM read quirk for 2-byte").
> You can fixes below as well. I think you can avoid this in commit message
> > 
> > In the previous commit, the value with one offset is being read into
> > offset_test, but the pointer used to match was still ep. After reading
> > with an offset 1, the second byte of the header is compared with the 1-byte
> > data read from EEPROM. This is taken care by comparing proper first byte
> > value from the header.
> Nice catch
> > 
> > Signed-off-by: Prasanth Babu Mantena 
> > Fixes: 9f393a2d7af8 (board: ti: common: board_detect: Fix EEPROM read quirk 
> > for 2-byte)
> Please consider Fixes line, first than Signed-off-by
> 
> 
> Please copy Nishanth in patch as well .
> 
Thanks for the review comments Udit.
Will send a v2 addressing them.

Regards,
Prasanth

> > ---
> > Resending due to incorrect patch tag last time.
> > 
> >   board/ti/common/board_detect.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
> > index 9a53884c98..17fe8f8069 100644
> > --- a/board/ti/common/board_detect.c
> > +++ b/board/ti/common/board_detect.c
> > @@ -128,7 +128,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int 
> > bus_addr, int dev_addr,
> > rc = dm_i2c_read(dev, 0x1, &offset_test, sizeof(offset_test));
> > -   if (*((u32 *)ep) != (header & 0xFF))
> > +   if (offset_test != ((header >> 8) & 0xFF))
> > one_byte_addressing = false;
> > /* Corrupted data??? */


Re: [PATCH v3] board: ti: common: board_detect: Fix EEPROM offset read for 1-byte

2023-12-14 Thread Prasanth Mantena
On 15:15-20231107, Neha Malcom Francis wrote:
> Hi Prasanth,
> 
> On 30/10/23 22:34, Prasanth Babu Mantena wrote:
> > EEPROM detection logic in ti_i2c_eeprom_get() involves reading
> > the total size and the 1-byte size with an offset 1. The commit
> > 9f393a2d7af8 ("board: ti: common: board_detect: Fix EEPROM read
> > quirk for 2-byte") that attempts to fix this uses a wrong pointer to
> > compare.
> > 
> > The value with one offset is read into offset_test, but the pointer
> > used to match was still ep, resulting in an invalid comparison of the
> > values. The intent is to identify bad 2-byte addressing eeproms that
> > get stuck on the successive reads.
> > 
> > Fixes: 9f393a2d7af8 (board: ti: common: board_detect: Fix EEPROM read quirk 
> > for 2-byte)
> > Signed-off-by: Prasanth Babu Mantena 
> > Tested-by: Matwey V. Kornilov 
> > ---
> > v3 <--> v2:
> > Improved and concise commit description.
> > 
> > v2 <--> v1:
> > Fix inplace for the else condition of CONFIG_IS_ENABLED(DM_I2C).
> > Improved commit message.
> > 
> >   board/ti/common/board_detect.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/board/ti/common/board_detect.c b/board/ti/common/board_detect.c
> > index 9a53884c98..869f7a47f8 100644
> > --- a/board/ti/common/board_detect.c
> > +++ b/board/ti/common/board_detect.c
> > @@ -128,7 +128,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int 
> > bus_addr, int dev_addr,
> > rc = dm_i2c_read(dev, 0x1, &offset_test, sizeof(offset_test));
> > -   if (*((u32 *)ep) != (header & 0xFF))
> > +   if (offset_test != ((header >> 8) & 0xFF))
> > one_byte_addressing = false;
> > /* Corrupted data??? */
> > @@ -180,7 +180,7 @@ static int __maybe_unused ti_i2c_eeprom_get(int 
> > bus_addr, int dev_addr,
> > rc = i2c_read(dev_addr, 0x1, byte, &offset_test, sizeof(offset_test));
> > -   if (*((u32 *)ep) != (header & 0xFF))
> > +   if (offset_test != ((header >> 8) & 0xFF))
> > one_byte_addressing = false;
> > /* Corrupted data??? */
> 
> Reviewed-by: Neha Malcom Francis 
> 

Can this patch be considered for merge considering no further comments.

Regards,
Prasanth



> -- 
> Thanking You
> Neha Malcom Francis


Re: [PATCH v2 0/4] Cleanup dma device in spl and move dma channel[0]

2024-10-14 Thread Prasanth Mantena
On 23:10, Peter Robinson wrote:
Hi Peter,

> Hi Prasanth,
> 
> > The channel allocation and deallocation for dma copy was happening on every
> > dma transfer. This is a overhead for transactions like NAND, which does
> > page reads recursively for complete data.
> >
> > So, moving the dma allocation to probe and implement corresponding
> > remove function and cleanup dma device while exiting from spl.
> >
> > Enable SPL_DM_DEVICE_REMOVE, for device removal capability in SPL.
> >
> > v2 <==> v1
> > ==
> > 1> Improve subject of commit.
> > 2> Add warning on dma device not found.
> > 3> Do channel allocation only for udma and bcdma. Passthrough for all other 
> > types.
> >
> > Prasanth Babu Mantena (2):
> >   mach-k3: common.c: Remove dma device in spl exit
> >   configs: k3: Enable device removal in SPL
> >
> > Santhosh Kumar K (2):
> >   dma: ti: k3-udma: Move udma_probe() below all APIs
> >   dma: ti: k3-udma: Move DMA channel[0] allocation to probe and add
> > udma_remove()
> >
> >  arch/arm/mach-k3/common.c|  25 ++-
> >  configs/am62ax_evm_a53_defconfig |   1 +
> >  configs/am62ax_evm_r5_defconfig  |   1 +
> >  configs/am62x_evm_a53_defconfig  |   1 +
> >  configs/am62x_evm_r5_defconfig   |   1 +
> >  configs/j7200_evm_a72_defconfig  |   1 +
> >  configs/j7200_evm_r5_defconfig   |   1 +
> >  configs/j721e_evm_a72_defconfig  |   1 +
> >  configs/j721e_evm_r5_defconfig   |   1 +
> >  configs/j721s2_evm_a72_defconfig |   1 +
> >  configs/j721s2_evm_r5_defconfig  |   1 +
> >  configs/j784s4_evm_a72_defconfig |   1 +
> >  configs/j784s4_evm_r5_defconfig  |   1 +
> 
> Is there any reason why devices like j721e_beagleboneai64 and
> m62x_beagleplay and other TI devices aren't changed as part of this
> change?

We donot have DMA enabled in SPL for these devices, as a result of which
doesn't require removal of them at spl exit in current context. We donot
have any onboard flashes on beagle boards, and hence DMA is not enabled in
SPL for these boards, which is generally required for flash boot.

Thanks
Prasanth

> 
> Peter


Re: [PATCH 1/4] mach-k3: common.c: Add dma device remove in spl exit

2024-10-06 Thread Prasanth Mantena
On 20:09-20241004, Kumar, Udit wrote:
Hi Udit,
> Hi Prasant,
> 
> Thanks for series,
> 
> Could we update the subject of patch something like
> 
> Remove dma device in spl exit, Sorry but
> 
> Add dma device remove in spl exit looks little confusing , are we adding or
> removing ?

Understood. Should've been more simpler like "Remove dma device in spl
exit". Will update in next version.

> 
> 
> On 10/4/2024 6:50 PM, Prasanth Babu Mantena wrote:
> > While exiting from spl, remove any dma device active through
> > spl_board_prepare_for_boot(). This is required for cleaning up
> > any dma channels being used in spl and avoid issues with overlapping
> > channel allocation in the next stage bootloaders.
> > 
> > Signed-off-by: Prasanth Babu Mantena 
> > ---
> >   arch/arm/mach-k3/common.c | 23 ++-
> >   1 file changed, 22 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm/mach-k3/common.c b/arch/arm/mach-k3/common.c
> > index df48ec8d47..982dc76b00 100644
> > --- a/arch/arm/mach-k3/common.c
> > +++ b/arch/arm/mach-k3/common.c
> > @@ -28,6 +28,8 @@
> >   #include 
> >   #include 
> >   #include 
> > +#include 
> > +#include 
> >   #include 
> > @@ -246,12 +248,31 @@ void spl_enable_cache(void)
> >   #endif
> >   }
> > -#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
> > +static __maybe_unused void k3_dma_remove(void)
> > +{
> > +   struct udevice *dev;
> > +   int rc;
> > +
> > +   rc = uclass_find_device(UCLASS_DMA, 0, &dev);
> 
> Are you thinking to have DMA device optional ?

Not really. Sorry, but didn't understand the question completely here.
> 
> Second question, if we enabled multiple DMAs in u-boot, then would you like
> to remove all ?

Considering this function is specific to k3, here we have channel
allocation done in probe and there is a need to clean that channel
resources which has been done in remove function in this series, call
that remove function here. If there are such allocations done for other
dma devices as well and have their remove functions implemented, then
they should consider adding those as well here. I don't think there is
any such case with other dmas.

> 
> 
> > +   if (!rc && dev) {
> > +   rc = device_remove(dev, DM_REMOVE_NORMAL);
> > +   if (rc)
> > +   pr_warn("Cannot remove dma device '%s' (err=%d)\n",
> > +   dev->name, rc);
> > +   }
> > +}
> > +
> >   void spl_board_prepare_for_boot(void)
> >   {
> > +#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
> > dcache_disable();
> > +#endif
> 
> Please suggest , why we are protecting dcache_disable with flags ?

This is carry forward which was already present. This patch removes
the protection from whole spl_board_prepare_for_boot() function to
dcache_disable() only. This alone looks like it has been added in this
patch, but if we see the whole snippet, it can be understood well.

Thanks,
Prasanth
> 
> 
> > +#if IS_ENABLED(CONFIG_SPL_DMA) && IS_ENABLED(CONFIG_SPL_DM_DEVICE_REMOVE)
> > +   k3_dma_remove();
> > +#endif
> >   }
> > +#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
> >   void spl_board_prepare_for_linux(void)
> >   {
> > dcache_disable();


Re: [PATCH 1/4] mach-k3: common.c: Add dma device remove in spl exit

2024-10-07 Thread Prasanth Mantena
On 09:59, Kumar, Udit wrote:
> Hi Prasanth,
> 
> On 10/6/2024 2:26 PM, Prasanth Mantena wrote:
> > On 20:09-20241004, Kumar, Udit wrote:
> > Hi Udit,
> > > Hi Prasant,
> > > 
> > > Thanks for series,
> > > 
> > > Could we update the subject of patch something like
> > > 
> > > Remove dma device in spl exit, Sorry but
> > > 
> > > Add dma device remove in spl exit looks little confusing , are we adding 
> > > or
> > > removing ?
> > Understood. Should've been more simpler like "Remove dma device in spl
> > exit". Will update in next version.
> > > 
> > > On 10/4/2024 6:50 PM, Prasanth Babu Mantena wrote:
> > > > [..]
> > > > Are you thinking to have DMA device optional ?
> > Not really. Sorry, but didn't understand the question completely here.
> 
> + rc = uclass_find_device(UCLASS_DMA, 0, &dev);
> + if (!rc && dev) {
> 
> 
> I meant , here if there is some return code or device is not found then
> at user level there is no notification.
> May be adding some error print in case either some return code or device is 
> NULL
> will help to user know, that DMA was not cleaned properly.

Will check and add an error notification here.

> 
> > > Second question, if we enabled multiple DMAs in u-boot, then would you 
> > > like
> > > to remove all ?
> > Considering this function is specific to k3, here we have channel
> > allocation done in probe and there is a need to clean that channel
> > resources which has been done in remove function in this series, call
> > that remove function here. If there are such allocations done for other
> > dma devices as well and have their remove functions implemented, then
> > they should consider adding those as well here. I don't think there is
> > any such case with other dmas.
> 
> Understood this is k3 specific , But here i meant multiple dma devices like
> udma, pdma etc.

In general there will only one dma either udma(flash boot) or pdma(ethboot)
being used in spl at boot time to load the next stage images. There is
no case where multiple dmas work at same time atleast for now. If such
case comes in future and need a remove, it should be taken care.

Thanks,
Prasanth

> 
> 
> > 
> > > 
> > > > +   if (!rc && dev) {
> > > > +   rc = device_remove(dev, DM_REMOVE_NORMAL);
> > > > +   if (rc)
> > > > +   pr_warn("Cannot remove dma device '%s' 
> > > > (err=%d)\n",
> > > > +   dev->name, rc);
> > > > +   }
> > > > +}
> > > > +
> > > >void spl_board_prepare_for_boot(void)
> > > >{
> > > > +#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
> > > > dcache_disable();
> > > > +#endif
> > > Please suggest , why we are protecting dcache_disable with flags ?
> > This is carry forward which was already present. This patch removes
> > the protection from whole spl_board_prepare_for_boot() function to
> > dcache_disable() only. This alone looks like it has been added in this
> > patch, but if we see the whole snippet, it can be understood well.
> 
> 
> Thanks
> 
> > 
> > Thanks,
> > Prasanth
> > > 
> > > > +#if IS_ENABLED(CONFIG_SPL_DMA) && 
> > > > IS_ENABLED(CONFIG_SPL_DM_DEVICE_REMOVE)
> > > > +   k3_dma_remove();
> > > > +#endif
> > > >}
> > > > +#if !(defined(CONFIG_SYS_ICACHE_OFF) && defined(CONFIG_SYS_DCACHE_OFF))
> > > >void spl_board_prepare_for_linux(void)
> > > >{
> > > > dcache_disable();


Re: [PATCH] mtd: spi-nor: Enable mt35xu512aba_fixups for all mt35xx flashes

2024-11-25 Thread Prasanth Mantena
On 14:56, Venkatesh Yadav Abbarapu wrote:
Hi Venkatesh, 
> Enable mt35xu512aba_fixups for all mt35 series flashes to work
> in DTR mode, and return after nor->fixups is updated, otherwise
> it will get overwritten with macronix_octal_fixups.
> This flash works in DTR mode only if CONFIG_SPI_FLASH_MT35XU
> is enabled and SPI_NOR_OCTAL_DTR_READ flag is set in id table.
> 
> Additionally, a new flag, "SPI_XFER_SET_DDR", has been introduced
> to instruct the OSPI controller driver to switch to DDR mode.
> 
> Signed-off-by: Ashok Reddy Soma 
> Signed-off-by: Venkatesh Yadav Abbarapu 
> ---
>  drivers/mtd/spi/spi-nor-core.c | 8 +++-
>  include/spi.h  | 1 +
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index ec841fb13b..8d201433d5 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -4073,6 +4073,7 @@ static int spi_nor_micron_octal_dtr_enable(struct 
> spi_nor *nor)
>   if (ret)
>   return ret;
>  
> + nor->spi->flags |= SPI_XFER_SET_DDR;
>   buf = SPINOR_MT_OCT_DTR;
>   op = (struct spi_mem_op)
>   SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_MT_WR_ANY_REG, 1),
> @@ -4404,8 +4405,13 @@ void spi_nor_set_fixups(struct spi_nor *nor)
>  #endif
>  
>  #ifdef CONFIG_SPI_FLASH_MT35XU
> - if (!strcmp(nor->info->name, "mt35xu512aba"))
> + if (!strcmp(nor->info->name, "mt35xu512aba") ||
> + !strcmp(nor->info->name, "mt35xl512aba") ||
> + !strcmp(nor->info->name, "mt35xu01g") ||
> + !strcmp(nor->info->name, "mt35xu02g")) {
>   nor->fixups = &mt35xu512aba_fixups;
> + return;
> + }
>  #endif
>  
>  #if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX)
> diff --git a/include/spi.h b/include/spi.h
> index 6944773b59..d7fef36662 100644
> --- a/include/spi.h
> +++ b/include/spi.h
> @@ -164,6 +164,7 @@ struct spi_slave {
>  #define SPI_XFER_U_PAGE  BIT(4)
>  #define SPI_XFER_STACKED BIT(5)
>  #define SPI_XFER_LOWER   BIT(6)
> +#define SPI_XFER_SET_DDR BIT(7)

Are we using this anywhere in the Controller driver ? What is the
significance of this flag, when we can send the DDR info through
the ops.

>  
>   /*
>* Flag indicating that the spi-controller has multi chip select
> -- 
> 2.25.1
> 
> 


Re: [PATCH] mtd: spi-nor: Send write disable cmd after every write enable

2024-11-19 Thread Prasanth Mantena
On 12:09, Venkatesh Yadav Abbarapu wrote:
> Write enable(06h) command will be sent to a flash device to
> set the write enable latch bit before every program, erase,
> write command. After that write disable command (04h) needs
> to be sent to clear the write enable latch.
> 
> This write_disable() is missing at the majority of the places
> in the driver, add it to clear write enable latch.
> 
> Signed-off-by: Ashok Reddy Soma 
> Signed-off-by: Venkatesh Yadav Abbarapu 
> ---
>  drivers/mtd/spi/spi-nor-core.c | 23 ---
>  1 file changed, 20 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index ec841fb13b..5977c634c2 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -944,6 +944,7 @@ static int spi_nor_erase_chip_wait_till_ready(struct 
> spi_nor *nor, unsigned long
>  static int clean_bar(struct spi_nor *nor)
>  {
>   u8 cmd, bank_sel = 0;
> + int ret;
>  
>   if (nor->bank_curr == 0)
>   return 0;
> @@ -951,7 +952,11 @@ static int clean_bar(struct spi_nor *nor)
>   nor->bank_curr = 0;
>   write_enable(nor);
>  
> - return nor->write_reg(nor, cmd, &bank_sel, 1);
> + ret = nor->write_reg(nor, cmd, &bank_sel, 1);
> + if (ret)
> + return ret;
> +
> + return write_disable(nor);
>  }
>  
>  static int write_bar(struct spi_nor *nor, u32 offset)
> @@ -1270,6 +1275,10 @@ static int write_sr_and_check(struct spi_nor *nor, u8 
> status_new, u8 mask)
>   if (ret)
>   return ret;
>  
> + ret = write_disable(nor);
> + if (ret)
> + return ret;
> +
>   ret = read_sr(nor);
>   if (ret < 0)
>   return ret;
> @@ -1796,13 +1805,18 @@ static int sst26_lock_ctl(struct spi_nor *nor, loff_t 
> ofs, uint64_t len, enum lo
>   if (ctl == SST26_CTL_CHECK)
>   return 0;
>  
> + /* Write latch enable before write operation */
> + ret = write_enable(nor);
> + if (ret)
> + return ret;
> +
>   ret = nor->write_reg(nor, SPINOR_OP_WRITE_BPR, bpr_buff, bpr_size);
>   if (ret < 0) {
>   dev_err(nor->dev, "fail to write block-protection register\n");
>   return ret;
>   }
>  
> - return 0;
> + return write_disable(nor);
>  }
>  
>  static int sst26_unlock(struct spi_nor *nor, loff_t ofs, uint64_t len)
> @@ -2204,7 +2218,7 @@ static int write_sr_cr(struct spi_nor *nor, u8 *sr_cr)
>   return ret;
>   }
>  
> - return 0;
> + return write_disable(nor);
>  }
>  
>  /**
> @@ -4273,6 +4287,9 @@ static int spi_nor_init(struct spi_nor *nor)
>   write_disable(nor);
>   }
>   }
> + err = write_disable(nor);
> + if (err)
> + return err;
>   }
>  
>   if (nor->quad_enable) {
> -- 
> 2.17.1
> 
>
Tested on j721s2-evm. 
Link: 
https://gist.github.com/PrasanthBabuMantena/c12f39744de188a9d08cd5ca51dc2a7b

Tested-by: Prasanth Babu Mantena 


Re: [PATCH] mtd: spi-nor: Enable mt35xu512aba_fixups for all mt35xx flashes

2024-11-25 Thread Prasanth Mantena
On 09:12, Abbarapu, Venkatesh wrote:
Hi Venkatesh,

> Hi Prasanth,
> 
> > -Original Message-
> > From: Prasanth Mantena 
> > Sent: Monday, November 25, 2024 2:19 PM
> > To: Abbarapu, Venkatesh 
> > Cc: u-boot@lists.denx.de; j-humphr...@ti.com; Simek, Michal
> > ; ja...@amarulasolutions.com; vigne...@ti.com; u-
> > kum...@ti.com; tr...@konsulko.com; sean...@gmail.com;
> > caleb.conno...@linaro.org; s...@chromium.org; william.zh...@broadcom.com;
> > stefa...@posteo.net; quentin.sch...@cherry.de; tudor.amba...@linaro.org;
> > takahiro.kuw...@infineon.com; git (AMD-Xilinx) ; Ashok Reddy
> > Soma 
> > Subject: Re: [PATCH] mtd: spi-nor: Enable mt35xu512aba_fixups for all mt35xx
> > flashes
> > 
> > On 14:56, Venkatesh Yadav Abbarapu wrote:
> > Hi Venkatesh,
> > > Enable mt35xu512aba_fixups for all mt35 series flashes to work in DTR
> > > mode, and return after nor->fixups is updated, otherwise it will get
> > > overwritten with macronix_octal_fixups.
> > > This flash works in DTR mode only if CONFIG_SPI_FLASH_MT35XU is
> > > enabled and SPI_NOR_OCTAL_DTR_READ flag is set in id table.
> > >
> > > Additionally, a new flag, "SPI_XFER_SET_DDR", has been introduced to
> > > instruct the OSPI controller driver to switch to DDR mode.
> > >
> > > Signed-off-by: Ashok Reddy Soma 
> > > Signed-off-by: Venkatesh Yadav Abbarapu 
> > > ---
> > >  drivers/mtd/spi/spi-nor-core.c | 8 +++-
> > >  include/spi.h  | 1 +
> > >  2 files changed, 8 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/mtd/spi/spi-nor-core.c
> > > b/drivers/mtd/spi/spi-nor-core.c index ec841fb13b..8d201433d5 100644
> > > --- a/drivers/mtd/spi/spi-nor-core.c
> > > +++ b/drivers/mtd/spi/spi-nor-core.c
> > > @@ -4073,6 +4073,7 @@ static int spi_nor_micron_octal_dtr_enable(struct
> > spi_nor *nor)
> > >   if (ret)
> > >   return ret;
> > >
> > > + nor->spi->flags |= SPI_XFER_SET_DDR;
> > >   buf = SPINOR_MT_OCT_DTR;
> > >   op = (struct spi_mem_op)
> > >
> > SPI_MEM_OP(SPI_MEM_OP_CMD(SPINOR_OP_MT_WR_ANY_REG, 1),
> > @@ -4404,8
> > > +4405,13 @@ void spi_nor_set_fixups(struct spi_nor *nor)  #endif
> > >
> > >  #ifdef CONFIG_SPI_FLASH_MT35XU
> > > - if (!strcmp(nor->info->name, "mt35xu512aba"))
> > > + if (!strcmp(nor->info->name, "mt35xu512aba") ||
> > > + !strcmp(nor->info->name, "mt35xl512aba") ||
> > > + !strcmp(nor->info->name, "mt35xu01g") ||
> > > + !strcmp(nor->info->name, "mt35xu02g")) {
> > >   nor->fixups = &mt35xu512aba_fixups;
> > > + return;
> > > + }
> > >  #endif
> > >
> > >  #if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX)
> > > diff --git a/include/spi.h b/include/spi.h index
> > > 6944773b59..d7fef36662 100644
> > > --- a/include/spi.h
> > > +++ b/include/spi.h
> > > @@ -164,6 +164,7 @@ struct spi_slave {
> > >  #define SPI_XFER_U_PAGE  BIT(4)
> > >  #define SPI_XFER_STACKED BIT(5)
> > >  #define SPI_XFER_LOWER   BIT(6)
> > > +#define SPI_XFER_SET_DDR BIT(7)
> > 
> > Are we using this anywhere in the Controller driver ? What is the 
> > significance of this
> > flag, when we can send the DDR info through the ops.
> Yes...we will be checking this 
> If (spi->flags & SPI_XFER_SET_DDR) && op->cmd.dtr) then 
>   setup the DDR mode in controller driver.

Is this change going to be for every read/write exec op or in some init
function. Current driver has this setup dtr mode on checking the opcode.
Curious to know, what does this extra flag bring something new to the driver.

Prasanth

> 
> I need to send the controller change series.
> 
> Thanks
> Venkatesh
> > 
> > >
> > >   /*
> > >* Flag indicating that the spi-controller has multi chip select
> > > --
> > > 2.25.1
> > >
> > >


Re: [PATCH v2] mtd: spi-nor: Enable mt35xu512aba_fixups for all mt35xx flashes

2024-12-03 Thread Prasanth Mantena
On 15:52, Venkatesh Yadav Abbarapu wrote:
> Enable mt35xu512aba_fixups for all mt35 series flashes to work
> in DTR mode, and return after nor->fixups is updated, otherwise
> it will get overwritten with macronix_octal_fixups.
> This flash works in DTR mode only if CONFIG_SPI_FLASH_MT35XU
> is enabled and SPI_NOR_OCTAL_DTR_READ flag is set in id table.
> 
> Signed-off-by: Ashok Reddy Soma 
> Signed-off-by: Venkatesh Yadav Abbarapu 
> ---
> Changes in v2:
> - Removed the SPI_XFER_SET_DDR flag.
> ---
>  drivers/mtd/spi/spi-nor-core.c | 7 ++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/mtd/spi/spi-nor-core.c b/drivers/mtd/spi/spi-nor-core.c
> index ec841fb13b..96f749f7a8 100644
> --- a/drivers/mtd/spi/spi-nor-core.c
> +++ b/drivers/mtd/spi/spi-nor-core.c
> @@ -4404,8 +4404,13 @@ void spi_nor_set_fixups(struct spi_nor *nor)
>  #endif
>  
>  #ifdef CONFIG_SPI_FLASH_MT35XU
> - if (!strcmp(nor->info->name, "mt35xu512aba"))
> + if (!strcmp(nor->info->name, "mt35xu512aba") ||
> + !strcmp(nor->info->name, "mt35xl512aba") ||
> + !strcmp(nor->info->name, "mt35xu01g") ||
> + !strcmp(nor->info->name, "mt35xu02g")) {
>   nor->fixups = &mt35xu512aba_fixups;
> + return;
> + }
>  #endif
>  
>  #if CONFIG_IS_ENABLED(SPI_FLASH_MACRONIX)
> -- 
> 2.34.1
> 

Reviewed-by: Prasanth Babu Mantena 


Re: [PATCH 1/4] mailbox: k3-sec-proxy: Add DM to DMSC communication thread for J722S

2024-12-18 Thread Prasanth Mantena
On 21:01, Kumar, Udit wrote:
Hi Udit,
> 
> On 12/10/2024 7:27 PM, Prasanth Babu Mantena wrote:
> > From: Vaishnav Achath 
> > 
> > J722S R5 SPL uses sec-proxy threads 28 and 29 for communication with
> > TIFS. Mark these as valid threads in the driver.
> 
> Please mention doc link also,
> 
> Also is this only for R5 SPL
> 

Yes this is only for R5, as DM is not active on dmsc, in R5 stage, use dm_tifs
whose sec-proxy threads are specified in documentation.
https://software-dl.ti.com/tisci/esd/latest/5_soc_doc/j722s/sec_proxy.html
Will add this doc link in v2.

Thanks,
Prasanth

> 
> > Signed-off-by: Vaishnav Achath 
> > Signed-off-by: Prasanth Babu Mantena 
> > ---
> >   drivers/mailbox/k3-sec-proxy.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/mailbox/k3-sec-proxy.c b/drivers/mailbox/k3-sec-proxy.c
> > index 5eafe46fd4d..6f5ad37919f 100644
> > --- a/drivers/mailbox/k3-sec-proxy.c
> > +++ b/drivers/mailbox/k3-sec-proxy.c
> > @@ -408,7 +408,7 @@ static int k3_sec_proxy_remove(struct udevice *dev)
> > return 0;
> >   }
> > -static const u32 am6x_valid_threads[] = { 0, 1, 4, 5, 6, 7, 8, 9, 11, 12, 
> > 13, 20, 21, 22, 23 };
> > +static const u32 am6x_valid_threads[] = { 0, 1, 4, 5, 6, 7, 8, 9, 11, 12, 
> > 13, 20, 21, 22, 23, 28, 29 };
> >   static const struct k3_sec_proxy_desc am654_desc = {
> > .thread_count = 90,