From: JC Kuo <jckuo@nvidia.com>
To: Thierry Reding <thierry.reding@gmail.com>
Cc: <gregkh@linuxfoundation.org>, <robh@kernel.org>,
<jonathanh@nvidia.com>, <kishon@ti.com>,
<linux-tegra@vger.kernel.org>, <linux-usb@vger.kernel.org>,
<linux-kernel@vger.kernel.org>, <devicetree@vger.kernel.org>,
<nkristam@nvidia.com>
Subject: Re: [PATCH v3 08/15] soc/tegra: pmc: Provide usb sleepwalk register map
Date: Wed, 14 Oct 2020 12:08:45 +0800 [thread overview]
Message-ID: <89c469f7-60a5-7cc7-253b-a6b98c86e3c9@nvidia.com> (raw)
In-Reply-To: <20200928131751.GI3065790@ulmo>
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 <jckuo@nvidia.com>
>> ---
>> 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 <treding@nvidia.com>
>
next prev parent reply other threads:[~2020-10-14 4:08 UTC|newest]
Thread overview: 39+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-09-09 8:10 [PATCH v3 00/15] Tegra XHCI controller ELPG support JC Kuo
2020-09-09 8:10 ` [PATCH v3 01/15] clk: tegra: Add PLLE HW power sequencer control JC Kuo
2020-09-28 12:51 ` Thierry Reding
2020-09-09 8:10 ` [PATCH v3 02/15] clk: tegra: Don't enable PLLE HW sequencer at init JC Kuo
2020-09-28 12:52 ` Thierry Reding
2020-09-09 8:10 ` [PATCH v3 03/15] phy: tegra: xusb: Move usb3 port init for Tegra210 JC Kuo
2020-09-28 13:03 ` Thierry Reding
2020-09-09 8:10 ` [PATCH v3 04/15] phy: tegra: xusb: tegra210: Do not reset UPHY PLL JC Kuo
2020-09-28 13:06 ` Thierry Reding
2020-10-14 3:21 ` JC Kuo
2020-09-09 8:10 ` [PATCH v3 05/15] phy: tegra: xusb: Rearrange UPHY init on Tegra210 JC Kuo
2020-09-28 13:08 ` Thierry Reding
2020-09-09 8:10 ` [PATCH v3 06/15] phy: tegra: xusb: Add Tegra210 lane_iddq operation JC Kuo
2020-09-28 13:10 ` Thierry Reding
2020-09-09 8:10 ` [PATCH v3 07/15] phy: tegra: xusb: Add sleepwalk and suspend/resume JC Kuo
2020-09-28 13:11 ` Thierry Reding
2020-09-09 8:10 ` [PATCH v3 08/15] soc/tegra: pmc: Provide usb sleepwalk register map JC Kuo
2020-09-28 13:17 ` Thierry Reding
2020-10-14 4:08 ` JC Kuo [this message]
2020-09-09 8:10 ` [PATCH v3 09/15] arm64: tegra210: XUSB PADCTL add "nvidia,pmc" prop JC Kuo
2020-09-28 13:18 ` Thierry Reding
2020-10-14 4:15 ` JC Kuo
2020-09-09 8:10 ` [PATCH v3 10/15] phy: tegra: xusb: Add wake/sleepwalk for Tegra210 JC Kuo
2020-09-28 13:40 ` Thierry Reding
2020-10-14 8:37 ` JC Kuo
2020-09-09 8:10 ` [PATCH v3 11/15] phy: tegra: xusb: Tegra210 host mode VBUS control JC Kuo
2020-09-28 13:42 ` Thierry Reding
2020-09-09 8:10 ` [PATCH v3 12/15] phy: tegra: xusb: Add wake/sleepwalk for Tegra186 JC Kuo
2020-09-28 13:50 ` Thierry Reding
2020-10-15 8:08 ` JC Kuo
2020-09-09 8:10 ` [PATCH v3 13/15] arm64: tegra210/tegra186/tegra194: XUSB PADCTL irq JC Kuo
2020-09-09 8:10 ` [PATCH v3 14/15] usb: host: xhci-tegra: Unlink power domain devices JC Kuo
2020-09-28 13:53 ` Thierry Reding
2020-10-15 8:09 ` JC Kuo
2020-09-09 8:10 ` [PATCH v3 15/15] xhci: tegra: Enable ELPG for runtime/system PM JC Kuo
2020-09-28 14:06 ` Thierry Reding
2020-10-15 8:12 ` JC Kuo
2020-09-28 12:54 ` [PATCH v3 00/15] Tegra XHCI controller ELPG support Thierry Reding
2020-10-14 2:26 ` JC Kuo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=89c469f7-60a5-7cc7-253b-a6b98c86e3c9@nvidia.com \
--to=jckuo@nvidia.com \
--cc=devicetree@vger.kernel.org \
--cc=gregkh@linuxfoundation.org \
--cc=jonathanh@nvidia.com \
--cc=kishon@ti.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-tegra@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=nkristam@nvidia.com \
--cc=robh@kernel.org \
--cc=thierry.reding@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).