From mboxrd@z Thu Jan 1 00:00:00 1970 From: Tero Kristo Subject: Re: [PATCH 6/8] soc: ti: omap_prm: add data for am33xx Date: Wed, 21 Aug 2019 10:23:21 +0300 Message-ID: <8f5f86db-270a-7278-9d9c-e84c0fa9b73c@ti.com> References: <1565164139-21886-1-git-send-email-t-kristo@ti.com> <1565164139-21886-7-git-send-email-t-kristo@ti.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: Content-Language: en-US List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=m.gmane.org@lists.infradead.org To: Suman Anna , ssantosh@kernel.org, linux-arm-kernel@lists.infradead.org, linux-omap@vger.kernel.org, robh+dt@kernel.org Cc: tony@atomide.com, devicetree@vger.kernel.org List-Id: devicetree@vger.kernel.org On 20.8.2019 21.48, Suman Anna wrote: > Hi Tero, > > On 8/7/19 2:48 AM, Tero Kristo wrote: >> Add PRM instance data for AM33xx SoC. Includes some basic register >> definitions and reset data for now. >> >> Signed-off-by: Tero Kristo >> --- >> drivers/soc/ti/omap_prm.c | 17 +++++++++++++++++ >> 1 file changed, 17 insertions(+) >> >> diff --git a/drivers/soc/ti/omap_prm.c b/drivers/soc/ti/omap_prm.c >> index 9b8d5945..fadfc7f 100644 >> --- a/drivers/soc/ti/omap_prm.c >> +++ b/drivers/soc/ti/omap_prm.c >> @@ -73,8 +73,25 @@ struct omap_prm_data omap4_prm_data[] = { >> { }, >> }; >> >> +struct omap_rst_map am3_wkup_rst_map[] = { >> + { .rst = 3, .st = 5 }, >> + { .rst = -1 }, >> +}; >> + >> +struct omap_prm_data am3_prm_data[] = { >> + { .name = "per", .base = 0x44e00c00, .pwstctrl = 0xc, .pwstst = 0x8, .flags = OMAP_PRM_NO_RSTST }, >> + { .name = "wkup", .base = 0x44e00d00, .pwstctrl = 0x4, .pwstst = 0x8, .rstst = 0xc, .rstmap = am3_wkup_rst_map }, >> + { .name = "mpu", .base = 0x44e00e00, .pwstst = 0x4 }, > > Has a rstst but no rstctrl, but your registration logic takes care of > this. Somewhat confusing, when you just look at the data. Should you > limit the check to only rstctrl and OMAP_PRM_NO_RSTST? I think its probably better I invert the flags and explicitly state OMAP_PRM_HAS_RSTST | OMAP_PRM_HAS_RSTCTRL, in case any zero value is used for these. > >> + { .name = "device", .base = 0x44e00f00, .rstctl = 0x0, .rstst = 0x8 }, > > No pwrstctrl and pwrstst registers, so same comment as on OMAP4 data. I should probably add some flag for this in future once the support for power domains is added. Anyway, I'll ditch all pwstctrl / pwstst data for now as it seems to bother you too much. -Tero > >> + { .name = "rtc", .base = 0x44e01000, .pwstst = 0x4 }, >> + { .name = "gfx", .base = 0x44e01100, .pwstst = 0x10, .rstctl = 0x4, .rstst = 0x14 }, >> + { .name = "cefuse", .base = 0x44e01200, .pwstst = 0x4 }, > > I am not sure if it is better to explicitly list the registers at 0 > offset rather than using the implied value of 0, since there are some > registers that do not exist on some PRM instances which are also not > defined. > > regards > Suman > >> + { }, >> +}; >> + >> static const struct of_device_id omap_prm_id_table[] = { >> { .compatible = "ti,omap4-prm-inst", .data = omap4_prm_data }, >> + { .compatible = "ti,am3-prm-inst", .data = am3_prm_data }, >> { }, >> }; >> >> > -- Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki