Re: [PATCH] mmc: remove another TMIO MMC variant usdhi6rol0.c

From: Simon Horman
Date: Fri Jun 21 2019 - 03:12:46 EST


On Fri, Jun 21, 2019 at 03:05:11PM +0900, Masahiro Yamada wrote:
> Renesas upstreamed two different drivers for (almost) the same hardware.
> usdhi6rol0.c is (what we call) "TMIO MMC", which I am 100% sure from the
> the register macros in usdhi6rol0.c.
>
> As commit b6147490e6aa ("mmc: tmio: split core functionality, DMA and
> MFD glue") said, the MMC controllers called tmio_mmc uses the IP from
> Panasonic. ('TMIO MMC' is a historical misnomer)
>
> The macros in usdhi6rol0.c exactly match to the original datasheet
> published by Panasonic. (except the 'USDHI6_' prefix, of course).
> I formerly worked for Panasonic, and Socionext was split out from
> Panasonic. I can get access to the IP datasheet.
>
> [The register comparison]
>
> tmio_mmc.h usdhi6rol0.c
>
> 0x00 CTL_SD_CMD 0x000 USDHI6_SD_CMD
> 0x004 USDHI6_SD_PORT_SEL
> 0x04 CTL_ARG_REG 0x008 USDHI6_SD_ARG
> 0x08 CTL_STOP_INTERNAL_ACTION 0x010 USDHI6_SD_STOP
> 0x0a CTL_XFER_BLK_COUNT 0x014 USDHI6_SD_SECCNT
> 0x0c CTL_RESPONSE 0x018 USDHI6_SD_RSP10
> 0x020 USDHI6_SD_RSP32
> 0x028 USDHI6_SD_RSP54
> 0x030 USDHI6_SD_RSP76
> 0x1c CTL_STATUS 0x038 USDHI6_SD_INFO1
> 0x03c USDHI6_SD_INFO2
> 0x20 CTL_IRQ_MASK 0x040 USDHI6_SD_INFO1_MASK
> 0x044 USDHI6_SD_INFO2_MASK
> 0x24 CTL_SD_CARD_CLK_CTL 0x048 USDHI6_SD_CLK_CTRL
> 0x26 CTL_SD_XFER_LEN 0x04c USDHI6_SD_SIZE
> 0x28 CTL_SD_MEM_CARD_OPT 0x050 USDHI6_SD_OPTION
> 0x2c CTL_SD_ERROR_DETAIL_STATUS 0x058 USDHI6_SD_ERR_STS1
> 0x05c USDHI6_SD_ERR_STS2
> 0x30 CTL_SD_DATA_PORT 0x060 USDHI6_SD_BUF0
> 0x34 CTL_TRANSACTION_CTL 0x068 USDHI6_SDIO_MODE
> 0x36 CTL_SDIO_STATUS 0x06c USDHI6_SDIO_INFO1
> 0x38 CTL_SDIO_IRQ_MASK 0x070 USDHI6_SDIO_INFO1_MASK
> 0xd8 CTL_DMA_ENABLE 0x1b0 USDHI6_CC_EXT_MODE
> 0xe0 CTL_RESET_SD 0x1c0 USDHI6_SOFT_RESET
> 0xe2 CTL_VERSION 0x1c4 USDHI6_VERSION
> 0x1c8 USDHI6_HOST_MODE
> 0xe6 CTL_SDIF_MODE 0x1cc USDHI6_SDIF_MODE
>
> The offsets for tmio_mmc.h are half of those of usdhi6rol0.c because
> tmio_mmc was originally written for 16-bit bus platforms. The register
> stride is adjusted by ->bus_shift for modern SoCs.
>
> The register names for usdhi6rol0.c are taken from the IP datasheet
> (with USDHI6_ prefixed). On the other hand, tmio_mmc largely renamed
> the registers.
>
> You may think some registers are missing on the tmio_mmc side.
> Actually, the registers exists. For example, tmio_mmc merged
> 'SD_INFO1' and 'SD_INFO2' into the single macro 'CTL_STATUS', then
> get access to it by the crappy helper, sd_ctrl_write32_as_16_and_16().
>
> As a summary, maintaining two drivers for the same hardware leads to
> maintenance nightmare.
>
> The naming and the code quality for TMIO is unfortunate, but we cannot
> kill it since it is widely used. On the other hand, usdhi6rol0.c just
> supports a single platform. In fact, there is no DT in upstream for
> "renesas,usdhi6rol0":

I am fine with removing this driver on the basis that it is not used
upstream. I agree it would be good to get an Ack from @renesas.com.

>
> $ git grep renesas,usdhi6rol0
> Documentation/devicetree/bindings/mmc/usdhi6rol0.txt: "renesas,usdhi6rol0"
> Documentation/devicetree/bindings/mmc/usdhi6rol0.txt: compatible = "renesas,usdhi6rol0";
> drivers/mmc/host/usdhi6rol0.c: {.compatible = "renesas,usdhi6rol0"},
>
> Delete this driver now. Please re-implement it based on tmio_mmc_core.c
> if needed.
>
> Perhaps, some code snippets in this driver might be useful for cleaning
> tmio_mmc. It will stay in git history forever, and you can dig for it
> whenever you need it.
>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>

...