I will amend commit accordingly and submit a new patch. Thanks for review. JC
On 9/28/20 9:17 PM, Thierry Reding wrote: > On Wed, Sep 09, 2020 at 04:10:34PM +0800, JC Kuo wrote: >> This commit implements a register map which grants USB (UTMI and HSIC) >> sleepwalk registers access to USB PHY drivers. The USB sleepwalk logic >> is in PMC hardware block but USB PHY drivers have the best knowledge >> of proper programming sequence. This approach prevents using custom >> pmc APIs. > > I don't think this final sentence is useful. The commit message should > explain what you're doing, but there's no need to enumerate any other > inferior solution you didn't choose to implement. > > If you do want to keep it: s/pmc/PMC/. > > While at it, perhaps replace "usb" by "USB" in the subject as well. > >> >> Signed-off-by: JC Kuo <[email protected]> >> --- >> v3: >> commit message improvement >> drop regmap_reg() usage >> rename 'reg' with 'offset' >> rename 'val' with 'value' >> drop '__force' when invokes devm_regmap_init() >> print error code of devm_regmap_init() >> move devm_regmap_init() a litter bit earlier >> explicitly set '.has_usb_sleepwalk=false' >> >> drivers/soc/tegra/pmc.c | 95 +++++++++++++++++++++++++++++++++++++++++ >> 1 file changed, 95 insertions(+) >> >> diff --git a/drivers/soc/tegra/pmc.c b/drivers/soc/tegra/pmc.c >> index d332e5d9abac..ff24891ce9ca 100644 >> --- a/drivers/soc/tegra/pmc.c >> +++ b/drivers/soc/tegra/pmc.c >> @@ -43,6 +43,7 @@ >> #include <linux/seq_file.h> >> #include <linux/slab.h> >> #include <linux/spinlock.h> >> +#include <linux/regmap.h> >> >> #include <soc/tegra/common.h> >> #include <soc/tegra/fuse.h> >> @@ -102,6 +103,9 @@ >> >> #define PMC_PWR_DET_VALUE 0xe4 >> >> +#define PMC_USB_DEBOUNCE_DEL 0xec >> +#define PMC_USB_AO 0xf0 >> + >> #define PMC_SCRATCH41 0x140 >> >> #define PMC_WAKE2_MASK 0x160 >> @@ -133,6 +137,13 @@ >> #define IO_DPD2_STATUS 0x1c4 >> #define SEL_DPD_TIM 0x1c8 >> >> +#define PMC_UTMIP_UHSIC_TRIGGERS 0x1ec >> +#define PMC_UTMIP_UHSIC_SAVED_STATE 0x1f0 >> + >> +#define PMC_UTMIP_TERM_PAD_CFG 0x1f8 >> +#define PMC_UTMIP_UHSIC_SLEEP_CFG 0x1fc >> +#define PMC_UTMIP_UHSIC_FAKE 0x218 >> + >> #define PMC_SCRATCH54 0x258 >> #define PMC_SCRATCH54_DATA_SHIFT 8 >> #define PMC_SCRATCH54_ADDR_SHIFT 0 >> @@ -145,8 +156,18 @@ >> #define PMC_SCRATCH55_CHECKSUM_SHIFT 16 >> #define PMC_SCRATCH55_I2CSLV1_SHIFT 0 >> >> +#define PMC_UTMIP_UHSIC_LINE_WAKEUP 0x26c >> + >> +#define PMC_UTMIP_BIAS_MASTER_CNTRL 0x270 >> +#define PMC_UTMIP_MASTER_CONFIG 0x274 >> +#define PMC_UTMIP_UHSIC2_TRIGGERS 0x27c >> +#define PMC_UTMIP_MASTER2_CONFIG 0x29c >> + >> #define GPU_RG_CNTRL 0x2d4 >> >> +#define PMC_UTMIP_PAD_CFG0 0x4c0 >> +#define PMC_UTMIP_UHSIC_SLEEP_CFG1 0x4d0 >> +#define PMC_UTMIP_SLEEPWALK_P3 0x4e0 >> /* Tegra186 and later */ >> #define WAKE_AOWAKE_CNTRL(x) (0x000 + ((x) << 2)) >> #define WAKE_AOWAKE_CNTRL_LEVEL (1 << 3) >> @@ -334,6 +355,7 @@ struct tegra_pmc_soc { >> const struct pmc_clk_init_data *pmc_clks_data; >> unsigned int num_pmc_clks; >> bool has_blink_output; >> + bool has_usb_sleepwalk; >> }; >> >> static const char * const tegra186_reset_sources[] = { >> @@ -2495,6 +2517,68 @@ static void tegra_pmc_clock_register(struct tegra_pmc >> *pmc, >> err); >> } >> >> +static const struct regmap_range pmc_usb_sleepwalk_ranges[] = { >> + regmap_reg_range(PMC_USB_DEBOUNCE_DEL, PMC_USB_AO), >> + regmap_reg_range(PMC_UTMIP_UHSIC_TRIGGERS, PMC_UTMIP_UHSIC_SAVED_STATE), >> + regmap_reg_range(PMC_UTMIP_TERM_PAD_CFG, PMC_UTMIP_UHSIC_FAKE), >> + regmap_reg_range(PMC_UTMIP_UHSIC_LINE_WAKEUP, >> PMC_UTMIP_UHSIC_LINE_WAKEUP), >> + regmap_reg_range(PMC_UTMIP_BIAS_MASTER_CNTRL, PMC_UTMIP_MASTER_CONFIG), >> + regmap_reg_range(PMC_UTMIP_UHSIC2_TRIGGERS, PMC_UTMIP_MASTER2_CONFIG), >> + regmap_reg_range(PMC_UTMIP_PAD_CFG0, PMC_UTMIP_UHSIC_SLEEP_CFG1), >> + regmap_reg_range(PMC_UTMIP_SLEEPWALK_P3, PMC_UTMIP_SLEEPWALK_P3), >> +}; >> + >> +static const struct regmap_access_table pmc_usb_sleepwalk_table = { >> + .yes_ranges = pmc_usb_sleepwalk_ranges, >> + .n_yes_ranges = ARRAY_SIZE(pmc_usb_sleepwalk_ranges), >> +}; >> + >> +static int tegra_pmc_regmap_readl(void *context, unsigned int offset, >> unsigned int *value) >> +{ >> + struct tegra_pmc *pmc = context; >> + >> + *value = tegra_pmc_readl(pmc, offset); >> + return 0; >> +} >> + >> +static int tegra_pmc_regmap_writel(void *context, unsigned int offset, >> unsigned int value) >> +{ >> + struct tegra_pmc *pmc = context; >> + >> + tegra_pmc_writel(pmc, value, offset); >> + return 0; >> +} >> + >> +static const struct regmap_config usb_sleepwalk_regmap_config = { >> + .name = "usb_sleepwalk", >> + .reg_bits = 32, >> + .val_bits = 32, >> + .reg_stride = 4, >> + .fast_io = true, >> + .rd_table = &pmc_usb_sleepwalk_table, >> + .wr_table = &pmc_usb_sleepwalk_table, >> + .reg_read = tegra_pmc_regmap_readl, >> + .reg_write = tegra_pmc_regmap_writel, >> +}; >> + >> +static int tegra_pmc_regmap_init(struct tegra_pmc *pmc) >> +{ >> + struct regmap *regmap; >> + int err; >> + >> + if (pmc->soc->has_usb_sleepwalk) { >> + regmap = devm_regmap_init(pmc->dev, NULL, (void *) pmc, > > I don't think you need that explicit cast there. > > With those minor comments addressed: > > Acked-by: Thierry Reding <[email protected]> >
