* Re: [PATCH v13 2/5] tee: generic TEE subsystem
From: Arnd Bergmann @ 2017-01-20 16:47 UTC (permalink / raw)
To: Jens Wiklander
Cc: valentin.manea, devicetree, javier, emmanuel.michel,
Greg Kroah-Hartman, Mark Rutland, Will Deacon, linux-kernel,
Wei Xu, Nishanth Menon, Jason Gunthorpe, Rob Herring, broonie,
Al Viro, Andrew F. Davis, Olof Johansson, Andrew Morton,
jean-michel.delorme, Michal Simek, linux-arm-kernel
In-Reply-To: <20170119164541.GA22094@jax>
On Thursday, January 19, 2017 5:45:43 PM CET Jens Wiklander wrote:
>
> On Wed, Jan 18, 2017 at 09:19:25PM +0100, Arnd Bergmann wrote:
> > On Friday, November 18, 2016 3:51:37 PM CET Jens Wiklander wrote:
> > > Initial patch for generic TEE subsystem.
> > > This subsystem provides:
> > > * Registration/un-registration of TEE drivers.
> > > * Shared memory between normal world and secure world.
> > > * Ioctl interface for interaction with user space.
> > > * Sysfs implementation_id of TEE driver
> > >
> > > A TEE (Trusted Execution Environment) driver is a driver that interfaces
> > > with a trusted OS running in some secure environment, for example,
> > > TrustZone on ARM cpus, or a separate secure co-processor etc.
> > >
> > > The TEE subsystem can serve a TEE driver for a Global Platform compliant
> > > TEE, but it's not limited to only Global Platform TEEs.
> > >
> > > This patch builds on other similar implementations trying to solve
> > > the same problem:
> > > * "optee_linuxdriver" by among others
> > > Jean-michel DELORME<jean-michel.delorme@st.com> and
> > > Emmanuel MICHEL <emmanuel.michel@st.com>
> > > * "Generic TrustZone Driver" by Javier González <javier@javigon.com>
> >
> > Can you give an example for a system that would contain more than one
> > TEE? I see that you support dynamic registration, and it's clear that
> > there can be more than one type of TEE, but why would one have more
> > than one at a time, and why not more than 32?
>
> I know that ST has systems where there's one TEE in TrustZone and
> another TEE on a separate secure co-processor. If you have several TEEs
> it's probably because they have different capabilities (performance
> versus level of security). Just going beyond two or three different
> levels of security with different TEEs sounds a bit extreme, so a
> maximum of 32 or 16 should be fairly safe. If it turns out I'm wrong in
> this assumption it's not that hard to correct it.
Ok
> >
> > > + if (copy_from_user(&arg, uarg, sizeof(arg)))
> > > + return -EFAULT;
> > > +
> > > + if (sizeof(arg) + TEE_IOCTL_PARAM_SIZE(arg.num_params) != buf.buf_len)
> > > + return -EINVAL;
> > > +
> > > + if (arg.num_params) {
> > > + params = kcalloc(arg.num_params, sizeof(struct tee_param),
> > > + GFP_KERNEL);
> > > + if (!params)
> > > + return -ENOMEM;
> >
> > It would be good to have an upper bound on the number of parameters
> > to limit the size of the memory allocation here.
>
> This is already limited due to:
>
> The test with: buf.buf_len > TEE_MAX_ARG_SIZE
>
> And then another test that the number of parameters matches the buffer size
> with: sizeof(arg) + TEE_IOCTL_PARAM_SIZE(arg.num_params) != buf.buf_len
Ok, makes sense.
> >
> > > +/**
> > > + * struct tee_ioctl_param - parameter
> > > + * @attr: attributes
> > > + * @memref: a memory reference
> > > + * @value: a value
> > > + *
> > > + * @attr & TEE_PARAM_ATTR_TYPE_MASK indicates if memref or value is used in
> > > + * the union. TEE_PARAM_ATTR_TYPE_VALUE_* indicates value and
> > > + * TEE_PARAM_ATTR_TYPE_MEMREF_* indicates memref. TEE_PARAM_ATTR_TYPE_NONE
> > > + * indicates that none of the members are used.
> > > + */
> > > +struct tee_ioctl_param {
> > > + __u64 attr;
> > > + union {
> > > + struct tee_ioctl_param_memref memref;
> > > + struct tee_ioctl_param_value value;
> > > + } u;
> > > +};
> > > +
> > > +#define TEE_IOCTL_UUID_LEN 16
> > > +
> >
> > Having a union in an ioctl argument seems odd. Have you considered
> > using two different ioctl command numbers depending on the type?
>
> struct tee_ioctl_param is used as an array and some parameters can be
> memrefs while other are values.
Got it. I still think it's a bit awkward on the user ABI side.
I also see that (unlike the in-kernel interface) tee_ioctl_param_memref
and tee_ioctl_param_value are both defined in terms of three __u64
members.
How about simply using one format here and making this
struct tee_ioctl_param {
__u64 attr;
__u64 a;
__u64 b;
__u64 c;
};
Given that you need a wrapper to set the pointer in memref anyway?
Having an ioctl with a variable number of variable type arguments
is really a weakness of the ABI, but I don't see a good way around
it either, the above would just make it slightly more direct.
> > > +/**
> > > + * struct tee_iocl_supp_send_arg - Send a response to a received request
> > > + * @ret: [out] return value
> > > + * @num_params [in] number of parameters following this struct
> > > + */
> > > +struct tee_iocl_supp_send_arg {
> > > + __u32 ret;
> > > + __u32 num_params;
> > > + /*
> > > + * this struct is 8 byte aligned since the 'struct tee_ioctl_param'
> > > + * which follows requires 8 byte alignment.
> > > + *
> > > + * Commented out element used to visualize the layout dynamic part
> > > + * of the struct. This field is not available at all if
> > > + * num_params == 0.
> > > + *
> > > + * struct tee_ioctl_param params[num_params];
> > > + */
> > > +} __aligned(8);
> >
> > I'd make that
> >
> > struct tee_ioctl_param params[0];
> >
> > as wel here, as I also commented in patch 3 that has a similar structure.
>
> I'm concerned that this may cause warnings when compiling for user space
> depending on compiler and options. Am I too cautious here?
See https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
I actually misremembered it and the syntax I listed is GCC specific,
but C99 allows "flexible arrays". I think there is no problem relying
on C99 here, we already rely on C99 features elsewhere in headers.
Arnd
^ permalink raw reply
* Re: [PATCH v3 2/4] dt-bindings: Add TI SCI PM Domains
From: Ulf Hansson @ 2017-01-20 16:52 UTC (permalink / raw)
To: Dave Gerlach
Cc: Nishanth Menon, Rob Herring, Santosh Shilimkar,
linux-pm@vger.kernel.org, Lokesh Vutla, Kevin Hilman,
Sudeep Holla, Rafael J . Wysocki, linux-kernel@vger.kernel.org,
Tero Kristo, devicetree@vger.kernel.org, Keerthy, Russell King,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <58821CA2.4050701@ti.com>
[...]
>>> Another option is create something new either common or TI SCI
>>> specific. It could be just a table of ids and phandles in the SCI
>>> node. I'm much more comfortable with an isolated property in one node
>>> than something scattered throughout the DT.
>>
>> To me, this seems like the best possible solution.
>>
>> However, perhaps we should also consider the SCPI Generic power domain
>> (drivers/firmware/scpi_pm_domain.c), because I believe it's closely
>> related.
>> To change the power state of a device, this PM domain calls
>> scpi_device_set|get_power_state() (drivers/firmware/arm_scpi.c), which
>> also needs a device id as a parameter. Very similar to our case with
>> the TI SCI domain.
>>
>> Currently these SCPI device ids lacks corresponding DT bindings, so
>> the scpi_pm_domain tries to work around it by assigning ids
>> dynamically at genpd creation time.
>>
>> That makes me wonder, whether we should think of something common/generic?
>
> When you say something common/generic, do you mean a better binding for genpd,
> or something bigger than that like a new driver? Because I do think a phandle
> cell left open for the genpd provider to interpret solves both the scpi and
> ti-sci problem we are facing here in the best way. Using generic PM domains lets
> us do exactly what we want apart from interpreting the phandle cell with our
> driver, and I feel like anything else we try at this point is just going to be
> to work around that. Is bringing back genpd xlate something we can discuss?
Bringing back xlate, how would that help? Wouldn't that just mean that
you will get one genpd per device? That's not an option, I think we
are all in agreement to that.
Or am I missing something here?
Regarding something "common/generic", I was merely thinking of some
common bindings describing the list of phandle to device ids mapping.
However, let's not make this more complicated than it is already. So
please just ignore my suggestion so we can make this work for your
case first.
However, maybe I didn't fully understood Rob's suggestion. Perhaps Rob
can clarify once more.
Anyway, this is how interpreted Rob's suggestion:
TISCI_PM_DOMAIN_DEVLIST: tisci-pm-domain-devlist {
devs = <&serial0>, <&mmc0>;
devids = <5 10>;
}
With this, you should be to extract the devid which corresponds to the
device that has been attached to the TI SCI PM domain. Don't you
think?
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH v14 3/5] tee: add OP-TEE driver
From: Arnd Bergmann @ 2017-01-20 16:57 UTC (permalink / raw)
To: Jens Wiklander
Cc: Greg Kroah-Hartman, Olof Johansson, Andrew Morton, Wei Xu,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, Al Viro,
valentin.manea-hv44wF8Li93QT0dZR+AlfA,
jean-michel.delorme-qxv4g6HH51o, emmanuel.michel-qxv4g6HH51o,
javier-5MUHepqpBA1BDgjK7y7TUQ, Jason Gunthorpe, Mark Rutland,
Michal Simek, Rob Herring, Will Deacon, Nishanth Menon,
Andrew F . Davis, broonie-DgEjT+Ai2ygdnm+yROfE0A,
scott.branden-dY08KVG/lbpWk0Htik3J/w
In-Reply-To: <20170119145621.GA4958@jax>
On Thursday, January 19, 2017 3:56:23 PM CET Jens Wiklander wrote:
> On Wed, Jan 18, 2017 at 05:28:17PM +0100, Arnd Bergmann wrote:
> > On Wednesday, January 18, 2017 1:58:14 PM CET Jens Wiklander wrote:
> > > +static void optee_cq_complete_one(struct optee_call_queue *cq)
> > > +{
> > > + struct optee_call_waiter *w;
> > > +
> > > + list_for_each_entry(w, &cq->waiters, list_node) {
> > > + if (!w->completed) {
> > > + complete(&w->c);
> > > + w->completed = true;
> > > + break;
> > > + }
> > > + }
> > > +}
> > > +
> > > +static void optee_cq_wait_final(struct optee_call_queue *cq,
> > > + struct optee_call_waiter *w)
> > > +{
> > > + mutex_lock(&cq->mutex);
> > > +
> > > + /* Get out of the list */
> > > + list_del(&w->list_node);
> > > +
> > > + optee_cq_complete_one(cq);
> > > + /*
> > > + * If we're completed we've got a completion that some other task
> > > + * could have used instead.
> > > + */
> > > + if (w->completed)
> > > + optee_cq_complete_one(cq);
> > > +
> > > + mutex_unlock(&cq->mutex);
> > > +}
> >
> > This deserves some more comments: the function name suggests that you are
> > waiting for a specific optee_call_waiter, but then it calls
> > optee_cq_complete_one(), which unconditionally completes the first
> > incomplete completion and it never waits.
>
> OK, I'm updating these functions with more comments. The purpose of
> these functions is to deal with resource shortage in secure world.
> There's a limit on how many threads that execute concurrently in secure
> world. optee_cq_wait_for_completion() waits for any task returning from
> a call to secure world, optee_cq_complete_one() doesn't care who's
> completed as long as tasks aren't stuck when there's available resources
> in secure world.
Ok, that is indeed not clear from the code but your explanation makes
perfect sense, thanks!
> > > +static int __init optee_driver_init(void)
> > > +{
> > > + struct device_node *node;
> > > +
> > > + /*
> > > + * Preferred path is /firmware/optee, but it's the matching that
> > > + * matters.
> > > + */
> > > + for_each_matching_node(node, optee_match)
> > > + of_platform_device_create(node, NULL, NULL);
> > > +
> > > + return platform_driver_register(&optee_driver);
> > > +}
> > > +module_init(optee_driver_init);
> > > +
> > > +static void __exit optee_driver_exit(void)
> > > +{
> > > + platform_driver_unregister(&optee_driver);
> > > +}
> > > +module_exit(optee_driver_exit);
> >
> > What is the platform driver good for if the same module has to create the
> > platform devices itself?
>
> The platform device(s) are created here because the optee node is below
> "/firmware" instead of the root where it would have had the platform
> device created automatically.
>
> I think it's useful to be able to unload the module, the early reviews
> of this patch set was much focused around that. Regardless I'll need
> some device as parent for the devices created during optee_probe() and
> using a platform device for that seems natural.
>
> I'd rather keep the platform driver. Perhaps some variant of the pattern
> in qcom_scm_init() (drivers/firmware/qcom_scm.c) is useful, except that
> I need to find out what to do about the life cycle of the objects
> created with of_platform_populate().
My point was that I don't think we need devices here at all. It's different
when you talk to external hardware that has register resource etc that
can be best abstracted as a real device, but for other firmware features
we don't normally add one.
Module unloading can also be done without the device.
> >
> > I'd just skip it and do
> >
> > for_each_matching_node(node, optee_match)
> > optee_probe(node);
> >
> > I also suspect that module unloading is broken here if you don't clean
> > up the platform devices in the end, so you should already remove the
> > exit function to prevent unloading.
>
> Does the platform devices really need cleaning? I mean
> of_platform_default_populate_init() creates a bunch of platform devices
> which are just left there even if unused. Here we're doing the same
> thing except that we're doing it for a specific node in the DT.
I think it will work if you don't clean them up, but it feels wrong
to have a loadable module that creates devices when loaded but doesn't
remove them when unloaded.
This could be done differently by having the device creation done in
one driver and the the user of that device in another driver, but I
think just killing off the device achieves the same in a simpler way.
> > > +/*
> > > + * Get revision of Trusted OS.
> > > + *
> > > + * Used by non-secure world to figure out which version of the Trusted OS
> > > + * is installed. Note that the returned revision is the revision of the
> > > + * Trusted OS, not of the API.
> > > + *
> > > + * Returns revision in 2 32-bit words in the same way as
> > > + * OPTEE_MSG_CALLS_REVISION described above.
> > > + */
> > > +#define OPTEE_MSG_OS_OPTEE_REVISION_MAJOR 1
> > > +#define OPTEE_MSG_OS_OPTEE_REVISION_MINOR 0
> > > +#define OPTEE_MSG_FUNCID_GET_OS_REVISION 0x0001
> >
> > Just for my understanding, what is the significance of these numbers,
> > i.e. which code (user space, kernel driver, trusted OS) provides
> > the uuid and which one provides the version? The code comments almost
> > make sense to me, but I don't see why specific versions are listed
> > in this header.
>
> You're right, OPTEE_MSG_OS_OPTEE_REVISION_* should be removed. The
> actual version the secure OS is of a mostly informational nature. The
> same goes the OS UUID, but I suppose the actual UUID used by the
> upstream version of OP-TEE OS could be interesting to know.
...
> > What is the expected behavior when one side reports a version that
> > is unknown? Can one side claim to be backwards compatible with
> > a previous version, or does each new version need support on
> > all three sides?
>
> The UUID and version of the message protocol are important to match
> correctly as otherwise it could mean that there's something unexpected
> in secure world that following the message protocol would be undefined
> behaviour. All changes to the message protocol should be backwards
> compatible in the sense that the driver and secure world need to
> negotiate eventual extensions while probing. That's what we're doing in
> optee_msg_exchange_capabilities().
Ok, then maybe the "compatible" identifier in DT should be sufficient
to ensure that the capability exchange works, and the rest be based
on that?
We tend to avoid version checks for APIs in the kernel because they
never work in practice, but the capability check should be fine.
> > > diff --git a/drivers/tee/optee/rpc.c b/drivers/tee/optee/rpc.c
> > > new file mode 100644
> > > index 000000000000..0b9c1a2accd0
> > > --- /dev/null
> > > +++ b/drivers/tee/optee/rpc.c
> > > +static void handle_rpc_func_cmd_wq(struct optee *optee,
> > > + struct optee_msg_arg *arg)
> > > +{
> > > + struct optee_msg_param *params;
> > > +
> > > + if (arg->num_params != 1)
> > > + goto bad;
> > > +
> > > + params = OPTEE_MSG_GET_PARAMS(arg);
> > > + if ((params->attr & OPTEE_MSG_ATTR_TYPE_MASK) !=
> > > + OPTEE_MSG_ATTR_TYPE_VALUE_INPUT)
> > > + goto bad;
> > > +
> > > + switch (params->u.value.a) {
> > > + case OPTEE_MSG_RPC_WAIT_QUEUE_SLEEP:
> > > + wq_sleep(&optee->wait_queue, params->u.value.b);
> > > + break;
> > > + case OPTEE_MSG_RPC_WAIT_QUEUE_WAKEUP:
> > > + wq_wakeup(&optee->wait_queue, params->u.value.b);
> > > + break;
> > > + default:
> > > + goto bad;
> > > + }
> > > +
> > > + arg->ret = TEEC_SUCCESS;
> > > + return;
> > > +bad:
> > > + arg->ret = TEEC_ERROR_BAD_PARAMETERS;
> > > +}
> > > +
> >
> > I'm trying to understand what this is good for. What I can see is that
> > you have a user space process calling into the kernel asking the tee
> > to do some command, and then the tee can ask the kernel to wait for
> > something to happen, or notify it that something has happened.
> >
> > If we wait here, the user process gets suspended until this has
> > actually happened.
> >
> > Am I reading this correctly? If yes, what is the intended use case?
> > Is there some process that is meant to always wait here? What
> > if we ever need to wait for more than one thing at a time (think
> > select or poll?)
>
> I'm updating the comments for OPTEE_MSG_RPC_CMD_WAIT_QUEUE with:
> "If secure world need to wait for a secure world mutex it issues a sleep
> request instead of spinning in secure world. Conversely is a wakeup
> request issued when a secure world mutex with a thread waiting thread is
> unlocked."
>
> The way we're waking up a sleeping thread is a bit limiting in some
> circumstances.
>
> One case is where we need to do it from a secure interrupt handler.
> Because there's no way of doing this kind of RPC to normal world from a
> secure interrupt handler. In that case it wouldn't be mutex the thread
> is waiting for though.
>
> Another case is where there's several guest running in the system and
> more than one guests has access to secure world. If guest2 waits for
> mutex which guest1 is releasing, how can guest2 be notified? Normal RPC
> is impossible here also.
>
> The only way around this limitation I've come up with so far is by doing
> the wakeup via a software generated interrupt destined to the correct
> guest. However this problem is beyond the scope of this patch set.
Ok, I see.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 2/9] ARM64: dts: meson-gxbb-p200: add the ethernet PHY's reset GPIO
From: Martin Blumenstingl @ 2017-01-20 17:07 UTC (permalink / raw)
To: Andrew Lunn
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
narmstrong-rdvid1DuHRBWk0Htik3J/w, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
jbrunet-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <20170120162138.GE25907-g2DYL2Zd6BY@public.gmane.org>
Hi Andrew,
On Fri, Jan 20, 2017 at 5:21 PM, Andrew Lunn <andrew-g2DYL2Zd6BY@public.gmane.org> wrote:
> On Fri, Jan 20, 2017 at 04:22:25PM +0100, Martin Blumenstingl wrote:
>> This resets the ethernet PHY during boot to get the PHY into a "clean"
>> state.
>> While here also specify the phy-handle of the ethmac node to make the
>> PHY configuration similar to the one we have on GXL devices. This will
>> allow us to specify OF-properties for the PHY itself.
>>
>> Signed-off-by: Martin Blumenstingl <martin.blumenstingl-gM/Ye1E23mwN+BqQ9rBEUg@public.gmane.org>
>> ---
>> arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts | 24 ++++++++++++++++++++++++
>> 1 file changed, 24 insertions(+)
>>
>> diff --git a/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts b/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
>> index 03e3d76626dd..fa0f84cfeaa9 100644
>> --- a/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
>> +++ b/arch/arm64/boot/dts/amlogic/meson-gxbb-p200.dts
>> @@ -51,6 +51,30 @@
>> model = "Amlogic Meson GXBB P200 Development Board";
>> };
>>
>> +ðmac {
>> + status = "okay";
>> + pinctrl-0 = <ð_rgmii_pins>;
>> + pinctrl-names = "default";
>> + phy-handle = <ð_phy0>;
>> + phy-mode = "rgmii";
>> +
>> + snps,reset-gpio = <&gpio GPIOZ_14 0>;
>> + snps,reset-delays-us = <0 10000 1000000>;
>> + snps,reset-active-low;
>> +
>> + mdio {
>> + compatible = "snps,dwmac-mdio";
>> + #address-cells = <1>;
>> + #size-cells = <0>;
>> +
>> + eth_phy0: ethernet-phy@0 {
>> + compatible = "ethernet-phy-id0022.1620",
>
> Hi Martin
>
> You don't need to specify the PHY ID. You really only need this when
> the PHY is reporting no ID, or a wrong ID. In fact, if a new revision
> of the board is made, with a different PHY, you might have a problem,
> the wrong PHY driver is loaded.
>
> Andrew
I though that this is good practice - the documentation doesn't say
that it should only be added in specific cases: [0]
my intention behind adding the PHY ID was to make it easier to see
which devices uses which PHY. this is especially useful for devices
with RTL8211F PHY (basically all devices with RGMII PHY except the
P200) as these may need similar fixes than [1]
regarding your other mail: you're absolutely right that ethernet-phy@0
should be ethernet-phy@3.
[0] http://lxr.free-electrons.com/source/Documentation/devicetree/bindings/net/phy.txt
[1] https://patchwork.kernel.org/patch/9528915/
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] arm64: dts: hisilicon: add dtsi file for Hisilicon Hi3660 SOC
From: Wei Xu @ 2017-01-20 17:10 UTC (permalink / raw)
To: Wang Xiaoyin, robh+dt, mark.rutland, catalin.marinas, will.deacon,
linux-arm-kernel, devicetree, linux-kernel
Cc: chenya99
In-Reply-To: <20170118100435.43318-1-hw.wangxiaoyin@hisilicon.com>
Hi Wang Xiaoyin,
On 2017/1/18 10:04, Wang Xiaoyin wrote:
> This patch adds pinctrl dtsi file
>
> Signed-off-by: Wang Xiaoyin <hw.wangxiaoyin@hisilicon.com>
> ---
> arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts | 3 +-
> .../arm64/boot/dts/hisilicon/hikey960-pinctrl.dtsi | 292 +++++++++++++++++++++
> 2 files changed, 293 insertions(+), 2 deletions(-)
> create mode 100644 arch/arm64/boot/dts/hisilicon/hikey960-pinctrl.dtsi
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
> index 9a1d36aa84ce..de21ebfa4bd8 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
> +++ b/arch/arm64/boot/dts/hisilicon/hi3660-hikey960.dts
> @@ -8,8 +8,7 @@
> /dts-v1/;
>
> #include "hi3660.dtsi"
> -/*#include "hi3660-ion.dtsi"*/
> -/*#include "hikey960-pinctrl.dtsi"*/
> +#include "hikey960-pinctrl.dtsi"
Can you please rebase to the 4.10-rc3 or mention which patch you are based on?
Thanks!
Best Regards,
Wei
>
> / {
> model = "HiKey960";
> diff --git a/arch/arm64/boot/dts/hisilicon/hikey960-pinctrl.dtsi b/arch/arm64/boot/dts/hisilicon/hikey960-pinctrl.dtsi
> new file mode 100644
> index 000000000000..98a0b78289b6
> --- /dev/null
> +++ b/arch/arm64/boot/dts/hisilicon/hikey960-pinctrl.dtsi
> @@ -0,0 +1,292 @@
> +/*
> + * pinctrl dts fils for Hislicon HiKey960 development board
> + *
> + */
> +
> +#include <dt-bindings/pinctrl/hisi.h>
> +
> +/ {
> + soc {
> + /* [IOMG_000, IOMG_123] */
> + range: gpio-range {
> + #pinctrl-single,gpio-range-cells = <3>;
> + };
> + pmx0: pinmux@e896c000 {
> + compatible = "pinctrl-single";
> + reg = <0x0 0xe896c000 0x0 0x1f0>;
> + #pinctrl-cells = <1>;
> + #gpio-range-cells = <0x3>;
> + pinctrl-single,register-width = <0x20>;
> + pinctrl-single,function-mask = <0x7>;
> + /* pin base, nr pins & gpio function */
> + pinctrl-single,gpio-range = <
> + &range 0 7 0
> + &range 8 116 0>;
> +
> + isp0_pmx_func: isp0_pmx_func {
> + pinctrl-single,pins = <
> + 0x058 MUX_M1 /* ISP_CLK0 */
> + 0x064 MUX_M1 /* ISP_SCL0 */
> + 0x068 MUX_M1 /* ISP_SDA0 */
> + >;
> + };
> +
> + isp1_pmx_func: isp1_pmx_func {
> + pinctrl-single,pins = <
> + 0x05c MUX_M1 /* ISP_CLK1 */
> + 0x06c MUX_M1 /* ISP_SCL1 */
> + 0x070 MUX_M1 /* ISP_SDA1 */
> + >;
> + };
> +
> + i2c3_pmx_func: i2c3_pmx_func {
> + pinctrl-single,pins = <
> + 0x02c MUX_M1 /* I2C3_SCL */
> + 0x030 MUX_M1 /* I2C3_SDA */
> + >;
> + };
> +
> + i2c4_pmx_func: i2c4_pmx_func {
> + pinctrl-single,pins = <
> + 0x090 MUX_M1 /* I2C4_SCL */
> + 0x094 MUX_M1 /* I2C4_SDA */
> + >;
> + };
> +
> + pcie_perstn_pmx_func: pcie_perstn_pmx_func {
> + pinctrl-single,pins = <
> + 0x15c MUX_M1 /* PCIE_PERST_N */
> + >;
> + };
> +
> + usbhub5734_pmx_func: usbhub5734_pmx_func {
> + pinctrl-single,pins = <
> + 0x11c MUX_M0 /* GPIO_073 */
> + 0x120 MUX_M0 /* GPIO_074 */
> + >;
> + };
> +
> + spi1_pmx_func: spi1_pmx_func {
> + pinctrl-single,pins = <
> + 0x034 MUX_M1 /* SPI1_CLK */
> + 0x038 MUX_M1 /* SPI1_DI */
> + 0x03c MUX_M1 /* SPI1_DO */
> + 0x040 MUX_M1 /* SPI1_CS_N */
> + >;
> + };
> +
> + uart0_pmx_func: uart0_pmx_func {
> + pinctrl-single,pins = <
> + 0x0cc MUX_M2 /* UART0_RXD */
> + 0x0d0 MUX_M2 /* UART0_TXD */
> + 0x0d4 MUX_M2 /* UART0_RXD_M */
> + 0x0d8 MUX_M2 /* UART0_TXD_M */
> + >;
> + };
> +
> + uart1_pmx_func: uart1_pmx_func {
> + pinctrl-single,pins = <
> + 0x0b0 MUX_M2 /* UART1_CTS_N */
> + 0x0b4 MUX_M2 /* UART1_RTS_N */
> + 0x0a8 MUX_M2 /* UART1_RXD */
> + 0x0ac MUX_M2 /* UART1_TXD */
> + >;
> + };
> +
> + uart2_pmx_func: uart2_pmx_func {
> + pinctrl-single,pins = <
> + 0x0bc MUX_M2 /* UART2_CTS_N */
> + 0x0c0 MUX_M2 /* UART2_RTS_N */
> + 0x0c8 MUX_M2 /* UART2_RXD */
> + 0x0c4 MUX_M2 /* UART2_TXD */
> + >;
> + };
> +
> + uart3_pmx_func: uart3_pmx_func {
> + pinctrl-single,pins = <
> + 0x0dc MUX_M1 /* UART3_CTS_N */
> + 0x0e0 MUX_M1 /* UART3_RTS_N */
> + 0x0e4 MUX_M1 /* UART3_RXD */
> + 0x0e8 MUX_M1 /* UART3_TXD */
> + >;
> + };
> +
> + uart4_pmx_func: uart4_pmx_func {
> + pinctrl-single,pins = <
> + 0x0ec MUX_M1 /* UART4_CTS_N */
> + 0x0f0 MUX_M1 /* UART4_RTS_N */
> + 0x0f4 MUX_M1 /* UART4_RXD */
> + 0x0f8 MUX_M1 /* UART4_TXD */
> + >;
> + };
> +
> + uart5_pmx_func: uart5_pmx_func {
> + pinctrl-single,pins = <
> + 0x0c4 MUX_M3 /* UART5_CTS_N */
> + 0x0c8 MUX_M3 /* UART5_RTS_N */
> + 0x0bc MUX_M3 /* UART5_RXD */
> + 0x0c0 MUX_M3 /* UART5_TXD */
> + >;
> + };
> +
> + uart6_pmx_func: uart6_pmx_func {
> + pinctrl-single,pins = <
> + 0x0cc MUX_M1 /* UART6_CTS_N */
> + 0x0d0 MUX_M1 /* UART6_RTS_N */
> + 0x0d4 MUX_M1 /* UART6_RXD */
> + 0x0d8 MUX_M1 /* UART6_TXD */
> + >;
> + };
> + };
> +
> + /* [IOMG_MMC0_000, IOMG_MMC0_005] */
> + pmx1: pinmux@ff37e000 {
> + compatible = "pinctrl-single";
> + reg = <0x0 0xff37e000 0x0 0x18>;
> + #gpio-range-cells = <0x3>;
> + #pinctrl-cells = <1>;
> + pinctrl-single,register-width = <0x20>;
> + pinctrl-single,function-mask = <0x7>;
> + /* pin base, nr pins & gpio function */
> + pinctrl-single,gpio-range = <&range 0 6 0>;
> +
> + sdcard_pmx_func: sdcard_pmx_func {
> + pinctrl-single,pins = <
> + 0x000 MUX_M1 /* SD_CLK */
> + 0x004 MUX_M1 /* SD_CMD */
> + 0x008 MUX_M1 /* SD_DATA0 */
> + 0x00c MUX_M1 /* SD_DATA1 */
> + 0x010 MUX_M1 /* SD_DATA2 */
> + 0x014 MUX_M1 /* SD_DATA3 */
> + >;
> + };
> + };
> +
> + /* [IOMG_FIX_000, IOMG_FIX_011] */
> + pmx2: pinmux@ff3b6000 {
> + compatible = "pinctrl-single";
> + reg = <0x0 0xff3b6000 0x0 0x30>;
> + #pinctrl-cells = <1>;
> + #gpio-range-cells = <0x3>;
> + pinctrl-single,register-width = <0x20>;
> + pinctrl-single,function-mask = <0x7>;
> + /* pin base, nr pins & gpio function */
> + pinctrl-single,gpio-range = <&range 0 12 0>;
> +
> + spi3_pmx_func: spi3_pmx_func {
> + pinctrl-single,pins = <
> + 0x008 MUX_M1 /* SPI3_CLK */
> + 0x00c MUX_M1 /* SPI3_DI */
> + 0x010 MUX_M1 /* SPI3_DO */
> + 0x014 MUX_M1 /* SPI3_CS0_N */
> + >;
> + };
> + };
> +
> + /* [IOMG_MMC1_000, IOMG_MMC1_005] */
> + pmx3: pinmux@ff3fd000 {
> + compatible = "pinctrl-single";
> + reg = <0x0 0xff3fd000 0x0 0x18>;
> + #pinctrl-cells = <1>;
> + #gpio-range-cells = <0x3>;
> + pinctrl-single,register-width = <0x20>;
> + pinctrl-single,function-mask = <0x7>;
> + /* pin base, nr pins & gpio function */
> + pinctrl-single,gpio-range = <&range 0 6 0>;
> +
> + sdio_pmx_func: sdio_pmx_func {
> + pinctrl-single,pins = <
> + 0x000 MUX_M1 /* SDIO_CLK */
> + 0x004 MUX_M1 /* SDIO_CMD */
> + 0x008 MUX_M1 /* SDIO_DATA0 */
> + 0x00c MUX_M1 /* SDIO_DATA1 */
> + 0x010 MUX_M1 /* SDIO_DATA2 */
> + 0x014 MUX_M1 /* SDIO_DATA3 */
> + >;
> + };
> + };
> +
> + /* [IOMG_AO_000, IOMG_AO_041] */
> + pmx4: pinmux@fff11000 {
> + compatible = "pinctrl-single";
> + reg = <0x0 0xfff11000 0x0 0xa8>;
> + #pinctrl-cells = <1>;
> + #gpio-range-cells = <0x3>;
> + pinctrl-single,register-width = <0x20>;
> + pinctrl-single,function-mask = <0x7>;
> + /* pin base in node, nr pins & gpio function */
> + pinctrl-single,gpio-range = <&range 0 42 0>;
> +
> + i2s2_pmx_func: i2s2_pmx_func {
> + pinctrl-single,pins = <
> + 0x044 MUX_M1 /* I2S2_DI */
> + 0x048 MUX_M1 /* I2S2_DO */
> + 0x04c MUX_M1 /* I2S2_XCLK */
> + 0x050 MUX_M1 /* I2S2_XFS */
> + >;
> + };
> +
> + slimbus_pmx_func: slimbus_pmx_func {
> + pinctrl-single,pins = <
> + 0x02c MUX_M1 /* SLIMBUS_CLK */
> + 0x030 MUX_M1 /* SLIMBUS_DATA */
> + >;
> + };
> +
> + i2c0_pmx_func: i2c0_pmx_func {
> + pinctrl-single,pins = <
> + 0x014 MUX_M1 /* I2C0_SCL */
> + 0x018 MUX_M1 /* I2C0_SDA */
> + >;
> + };
> +
> + i2c1_pmx_func: i2c1_pmx_func {
> + pinctrl-single,pins = <
> + 0x01c MUX_M1 /* I2C1_SCL */
> + 0x020 MUX_M1 /* I2C1_SDA */
> + >;
> + };
> +
> + i2c2_pmx_func: i2c2_pmx_func {
> + pinctrl-single,pins = <
> + 0x024 MUX_M1 /* I2C2_SCL */
> + 0x028 MUX_M1 /* I2C2_SDA */
> + >;
> + };
> +
> + i2c7_pmx_func: i2c7_pmx_func {
> + pinctrl-single,pins = <
> + 0x024 MUX_M3 /* I2C7_SCL */
> + 0x028 MUX_M3 /* I2C7_SDA */
> + >;
> + };
> +
> + spi2_pmx_func: spi2_pmx_func {
> + pinctrl-single,pins = <
> + 0x08c MUX_M1 /* SPI2_CLK */
> + 0x090 MUX_M1 /* SPI2_DI */
> + 0x094 MUX_M1 /* SPI2_DO */
> + 0x098 MUX_M1 /* SPI2_CS0_N */
> + >;
> + };
> +
> + spi4_pmx_func: spi4_pmx_func {
> + pinctrl-single,pins = <
> + 0x08c MUX_M4 /* SPI4_CLK */
> + 0x090 MUX_M4 /* SPI4_DI */
> + 0x094 MUX_M4 /* SPI4_DO */
> + 0x098 MUX_M4 /* SPI4_CS0_N */
> + >;
> + };
> +
> + i2s0_pmx_func: i2s0_pmx_func {
> + pinctrl-single,pins = <
> + 0x034 MUX_M1 /* I2S0_DI */
> + 0x038 MUX_M1 /* I2S0_DO */
> + 0x03c MUX_M1 /* I2S0_XCLK */
> + 0x040 MUX_M1 /* I2S0_XFS */
> + >;
> + };
> + };
> + };
> +};
>
^ permalink raw reply
* Re: [PATCH v3 2/3] mmc: sh_mobile_sdhi: explain clock bindings
From: Ulf Hansson @ 2017-01-20 17:12 UTC (permalink / raw)
To: Chris Brandt
Cc: Rob Herring, Mark Rutland, Simon Horman, Wolfram Sang,
Geert Uytterhoeven, devicetree@vger.kernel.org,
linux-mmc@vger.kernel.org, Linux-Renesas
In-Reply-To: <SG2PR06MB1165B5BF4296DB866F3D418B8A710@SG2PR06MB1165.apcprd06.prod.outlook.com>
On 20 January 2017 at 17:05, Chris Brandt <Chris.Brandt@renesas.com> wrote:
> Hello Ulf,
>
> Friday, January 20, 2017, Ulf Hansson wrote:
>> > +- clocks: Most controllers only have 1 clock source per channel.
>> However, some
>> > + have a second clock dedicated to card detection. If 2 clocks
>> are
>> > + specified, you must name them as "core" and "cd". If the
>> controller
>> > + only has 1 clock, naming is not required.
>>
>> Could you please elaborate a bit on the card detection clock?
>>
>> I guess that there is some kind of internal card detection logic (native
>> card detect) in the SDHI IP, which requires a separate clock for it to
>> work? Perhaps you can state that somehow?
>
>
> The reality is that the chip guys cut up the standard SDHI IP to add a
> 'cool new feature', but all I want to do is put it back the way it was.
>
> NOTE: The design guys like to reuse IP blocks from previous designs that they
> know worked and didn't have bugs. So, there is a good chance you will see this
> change in future RZ/A devices (or even other future Renesas SoC devices).
> To remove an unwanted feature adds additional design risk of breaking
> something else....and given the cost of redoing silicon mask layers...no
> engineer wants that mistake on their hands.
So, one should be aware of that runtime PM support in these case is
going to be suboptimal.
For example, when using this native card detect, you will need to keep
the controller runtime PM resumed as the be able to keep the clock on
and to be able to fetch the irq. Wasting power.
Most SoC vendors are therefore using a GPIO card detect instead,
although I assume you already knew that. :-)
[...]
Kind regards
Uffe
^ permalink raw reply
* Re: [PATCH 2/2] ARM: dts: exynos: Use correct mfc_pd async-bridge clock for Exynos5420
From: Javier Martinez Canillas @ 2017-01-20 17:15 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: linux-kernel, Inki Dae, Andi Shyti, Shuah Khan, Marek Szyprowski,
Andrzej Hajda, devicetree, Kukjin Kim, Russell King,
linux-samsung-soc, Rob Herring, Mark Rutland, linux-arm-kernel
In-Reply-To: <20170120162846.x766u3nzblqywggj@kozik-lap>
Hello Krzysztof,
On 01/20/2017 01:28 PM, Krzysztof Kozlowski wrote:
> On Thu, Jan 19, 2017 at 07:29:55PM -0300, Javier Martinez Canillas wrote:
>> Commit 94aed538e032 ("ARM: dts: exynos: Add async-bridge clock to MFC
>> power domain for Exynos5420") fixed an imprecise external abort error
>> when the MFC registers were tried to be accessed and the needed clock
>> for the asynchronous bridges were gated.
>>
>> But according to the Exynos5420 manual the "Gating AXI clock for MFC"
>> is not CLK_ACLK333 but CLK_MFC.
>>
>> The end effect is the same because CLK_ACLK333 is a parent of CLK_MFC
>> but the correct clock should be used instead.
>>
>> Signed-off-by: Javier Martinez Canillas <javier@osg.samsung.com>
>>
>> ---
>>
>> arch/arm/boot/dts/exynos5420.dtsi | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> Is this still needed?
>
Not really, only if we care about correctness in the existing power domains
that have their clocks defined. But as said, even currently with CLK_ACLK333
works due to the clock hierarchy.
I think is less of an issue now that we prefer to mark clocks that needs to
be ungated as critical instead of growing the DT ABI.
> Best regards,
> Krzysztof
>
Best regards,
--
Javier Martinez Canillas
Open Source Group
Samsung Research America
^ permalink raw reply
* Re: [PATCH 2/2] ARM: dts: exynos: Use correct mfc_pd async-bridge clock for Exynos5420
From: Krzysztof Kozlowski @ 2017-01-20 17:32 UTC (permalink / raw)
To: Javier Martinez Canillas
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Inki Dae, Andi Shyti,
Shuah Khan, Marek Szyprowski, Andrzej Hajda,
devicetree-u79uwXL29TY76Z2rM5mHXA, Kukjin Kim, Russell King,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA, Rob Herring,
Mark Rutland, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
In-Reply-To: <55cdceb9-d768-d5a9-7bc2-d4ef26d2e5e8-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
On Fri, Jan 20, 2017 at 02:15:40PM -0300, Javier Martinez Canillas wrote:
> Hello Krzysztof,
>
> On 01/20/2017 01:28 PM, Krzysztof Kozlowski wrote:
> > On Thu, Jan 19, 2017 at 07:29:55PM -0300, Javier Martinez Canillas wrote:
> >> Commit 94aed538e032 ("ARM: dts: exynos: Add async-bridge clock to MFC
> >> power domain for Exynos5420") fixed an imprecise external abort error
> >> when the MFC registers were tried to be accessed and the needed clock
> >> for the asynchronous bridges were gated.
> >>
> >> But according to the Exynos5420 manual the "Gating AXI clock for MFC"
> >> is not CLK_ACLK333 but CLK_MFC.
> >>
> >> The end effect is the same because CLK_ACLK333 is a parent of CLK_MFC
> >> but the correct clock should be used instead.
> >>
> >> Signed-off-by: Javier Martinez Canillas <javier-JPH+aEBZ4P+UEJcrhfAQsw@public.gmane.org>
> >>
> >> ---
> >>
> >> arch/arm/boot/dts/exynos5420.dtsi | 2 +-
> >> 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > Is this still needed?
> >
>
> Not really, only if we care about correctness in the existing power domains
> that have their clocks defined. But as said, even currently with CLK_ACLK333
> works due to the clock hierarchy.
>
> I think is less of an issue now that we prefer to mark clocks that needs to
> be ungated as critical instead of growing the DT ABI.
Okay then, I'll skip the patch. Resend if it turns out to be needed.
Best regards,
Krzysztof
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] of/platform: dynamic: Use of_platform_bus_create() to create devices
From: Alexander Sverdlin @ 2017-01-20 17:38 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Frank Rowand,
Pantelis Antoniou, Grant Likely
In-Reply-To: <CAL_JsqJZ__mUwRT14BmNQKaa_mAPsJm0iWP8sb6Oqs+_-Gu5hw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hello Rob,
On 27/10/16 15:00, Rob Herring wrote:
>> @@ -534,15 +535,15 @@ static int of_platform_notify(struct notifier_block *nb,
>>
>> /* pdev_parent may be NULL when no bus platform device */
>> pdev_parent = of_find_device_by_node(rd->dn->parent);
>> - pdev = of_platform_device_create(rd->dn, NULL,
>> - pdev_parent ? &pdev_parent->dev : NULL);
>> + ret = of_platform_bus_create(rd->dn, of_default_bus_match_table,
>> + NULL, pdev_parent ?
>> + &pdev_parent->dev : NULL, true);
> I think this should be of_platform_default_populate() instead.
I've just tested: it cannot be. It calls of_platform_populate(), which ignores "status" property.
This not only brakes half of the unit tests, but also would change the existing behavior of
overlays (they will completely ignore "status" too).
--
Best regards,
Alexander Sverdlin.
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 2/9] ARM64: dts: meson-gxbb-p200: add the ethernet PHY's reset GPIO
From: Andrew Lunn @ 2017-01-20 17:46 UTC (permalink / raw)
To: Martin Blumenstingl
Cc: linux-amlogic-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
khilman-rdvid1DuHRBWk0Htik3J/w, carlo-KA+7E9HrN00dnm+yROfE0A,
mark.rutland-5wv7dgnIgG8, devicetree-u79uwXL29TY76Z2rM5mHXA,
narmstrong-rdvid1DuHRBWk0Htik3J/w, catalin.marinas-5wv7dgnIgG8,
will.deacon-5wv7dgnIgG8, robh+dt-DgEjT+Ai2ygdnm+yROfE0A,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
jbrunet-rdvid1DuHRBWk0Htik3J/w
In-Reply-To: <CAFBinCA47Nng_jjxKrWrV_YBHr0bg+7xPaJrVswt_WnZ6wtyjA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Fri, Jan 20, 2017 at 06:07:20PM +0100, Martin Blumenstingl wrote:
> I though that this is good practice - the documentation doesn't say
> that it should only be added in specific cases: [0]
Agreed, the text should be better, listing when it is appropriate to
use.
> my intention behind adding the PHY ID was to make it easier to see
> which devices uses which PHY. this is especially useful for devices
> with RTL8211F PHY (basically all devices with RGMII PHY except the
> P200) as these may need similar fixes than [1]
I would not say it is wrong to list the PHY ID, it just has
consequences if the PHY ever changes.
Andrew
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 4/8] clk: renesas: cpg-mssr: Add support for reset control
From: Geert Uytterhoeven @ 2017-01-20 18:03 UTC (permalink / raw)
To: Philipp Zabel
Cc: Geert Uytterhoeven, Simon Horman, Magnus Damm, Michael Turquette,
Stephen Boyd, Rob Herring, Mark Rutland, linux-clk,
devicetree@vger.kernel.org, Linux-Renesas,
linux-kernel@vger.kernel.org,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <1484927830.2897.51.camel@pengutronix.de>
Hi Philipp,
On Fri, Jan 20, 2017 at 4:57 PM, Philipp Zabel <p.zabel@pengutronix.de> wrote:
> On Fri, 2017-01-20 at 15:08 +0100, Geert Uytterhoeven wrote:
>> Add optional support for the Reset Control feature of the Renesas Clock
>> Pulse Generator / Module Standby and Software Reset module on R-Car
>> Gen2, R-Car Gen3, and RZ/G1 SoCs.
>
> Is there a reason to make this optional?
With "optional", I mean that I don't select CONFIG_RESET_CONTROLLER, and
make the reset controller code depend on CONFIG_RESET_CONTROLLER.
So far we don't have any mandatory users.
>> This allows to reset SoC devices using the Reset Controller API.
>>
>> Signed-off-by: Geert Uytterhoeven <geert+renesas@glider.be>
>
> Looks good to me,
>
> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
Thanks!
> Just a small issue below,
>
>> ---
>> drivers/clk/renesas/renesas-cpg-mssr.c | 122 +++++++++++++++++++++++++++++++++
>> 1 file changed, 122 insertions(+)
>>
>> diff --git a/drivers/clk/renesas/renesas-cpg-mssr.c b/drivers/clk/renesas/renesas-cpg-mssr.c
>> index f1161a585c57e433..ea4af714ac14603a 100644
>> --- a/drivers/clk/renesas/renesas-cpg-mssr.c
>> +++ b/drivers/clk/renesas/renesas-cpg-mssr.c
> [...]
>> +static int cpg_mssr_reset(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
>> + unsigned int reg = id / 32;
>> + unsigned int bit = id % 32;
>> + u32 bitmask = BIT(bit);
>
> Here you have a bitmask = BIT(bit) variable.
Because there are two users in the function.
>> + unsigned long flags;
>> + u32 value;
>> +
>> + dev_dbg(priv->dev, "reset %u%02u\n", reg, bit);
>> +
>> + /* Reset module */
>> + spin_lock_irqsave(&priv->rmw_lock, flags);
>> + value = readl(priv->base + SRCR(reg));
>> + value |= bitmask;
>
> Here you use it.
>
>> + writel(value, priv->base + SRCR(reg));
>> + spin_unlock_irqrestore(&priv->rmw_lock, flags);
>> +
>> + /* Wait for at least one cycle of the RCLK clock (@ ca. 32 kHz) */
>> + udelay(35);
>> +
>> + /* Release module from reset state */
>> + writel(bitmask, priv->base + SRSTCLR(reg));
>> +
>> + return 0;
>> +}
>> +
>> +static int cpg_mssr_assert(struct reset_controller_dev *rcdev, unsigned long id)
>> +{
>> + struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
>> + unsigned int reg = id / 32;
>> + unsigned int bit = id % 32;
>
> Here you haven't.
>
>> + unsigned long flags;
>> + u32 value;
>> +
>> + dev_dbg(priv->dev, "assert %u%02u\n", reg, bit);
>> +
>> + spin_lock_irqsave(&priv->rmw_lock, flags);
>> + value = readl(priv->base + SRCR(reg));
>> + writel(value | BIT(bit), priv->base + SRCR(reg));
>
> Here you don't.
Because there's a single user in the function.
>> + spin_unlock_irqrestore(&priv->rmw_lock, flags);
>> + return 0;
>> +}
>> +
>> +static int cpg_mssr_deassert(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
>> + unsigned int reg = id / 32;
>> + unsigned int bit = id % 32;
>> +
>> + dev_dbg(priv->dev, "deassert %u%02u\n", reg, bit);
>> +
>> + writel(BIT(bit), priv->base + SRSTCLR(reg));
>
> And here ...
>
>> + return 0;
>> +}
>> +
>> +static int cpg_mssr_status(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + struct cpg_mssr_priv *priv = rcdev_to_priv(rcdev);
>> + unsigned int reg = id / 32;
>> + unsigned int bit = id % 32;
>> +
>> + return !!(readl(priv->base + SRCR(reg)) & BIT(bit));
>
> And here neither.
>
> I'd choose one variant over the other for consistency.
OK, I'll use the "bitmask" variable in all functions.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH 36/37] ARM: DRA7: clockdomain: Change the CLKTRCTRL of CM_PCIE_CLKSTCTRL to SW_WKUP
From: Tony Lindgren @ 2017-01-20 18:28 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Bjorn Helgaas, Jingoo Han, Joao Pinto, Arnd Bergmann, linux-pci,
linux-doc, linux-kernel, devicetree, linux-omap, linux-arm-kernel,
linux-samsung-soc, linuxppc-dev, linux-arm-kernel, linux-arm-msm,
nsekhar
In-Reply-To: <587C628E.7070804@ti.com>
* Kishon Vijay Abraham I <kishon@ti.com> [170115 22:06]:
> Hi Tony,
>
> On Friday 13 January 2017 10:45 PM, Tony Lindgren wrote:
> > * Kishon Vijay Abraham I <kishon@ti.com> [170112 02:35]:
> >> The PCIe programming sequence in TRM suggests CLKSTCTRL of PCIe should
> >> be set to SW_WKUP. There are no issues when CLKSTCTRL is set to HW_AUTO
> >> in RC mode. However in EP mode, the host system is not able to access the
> >> MEMSPACE and setting the CLKSTCTRL to SW_WKUP fixes it.
> >
> > I guess ideally in the long run we would set this dynamically based on
> > the selected mode, right?
>
> The programming sequence mentioned in the TRM w.r.t clock programming is same
> for both host mode or device mode. Though we never faced any issues in host
> mode when HW_AUTO is set, it's better to follow TRM recommended settings IMHO.
OK best to merge this with the whole series:
Acked-by: Tony Lindgren <tony@atomide.com>
^ permalink raw reply
* Re: [PATCH 37/37] ARM: dts: DRA7: Add pcie1 dt node for EP mode
From: Tony Lindgren @ 2017-01-20 18:30 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: Bjorn Helgaas, Jingoo Han, Joao Pinto, Arnd Bergmann, linux-pci,
linux-doc, linux-kernel, devicetree, linux-omap, linux-arm-kernel,
linux-samsung-soc, linuxppc-dev, linux-arm-kernel, linux-arm-msm,
nsekhar
In-Reply-To: <1484216786-17292-38-git-send-email-kishon@ti.com>
* Kishon Vijay Abraham I <kishon@ti.com> [170112 02:34]:
> Add pcie1 dt node in order for the controller to operate in
> endpoint mode. However since none of the dra7 based boards have
> slots configured to operate in endpoint mode, keep EP mode
> disabled.
Can this be merged separately later on without breaking anything?
Regards,
Tony
^ permalink raw reply
* Re: [PATCH 0/6] ARM: dts: am335x-sl50: Update to new hardware
From: Tony Lindgren @ 2017-01-20 18:35 UTC (permalink / raw)
To: Enric Balletbo Serra
Cc: Enric Balletbo i Serra, Rob Herring, Russell King,
linux-omap-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel
In-Reply-To: <CAFqH_50s3oMsMK_QPFSKfOdrQ5gLcPp_PNM2emWfXO086_T8oQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
* Enric Balletbo Serra <eballetbo-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> [170116 12:54]:
> Hi Tony,
>
> 2017-01-16 18:41 GMT+01:00 Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org>:
> > * Enric Balletbo i Serra <enric.balletbo-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org> [170116 08:58]:
> >> Hi Tony,
> >>
> >> The following patches updates the device tree file for the SL50 device, there
> >> are few v1 boards and IMHO doesn't make sense to maintain the device tree
> >> for these old devices, so mostly of the updates are related to changes done in
> >> the next hardware revision v2 (the one that will go out).
> >
> > This is only if the v1 boards are company internal and can be easily
> > replaced. As long as that's the case I'm fine with that.
> >
>
> Yes thats the case, I don't expect see v1 board outside.
OK applying all into omap-for-v4.11/dt thanks.
Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 2/3] NFC: trf7970a: Add device tree option of 1.8 Volt IO voltage
From: Mark Greer @ 2017-01-20 18:37 UTC (permalink / raw)
To: Geoff Lansberry
Cc: linux-wireless, lauro.venancio, aloisio.almeida, sameo, robh+dt,
mark.rutland, netdev, devicetree, linux-kernel, justin
In-Reply-To: <1482380314-16440-2-git-send-email-geoff@kuvee.com>
On Wed, Dec 21, 2016 at 11:18:33PM -0500, Geoff Lansberry wrote:
> The TRF7970A has configuration options for supporting hardware designs
> with 1.8 Volt or 3.3 Volt IO. This commit adds a device tree option,
> using a fixed regulator binding, for setting the io voltage to match
> the hardware configuration. If no option is supplied it defaults to
> 3.3 volt configuration.
>
> Signed-off-by: Geoff Lansberry <geoff@kuvee.com>
> ---
> .../devicetree/bindings/net/nfc/trf7970a.txt | 2 ++
> drivers/nfc/trf7970a.c | 26 +++++++++++++++++++++-
Acked-by: Mark Greer <mgreer@animalcreek.com>
^ permalink raw reply
* Re: [PATCH v2 0/5] ARM: OMAP2+: Add earlycon support
From: Tony Lindgren @ 2017-01-20 18:39 UTC (permalink / raw)
To: Lokesh Vutla
Cc: Linux OMAP Mailing List, Device Tree Mailing List, Rob Herring,
Mark Rutland, Tero Kristo, Sekhar Nori, Nishanth Menon, Vignesh R
In-Reply-To: <20170118040326.29259-1-lokeshvutla-l0cyMroinI0@public.gmane.org>
* Lokesh Vutla <lokeshvutla-l0cyMroinI0@public.gmane.org> [170117 20:06]:
> This series adds earlycon support for all am33xx, am43xx, am57xx, dra7xx
> based TI platforms. With this series just passing "earlycon" in bootargs
> is sufficient for early debug.
Applying to omap-for-v4.11/soc and the dts into omap-for-v4.11/dt.
Thanks,
Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v3 00/24] i.MX Media Driver
From: Steve Longerbeam @ 2017-01-20 18:40 UTC (permalink / raw)
To: Philipp Zabel, Hans Verkuil
Cc: robh+dt, mark.rutland, shawnguo, kernel, fabio.estevam, linux,
mchehab, nick, markus.heiser, laurent.pinchart+renesas, bparrot,
geert, arnd, sudipm.mukherjee, minghsiu.tsai, tiffany.lin,
jean-christophe.trotin, horms+renesas, niklas.soderlund+renesas,
robert.jarzmik, songjun.wu, andrew-ct.chen, gregkh, devicetree,
linux-kernel, linux-arm-kernel, linux-media, devel,
Steve Longerbeam
In-Reply-To: <1484929911.2897.70.camel@pengutronix.de>
Hi Hans, Philipp,
On 01/20/2017 08:31 AM, Philipp Zabel wrote:
> Hi Hans,
>
> On Fri, 2017-01-20 at 14:52 +0100, Hans Verkuil wrote:
>> Hi Steve, Philipp,
>>
>> On 01/07/2017 03:11 AM, Steve Longerbeam wrote:
>>> In version 3:
>>>
>>> Changes suggested by Rob Herring <robh@kernel.org>:
>>>
>>> - prepended FIM node properties with vendor prefix "fsl,".
>>>
>>> - make mipi csi-2 receiver compatible string SoC specific:
>>> "fsl,imx6-mipi-csi2" instead of "fsl,imx-mipi-csi2".
>>>
>>> - redundant "_clk" removed from mipi csi-2 receiver clock-names property.
>>>
>>> - removed board-specific info from the media driver binding doc. These
>>> were all related to sensor bindings, which already are (adv7180)
>>> or will be (ov564x) covered in separate binding docs. All reference
>>> board info not related to DT bindings has been moved to
>>> Documentation/media/v4l-drivers/imx.rst.
>>>
>>> - removed "_mipi" from the OV5640 compatible string.
>>>
>>> Changes suggested by Vladimir Zapolskiy <vladimir_zapolskiy@mentor.com>:
>>>
>>> Mostly cosmetic/non-functional changes which I won't list here, except
>>> for the following:
>>>
>>> - spin_lock_irqsave() changed to spin_lock() in a couple interrupt handlers.
>>>
>>> - fixed some unnecessary of_node_put()'s in for_each_child_of_node() loops.
>>>
>>> - check/handle return code from required reg property of CSI port nodes.
>>>
>>> - check/handle return code from clk_prepare_enable().
>>>
>>> Changes suggested by Fabio Estevam <festevam@gmail.com>:
>>>
>>> - switch to VGEN3 Analog Vdd supply assuming rev. C SabreSD boards.
>>>
>>> - finally got around to passing valid IOMUX pin config values to the
>>> pin groups.
>>>
>>> Other changes:
>>>
>>> - removed the FIM properties that overrided the v4l2 FIM control defaults
>>> values. This was left-over from a requirement of a customer and is not
>>> necessary here.
>>>
>>> - The FIM must be explicitly enabled in the fim child node under the CSI
>>> port nodes, using the status property. If not enabled, FIM v4l2 controls
>>> will not appear in the video capture driver.
>>>
>>> - brought in additional media types patch from Philipp Zabel. Use new
>>> MEDIA_ENT_F_VID_IF_BRIDGE in mipi csi-2 receiver subdev.
>>>
>>> - brought in latest platform generic video multiplexer subdevice driver
>>> from Philipp Zabel (squashed with patch that uses new MEDIA_ENT_F_MUX).
>>>
>>> - removed imx-media-of.h, moved those prototypes into imx-media.h.
>> Based on the discussion on the mailinglist it seems everyone agrees that this
>> is the preferred driver, correct?
> No. I have some major reservations against the custom mem2mem framework
> embedded in Steve's driver.
> I think it is a misuse of the media entity links (which should describe
> hardware connections) for something that should be done at the vb2 level
> (letting one device's capture EOF interrupt trigger the next device's
> m2m device_run without going through userspace).
> Steve and I disagree on that point, so we'd appreciate if we could get
> some more eyes on the above issue.
This needs some background first, so let me first describe one example
pipeline in this driver.
There is a VDIC entity in the i.MX IPU that performs de-interlacing with
hardware filters for motion compensation. Some of the motion compensation
modes ("low" and "medium" motion) require that the VDIC receive video
frame fields from memory buffers (dedicated dma channels in the
IPU are used to transfer those buffers into the VDIC).
So one option to support those modes would be to pass the raw buffers
from a camera sensor up to userspace to a capture device, and then pass
them back to the VDIC for de-interlacing using a mem2mem device.
Philipp and I are both in agreement that, since userland is not interested
in the intermediate interlaced buffers in this case, but only the final
result (motion compensated, de-interlaced frames), it is more efficient
to provide a media link that allows passing those intermediate frames
directly from a camera source pad to VDIC sink pad, without having
to route them through userspace.
So in order to support that, I've implemented a simple FIFO dma buffer
queue in the driver to allow passing video buffers directly from a source
to a sink. It is modeled loosely off the vb2 state machine and API, but
simpler (for instance it only allows contiguous, cache-coherent buffers).
This is where Philipp has an argument, that this should be done with a
new API in videobuf2.
And I'm actually in total agreement with that. I definitely agree that there
should be a mechanism in the media framework that allows passing video
buffers from a source pad to a sink pad using a software queue, with no
involvement from userland.
My only disagreement is when this should be implemented. I think it is
fine to keep my custom implementation of this in the driver for now. Once
an extension of vb2 is ready to support this feature, it would be fairly
straightforward to strip out my custom implementation and go with the
new API.
Steve
^ permalink raw reply
* Re: [PATCH v4 2/4] Documentation: dt: add bindings for ti-cpufreq
From: Tony Lindgren @ 2017-01-20 18:43 UTC (permalink / raw)
To: Dave Gerlach
Cc: Rob Herring, Viresh Kumar, Rafael J . Wysocki,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-omap-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA, Nishanth Menon
In-Reply-To: <b7d11925-879a-9be0-fdd8-45b70e642ce1-l0cyMroinI0@public.gmane.org>
* Dave Gerlach <d-gerlach-l0cyMroinI0@public.gmane.org> [170119 10:32]:
> On 01/19/2017 12:07 PM, Rob Herring wrote:
> > > + /*
> > > + * The three following nodes are marked with opp-suspend
> > > + * because they can not be enabled simultaneously on a
> > > + * single SoC.
> > > + */
> > > + opp50@300000000 {
> >
> > What's the 50, 100, 120 in the names?
>
> Those are the names of the OPPs given in Table 5-7 of the AM335x data
> manual, seen here http://www.ti.com/lit/ds/symlink/am3359.pdf . I typically
> reference the table and document in the commit message of the actual DT
> patches but didn't here for the binding.
If you don't need the names, you could maybe use:
opp100: opp@275000000 {
...
};
opp200: opp@300000000 {
...
};
...
Regards,
Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v7 0/5] Support qcom's HSIC USB and rewrite USB2 HS support
From: Stephen Boyd @ 2017-01-20 18:50 UTC (permalink / raw)
To: Peter Chen, Kishon Vijay Abraham I
Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, linux-usb,
Andy Gross, Bjorn Andersson, Neil Armstrong, Arnd Bergmann,
Felipe Balbi, Greg Kroah-Hartman, devicetree
This patch series continues the usb chipidea rewrite for
Qualcommm platforms. I've dropped the patches that were applied
to Peter's tree for chipidea. Now the phy drivers are left,
along with a new hook to set the vbus in the phy and changes
to chipidea to call the new hook.
I've left the HSIC phy driver here, because it wasn't merged in
the last round. Nothing has changed in that driver, so I believe
it is ready to be merged now. The real changes are the new
set_vbus() hook and how we're supposed to handle that in the HS
phy driver.
The only dependencies between subsystems is the set_vbus callback.
If that can go into some stable branch in the phy tree, then Kishon
can apply the phy patches on top of that and Peter can apply the small
chipidea patch on top of that. Or everything can go in parallel, except
for the chipidea patch that adds the set_vbus hook. That hook can be
added after rc1 when dependencies merge.
Patches based on v4.10-rc1 + first 22 patches from v5. Full
branch is here[1].
Changes from v6:
* Dropped first 22 applied patches
* Rewrote phy_set_mode() patch to be msm specific
* New set_vbus() callback in phy framework
* Updated HS phy driver for set_vbus() callback
Changes from v5:
* Replaced "Emulate OTGSC interrupt enable path" patch with a patch
from Peter
* Updated HS phy driver to support set_mode callback to handle pullup
* New patch to set the mode to device or host in chipidea udc pullup
function to toggle the pullup for HS mode
* New patch to drop lock around event_notify callback to avoid lockdep
issues
* Removal of extcon usage from HS phy driver
* Picked up acks from Heikki and Peter on ULPI core patch
Changes from v4:
* Picked up Acks from Rob
* Updated HS phy init sequence DT property to restrict it to offsets
Changes from v3:
* Picked up Acks from Peter
* Updated extcon consolidation patch per Peter's comments
* Folded in simplification from Heikki for ULPI DT matching
Changes from v2:
* Added SoC specific compatibles in phy bindings
* Dropped AVVIS patch for OTG statemachine
* New patch to consolidate extcon handlers
* Picked up Acks from Peter
* Rebased onto v4.8-rc1
* Reworked ULPI OF code to look at vid == 0 instead of pid == 0
* Dropped ULPI bindings for vid and pid overrides
Changes from v1:
* Reworked ULPI device probing to keep using vendor/product ids that
come from DT if needed and falls back to OF style match when product id
is 0
* PHY init later patch was rejected so that moved to a quirk flag and
the msm wrapper started managing the phy on/off
* Updated clk requirements for HSIC phy in binding doc
* Added optional clk in wrapper for "housekeeping" found on older qcom
platforms
* Bug fix to OTGSC polling function
* Changed runtime PM patch to set as active instead of get/put
TODO:
* DMA fails on arm64 so we need something like [2] to make it work.
* The db410c needs a driver to toggle the onboard switch to connect
the usb hub instead of micro port when the usb cable is disconnected.
I've sent a patch set for this[3], which needs some further
discussion/development. The current plan is to reintroduce the usb
mux framework.
* apq8064 platforms need a vbus regulator to really use otg and I haven't
tried out the RPM based regulators yet
* The HSIC phy on the apq8074 dragonboard is connected to a usb4604
device which requires the i2c driver to probe and send an i2c
sequence before the HSIC controller enumerates or HSIC doesn't work.
Right now I have a hack to force the controller to probe defer
once so that usb4604 probes first. This needs a more proper solution
like having the DT describe a linkage between the controller and
the usb device so we can enforce probe ordering. My current plan
is to use DT graphs/endpoints for this.
[1] https://git.linaro.org/people/stephen.boyd/linux.git/log/?h=usb-hsic-8074
[2] https://patchwork.kernel.org/patch/9319527/
[3] https://lkml.kernel.org/r/20160914014246.31847-1-stephen.boyd@linaro.org
Stephen Boyd (5):
phy: Add support for Qualcomm's USB HSIC phy
usb: chipidea: msm: Configure phy for appropriate mode
phy: Add set_vbus callback
usb: chipidea: Signal vbus state to phy
phy: Add support for Qualcomm's USB HS phy
.../devicetree/bindings/phy/qcom,usb-hs-phy.txt | 78 +++++++
.../devicetree/bindings/phy/qcom,usb-hsic-phy.txt | 65 ++++++
drivers/phy/Kconfig | 15 ++
drivers/phy/Makefile | 2 +
drivers/phy/phy-core.c | 15 ++
drivers/phy/phy-qcom-usb-hs.c | 256 +++++++++++++++++++++
drivers/phy/phy-qcom-usb-hsic.c | 160 +++++++++++++
drivers/usb/chipidea/ci.h | 7 +-
drivers/usb/chipidea/ci_hdrc_msm.c | 4 +
drivers/usb/chipidea/otg.c | 7 +-
include/linux/phy/phy.h | 10 +
11 files changed, 615 insertions(+), 4 deletions(-)
create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
create mode 100644 drivers/phy/phy-qcom-usb-hs.c
create mode 100644 drivers/phy/phy-qcom-usb-hsic.c
--
2.10.0.297.gf6727b0
^ permalink raw reply
* [PATCH v7 1/5] phy: Add support for Qualcomm's USB HSIC phy
From: Stephen Boyd @ 2017-01-20 18:50 UTC (permalink / raw)
To: Peter Chen, Kishon Vijay Abraham I
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Andy Gross, Bjorn Andersson,
Neil Armstrong, Arnd Bergmann, Felipe Balbi,
linux-usb-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20170120185057.16206-1-stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
The HSIC USB controller on qcom SoCs has an integrated all
digital phy controlled via the ULPI viewport.
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
Cc: <devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>
Signed-off-by: Stephen Boyd <stephen.boyd-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
No changes.
.../devicetree/bindings/phy/qcom,usb-hsic-phy.txt | 65 +++++++++
drivers/phy/Kconfig | 7 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-qcom-usb-hsic.c | 160 +++++++++++++++++++++
4 files changed, 233 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
create mode 100644 drivers/phy/phy-qcom-usb-hsic.c
diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt b/Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
new file mode 100644
index 000000000000..3c7cb2be4b12
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,usb-hsic-phy.txt
@@ -0,0 +1,65 @@
+Qualcomm's USB HSIC PHY
+
+PROPERTIES
+
+- compatible:
+ Usage: required
+ Value type: <string>
+ Definition: Should contain "qcom,usb-hsic-phy" and more specifically one of the
+ following:
+
+ "qcom,usb-hsic-phy-mdm9615"
+ "qcom,usb-hsic-phy-msm8974"
+
+- #phy-cells:
+ Usage: required
+ Value type: <u32>
+ Definition: Should contain 0
+
+- clocks:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: Should contain clock specifier for phy, calibration and
+ a calibration sleep clock
+
+- clock-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: Should contain "phy, "cal" and "cal_sleep"
+
+- pinctrl-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: Should contain "init" and "default" in that order
+
+- pinctrl-0:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: List of pinctrl settings to apply to keep HSIC pins in a glitch
+ free state
+
+- pinctrl-1:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: List of pinctrl settings to apply to mux out the HSIC pins
+
+EXAMPLE
+
+usb-controller {
+ ulpi {
+ phy {
+ compatible = "qcom,usb-hsic-phy-msm8974",
+ "qcom,usb-hsic-phy";
+ #phy-cells = <0>;
+ pinctrl-names = "init", "default";
+ pinctrl-0 = <&hsic_sleep>;
+ pinctrl-1 = <&hsic_default>;
+ clocks = <&gcc GCC_USB_HSIC_CLK>,
+ <&gcc GCC_USB_HSIC_IO_CAL_CLK>,
+ <&gcc GCC_USB_HSIC_IO_CAL_SLEEP_CLK>;
+ clock-names = "phy", "cal", "cal_sleep";
+ assigned-clocks = <&gcc GCC_USB_HSIC_IO_CAL_CLK>;
+ assigned-clock-rates = <960000>;
+ };
+ };
+};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index e8eb7f225a88..a430a64981d5 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -437,6 +437,13 @@ config PHY_QCOM_UFS
help
Support for UFS PHY on QCOM chipsets.
+config PHY_QCOM_USB_HSIC
+ tristate "Qualcomm USB HSIC ULPI PHY module"
+ depends on USB_ULPI_BUS
+ select GENERIC_PHY
+ help
+ Support for the USB HSIC ULPI compliant PHY on QCOM chipsets.
+
config PHY_TUSB1210
tristate "TI TUSB1210 ULPI PHY module"
depends on USB_ULPI_BUS
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index 65eb2f436a41..c43c9df5d301 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_PHY_STIH407_USB) += phy-stih407-usb.o
obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o
obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o
obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-14nm.o
+obj-$(CONFIG_PHY_QCOM_USB_HSIC) += phy-qcom-usb-hsic.o
obj-$(CONFIG_PHY_TUSB1210) += phy-tusb1210.o
obj-$(CONFIG_PHY_BRCM_SATA) += phy-brcm-sata.o
obj-$(CONFIG_PHY_PISTACHIO_USB) += phy-pistachio-usb.o
diff --git a/drivers/phy/phy-qcom-usb-hsic.c b/drivers/phy/phy-qcom-usb-hsic.c
new file mode 100644
index 000000000000..47690f9945b9
--- /dev/null
+++ b/drivers/phy/phy-qcom-usb-hsic.c
@@ -0,0 +1,160 @@
+/**
+ * Copyright (C) 2016 Linaro Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/ulpi/driver.h>
+#include <linux/ulpi/regs.h>
+#include <linux/pinctrl/consumer.h>
+#include <linux/pinctrl/pinctrl-state.h>
+#include <linux/delay.h>
+#include <linux/clk.h>
+
+#include "ulpi_phy.h"
+
+#define ULPI_HSIC_CFG 0x30
+#define ULPI_HSIC_IO_CAL 0x33
+
+struct qcom_usb_hsic_phy {
+ struct ulpi *ulpi;
+ struct phy *phy;
+ struct pinctrl *pctl;
+ struct clk *phy_clk;
+ struct clk *cal_clk;
+ struct clk *cal_sleep_clk;
+};
+
+static int qcom_usb_hsic_phy_power_on(struct phy *phy)
+{
+ struct qcom_usb_hsic_phy *uphy = phy_get_drvdata(phy);
+ struct ulpi *ulpi = uphy->ulpi;
+ struct pinctrl_state *pins_default;
+ int ret;
+
+ ret = clk_prepare_enable(uphy->phy_clk);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(uphy->cal_clk);
+ if (ret)
+ goto err_cal;
+
+ ret = clk_prepare_enable(uphy->cal_sleep_clk);
+ if (ret)
+ goto err_sleep;
+
+ /* Set periodic calibration interval to ~2.048sec in HSIC_IO_CAL_REG */
+ ret = ulpi_write(ulpi, ULPI_HSIC_IO_CAL, 0xff);
+ if (ret)
+ goto err_ulpi;
+
+ /* Enable periodic IO calibration in HSIC_CFG register */
+ ret = ulpi_write(ulpi, ULPI_HSIC_CFG, 0xa8);
+ if (ret)
+ goto err_ulpi;
+
+ /* Configure pins for HSIC functionality */
+ pins_default = pinctrl_lookup_state(uphy->pctl, PINCTRL_STATE_DEFAULT);
+ if (IS_ERR(pins_default))
+ return PTR_ERR(pins_default);
+
+ ret = pinctrl_select_state(uphy->pctl, pins_default);
+ if (ret)
+ goto err_ulpi;
+
+ /* Enable HSIC mode in HSIC_CFG register */
+ ret = ulpi_write(ulpi, ULPI_SET(ULPI_HSIC_CFG), 0x01);
+ if (ret)
+ goto err_ulpi;
+
+ /* Disable auto-resume */
+ ret = ulpi_write(ulpi, ULPI_CLR(ULPI_IFC_CTRL),
+ ULPI_IFC_CTRL_AUTORESUME);
+ if (ret)
+ goto err_ulpi;
+
+ return ret;
+err_ulpi:
+ clk_disable_unprepare(uphy->cal_sleep_clk);
+err_sleep:
+ clk_disable_unprepare(uphy->cal_clk);
+err_cal:
+ clk_disable_unprepare(uphy->phy_clk);
+ return ret;
+}
+
+static int qcom_usb_hsic_phy_power_off(struct phy *phy)
+{
+ struct qcom_usb_hsic_phy *uphy = phy_get_drvdata(phy);
+
+ clk_disable_unprepare(uphy->cal_sleep_clk);
+ clk_disable_unprepare(uphy->cal_clk);
+ clk_disable_unprepare(uphy->phy_clk);
+
+ return 0;
+}
+
+static const struct phy_ops qcom_usb_hsic_phy_ops = {
+ .power_on = qcom_usb_hsic_phy_power_on,
+ .power_off = qcom_usb_hsic_phy_power_off,
+ .owner = THIS_MODULE,
+};
+
+static int qcom_usb_hsic_phy_probe(struct ulpi *ulpi)
+{
+ struct qcom_usb_hsic_phy *uphy;
+ struct phy_provider *p;
+ struct clk *clk;
+
+ uphy = devm_kzalloc(&ulpi->dev, sizeof(*uphy), GFP_KERNEL);
+ if (!uphy)
+ return -ENOMEM;
+ ulpi_set_drvdata(ulpi, uphy);
+
+ uphy->ulpi = ulpi;
+ uphy->pctl = devm_pinctrl_get(&ulpi->dev);
+ if (IS_ERR(uphy->pctl))
+ return PTR_ERR(uphy->pctl);
+
+ uphy->phy_clk = clk = devm_clk_get(&ulpi->dev, "phy");
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ uphy->cal_clk = clk = devm_clk_get(&ulpi->dev, "cal");
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ uphy->cal_sleep_clk = clk = devm_clk_get(&ulpi->dev, "cal_sleep");
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ uphy->phy = devm_phy_create(&ulpi->dev, ulpi->dev.of_node,
+ &qcom_usb_hsic_phy_ops);
+ if (IS_ERR(uphy->phy))
+ return PTR_ERR(uphy->phy);
+ phy_set_drvdata(uphy->phy, uphy);
+
+ p = devm_of_phy_provider_register(&ulpi->dev, of_phy_simple_xlate);
+ return PTR_ERR_OR_ZERO(p);
+}
+
+static const struct of_device_id qcom_usb_hsic_phy_match[] = {
+ { .compatible = "qcom,usb-hsic-phy", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, qcom_usb_hsic_phy_match);
+
+static struct ulpi_driver qcom_usb_hsic_phy_driver = {
+ .probe = qcom_usb_hsic_phy_probe,
+ .driver = {
+ .name = "qcom_usb_hsic_phy",
+ .of_match_table = qcom_usb_hsic_phy_match,
+ },
+};
+module_ulpi_driver(qcom_usb_hsic_phy_driver);
+
+MODULE_DESCRIPTION("Qualcomm USB HSIC phy");
+MODULE_LICENSE("GPL v2");
--
2.10.0.297.gf6727b0
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
* [PATCH v7 5/5] phy: Add support for Qualcomm's USB HS phy
From: Stephen Boyd @ 2017-01-20 18:50 UTC (permalink / raw)
To: Peter Chen, Kishon Vijay Abraham I
Cc: linux-arm-kernel, linux-kernel, linux-arm-msm, Andy Gross,
Bjorn Andersson, Neil Armstrong, Arnd Bergmann, Felipe Balbi,
linux-usb, devicetree
In-Reply-To: <20170120185057.16206-1-stephen.boyd@linaro.org>
The high-speed phy on qcom SoCs is controlled via the ULPI
viewport.
Cc: <devicetree@vger.kernel.org>
Acked-by: Rob Herring <robh@kernel.org>
Signed-off-by: Stephen Boyd <stephen.boyd@linaro.org>
---
phy_set_mode() hook split up to toggle two bits independently
with new set_vbus() hook.
.../devicetree/bindings/phy/qcom,usb-hs-phy.txt | 78 +++++++
drivers/phy/Kconfig | 8 +
drivers/phy/Makefile | 1 +
drivers/phy/phy-qcom-usb-hs.c | 256 +++++++++++++++++++++
4 files changed, 343 insertions(+)
create mode 100644 Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
create mode 100644 drivers/phy/phy-qcom-usb-hs.c
diff --git a/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
new file mode 100644
index 000000000000..bec77a74bd39
--- /dev/null
+++ b/Documentation/devicetree/bindings/phy/qcom,usb-hs-phy.txt
@@ -0,0 +1,78 @@
+Qualcomm's USB HS PHY
+
+PROPERTIES
+
+- compatible:
+ Usage: required
+ Value type: <string>
+ Definition: Should contain "qcom,usb-hs-phy" and more specifically one of the
+ following:
+
+ "qcom,usb-hs-phy-apq8064"
+ "qcom,usb-hs-phy-msm8916"
+ "qcom,usb-hs-phy-msm8974"
+
+- #phy-cells:
+ Usage: required
+ Value type: <u32>
+ Definition: Should contain 0
+
+- clocks:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: Should contain clock specifier for the reference and sleep
+ clocks
+
+- clock-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: Should contain "ref" and "sleep" for the reference and sleep
+ clocks respectively
+
+- resets:
+ Usage: required
+ Value type: <prop-encoded-array>
+ Definition: Should contain the phy and POR resets
+
+- reset-names:
+ Usage: required
+ Value type: <stringlist>
+ Definition: Should contain "phy" and "por" for the phy and POR resets
+ respectively
+
+- v3p3-supply:
+ Usage: required
+ Value type: <phandle>
+ Definition: Should contain a reference to the 3.3V supply
+
+- v1p8-supply:
+ Usage: required
+ Value type: <phandle>
+ Definition: Should contain a reference to the 1.8V supply
+
+- qcom,init-seq:
+ Usage: optional
+ Value type: <u8 array>
+ Definition: Should contain a sequence of ULPI address and value pairs to
+ program into the ULPI_EXT_VENDOR_SPECIFIC area. This is related
+ to Device Mode Eye Diagram test. The addresses are offsets
+ from the ULPI_EXT_VENDOR_SPECIFIC address, for example,
+ <0x1 0x53> would mean "write the value 0x53 to address 0x81".
+
+EXAMPLE
+
+otg: usb-controller {
+ ulpi {
+ phy {
+ compatible = "qcom,usb-hs-phy-msm8974", "qcom,usb-hs-phy";
+ #phy-cells = <0>;
+ clocks = <&xo_board>, <&gcc GCC_USB2A_PHY_SLEEP_CLK>;
+ clock-names = "ref", "sleep";
+ resets = <&gcc GCC_USB2A_PHY_BCR>, <&otg 0>;
+ reset-names = "phy", "por";
+ v3p3-supply = <&pm8941_l24>;
+ v1p8-supply = <&pm8941_l6>;
+ qcom,init-seq = /bits/ 8 <0x1 0x63>;
+ };
+ };
+};
diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig
index a430a64981d5..61a22e985831 100644
--- a/drivers/phy/Kconfig
+++ b/drivers/phy/Kconfig
@@ -437,6 +437,14 @@ config PHY_QCOM_UFS
help
Support for UFS PHY on QCOM chipsets.
+config PHY_QCOM_USB_HS
+ tristate "Qualcomm USB HS PHY module"
+ depends on USB_ULPI_BUS
+ select GENERIC_PHY
+ help
+ Support for the USB high-speed ULPI compliant phy on Qualcomm
+ chipsets.
+
config PHY_QCOM_USB_HSIC
tristate "Qualcomm USB HSIC ULPI PHY module"
depends on USB_ULPI_BUS
diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile
index c43c9df5d301..0e4259473d28 100644
--- a/drivers/phy/Makefile
+++ b/drivers/phy/Makefile
@@ -52,6 +52,7 @@ obj-$(CONFIG_PHY_STIH407_USB) += phy-stih407-usb.o
obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs.o
obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-20nm.o
obj-$(CONFIG_PHY_QCOM_UFS) += phy-qcom-ufs-qmp-14nm.o
+obj-$(CONFIG_PHY_QCOM_USB_HS) += phy-qcom-usb-hs.o
obj-$(CONFIG_PHY_QCOM_USB_HSIC) += phy-qcom-usb-hsic.o
obj-$(CONFIG_PHY_TUSB1210) += phy-tusb1210.o
obj-$(CONFIG_PHY_BRCM_SATA) += phy-brcm-sata.o
diff --git a/drivers/phy/phy-qcom-usb-hs.c b/drivers/phy/phy-qcom-usb-hs.c
new file mode 100644
index 000000000000..50cb40977737
--- /dev/null
+++ b/drivers/phy/phy-qcom-usb-hs.c
@@ -0,0 +1,256 @@
+/**
+ * Copyright (C) 2016 Linaro Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#include <linux/module.h>
+#include <linux/ulpi/driver.h>
+#include <linux/ulpi/regs.h>
+#include <linux/clk.h>
+#include <linux/regulator/consumer.h>
+#include <linux/of_device.h>
+#include <linux/reset.h>
+#include <linux/usb/of.h>
+
+#include "ulpi_phy.h"
+
+#define ULPI_PWR_CLK_MNG_REG 0x88
+# define ULPI_PWR_OTG_COMP_DISABLE BIT(0)
+
+#define ULPI_MISC_A 0x96
+# define ULPI_MISC_A_VBUSVLDEXTSEL BIT(1)
+# define ULPI_MISC_A_VBUSVLDEXT BIT(0)
+
+
+struct ulpi_seq {
+ u8 addr;
+ u8 val;
+};
+
+struct qcom_usb_hs_phy {
+ struct ulpi *ulpi;
+ struct phy *phy;
+ struct clk *ref_clk;
+ struct clk *sleep_clk;
+ struct regulator *v1p8;
+ struct regulator *v3p3;
+ struct reset_control *reset;
+ struct ulpi_seq *init_seq;
+ enum usb_dr_mode dr_mode;
+};
+
+static int qcom_usb_hs_phy_set_mode(struct phy *phy, enum phy_mode mode)
+{
+ struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
+ u8 addr, val = 0;
+ int ret;
+
+ switch (mode) {
+ case PHY_MODE_USB_OTG:
+ switch (uphy->dr_mode) {
+ case USB_DR_MODE_OTG:
+ val |= ULPI_INT_IDGRD;
+ case USB_DR_MODE_PERIPHERAL:
+ val |= ULPI_INT_SESS_VALID;
+ default:
+ break;
+ }
+
+ ret = ulpi_write(uphy->ulpi, ULPI_USB_INT_EN_RISE, val);
+ if (ret)
+ return ret;
+ return ulpi_write(uphy->ulpi, ULPI_USB_INT_EN_FALL, val);
+ case PHY_MODE_USB_DEVICE:
+ addr = ULPI_SET(ULPI_MISC_A);
+ break;
+ case PHY_MODE_USB_HOST:
+ addr = ULPI_CLR(ULPI_MISC_A);
+ break;
+ default:
+ return -EINVAL;
+ }
+
+ ulpi_write(uphy->ulpi, ULPI_SET(ULPI_PWR_CLK_MNG_REG),
+ ULPI_PWR_OTG_COMP_DISABLE);
+ return ulpi_write(uphy->ulpi, addr, ULPI_MISC_A_VBUSVLDEXTSEL);
+}
+
+static int qcom_usb_hs_phy_set_vbus(struct phy *phy, int on)
+{
+ struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
+ u8 addr, val;
+ int ret;
+
+ if (on)
+ addr = ULPI_SET(ULPI_MISC_A);
+ else
+ addr = ULPI_CLR(ULPI_MISC_A);
+
+ return ulpi_write(uphy->ulpi, addr, ULPI_MISC_A_VBUSVLDEXT);
+}
+
+static int qcom_usb_hs_phy_power_on(struct phy *phy)
+{
+ struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
+ struct ulpi *ulpi = uphy->ulpi;
+ const struct ulpi_seq *seq;
+ int ret;
+
+ ret = clk_prepare_enable(uphy->ref_clk);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(uphy->sleep_clk);
+ if (ret)
+ goto err_sleep;
+
+ ret = regulator_set_load(uphy->v1p8, 50000);
+ if (ret < 0)
+ goto err_1p8;
+
+ ret = regulator_enable(uphy->v1p8);
+ if (ret)
+ goto err_1p8;
+
+ ret = regulator_set_voltage_triplet(uphy->v3p3, 3050000, 3300000,
+ 3300000);
+ if (ret)
+ goto err_3p3;
+
+ ret = regulator_set_load(uphy->v3p3, 50000);
+ if (ret < 0)
+ goto err_3p3;
+
+ ret = regulator_enable(uphy->v3p3);
+ if (ret)
+ goto err_3p3;
+
+ for (seq = uphy->init_seq; seq->addr; seq++) {
+ ret = ulpi_write(ulpi, ULPI_EXT_VENDOR_SPECIFIC + seq->addr,
+ seq->val);
+ if (ret)
+ goto err_ulpi;
+ }
+
+ if (uphy->reset) {
+ ret = reset_control_reset(uphy->reset);
+ if (ret)
+ goto err_ulpi;
+ }
+
+ return 0;
+err_ulpi:
+ regulator_disable(uphy->v3p3);
+err_3p3:
+ regulator_disable(uphy->v1p8);
+err_1p8:
+ clk_disable_unprepare(uphy->sleep_clk);
+err_sleep:
+ clk_disable_unprepare(uphy->ref_clk);
+ return ret;
+}
+
+static int qcom_usb_hs_phy_power_off(struct phy *phy)
+{
+ struct qcom_usb_hs_phy *uphy = phy_get_drvdata(phy);
+
+ regulator_disable(uphy->v3p3);
+ regulator_disable(uphy->v1p8);
+ clk_disable_unprepare(uphy->sleep_clk);
+ clk_disable_unprepare(uphy->ref_clk);
+
+ return 0;
+}
+
+static const struct phy_ops qcom_usb_hs_phy_ops = {
+ .power_on = qcom_usb_hs_phy_power_on,
+ .power_off = qcom_usb_hs_phy_power_off,
+ .set_mode = qcom_usb_hs_phy_set_mode,
+ .set_vbus = qcom_usb_hs_phy_set_vbus,
+ .owner = THIS_MODULE,
+};
+
+static int qcom_usb_hs_phy_probe(struct ulpi *ulpi)
+{
+ struct qcom_usb_hs_phy *uphy;
+ struct phy_provider *p;
+ struct clk *clk;
+ struct regulator *reg;
+ struct reset_control *reset;
+ int size;
+ int ret;
+
+ uphy = devm_kzalloc(&ulpi->dev, sizeof(*uphy), GFP_KERNEL);
+ if (!uphy)
+ return -ENOMEM;
+ ulpi_set_drvdata(ulpi, uphy);
+ uphy->ulpi = ulpi;
+ uphy->dr_mode = of_usb_get_dr_mode_by_phy(ulpi->dev.of_node, -1);
+
+ size = of_property_count_u8_elems(ulpi->dev.of_node, "qcom,init-seq");
+ if (size < 0)
+ size = 0;
+ uphy->init_seq = devm_kmalloc_array(&ulpi->dev, (size / 2) + 1,
+ sizeof(*uphy->init_seq), GFP_KERNEL);
+ if (!uphy->init_seq)
+ return -ENOMEM;
+ ret = of_property_read_u8_array(ulpi->dev.of_node, "qcom,init-seq",
+ (u8 *)uphy->init_seq, size);
+ if (ret && size)
+ return ret;
+ /* NUL terminate */
+ uphy->init_seq[size / 2].addr = uphy->init_seq[size / 2].val = 0;
+
+ uphy->ref_clk = clk = devm_clk_get(&ulpi->dev, "ref");
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ uphy->sleep_clk = clk = devm_clk_get(&ulpi->dev, "sleep");
+ if (IS_ERR(clk))
+ return PTR_ERR(clk);
+
+ uphy->v1p8 = reg = devm_regulator_get(&ulpi->dev, "v1p8");
+ if (IS_ERR(reg))
+ return PTR_ERR(reg);
+
+ uphy->v3p3 = reg = devm_regulator_get(&ulpi->dev, "v3p3");
+ if (IS_ERR(reg))
+ return PTR_ERR(reg);
+
+ uphy->reset = reset = devm_reset_control_get(&ulpi->dev, "por");
+ if (IS_ERR(reset)) {
+ if (PTR_ERR(reset) == -EPROBE_DEFER)
+ return PTR_ERR(reset);
+ uphy->reset = NULL;
+ }
+
+ uphy->phy = devm_phy_create(&ulpi->dev, ulpi->dev.of_node,
+ &qcom_usb_hs_phy_ops);
+ if (IS_ERR(uphy->phy))
+ return PTR_ERR(uphy->phy);
+
+ phy_set_drvdata(uphy->phy, uphy);
+
+ p = devm_of_phy_provider_register(&ulpi->dev, of_phy_simple_xlate);
+ return PTR_ERR_OR_ZERO(p);
+}
+
+static const struct of_device_id qcom_usb_hs_phy_match[] = {
+ { .compatible = "qcom,usb-hs-phy", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, qcom_usb_hs_phy_match);
+
+static struct ulpi_driver qcom_usb_hs_phy_driver = {
+ .probe = qcom_usb_hs_phy_probe,
+ .driver = {
+ .name = "qcom_usb_hs_phy",
+ .of_match_table = qcom_usb_hs_phy_match,
+ },
+};
+module_ulpi_driver(qcom_usb_hs_phy_driver);
+
+MODULE_DESCRIPTION("Qualcomm USB HS phy");
+MODULE_LICENSE("GPL v2");
--
2.10.0.297.gf6727b0
^ permalink raw reply related
* RE: [PATCH v3 2/3] mmc: sh_mobile_sdhi: explain clock bindings
From: Chris Brandt @ 2017-01-20 18:51 UTC (permalink / raw)
To: Ulf Hansson
Cc: Rob Herring, Mark Rutland, Simon Horman, Wolfram Sang,
Geert Uytterhoeven, devicetree@vger.kernel.org,
linux-mmc@vger.kernel.org, Linux-Renesas
In-Reply-To: <CAPDyKFqCa_evsr3a_xHNy2joodgP9xiOQ1jdD0=nY-cGTWd_Hg@mail.gmail.com>
Hello Uffe,
On Friday, January 20, 2017, Ulf Hansson wrote:
> > The reality is that the chip guys cut up the standard SDHI IP to add a
> > 'cool new feature', but all I want to do is put it back the way it was.
> >
> > NOTE: The design guys like to reuse IP blocks from previous designs
> > that they know worked and didn't have bugs. So, there is a good chance
> > you will see this change in future RZ/A devices (or even other future
> Renesas SoC devices).
> > To remove an unwanted feature adds additional design risk of breaking
> > something else....and given the cost of redoing silicon mask
> > layers...no engineer wants that mistake on their hands.
>
> So, one should be aware of that runtime PM support in these case is going
> to be suboptimal.
> For example, when using this native card detect, you will need to keep the
> controller runtime PM resumed as the be able to keep the clock on and to
> be able to fetch the irq. Wasting power.
Wolfram already pointed that out in a reply to Geert:
On Tuesday, January 17, 2017, Wolfram Sang wrote:
> > If we handle them as one, won't we miss card detect events due to the
> > card detect clock being disabled while SDHI is idle?
>
> You mean this?
>
> 1208 /*
> 1209 * While using internal tmio hardware logic for card
> detection, we need
> 1210 * to ensure it stays powered for it to work.
> 1211 */
> 1212 if (_host->native_hotplug)
> 1213 pm_runtime_get_noresume(&pdev->dev);
As per your request, I'll go back and add some text to describing that even though
this specific HW has a separate clock for card detect for PM, the existing driver
infrastructure doesn't handle that so both clocks need to be treated as one.
> Most SoC vendors are therefore using a GPIO card detect instead, although
> I assume you already knew that. :-)
My first objective for the RZ/A1 platform is to move things from a local
custom BSP into upstream and get things 'working'. Later I can go back
and tweak things here and there for runtime PM and such.
Thank you,
Chris
^ permalink raw reply
* Re: [PATCH v3 0/3] input: pwm-beeper: add feature to set volume level
From: David Lechner @ 2017-01-20 19:11 UTC (permalink / raw)
To: Dmitry Torokhov, Frieder Schrempf
Cc: robh, pawel.moll, ijc+devicetree, galak, luis, linux-input,
devicetree, linux-kernel
In-Reply-To: <20170119213700.GC13542@dtor-ws>
On 01/19/2017 03:37 PM, Dmitry Torokhov wrote:
> On Thu, Jan 19, 2017 at 04:24:07PM +0100, Frieder Schrempf wrote:
>> Make the driver accept switching volume levels via sysfs.
>> This can be helpful if the beep/bell sound intensity needs
>> to be adapted to the environment of the device.
>>
>> The number of volume levels available and their values can
>> be specified via device tree (similar to pwm-backlight).
>>
>> The volume adjustment is done by changing the duty cycle of
>> the pwm signal.
>
> I wonder how this all will mesh up with beepers that have dedicated
> amplifiers (support is being added by David Lechner).
This will work very well with it. I fact, it is a feature I would like
to have but I was not sure about a good way to implement it. Please Cc:
me on future revisions of this series and I will be glad to test it.
One thing that would be nice would be for a more generic way to change
the volume from userspace. Having a sysfs attribute on the platform
device will work, but it requires very specific knowledge from any
userspace program that wants to control the volume. Would it be possible
to add an alsa mixer volume control or something like that?
^ permalink raw reply
* [PATCH v3 0/7] NFC: trf7970a: Fixups & convert to desc-based GPIO
From: Mark Greer @ 2017-01-20 19:17 UTC (permalink / raw)
To: Samuel Ortiz, Lauro Ramos Venancio, Aloisio Almeida Jr
Cc: linux-nfc-hn68Rpc1hR1g9hUCZPvPmw,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Greer
These trf7970a driver patches do the following things:
- a couple minor fixups
- allow EN2 to not be managed by the driver (e.g., when its tied low by
hardware
- convert the driver to use the descriptor-based GPIO interface
- remove support for 'vin-voltage-override' DT property
- change the DTS example to indicate that EN and EN2 are active high GPIOs
- add Mark Greer as the maintainer of the trf7970a driver
Based on k.o. 44b4b46 (Merge tag 'armsoc-fixes' of
git://git.kernel.org/pub/scm/linux/kernel/git/arm/arm-soc)
v2->v3:
- Removed "[PATCH v2 5/7] NFC: trf7970a: Clean up coding style issues"
because it will make merging patches from Geoff Lansberry and others
hard to apply. I will resubmit once those patches have been merged
or rejected.
- Added a patch to remove 'vin-voltage-override' DT property support as
proper DT regulator set up makes it unnecessary.
v1->v2:
- Commit description fixups only; no functional changes.
Mark Greer (7):
NFC: trf7970a: Don't de-assert EN2 unless it was asserted
NFC: trf7970a: Don't manage EN2 when not specified in DT
NFC: trf7970a: Convert to descriptor based GPIO interface
NFC: trf7970a: Remove useless comment
NFC: trf7970a: Remove support for 'vin-voltage-override' DT property
NFC: trf7970a: Enable pins are active high not active low
MAINTAINERS: NFC: trf7970a: Add Mark Greer as maintainer
.../devicetree/bindings/net/nfc/trf7970a.txt | 6 +-
MAINTAINERS | 7 ++
drivers/nfc/Kconfig | 2 +-
drivers/nfc/trf7970a.c | 75 +++++++++-------------
4 files changed, 40 insertions(+), 50 deletions(-)
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH v3 1/7] NFC: trf7970a: Don't de-assert EN2 unless it was asserted
From: Mark Greer @ 2017-01-20 19:17 UTC (permalink / raw)
To: Samuel Ortiz, Lauro Ramos Venancio, Aloisio Almeida Jr
Cc: linux-nfc-hn68Rpc1hR1g9hUCZPvPmw,
devicetree-u79uwXL29TY76Z2rM5mHXA, Mark Greer
In-Reply-To: <20170120191745.29255-1-mgreer-luAo+O/VEmrlveNOaEYElw@public.gmane.org>
When the trf7970a part has the bug related to 'en2-rf-quirk',
the GPIO connected to the EN2 pin will not be asserted by the
driver when powering up so it shouldn't be de-asserted when
powering down.
Signed-off-by: Mark Greer <mgreer-luAo+O/VEmrlveNOaEYElw@public.gmane.org>
---
drivers/nfc/trf7970a.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/drivers/nfc/trf7970a.c b/drivers/nfc/trf7970a.c
index 26c9dbbccb0c..92f3cf3e012a 100644
--- a/drivers/nfc/trf7970a.c
+++ b/drivers/nfc/trf7970a.c
@@ -1914,7 +1914,9 @@ static int trf7970a_power_down(struct trf7970a *trf)
}
gpio_set_value(trf->en_gpio, 0);
- gpio_set_value(trf->en2_gpio, 0);
+
+ if (!(trf->quirks & TRF7970A_QUIRK_EN2_MUST_STAY_LOW))
+ gpio_set_value(trf->en2_gpio, 0);
ret = regulator_disable(trf->regulator);
if (ret)
--
2.11.0
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox