Re: [PATCH v2 2/4] phy: axiado: add Axiado eMMC PHY driver

[Tzu-Hao Wei <twei@axiado.com>]

On 2/27/2026 6:53 AM, Vinod Koul wrote:
>> --- /dev/null
>> +++ b/drivers/phy/axiado/phy-axiado-emmc.c
>> @@ -0,0 +1,221 @@
>> +// SPDX-License-Identifier: GPL-2.0-or-later
>> +/*
>> + * Axiado eMMC PHY driver
>> + *
>> + * Copyright (C) 2017 Arasan Chip Systems Inc.
>> + * Copyright (C) 2022-2025 Axiado Corporation (or its affiliates).
> 
> 2026
> 
Thanks. Will fix in the next version.

>> + *
>> + * Based on Arasan Driver (sdhci-pci-arasan.c)
>> + * sdhci-pci-arasan.c - Driver for Arasan PCI Controller with integrated phy.
>> + */
>> +#include <linux/bitfield.h>
>> +#include <linux/delay.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/phy/phy.h>
>> +#include <linux/platform_device.h>
>> +
>> +/* Arasan eMMC 5.1 - PHY configuration registers */
>> +#define CAP_REG_IN_S1_LSB            0x00
>> +#define CAP_REG_IN_S1_MSB            0x04
>> +#define PHY_CTRL_1                   0x38
>> +#define PHY_CTRL_2                   0x3C
> 
> smaller hex case please, here and other places
> 
Thanks. Will fix in the next version.

>> +#define PHY_CTRL_3                   0x40
>> +#define STATUS                               0x50
>> +
>> +#define DLL_ENBL     BIT(26)
>> +#define RTRIM_EN     BIT(21)
>> +#define PDB_ENBL     BIT(23)
>> +#define RETB_ENBL    BIT(1)
>> +
>> +#define REN_STRB     BIT(27)
>> +#define REN_CMD              BIT(12)
>> +#define REN_DAT0     BIT(13)
>> +#define REN_DAT1     BIT(14)
>> +#define REN_DAT2     BIT(15)
>> +#define REN_DAT3     BIT(16)
>> +#define REN_DAT4     BIT(17)
>> +#define REN_DAT5     BIT(18)
>> +#define REN_DAT6     BIT(19)
>> +#define REN_DAT7     BIT(20)
>> +#define REN_CMD_EN   (REN_CMD | REN_DAT0 | REN_DAT1 | REN_DAT2 | \
>> +             REN_DAT3 | REN_DAT4 | REN_DAT5 | REN_DAT6 | REN_DAT7)
>> +
>> +/* Pull-UP Enable on CMD Line */
>> +#define PU_CMD               BIT(3)
>> +#define PU_DAT0              BIT(4)
>> +#define PU_DAT1              BIT(5)
>> +#define PU_DAT2              BIT(6)
>> +#define PU_DAT3              BIT(7)
>> +#define PU_DAT4              BIT(8)
>> +#define PU_DAT5              BIT(9)
>> +#define PU_DAT6              BIT(10)
>> +#define PU_DAT7              BIT(11)
>> +#define PU_CMD_EN (PU_CMD | PU_DAT0 | PU_DAT1 | PU_DAT2 | PU_DAT3 | \
>> +             PU_DAT4 | PU_DAT5 | PU_DAT6 | PU_DAT7)
> 
> The bit define are used only once, why not define the cmd with
> respective bits here
> 
Will replace it by
#define PU_CMD_EN GENMASK(11, 3)

>> +static const struct phy_ops axiado_emmc_phy_ops = {
>> +     .init = axiado_emmc_phy_init,
>> +     .power_on = axiado_emmc_phy_power_on,
> 
> no power_off?
> 
Thanks for reminding, will add power_off.

> --
> ~Vinod