RE: [PATCH v20 08/12] mmc: renesas_sdhi: Add RZ/G3L SDHI support

Biju Das <[email protected]>
Newsgroups org.kernel.vger.linux-renesas-soc,org.kernel.vger.linux-kernel,org.kernel.vger.linux-mmc
Message-ID <TY3PR01MB113465E307DC9AA067A1B1D1F86F92@TY3PR01MB11346.jpnprd01.prod.outlook.com>
Hi Geert,

Thanks for testing.

> -----Original Message-----
> From: Geert Uytterhoeven <[email protected]>
> Sent: 14 July 2026 16:43
> Subject: Re: [PATCH v20 08/12] mmc: renesas_sdhi: Add RZ/G3L SDHI support
> 
> Hi Biju,
> 
> On Mon, 13 Jul 2026 at 19:52, Biju <[email protected]> wrote:
> > From: Biju Das <[email protected]>
> >
> > The RZ/G3L SoC (r9a08g046) has an SDHI controller with several
> > differences from existing platforms, requiring dedicated handling.
> >
> > Utilise the TMIO_MMC_HWADJ flag to identify controllers that carry a
> > second hardware adjustment register (SDm_SCC_HWADJ2 at offset 0x010).
> > When both TMIO_MMC_TUNING_DELAY and TMIO_MMC_HWADJ are set,
> > renesas_sdhi_set_hw_adjustment_delay() additionally programs HWADJ2
> > with 0x3FFF at 3.3 V and 0xFF at 1.8 V. A new RZG3L_SDHI_SCC_HWADJ4
> > register (offset 0x022) is also cleared at the start of tuning when
> > TMIO_MMC_INTERNAL_DIVIDER is set.
> >
> > Add a new version constant SDHI_VER_RZ_G3L_SDMMC (0xce10) and extend
> > renesas_sdhi_sdbuf_width() to handle it alongside the existing Gen3
> > variants.
> >
> > Introduce of_data_rzg3l with RZ/G3L-specific parameters: a wider
> > clk_mask of 0x200000200, a max_divider of 2048 (reflecting the 11-bit
> > divider), a dedicated rzg3l_scc_taps table, and all relevant
> > tmio_flags, including TMIO_MMC_INTERNAL_DIVIDER and TMIO_MMC_HWADJ.
> > Wire it to a new of_rzg3l_compatible entry using sdhi_quirks_rzg3l
> > (which sets fixed_addr_mode), and register the
> > "renesas,sdhi-r9a08g046" compatible string in the OF match table.
> >
> > Signed-off-by: Biju Das <[email protected]>
> 
> Thanks for your patch!
> 
> This causes a division by zero on Armadillo (R-Mobile A1):
> 
>     Division by zero in kernel.
>     CPU: 0 UID: 0 PID: 22 Comm: kworker/0:2 Not tainted 7.2.0-rc3-armadillo-06338-gd1557670ab0c #976
> VOLUNTARY
>     Hardware name: Generic R8A7740 (Flattened Device Tree)
>     Workqueue: events_freezable mmc_rescan
>     Call trace:
>      unwind_backtrace from show_stack+0x10/0x14
>      show_stack from dump_stack_lvl+0x44/0x74
>      dump_stack_lvl from Ldiv0+0x8/0x10
>      Ldiv0 from tmio_mmc_set_ios+0x11c/0x1bc
>      tmio_mmc_set_ios from mmc_sdio_init_card+0xa40/0xab8
>      mmc_sdio_init_card from mmc_attach_sdio+0x90/0x32c
>      mmc_attach_sdio from mmc_rescan+0x1d8/0x2a4
>      mmc_rescan from process_scheduled_works+0x2bc/0x45c
>      process_scheduled_works from worker_thread+0x170/0x214
>      worker_thread from kthread+0x114/0x124
>      kthread from ret_from_fork+0x14/0x28
>     Exception stack(0xe0945fb0 to 0xe0945ff8)
>     5fa0:                                     00000000 00000000
> 00000000 00000000
>     5fc0: 00000000 00000000 00000000 00000000 00000000 00000000
> 00000000 00000000
>     5fe0: 00000000 00000000 00000000 00000000 00000013 00000000
>     Division by zero in kernel.
>     CPU: 0 UID: 0 PID: 22 Comm: kworker/0:2 Not tainted 7.2.0-rc3-armadillo-06338-gd1557670ab0c #976
> VOLUNTARY
>     Hardware name: Generic R8A7740 (Flattened Device Tree)
>     Workqueue: events_freezable mmc_rescan
>     Call trace:
>      unwind_backtrace from show_stack+0x10/0x14
>      show_stack from dump_stack_lvl+0x44/0x74
>      dump_stack_lvl from Ldiv0+0x8/0x10
>      Ldiv0 from tmio_mmc_set_ios+0x11c/0x1bc
>      tmio_mmc_set_ios from sdio_enable_4bit_bus+0x120/0x138
>      sdio_enable_4bit_bus from mmc_sdio_init_card+0xa48/0xab8
>      mmc_sdio_init_card from mmc_attach_sdio+0x90/0x32c
>      mmc_attach_sdio from mmc_rescan+0x1d8/0x2a4
>      mmc_rescan from process_scheduled_works+0x2bc/0x45c
> 
> > --- a/drivers/mmc/host/renesas_sdhi_core.c
> > +++ b/drivers/mmc/host/renesas_sdhi_core.c
> > @@ -235,9 +238,18 @@ static void renesas_sdhi_set_clock(struct
> > tmio_mmc_host *host,
> >
> >         clock = clk & CLK_CTL_DIV_MASK;
> >         if (clock != CLK_CTL_DIV_MASK)
> > -               host->mmc->actual_clock /= (1 << (ffs(clock) + 1));
> > +               host->mmc->actual_clock /= (1 << (ffs(clk) + 1));
> 
> Is this change intentional?
> 
>     clock = 0, ffs(clock) = 0
>     clk = 8388608, ffs(clk) = 24

Oops, I missed to check the actual clk [1] in v20 while focussing on fixing
sashiko's comment.

 Actually,
 clk = clk & GENMASK(7,0) --> For Non-RZ/G3L and
 clk = clk & GENMASK(9,0) --> For RZ/G3L 

I need to add the clk_divider_mask variable to of_data to fix this issue.

[1]
root@smarc-rzg3l:~# cat /sys/kernel/debug/mmc*/ios
clock:          150000000 Hz
actual clock:   8 Hz --> It is wrong here (150 MHz / (1 << 25)).
vdd:            21 (3.3 ~ 3.4 V)
bus mode:       2 (push-pull)
chip select:    0 (don't care)
power mode:     2 (on)
bus width:      3 (8 bits)
timing spec:    10 (mmc HS400 enhanced strobe)
signal voltage: 1 (1.80 V)
driver type:    1 (driver type A)
clock:          150000000 Hz
actual clock:   150000000 Hz
vdd:            21 (3.3 ~ 3.4 V)
bus mode:       2 (push-pull)
chip select:    0 (don't care)
power mode:     2 (on)
bus width:      2 (4 bits)
timing spec:    6 (sd uhs SDR104)
signal voltage: 1 (1.80 V)
driver type:    0 (driver type B)

Cheers,
Biju

> 
> Later:
> 
>     static void tmio_mmc_max_busy_timeout(struct tmio_mmc_host *host)
>     {
>             unsigned int clk_rate = host->mmc->actual_clock ?: host->mmc->f_max;
> 
>             host->mmc->max_busy_timeout = host->get_timeout_cycles(host) /
>                                           (clk_rate / MSEC_PER_SEC);
> 
> clk_rate = 2, BOOM!
> 
>    }
> 
> Gr{oetje,eeting}s,
> 
>                         Geert
> 
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]
> 
> In personal conversations with technical people, I call myself a hacker. But when I'm talking to
> journalists I just say "programmer" or something like that.
>                                 -- Linus Torvalds
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.