devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jolly Shah <JOLLYS@xilinx.com>
To: Shubhrajyoti Datta <shubhrajyoti.datta@gmail.com>
Cc: "ard.biesheuvel@linaro.org" <ard.biesheuvel@linaro.org>,
	"mingo@kernel.org" <mingo@kernel.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"matt@codeblueprint.co.uk" <matt@codeblueprint.co.uk>,
	"sudeep.holla@arm.com" <sudeep.holla@arm.com>,
	"hkallweit1@gmail.com" <hkallweit1@gmail.com>,
	"keescook@chromium.org" <keescook@chromium.org>,
	"dmitry.torokhov@gmail.com" <dmitry.torokhov@gmail.com>,
	Michal Simek <michal.simek@xilinx.com>,
	Rob Herring <robh+dt@kernel.org>,
	Mark Rutland <mark.rutland@arm.com>,
	"linux-arm-kernel@lists.infradead.org"
	<linux-arm-kernel@lists.infradead.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"devicetree@vger.kernel.org" <devicetree@vger.kernel.org>,
	Rajan Vaja <RAJANV@xilinx.com>
Subject: RE: [PATCH v3 2/4] drivers: firmware: xilinx: Add ZynqMP firmware driver
Date: Wed, 31 Jan 2018 19:46:20 +0000	[thread overview]
Message-ID: <DM2PR0201MB07676D107FA004D43F1FC49AB8FB0@DM2PR0201MB0767.namprd02.prod.outlook.com> (raw)
In-Reply-To: <CAKfKVtGGf8AexOTEvyyi7cmQ_Xopa3Fszf-jOuNSfBJpE2EzCg@mail.gmail.com>

Hi Shubhrajyoti,
Thanks for the review,

> -----Original Message-----
> From: Shubhrajyoti Datta [mailto:shubhrajyoti.datta@gmail.com]
> Sent: Monday, January 29, 2018 9:06 PM
> To: Jolly Shah <JOLLYS@xilinx.com>
> Cc: ard.biesheuvel@linaro.org; mingo@kernel.org; Greg Kroah-Hartman
> <gregkh@linuxfoundation.org>; matt@codeblueprint.co.uk;
> sudeep.holla@arm.com; hkallweit1@gmail.com; keescook@chromium.org;
> dmitry.torokhov@gmail.com; Michal Simek <michal.simek@xilinx.com>; Rob
> Herring <robh+dt@kernel.org>; Mark Rutland <mark.rutland@arm.com>; linux-
> arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org; Jolly Shah <JOLLYS@xilinx.com>; Rajan Vaja
> <RAJANV@xilinx.com>
> Subject: Re: [PATCH v3 2/4] drivers: firmware: xilinx: Add ZynqMP firmware
> driver
> 
> Hi,
> 
> Thanks for the patch.
> A few questions below.
> 
> 
> On Thu, Jan 25, 2018 at 4:51 AM, Jolly Shah <jolly.shah@xilinx.com> wrote:
> > This patch is adding communication layer with firmware.
> > Firmware driver provides an interface to firmware APIs.
> > Interface APIs can be used by any driver to communicate to
> > PMUFW(Platform Management Unit). All requests go through ATF.
> >
> > Signed-off-by: Jolly Shah <jollys@xilinx.com>
> > Signed-off-by: Rajan Vaja <rajanv@xilinx.com>
> > ---
> <snip>
> >
> 
> > +/**
> > + * zynqmp_pm_clock_enable - Enable the clock for given id
> > + * @clock_id:  ID of the clock to be enabled
> 
> Does it enable all the parents also if they are disabled?
Current solution enables specified clock only. We are working on enhancing the solution to take care of other dependent clocks.

> 
> > + *
> > + * This function is used by master to enable the clock
> > + * including peripherals and PLL clocks.
> > + *
> > + * Return:     Returns status, either success or error+reason.
> > + */
> > +static int zynqmp_pm_clock_enable(u32 clock_id) {
> > +       return invoke_pm_fn(PM_CLOCK_ENABLE, clock_id, 0, 0, 0, NULL);
> > +}
> > +
> > +/**
> > + * zynqmp_pm_clock_disable - Disable the clock for given id
> > + * @clock_id:  ID of the clock to be disable
> > + *
> > + * This function is used by master to disable the clock
> > + * including peripherals and PLL clocks.
> > + *
> > + * Return:     Returns status, either success or error+reason.
> > + */
> > +static int zynqmp_pm_clock_disable(u32 clock_id) {
> > +       return invoke_pm_fn(PM_CLOCK_DISABLE, clock_id, 0, 0, 0,
> > +NULL); }
> > +
> > +/**
> > + * zynqmp_pm_clock_getstate - Get the clock state for given id
> > + * @clock_id:  ID of the clock to be queried
> > + * @state:     1/0 (Enabled/Disabled)
> > + *
> > + * This function is used by master to get the state of clock
> > + * including peripherals and PLL clocks.
> > + *
> > + * Return:     Returns status, either success or error+reason.
> > + */
> > +static int zynqmp_pm_clock_getstate(u32 clock_id, u32 *state) {
> > +       u32 ret_payload[PAYLOAD_ARG_CNT];
> > +       int ret;
> > +
> > +       ret = invoke_pm_fn(PM_CLOCK_GETSTATE, clock_id, 0, 0, 0,
> ret_payload);
> > +       *state = ret_payload[1];
> > +
> > +       return ret;
> > +}
> > +
> > +/**
> > + * zynqmp_pm_clock_setdivider - Set the clock divider for given id
> > + * @clock_id:  ID of the clock
> > + * @div_type:  TYPE_DIV1: div1
> > + *             TYPE_DIV2: div2
> div type didnt see in the signature.


Will be fixed in next version.

> 
> 
> 
> > + * @divider:   divider value.
> > + *
> > + * This function is used by master to set divider for any clock
> > + * to achieve desired rate.
> > + *
> > + * Return:     Returns status, either success or error+reason.
> > + */
> > +static int zynqmp_pm_clock_setdivider(u32 clock_id, u32 divider) {
> > +       return invoke_pm_fn(PM_CLOCK_SETDIVIDER, clock_id, divider, 0,
> > +0, NULL); }
> > +
> > +/**
> > + * zynqmp_pm_clock_getdivider - Get the clock divider for given id
> > + * @clock_id:  ID of the clock
> > + * @div_type:  TYPE_DIV1: div1
> > + *             TYPE_DIV2: div2
> didnt see this  below.
Will be fixed in next version.


> 
> > + * @divider:   divider value.
> > + *
> > + * This function is used by master to get divider values
> > + * for any clock.
> > + *
> > + * Return:     Returns status, either success or error+reason.
> > + */
> > +static int zynqmp_pm_clock_getdivider(u32 clock_id, u32 *divider) {
> > +       u32 ret_payload[PAYLOAD_ARG_CNT];
> > +       int ret;
> > +
> > +       ret = invoke_pm_fn(PM_CLOCK_GETDIVIDER, clock_id, 0, 0, 0,
> ret_payload);
> > +       *divider = ret_payload[1];
> > +
> > +       return ret;
> > +}
> > +
> > +/**
> > + * zynqmp_pm_clock_setrate - Set the clock rate for given id
> > + * @clock_id:  ID of the clock
> > + * @rate:      rate value in hz
> > + *
> > + * This function is used by master to set rate for any clock.
> > + *
> > + * Return:     Returns status, either success or error+reason.
> > + */
> So this can set rate only 4G max ?
Need to fix this to have u64 rate.

> 
> > +static int zynqmp_pm_clock_setrate(u32 clock_id, u32 rate) {
> > +       return invoke_pm_fn(PM_CLOCK_SETRATE, clock_id, rate, 0, 0,
> > +NULL); }
> > +
> > +/**
> > + * zynqmp_pm_clock_getrate - Get the clock rate for given id
> > + * @clock_id:  ID of the clock
> > + * @rate:      rate value in hz
> > + *
> > + * This function is used by master to get rate
> > + * for any clock.
> > + *
> > + * Return:     Returns status, either success or error+reason.
> > + */
> Same question here?
Need to fix this to have u64 rate.

> 
> > +static int zynqmp_pm_clock_getrate(u32 clock_id, u32 *rate) {
> > +       u32 ret_payload[PAYLOAD_ARG_CNT];
> > +       int ret;
> > +
> > +       ret = invoke_pm_fn(PM_CLOCK_GETRATE, clock_id, 0, 0, 0, ret_payload);
> > +       *rate = ret_payload[1];
> > +
> > +       return ret;
> > +}
> > +
> Also  what is the difference between set rate and set divider?
Set rate takes rate as input and dividers are calculated by FW. 
Set divider takes dividers as input and sets them directly.
With linux, it is recommended to use set divider only. Set rate API is mainly for baremetal case.


Thanks,
Jolly Shah

  reply	other threads:[~2018-01-31 19:46 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-24 23:21 [PATCH v3 0/4] drivers: firmware: xilinx: Add firmware driver support Jolly Shah
     [not found] ` <1516836074-4149-1-git-send-email-jollys-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
2018-01-24 23:21   ` [PATCH v3 1/4] dt-bindings: firmware: Add bindings for ZynqMP firmware Jolly Shah
2018-01-30 17:08     ` Rob Herring
2018-01-31 18:03       ` Jolly Shah
2018-02-01 15:11         ` Rob Herring
2018-01-31 18:03     ` Mark Rutland
     [not found]       ` <20180131180354.mqf4gvvprdtycbn5-agMKViyK24J5pKCnmE3YQBJ8xKzm50AiAL8bYrjMMd8@public.gmane.org>
2018-02-01  1:04         ` Jolly Shah
     [not found]           ` <CY1PR0201MB076454E915A6E6947A39F04BB8FA0-w5HSbVcoX6AJQdFqRSmStRrHTHEw16jenBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
2018-02-01 10:27             ` Mark Rutland
2018-02-02 19:50               ` Jolly Shah
2018-01-24 23:21 ` [PATCH v3 2/4] drivers: firmware: xilinx: Add ZynqMP firmware driver Jolly Shah
     [not found]   ` <1516836074-4149-3-git-send-email-jollys-gjFFaj9aHVfQT0dZR+AlfA@public.gmane.org>
2018-01-30  5:05     ` Shubhrajyoti Datta
2018-01-31 19:46       ` Jolly Shah [this message]
2018-01-31 18:20     ` Mark Rutland
     [not found]       ` <20180131182012.oshjmvahetaizlbu-agMKViyK24J5pKCnmE3YQBJ8xKzm50AiAL8bYrjMMd8@public.gmane.org>
2018-02-01  1:23         ` Jolly Shah
     [not found]           ` <CY1PR0201MB0764D88D409A35B8379323B4B8FA0-w5HSbVcoX6AJQdFqRSmStRrHTHEw16jenBOFsp37pqbUKgpGm//BTAC/G2K4zDHf@public.gmane.org>
2018-02-01 10:33             ` Mark Rutland
     [not found]               ` <20180201103321.fftn37mgzbk6oltl-agMKViyK24J5pKCnmE3YQBJ8xKzm50AiAL8bYrjMMd8@public.gmane.org>
2018-02-01 23:54                 ` Jolly Shah
2018-01-24 23:21 ` [PATCH v3 3/4] drivers: firmware: xilinx: Add sysfs interface Jolly Shah
2018-01-24 23:21 ` [PATCH v3 4/4] drivers: firmware: xilinx: Add debugfs interface Jolly Shah
2018-01-25  9:30   ` Greg KH
     [not found]     ` <20180125093057.GA1936-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
2018-01-31 19:48       ` Jolly Shah

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=DM2PR0201MB07676D107FA004D43F1FC49AB8FB0@DM2PR0201MB0767.namprd02.prod.outlook.com \
    --to=jollys@xilinx.com \
    --cc=RAJANV@xilinx.com \
    --cc=ard.biesheuvel@linaro.org \
    --cc=devicetree@vger.kernel.org \
    --cc=dmitry.torokhov@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=hkallweit1@gmail.com \
    --cc=keescook@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mark.rutland@arm.com \
    --cc=matt@codeblueprint.co.uk \
    --cc=michal.simek@xilinx.com \
    --cc=mingo@kernel.org \
    --cc=robh+dt@kernel.org \
    --cc=shubhrajyoti.datta@gmail.com \
    --cc=sudeep.holla@arm.com \
    --subject='RE: [PATCH v3 2/4] drivers: firmware: xilinx: Add ZynqMP firmware driver' \
    /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

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).