* Re: [PATCH v2 3/3] reset: Add the TI SCI reset driver
From: Andrew F. Davis @ 2016-10-28 18:30 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Nishanth Menon, Tero Kristo, Santosh Shilimkar, Philipp Zabel,
Rob Herring, Mark Rutland, Suman Anna,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CANLsYkyiC9o9h_mq4D-6pMKNVAKQEnXR5rswiCVa8-mbtgRoJg-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On 10/28/2016 12:43 PM, Mathieu Poirier wrote:
> On 27 October 2016 at 15:49, Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> wrote:
>> Some TI Keystone family of SoCs contain a system controller (like the
>> Power Management Micro Controller (PMMC) on K2G SoCs) that manage the
>> low-level device control (like clocks, resets etc) for the various
>> hardware modules present on the SoC. These device control operations
>> are provided to the host processor OS through a communication protocol
>> called the TI System Control Interface (TI SCI) protocol.
>>
>> This patch adds a reset driver that communicates to the system
>> controller over the TI SCI protocol for performing reset management
>> of various devices present on the SoC. Various reset functionalities
>> are achieved by the means of different TI SCI device operations
>> provided by the TI SCI framework.
>>
>> Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>
>> [s-anna-l0cyMroinI0@public.gmane.org: documentation changes, revised commit message]
>> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
>> Signed-off-by: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>
>> ---
>> MAINTAINERS | 1 +
>> drivers/reset/Kconfig | 9 ++
>> drivers/reset/Makefile | 1 +
>> drivers/reset/reset-ti-sci.c | 262 +++++++++++++++++++++++++++++++++++++++++++
>> 4 files changed, 273 insertions(+)
>> create mode 100644 drivers/reset/reset-ti-sci.c
>>
>> diff --git a/MAINTAINERS b/MAINTAINERS
>> index accf991..b93d91a 100644
>> --- a/MAINTAINERS
>> +++ b/MAINTAINERS
>> @@ -11901,6 +11901,7 @@ F: include/dt-bindings/clock/k2g.h
>> F: drivers/clk/keystone/sci-clk.c
>> F: Documentation/devicetree/bindings/reset/ti,sci-reset.txt
>> F: include/dt-bindings/reset/k2g.h
>> +F: drivers/reset/reset-ti-sci.c
>>
>> THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
>> M: Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
>> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
>> index 06d9fa2..4c21c9d 100644
>> --- a/drivers/reset/Kconfig
>> +++ b/drivers/reset/Kconfig
>> @@ -66,6 +66,15 @@ config RESET_SUNXI
>> help
>> This enables the reset driver for Allwinner SoCs.
>>
>> +config RESET_TI_SCI
>> + tristate "TI System Control Interface (TI-SCI) reset driver"
>> + depends on RESET_CONTROLLER
>> + depends on TI_SCI_PROTOCOL
>> + help
>> + This enables the reset driver support over TI System Control Interface
>> + available on some new TI SoCs. If you wish to use reset resources
>> + managed by the TI System Controller, say Y here. Otherwise, say N.
>> +
>> config TI_SYSCON_RESET
>> tristate "TI SYSCON Reset Driver"
>> depends on HAS_IOMEM
>> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
>> index bbe7026..36321f2 100644
>> --- a/drivers/reset/Makefile
>> +++ b/drivers/reset/Makefile
>> @@ -10,6 +10,7 @@ obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
>> obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
>> obj-$(CONFIG_RESET_STM32) += reset-stm32.o
>> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
>> +obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
>> obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
>> obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
>> obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
>> diff --git a/drivers/reset/reset-ti-sci.c b/drivers/reset/reset-ti-sci.c
>> new file mode 100644
>> index 0000000..42ccf12
>> --- /dev/null
>> +++ b/drivers/reset/reset-ti-sci.c
>> @@ -0,0 +1,262 @@
>> +/*
>> + * Texas Instrument's System Control Interface (TI-SCI) reset driver
>> + *
>> + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
>> + * Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>
>> + *
>> + * 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.
>> + *
>> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
>> + * kind, whether express or implied; without even the implied warranty
>> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/idr.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/reset-controller.h>
>> +#include <linux/soc/ti/ti_sci_protocol.h>
>> +
>> +/**
>> + * struct ti_sci_reset_control - reset control structure
>> + * @dev_id: SoC-specific device identifier
>> + * @reset_mask: reset mask to use for toggling reset
>> + */
>> +struct ti_sci_reset_control {
>> + u32 dev_id;
>> + u32 reset_mask;
>> +};
>> +
>> +/**
>> + * struct ti_sci_reset_data - reset controller information structure
>> + * @rcdev: reset controller entity
>> + * @dev: reset controller device pointer
>> + * @sci: TI SCI handle used for communication with system controller
>> + * @idr: idr structure for mapping ids to reset control structures
>> + */
>> +struct ti_sci_reset_data {
>> + struct reset_controller_dev rcdev;
>> + struct device *dev;
>> + const struct ti_sci_handle *sci;
>> + struct idr idr;
>> +};
>> +
>> +#define to_ti_sci_reset_data(p) \
>> + container_of((p), struct ti_sci_reset_data, rcdev)
>> +
>> +/**
>> + * ti_sci_reset_set() - program a device's reset
>> + * @rcdev: reset controller entity
>> + * @id: ID of the reset to toggle
>> + * @assert: boolean flag to indicate assert or deassert
>> + *
>> + * This is a common internal function used to assert or deassert a device's
>> + * reset using the TI SCI protocol. The device's reset is asserted if the
>> + * @assert argument is true, or deasserted if @assert argument is false.
>> + * The mechanism itself is a read-modify-write procedure, the current device
>> + * reset register is read using a TI SCI device operation, the new value is
>> + * set or un-set using the reset's mask, and the new reset value written by
>> + * using another TI SCI device operation.
>> + *
>> + * Return: 0 for successful request, else a corresponding error value
>> + */
>> +static int ti_sci_reset_set(struct reset_controller_dev *rcdev,
>> + unsigned long id, bool assert)
>> +{
>> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
>> + const struct ti_sci_handle *sci = data->sci;
>> + const struct ti_sci_dev_ops *dev_ops = &sci->ops.dev_ops;
>> + struct ti_sci_reset_control *control;
>> + u32 reset_state;
>> + int ret;
>> +
>> + control = idr_find(&data->idr, id);
>> + if (!control)
>> + return -EINVAL;
>> +
>> + ret = dev_ops->get_device_resets(sci, control->dev_id,
>> + &reset_state);
>> + if (ret)
>> + return ret;
>> +
>> + if (assert)
>> + reset_state |= control->reset_mask;
>> + else
>> + reset_state &= ~control->reset_mask;
>> +
>> + return dev_ops->set_device_resets(sci, control->dev_id,
>> + reset_state);
>> +}
>> +
>> +/**
>> + * ti_sci_reset_assert() - assert device reset
>> + * @rcdev: reset controller entity
>> + * @id: ID of the reset to be asserted
>> + *
>> + * This function implements the reset driver op to assert a device's reset
>> + * using the TI SCI protocol. This invokes the function ti_sci_reset_set()
>> + * with the corresponding parameters as passed in, but with the @assert
>> + * argument set to true for asserting the reset.
>> + *
>> + * Return: 0 for successful request, else a corresponding error value
>> + */
>> +static int ti_sci_reset_assert(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + return ti_sci_reset_set(rcdev, id, true);
>> +}
>> +
>> +/**
>> + * ti_sci_reset_deassert() - deassert device reset
>> + * @rcdev: reset controller entity
>> + * @id: ID of the reset to be deasserted
>> + *
>> + * This function implements the reset driver op to deassert a device's reset
>> + * using the TI SCI protocol. This invokes the function ti_sci_reset_set()
>> + * with the corresponding parameters as passed in, but with the @assert
>> + * argument set to false for deasserting the reset.
>> + *
>> + * Return: 0 for successful request, else a corresponding error value
>> + */
>> +static int ti_sci_reset_deassert(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + return ti_sci_reset_set(rcdev, id, false);
>> +}
>> +
>> +/**
>> + * ti_sci_reset_status() - check device reset status
>> + * @rcdev: reset controller entity
>> + * @id: ID of reset to be checked
>> + *
>> + * This function implements the reset driver op to return the status of a
>> + * device's reset using the TI SCI protocol. The reset register value is read
>> + * by invoking the TI SCI device opertation .get_device_resets(), and the
>> + * status of the specific reset is extracted and returned using this reset's
>> + * reset mask.
>> + *
>> + * Return: 0 if reset is deasserted, or a non-zero value if reset is asserted
>> + */
>> +static int ti_sci_reset_status(struct reset_controller_dev *rcdev,
>> + unsigned long id)
>> +{
>> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
>> + const struct ti_sci_handle *sci = data->sci;
>> + const struct ti_sci_dev_ops *dev_ops = &sci->ops.dev_ops;
>> + struct ti_sci_reset_control *control;
>> + u32 reset_state;
>> + int ret;
>> +
>> + control = idr_find(&data->idr, id);
>> + if (!control)
>> + return -EINVAL;
>> +
>> + ret = dev_ops->get_device_resets(sci, control->dev_id,
>> + &reset_state);
>> + if (ret)
>> + return ret;
>> +
>> + return reset_state & control->reset_mask;
>> +}
>> +
>> +static struct reset_control_ops ti_sci_reset_ops = {
>> + .assert = ti_sci_reset_assert,
>> + .deassert = ti_sci_reset_deassert,
>> + .status = ti_sci_reset_status,
>> +};
>> +
>> +/**
>> + * ti_sci_reset_of_xlate() - translate a set of OF arguments to a reset ID
>> + * @rcdev: reset controller entity
>> + * @reset_spec: OF reset argument specifier
>> + *
>> + * This function performs the translation of the reset argument specifier
>> + * values defined in a reset consumer device node. The function allocates a
>> + * reset control structure for that device reset, and will be used by the
>> + * driver for performing any reset functions on that reset. An idr structure
>> + * is allocated and used to map to the reset control structure. This idr
>> + * is used by the driver to do reset lookups.
>> + *
>> + * Return: 0 for successful request, else a corresponding error value
>> + */
>> +static int ti_sci_reset_of_xlate(struct reset_controller_dev *rcdev,
>> + const struct of_phandle_args *reset_spec)
>> +{
>> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
>> + struct ti_sci_reset_control *control;
>> +
>> + if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
>> + return -EINVAL;
>> +
>> + control = devm_kzalloc(data->dev, sizeof(*control), GFP_KERNEL);
>> + if (!control)
>> + return -ENOMEM;
>> +
>> + control->dev_id = reset_spec->args[0];
>> + control->reset_mask = reset_spec->args[1];
>> +
>> + return idr_alloc(&data->idr, control, 0, 0, GFP_KERNEL);
>> +}
>> +
>> +static const struct of_device_id ti_sci_reset_of_match[] = {
>> + { .compatible = "ti,sci-reset", },
>> + { /* sentinel */ },
>> +};
>> +MODULE_DEVICE_TABLE(of, ti_sci_reset_of_match);
>> +
>> +static int ti_sci_reset_probe(struct platform_device *pdev)
>> +{
>> + struct ti_sci_reset_data *data;
>> +
>> + if (!pdev->dev.of_node)
>> + return -ENODEV;
>> +
>> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
>> + if (!data)
>> + return -ENOMEM;
>> +
>> + data->sci = devm_ti_sci_get_handle(&pdev->dev);
>> + if (IS_ERR(data->sci))
>> + return PTR_ERR(data->sci);
>> +
>> + data->rcdev.ops = &ti_sci_reset_ops;
>> + data->rcdev.owner = THIS_MODULE;
>> + data->rcdev.of_node = pdev->dev.of_node;
>> + data->rcdev.of_reset_n_cells = 2;
>> + data->rcdev.of_xlate = ti_sci_reset_of_xlate;
>> + data->dev = &pdev->dev;
>> + idr_init(&data->idr);
>
> Hello Andrew,
>
> For my own education, is there a specific reason to use a struct idr
> as opposed to keeping a pointer to a struct ti_sci_reset_control in
> truct ti_sci_reset_data? I'm not opposed to the way you've done
> things but simply keeping a pointer sound more intuitive to me.
>
> Thanks,
> Mathieu
>
Hi Mathieu,
There is one ti_sci_reset_data per reset controller, and each reset
controller can have many resets (each described by a
ti_sci_reset_control instance). When a consumer requests a reset we need
to both generate a unique ID to give back to the consumer to reference
this reset and then internally map this ID to a new ti_sci_reset_control
instance for later look-up when the consumer uses this reset, IDR does
both of these for us.
Thanks,
Andrew
>> +
>> + platform_set_drvdata(pdev, data);
>> +
>> + return reset_controller_register(&data->rcdev);
>> +}
>> +
>> +static int ti_sci_reset_remove(struct platform_device *pdev)
>> +{
>> + struct ti_sci_reset_data *data = platform_get_drvdata(pdev);
>> +
>> + reset_controller_unregister(&data->rcdev);
>> +
>> + idr_destroy(&data->idr);
>> +
>> + return 0;
>> +}
>> +
>> +static struct platform_driver ti_sci_reset_driver = {
>> + .probe = ti_sci_reset_probe,
>> + .remove = ti_sci_reset_remove,
>> + .driver = {
>> + .name = "ti-sci-reset",
>> + .of_match_table = ti_sci_reset_of_match,
>> + },
>> +};
>> +module_platform_driver(ti_sci_reset_driver);
>> +
>> +MODULE_AUTHOR("Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>");
>> +MODULE_DESCRIPTION("TI System Control Interface (TI SCI) Reset driver");
>> +MODULE_LICENSE("GPL v2");
>> --
>> 2.10.1
>>
--
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 v12 RESEND 0/4] generic TEE subsystem
From: Mark Brown @ 2016-10-28 18:19 UTC (permalink / raw)
To: Andrew F. Davis
Cc: Jens Wiklander, linux-kernel, linux-arm-kernel, devicetree,
Greg Kroah-Hartman, Al Viro, valentin.manea, jean-michel.delorme,
emmanuel.michel, javier, Jason Gunthorpe, Mark Rutland,
Michal Simek, Rob Herring, Will Deacon, Arnd Bergmann,
Nishanth Menon
In-Reply-To: <1e532aeb-4944-62e4-c8c4-1e23438b92cd@ti.com>
[-- Attachment #1: Type: text/plain, Size: 523 bytes --]
On Fri, Oct 28, 2016 at 10:43:24AM -0500, Andrew F. Davis wrote:
> Do we see this as a chicken and egg situation, or is there any harm
> beyond the pains of supporting an out-of-tree driver for a while, to
> wait until we have at least one other TEE to add to this subsystem
> before merging?
We haven't been overburneded with TEE vendors wanting to get their
driver code into mainline - do we have any reasonable prospect of other
TEE vendors with an interest in mainline turning up in any kind of
reasonable timeframe?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH v2] spi: spi-fsl-dspi: Add DMA support for Vybrid
From: Mark Brown @ 2016-10-28 18:16 UTC (permalink / raw)
To: maitysanchayan
Cc: devicetree, bhuvanchandra.dv, linux-kernel, stefan, linux-spi,
shawnguo, linux-arm-kernel
In-Reply-To: <20161017062308.GA3478@Sanchayan-Arch.localdomain>
[-- Attachment #1.1: Type: text/plain, Size: 868 bytes --]
On Mon, Oct 17, 2016 at 11:53:08AM +0530, maitysanchayan@gmail.com wrote:
> Hello,
>
> Ping?
Please don't send content free pings and please allow a reasonable time
for review. People get busy, go on holiday, attend conferences and so
on so unless there is some reason for urgency (like critical bug fixes)
please allow at least a couple of weeks for review. If there have been
review comments then people may be waiting for those to be addressed.
Sending content free pings just adds to the mail volume (if they are
seen at all) and if something has gone wrong you'll have to resend the
patches anyway.
Please don't top post, reply in line with needed context. This allows
readers to readily follow the flow of conversation and understand what
you are talking about and also helps ensure that everything in the
discussion is being addressed.
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
[-- Attachment #2: Type: text/plain, Size: 176 bytes --]
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v4 4/4] regulator: Prevent falling too fast
From: Mark Brown @ 2016-10-28 18:15 UTC (permalink / raw)
To: Doug Anderson
Cc: Matthias Kaehlcke, Liam Girdwood, Brian Norris,
Javier Martinez Canillas, Rob Herring, Mark Rutland,
linux-kernel@vger.kernel.org, devicetree@vger.kernel.org
In-Reply-To: <CAD=FV=V_ncaKDzwEyCrZNqicoE+dGXN8CJ8dDuvxQ6XP8uxsDg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2390 bytes --]
On Mon, Sep 26, 2016 at 10:41:59AM -0700, Doug Anderson wrote:
> I guess I think of the whole network of components as the PWM
> regulator and not the individual discreet BUCK. I'm also not quite
> sure how you would model it as you're asking. I suppose you could say
> that all of the resistors / capacitors / inductors end up producing a
> voltage and this voltage is an input to the BUCK. ...then the BUCK
Yes, that's what's happening.
> I know for sure that our EEs have massively modified the behavior of
> the whole thing by just changing the resistors / capacitors /
> inductors, changing the undershoot, OVP issue, voltage ranges, default
> voltage, etc. That's what leads me to believe it's not so separable.
What you're describing to me is a discrete DCDC that has an input
voltage that sets the output voltage which happens to be set with a PWM.
It's of course going to be the case that the passives are important to
the system performance but it seems we have two bits here - the PWM
regulator providing an input to the DCDC and the DCDC itself which is
sensitive to rate changes.
> You could possible include some sort of string indicating what the
> model of the BUCK is, but I'm not sure how you would use it at the
> moment.
Well, the main thing it's apparently doing is providing this over
voltage protection... That's the bit that seems to warrant being
captured in this separate device.
> As I heard it described, the whole PWM regulator concept allows you to
> take relatively low cost BUCKs and make them easy to adjust up or down
> in software. It may have its downsides, but if it is inexpensive and
> can be made to work by adding a few delays for downward transitions I
> have a feeling that people will want to use it.
If they were easy to adjust up or down in software there wouldn't be any
issue! There doesn't seem to be anything PWM specific in the false
positive OVP issue, we could equally imagine someone shoving a regulator
like this on an otherwise unused LDO and experiencing the same problem.
The fact that a PWM is being used to generate the input voltage seems
like just a decision this particular system took to pair a cheap
controllable regualator with a not quite system appropriate high current
regulator, if this pattern does start to get wider use I'd expect to see
other systems using other regulators to set the input voltage.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH RESEND 1/2] dt: bindings: add allwinner,otg-routed property for phy-sun4i-usb
From: Hans de Goede @ 2016-10-28 18:13 UTC (permalink / raw)
To: Icenowy Zheng, Rob Herring, Maxime Ripard, Chen-Yu Tsai,
Kishon Vijay Abraham I
Cc: Mark Rutland, Reinder de Haan,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw@public.gmane.org
In-Reply-To: <86c3fad4-e0c1-9aaf-76c5-b9428110464f-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
HI,
On 26-10-16 12:14, Hans de Goede wrote:
> Hi,
>
> On 26-10-16 10:52, Icenowy Zheng wrote:
>>
>>
>> 26.10.2016, 16:28, "Hans de Goede" <hdegoede-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>:
>>> Hi,
>>>
>>> On 25-10-16 06:11, Icenowy Zheng wrote:
>>>> On some newer Allwinner SoCs (H3 or A64), the PHY0 can be either routed to
>>>> the MUSB controller (which is an OTG controller) or the OHCI/EHCI pair
>>>> (which is a Host-only controller, but more stable and easy to implement).
>>>>
>>>> This property marks whether on a certain board which controller should be
>>>> attached to the PHY.
>>>>
>>>> Signed-off-by: Icenowy Zheng <icenowy-ymACFijhrKM@public.gmane.org>
>>>
>>> Icenowy, I appreciate your work on this, but we really need full otg
>>> support with dynamic switching rather then hardwiring the routing, so
>>> this cannot go in as is.
>>
>> Now I have both PHY0 controllers' drivers.
>>
>> In the tree of https://github.com/Icenowy/linux/tree/ice-a64-v6.1 , I have already
>> enabled MUSB controller.
>>
>> And this patchset is for those prefer a stable USB host implement to dual-role
>> implementation. MUSB is a good UDC, but not a good host controller. My USB
>> sound card cannot work on MUSB on A33. Even connecting a R8's MUSB (Serial
>> Gadget) to an A33's MUSB cannot work.
>
> The idea is for dual-role setups to used the MUSB in gadget mode and the EHCI/OHCI
> pair when in host mode. So for otg setups you would runtime change the mux
> from one controller to the other based on the id pin value.
>
> Take a look at drivers/phy/phy-sun4i-usb.c, around line 512:
>
> if (id_det != data->id_det) {
> ...
> }
>
> This deals with id_det changes (including the initial id_det "change"
> for hardwired host-only ports). This currently assumes that the musb
> will be used for host mode too, we will want to change this to
> something like this:
>
> if (id_det != data->id_det) {
> if (data->cfg->separate_phy0_host_controller) {
> if (id_det) {
> /* Change to gadget mode (id_det == 1), switch phy mux to musb */
> actual code to switch phy mux to musb...
> } else {
> /* Change to host mode (id_det == 0), switch phy mux to ehci/ohci */
> actual code to switch phy mux to ehci/ohci...
> }
> }
> /* old code */
> }
>
> Note this will then still rely on the musb code to actually turn
> the regulator on, so you do need to have the musb driver build and
> loaded. This can be fixed but lets start with the above.
>
> If you combine this with dr_mode = "host"; in the dts, then
> sun4i_usb_phy0_get_id_det() will return 0 so on its first run
> sun4i_usb_phy0_id_vbus_det_scan() will throw the mux to the ehci/ohci
> and everything should work as you want without needing the custom
> "allwinner,otg-routed" property, and we should be more or less
> ready to support full otg on other boards.
I've just found further proof that the musb on the H3 at least
only is intended for gadget mode and that we must dynamically
switch for host-mode. If you look at:
drivers/usb/sunxi_usb/include/sunxi_udc.h
In the h3 sdk then you will see that for the H3 a different fifo
endpoint table is used, as the total fifo space is only 4k where
as previous SoCs had 8k. This means that we need to have 2
different ep tables in drivers/usb/musb/sunxi.c and select by
compatible.
Regards,
Hans
^ permalink raw reply
* Re: [PATCH V2 06/10] regulator: da9061: BUCK and LDO regulator driver
From: Mark Brown @ 2016-10-28 17:59 UTC (permalink / raw)
To: Steve Twiss
Cc: LINUX-KERNEL, Liam Girdwood, DEVICETREE, Dmitry Torokhov,
Eduardo Valentin, Guenter Roeck, LINUX-INPUT, LINUX-PM,
LINUX-WATCHDOG, Lee Jones, Mark Rutland, Rob Herring,
Support Opensource, Wim Van Sebroeck, Zhang Rui
In-Reply-To: <c8529abdbe8a07ac0d7b09ab24fad73ab56116be.1477501000.git.stwiss.opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 675 bytes --]
On Wed, Oct 26, 2016 at 05:56:38PM +0100, Steve Twiss wrote:
> From: Steve Twiss <stwiss.opensource-WBD+wuPFNBhBDgjK7y7TUQ@public.gmane.org>
>
> Regulator support for the DA9061 is added into the DA9062 regulator driver.
>
> The regulators for DA9061 differ from those of DA9062.
> A new DA9061 enumeration list for the LDOs and Bucks supported by this
> device is added. Regulator information added: the old regulator
> information for DA9062 is renamed from local_regulator_info[] to
> local_da9062_regulator_info[] and a new array is added to support
> local_da9061_regulator_info[].
Acked-by: Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]
^ permalink raw reply
* Re: [PATCH 1/2] PCI: hisi: add PCIe driver support for HiSilicon STB SoCs
From: Bjorn Helgaas @ 2016-10-28 17:53 UTC (permalink / raw)
To: Ruqiang Ju
Cc: bhelgaas, robh+dt, mark.rutland, linux-pci, devicetree,
linux-kernel, bin.chen, elder, hermit.wangheming, yanhaifeng,
xuejiancheng
In-Reply-To: <1477014336-12385-1-git-send-email-juruqiang@huawei.com>
Hi Ruqiang,
On Fri, Oct 21, 2016 at 09:45:36AM +0800, Ruqiang Ju wrote:
> Add PCIe controller drvier for HiSilicon STB SoCs,
s/drvier/driver/
> the controller is based on the DesignWare's PCIe core.
>
> Signed-off-by: Ruqiang Ju <juruqiang@huawei.com>
> ---
> .../bindings/pci/hisilicon-histb-pcie.txt | 66 +++
> drivers/pci/host/Kconfig | 8 +
> drivers/pci/host/Makefile | 1 +
> drivers/pci/host/pcie-histb.c | 448 +++++++++++++++++++++
> 4 files changed, 523 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/pci/hisilicon-histb-pcie.txt
> create mode 100644 drivers/pci/host/pcie-histb.c
>
> diff --git a/Documentation/devicetree/bindings/pci/hisilicon-histb-pcie.txt b/Documentation/devicetree/bindings/pci/hisilicon-histb-pcie.txt
> new file mode 100644
> index 0000000..952f1db
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/pci/hisilicon-histb-pcie.txt
> @@ -0,0 +1,66 @@
> +HiSilicon STB PCIe host bridge DT description
> +
> +HiSilicon PCIe host controller is based on Designware PCI core.
> +It shares common functions with PCIe Designware core driver and inherits
> +common properties defined in
> +Documentation/devicetree/bindings/pci/designware-pci.txt.
Documentation/devicetree/bindings/pci/designware-pcie.txt ("pcie" at
the end, not "pci").
> +
> +Additional properties are described here:
> +
> +Required properties
> +- compatible: Should be one of the following strings
> + "hisilicon,histb-pcie",
> + "hisilicon,hi3798cv200-pcie"
> +- reg: Should contain sysctl, rc_dbi, config registers location and length.
> +- reg-names: Must include the following entries:
> + "sysctrl": system control registers of PCIe controller;
> + "rc_dbi": configuration space of PCIe controller;
> + "config": configuration transaction space of PCIe controller.
> +- interrupts: MSI interrupt.
> +- interrupt-names: Must include "msi" entries.
> +- clocks: List of phandle and clock specifier pairs as listed
> + in clock-names property.
> +- clock-name: Must include the following entries:
> + "aux_clk": auxiliary gate clock;
> + "pipe_clk": pipe gate clock;
> + "sys_clk": sys gate clock;
> + "bus_clk": bus gate clock.
> +- resets: List of phandle and reset specifier pairs as listed
> + in reset-names property
> +- reset-names: Must include the following entries:
> + "soft_reset": soft reset;
> + "sys_reset": sys reset;
> + "bus_rest": bus reset.
Please add "bus-range" as a required property so we don't have to
assume the 00-ff range. Documented in
Documentation/devicetree/bindings/pci/designware-pcie.txt.
> +
> +Optional properties:
> +- power-gpios: pcie device power control gpio if needed;
> +- power-gpios-active-high: must include this propty
> + if active level is high.
> +- status: Either "ok" or "disabled".
> +
> +Example:
> + pcie@f9860000 {
> + compatible = "hisilicon,histb-pcie", "snps,dw-pcie";
> + reg = <0xf9860000 0x1000>,
> + <0xf0000000 0x2000>,
> + <0xf2000000 0x01000000>;
> + reg-names = "sysctrl", "rc_dbi", "config";
> + #address-cells = <3>;
> + #size-cells = <2>;
> + device_type = "pci";
> + num-lanes = <1>;
> + ranges=<0x81000000 0 0 0xf4000000 0 0x00010000
> + 0x82000000 0 0xf3000000 0xf3000000 0 0x01000000>;
> + interrupts = <GIC_SPI 128 IRQ_TYPE_LEVEL_HIGH>;
> + interrupt-names = "msi";
> + #interrupt-cells = <1>;
> + interrupt-map-mask = <0 0 0 7>;
> + interrupt-map = <0x0 0 0 1 &gic 0 131 4>;
> + clocks = <&crg PCIE_AUX_CLK>,
> + <&crg PCIE_PIPE_CLK>,
> + <&crg PCIE_SYS_CLK>,
> + <&crg PCIE_BUS_CLK>;
> + clock-names = "aux_clk", "pipe_clk", "sys_clk", "bus_clk";
> + resets = <&crg 0x18c 6>, <&crg 0x18c 5>, <&crg 0x18c 4>;
> + reset-names = "soft_reset", "sys_reset", "bus_reset";
> + };
> diff --git a/drivers/pci/host/Kconfig b/drivers/pci/host/Kconfig
> index d7e7c0a..57ce4df 100644
> --- a/drivers/pci/host/Kconfig
> +++ b/drivers/pci/host/Kconfig
> @@ -227,6 +227,14 @@ config PCI_HISI
> Say Y here if you want PCIe controller support on HiSilicon
> Hip05 and Hip06 SoCs
>
> +config PCIE_HISI_STB
> + depends on OF
> + bool "HiSilicon STB SoCs PCIe controllers"
> + select PCIEPORTBUS
> + select PCIE_DW
> + help
> + Say Y here if you want PCIe controller support on HiSilicon STB SoCs
> +
> config PCIE_QCOM
> bool "Qualcomm PCIe controller"
> depends on ARCH_QCOM && OF
> diff --git a/drivers/pci/host/Makefile b/drivers/pci/host/Makefile
> index 084cb49..55f4fe7 100644
> --- a/drivers/pci/host/Makefile
> +++ b/drivers/pci/host/Makefile
> @@ -26,6 +26,7 @@ obj-$(CONFIG_PCIE_IPROC_BCMA) += pcie-iproc-bcma.o
> obj-$(CONFIG_PCIE_ALTERA) += pcie-altera.o
> obj-$(CONFIG_PCIE_ALTERA_MSI) += pcie-altera-msi.o
> obj-$(CONFIG_PCI_HISI) += pcie-hisi.o
> +obj-$(CONFIG_PCIE_HISI_STB) += pcie-histb.o
> obj-$(CONFIG_PCIE_QCOM) += pcie-qcom.o
> obj-$(CONFIG_PCI_HOST_THUNDER_ECAM) += pci-thunder-ecam.o
> obj-$(CONFIG_PCI_HOST_THUNDER_PEM) += pci-thunder-pem.o
> diff --git a/drivers/pci/host/pcie-histb.c b/drivers/pci/host/pcie-histb.c
> new file mode 100644
> index 0000000..71e344f
> --- /dev/null
> +++ b/drivers/pci/host/pcie-histb.c
> @@ -0,0 +1,448 @@
> +/*
> + * PCIe host controller driver for HiSilicon STB SoCs
> + *
> + * Copyright (C) 2016 HiSilicon Co., Ltd. http://www.hisilicon.com
> + *
> + * Authors: RuQiang Ju <juruqiang@huawei.com>
> + *
> + * 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/clk.h>
> +#include <linux/delay.h>
> +#include <linux/interrupt.h>
> +#include <linux/kernel.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/of_gpio.h>
> +#include <linux/pci.h>
> +#include <linux/platform_device.h>
> +#include <linux/resource.h>
> +#include <linux/reset.h>
> +
> +#include "pcie-designware.h"
> +
> +#define PCIE_SYS_CTRL0 0x0000
> +#define PCIE_SYS_CTRL1 0x0004
> +#define PCIE_SYS_CTRL7 0x001C
> +#define PCIE_SYS_CTRL13 0x0034
> +#define PCIE_SYS_CTRL15 0x003c
> +#define PCIE_SYS_CTRL16 0x0040
> +#define PCIE_SYS_CTRL17 0x0044
> +
> +#define PCIE_SYS_STAT0 0x0100
> +#define PCIE_SYS_STAT4 0x0110
> +
> +#define PCIE_RDLH_LINK_UP BIT(5)
> +#define PCIE_XMLH_LINK_UP BIT(15)
> +#define PCIE_ELBI_SLV_DBI_ENABLE BIT(21)
> +#define PCIE_APP_LTSSM_ENABLE BIT(11)
> +
> +#define PCIE_DEVICE_TYPE_MASK GENMASK(31, 28)
> +#define PCIE_WM_EP 0
> +#define PCIE_WM_LEGACY BIT(1)
> +#define PCIE_WM_RC BIT(30)
> +
> +#define PCIE_LTSSM_STATE_MASK GENMASK(5, 0)
> +#define PCIE_LTSSM_STATE_ACTIVE 0x11
> +
> +struct histb_pcie_host {
struct histb_pcie (without "_host") should be enough.
> + void __iomem *sysctrl;
> + int power_gpio;
> + bool gpio_active_high;
> + struct clk *aux_clk;
> + struct clk *pipe_clk;
> + struct clk *sys_clk;
> + struct clk *bus_clk;
> + struct reset_control *soft_reset;
> + struct reset_control *sys_reset;
> + struct reset_control *bus_reset;
> + struct pcie_port pp;
Reorder as in f84cfdf72109 ("PCI: hisi: Reorder struct hisi_pcie") and
add comment(s) about related DT property names.
> +};
> +
> +#define to_histb_pcie(x) container_of(x, struct histb_pcie_host, pp)
> +
Can you add accessors, e.g.,
static u32 histb_pcie_readl(struct histb_pcie *histb_pcie, u32 reg)
{
return readl(histb_pcie->sysctrl + reg);
}
static void histb_pcie_writel(struct histb_pcie *histb_pcie, u32 reg, u32 val)
{
writel(val, histb_pcie->sysctrl + reg);
}
> +static void histb_pcie_dbi_w_mode(struct pcie_port *pp, bool enable)
Rework this and other internal interfaces as in e9480b5a7f34 ("PCI:
hisi: Pass device-specific struct to internal functions").
> +{
> + struct histb_pcie_host *hipcie = to_histb_pcie(pp);
struct histb_pcie *histb_pcie;
> + u32 val;
> +
> + if (enable) {
> + val = readl(hipcie->sysctrl + PCIE_SYS_CTRL0);
> + val |= PCIE_ELBI_SLV_DBI_ENABLE;
> + writel(val, hipcie->sysctrl + PCIE_SYS_CTRL0);
> + } else {
> + val = readl(hipcie->sysctrl + PCIE_SYS_CTRL0);
> + val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
> + writel(val, hipcie->sysctrl + PCIE_SYS_CTRL0);
val = histb_pcie_readl(hipcie, PCIE_SYS_CTRL0);
if (enable)
val |= PCIE_ELBI_SLV_DBI_ENABLE;
else
val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
histb_pcie_writel(hipcie, PCIE_SYS_CTRL0, val);
We should do the same to exynos_pcie_sideband_dbi_w_mode() and
exynos_pcie_sideband_dbi_r_mode().
> + }
> +}
> +
> +static void histb_pcie_dbi_r_mode(struct pcie_port *pp, bool enable)
> +{
> + struct histb_pcie_host *hipcie = to_histb_pcie(pp);
> + u32 val;
> +
> + if (enable) {
> + val = readl(hipcie->sysctrl + PCIE_SYS_CTRL1);
> + val |= PCIE_ELBI_SLV_DBI_ENABLE;
> + writel(val, hipcie->sysctrl + PCIE_SYS_CTRL1);
> + } else {
> + val = readl(hipcie->sysctrl + PCIE_SYS_CTRL1);
> + val &= ~PCIE_ELBI_SLV_DBI_ENABLE;
> + writel(val, hipcie->sysctrl + PCIE_SYS_CTRL1);
> + }
Similar to above.
> +}
> +
> +static inline u32 histb_pcie_readl_rc(struct pcie_port *pp, u32 reg)
It's pointless to mark these "inline". They can't be inline in any case
because the only use is to save a pointer to them in the
histb_pcie_host_ops structure.
> +{
> + u32 val;
> +
> + histb_pcie_dbi_r_mode(pp, true);
> + val = readl(pp->dbi_base + reg);
> + histb_pcie_dbi_r_mode(pp, false);
> +
> + return val;
> +}
> +
> +static inline void histb_pcie_writel_rc(struct pcie_port *pp,
> + u32 reg, u32 val)
> +{
> + histb_pcie_dbi_w_mode(pp, true);
> + writel(val, pp->dbi_base + reg);
> + histb_pcie_dbi_w_mode(pp, false);
> +}
> +
> +static int histb_pcie_rd_own_conf(struct pcie_port *pp,
> + int where, int size, u32 *val)
> +{
> + int ret;
> +
> + histb_pcie_dbi_r_mode(pp, true);
> + ret = dw_pcie_cfg_read(pp->dbi_base + where, size, val);
> + histb_pcie_dbi_r_mode(pp, false);
> +
> + return ret;
> +}
> +
> +static int histb_pcie_wr_own_conf(struct pcie_port *pp,
> + int where, int size, u32 val)
> +{
> + int ret;
> +
> + histb_pcie_dbi_w_mode(pp, true);
> + ret = dw_pcie_cfg_write(pp->dbi_base + where, size, val);
> + histb_pcie_dbi_w_mode(pp, false);
> +
> + return ret;
> +}
> +
> +static int histb_pcie_link_up(struct pcie_port *pp)
> +{
> + struct histb_pcie_host *hipcie = to_histb_pcie(pp);
> + u32 regval;
> + u32 status;
> +
> + regval = readl(hipcie->sysctrl + PCIE_SYS_STAT0);
> + status = readl(hipcie->sysctrl + PCIE_SYS_STAT4);
> + status &= PCIE_LTSSM_STATE_MASK;
> + if ((regval & PCIE_XMLH_LINK_UP) &&
> + (regval & PCIE_RDLH_LINK_UP) &&
> + (status == PCIE_LTSSM_STATE_ACTIVE))
> + return 1;
> +
> + return 0;
> +}
> +
> +static int histb_pcie_establish_link(struct pcie_port *pp)
> +{
> + struct histb_pcie_host *hipcie = to_histb_pcie(pp);
> + u32 regval;
> + int count = 0;
> +
> + if (dw_pcie_link_up(pp)) {
> + dev_err(pp->dev, "Link already up\n");
This doesn't seem like an error condition to me; maybe dev_info() if
we need a message at all. We should also change the other drivers
that do the same thing.
> + return 0;
> + }
> +
> + /* PCIe RC work mode */
> + regval = readl(hipcie->sysctrl + PCIE_SYS_CTRL0);
> + regval &= ~PCIE_DEVICE_TYPE_MASK;
> + regval |= PCIE_WM_RC;
> + writel(regval, hipcie->sysctrl + PCIE_SYS_CTRL0);
> +
> + /* setup root complex */
> + dw_pcie_setup_rc(pp);
> +
> + /* assert LTSSM enable */
> + regval = readl(hipcie->sysctrl + PCIE_SYS_CTRL7);
> + regval |= PCIE_APP_LTSSM_ENABLE;
> + writel(regval, hipcie->sysctrl + PCIE_SYS_CTRL7);
> +
> + /* check if the link is up or not */
> + while (!dw_pcie_link_up(pp)) {
> + mdelay(10);
> + count++;
> + if (count == 50) {
> + dev_err(pp->dev, "PCIe Link Fail\n");
> + return -EINVAL;
> + }
> + }
Use dw_pcie_wait_for_link().
> +
> + dev_info(pp->dev, "Link up\n");
> +
> + return 0;
> +}
> +
> +static void histb_pcie_host_init(struct pcie_port *pp)
> +{
> + histb_pcie_establish_link(pp);
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI))
> + dw_pcie_msi_init(pp);
> +}
> +
> +static struct pcie_host_ops histb_pcie_host_ops = {
> + .readl_rc = histb_pcie_readl_rc,
> + .writel_rc = histb_pcie_writel_rc,
> + .rd_own_conf = histb_pcie_rd_own_conf,
> + .wr_own_conf = histb_pcie_wr_own_conf,
> + .link_up = histb_pcie_link_up,
> + .host_init = histb_pcie_host_init,
> +};
> +
> +#ifdef CONFIG_PCI_MSI
> +static irqreturn_t histb_pcie_msi_irq_handler(int irq, void *arg)
> +{
> + struct pcie_port *pp = arg;
> +
> + return dw_handle_msi_irq(pp);
> +}
> +#endif
> +
> +static void histb_pcie_host_disable(struct histb_pcie_host *hipcie)
> +{
> + reset_control_assert(hipcie->soft_reset);
> + reset_control_assert(hipcie->sys_reset);
> + reset_control_assert(hipcie->bus_reset);
> +
> + clk_disable_unprepare(hipcie->aux_clk);
> + clk_disable_unprepare(hipcie->pipe_clk);
> + clk_disable_unprepare(hipcie->sys_clk);
> + clk_disable_unprepare(hipcie->bus_clk);
> +
> + if (gpio_is_valid(hipcie->power_gpio))
> + gpio_set_value_cansleep(hipcie->power_gpio,
> + !hipcie->gpio_active_high);
> +}
> +
> +static int histb_pcie_host_enable(struct pcie_port *pp)
> +{
> + struct histb_pcie_host *hipcie = to_histb_pcie(pp);
> + struct device *dev = pp->dev;
> + int ret;
> +
> + /* power on pcie device if have */
> + if (gpio_is_valid(hipcie->power_gpio))
> + gpio_set_value_cansleep(hipcie->power_gpio,
> + hipcie->gpio_active_high);
> +
Thanks for splitting all this clock and reset management into its own
function. Many of the other drivers mix it in with discovery of
resources from DT, which I think is a mistake.
> + ret = clk_prepare_enable(hipcie->bus_clk);
> + if (ret) {
> + dev_err(dev, "cannot prepare/enable bus_clk\n");
> + goto err_bus_clk;
> + }
> + ret = clk_prepare_enable(hipcie->sys_clk);
> + if (ret) {
> + dev_err(dev, "cannot prepare/enable sys_clk\n");
> + goto err_sys_clk;
> + }
> + ret = clk_prepare_enable(hipcie->pipe_clk);
> + if (ret) {
> + dev_err(dev, "cannot prepare/enable pipe_clk\n");
> + goto err_pipe_clk;
> + }
> + ret = clk_prepare_enable(hipcie->aux_clk);
> + if (ret) {
> + dev_err(dev, "cannot prepare/enable aux_clk\n");
> + goto err_aux_clk;
> + }
> +
> + reset_control_assert(hipcie->soft_reset);
> + reset_control_deassert(hipcie->soft_reset);
> +
> + reset_control_assert(hipcie->sys_reset);
> + reset_control_deassert(hipcie->sys_reset);
> +
> + reset_control_assert(hipcie->bus_reset);
> + reset_control_deassert(hipcie->bus_reset);
> +
> + return 0;
> +
> +err_aux_clk:
> + clk_disable_unprepare(hipcie->aux_clk);
> +err_pipe_clk:
> + clk_disable_unprepare(hipcie->pipe_clk);
> +err_sys_clk:
> + clk_disable_unprepare(hipcie->sys_clk);
> +err_bus_clk:
> + clk_disable_unprepare(hipcie->bus_clk);
> +
> + return ret;
> +}
> +
> +
> +static int histb_pcie_probe(struct platform_device *pdev)
> +{
> + struct histb_pcie_host *hipcie;
> + struct pcie_port *pp;
> + struct resource *res;
> + struct device_node *np = pdev->dev.of_node;
> + struct device *dev = &pdev->dev;
> + int ret;
> +
> + hipcie = devm_kzalloc(dev, sizeof(*hipcie), GFP_KERNEL);
> + if (!hipcie)
> + return -ENOMEM;
> +
> + pp = &hipcie->pp;
> + pp->dev = dev;
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "sysctrl");
> + hipcie->sysctrl = devm_ioremap_resource(dev, res);
> + if (IS_ERR(hipcie->sysctrl)) {
> + dev_err(dev, "cannot get sysctrl base\n");
> + return PTR_ERR(hipcie->sysctrl);
> + }
> +
> + res = platform_get_resource_byname(pdev, IORESOURCE_MEM, "rc_dbi");
> + pp->dbi_base = devm_ioremap_resource(dev, res);
> + if (IS_ERR(pp->dbi_base)) {
> + dev_err(dev, "cannot get rc_dbi base\n");
> + return PTR_ERR(pp->dbi_base);
> + }
> +
> + hipcie->power_gpio = of_get_named_gpio(np, "power-gpios", 0);
> + hipcie->gpio_active_high = of_property_read_bool(np,
> + "power-gpios-active-high");
> + if (gpio_is_valid(hipcie->power_gpio)) {
> + ret = devm_gpio_request_one(dev, hipcie->power_gpio,
> + hipcie->gpio_active_high ?
> + GPIOF_OUT_INIT_HIGH :
> + GPIOF_OUT_INIT_LOW,
> + "PCIe Dev power");
> + if (ret) {
> + dev_err(dev, "unable to request gpio\n");
> + return ret;
> + }
> + }
> +
> + hipcie->aux_clk = devm_clk_get(dev, "aux_clk");
> + if (IS_ERR(hipcie->aux_clk)) {
> + dev_err(dev, "Failed to get pcie aux_clk clock\n");
> + return PTR_ERR(hipcie->aux_clk);
> + }
> +
> + hipcie->pipe_clk = devm_clk_get(dev, "pipe_clk");
> + if (IS_ERR(hipcie->pipe_clk)) {
> + dev_err(dev, "Failed to get pcie pipe_clk clock\n");
> + return PTR_ERR(hipcie->pipe_clk);
> + }
> +
> + hipcie->sys_clk = devm_clk_get(dev, "sys_clk");
> + if (IS_ERR(hipcie->sys_clk)) {
> + dev_err(dev, "Failed to get pcie sys_clk clock\n");
> + return PTR_ERR(hipcie->sys_clk);
> + }
> +
> + hipcie->bus_clk = devm_clk_get(dev, "bus_clk");
> + if (IS_ERR(hipcie->bus_clk)) {
> + dev_err(dev, "Failed to get pcie bus_clk clock\n");
> + return PTR_ERR(hipcie->bus_clk);
> + }
> +
> + hipcie->soft_reset = devm_reset_control_get(dev, "soft_reset");
> + if (IS_ERR(hipcie->soft_reset)) {
> + dev_err(dev, "couldn't get soft_reset\n");
> + return PTR_ERR(hipcie->soft_reset);
> + }
> +
> + hipcie->sys_reset = devm_reset_control_get(dev, "sys_reset");
> + if (IS_ERR(hipcie->sys_reset)) {
> + dev_err(dev, "couldn't get sys_reset\n");
> + return PTR_ERR(hipcie->sys_reset);
> + }
> +
> + hipcie->bus_reset = devm_reset_control_get(dev, "bus_reset");
> + if (IS_ERR(hipcie->bus_reset)) {
> + dev_err(dev, "couldn't get bus_reset\n");
> + return PTR_ERR(hipcie->bus_reset);
> + }
> +
> + if (IS_ENABLED(CONFIG_PCI_MSI)) {
> + pp->msi_irq = platform_get_irq_byname(pdev, "msi");
> + if (pp->msi_irq < 0) {
> + dev_err(dev, "Failed to get msi irq\n");
> + return pp->msi_irq;
> + }
> + ret = devm_request_irq(dev, pp->msi_irq,
> + histb_pcie_msi_irq_handler,
> + IRQF_SHARED, "histb-pcie-msi", pp);
> + if (ret) {
> + dev_err(dev, "cannot request msi irq\n");
> + return ret;
> + }
> + }
> +
> + pp->root_bus_nr = -1;
> + pp->ops = &histb_pcie_host_ops;
> +
> + ret = histb_pcie_host_enable(pp);
> + if (ret) {
> + dev_err(dev, "failed to enable host\n");
> + return ret;
> + }
> +
> + ret = dw_pcie_host_init(pp);
> + if (ret) {
> + dev_err(dev, "failed to initialize host\n");
> + return ret;
> + }
> +
> + platform_set_drvdata(pdev, hipcie);
> +
> + return 0;
> +}
> +
> +static int histb_pcie_remove(struct platform_device *pdev)
> +{
> + struct histb_pcie_host *hipcie = platform_get_drvdata(pdev);
> +
> + histb_pcie_host_disable(hipcie);
> +
> + return 0;
> +}
> +
> +static const struct of_device_id histb_pcie_of_match[] = {
> + { .compatible = "hisilicon,histb-pcie", },
> + { .compatible = "hisilicon,hi3798cv200-pcie", },
> + {},
> +};
> +
> +MODULE_DEVICE_TABLE(of, histb_pcie_of_match);
> +
> +static struct platform_driver histb_pcie_platform_driver = {
> + .probe = histb_pcie_probe,
> + .remove = histb_pcie_remove,
> + .driver = {
> + .name = "histb-pcie",
> + .of_match_table = of_match_ptr(histb_pcie_of_match),
> + },
> +};
> +
> +module_platform_driver(histb_pcie_platform_driver);
> +
> +MODULE_AUTHOR("RuQiang Ju <juruqiang@huawei.com>");
> +MODULE_DESCRIPTION("HiSilicon STB PCIe host controller driver");
> +MODULE_LICENSE("GPL v2");
> --
> 1.9.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 3/3] reset: Add the TI SCI reset driver
From: Mathieu Poirier @ 2016-10-28 17:43 UTC (permalink / raw)
To: Andrew F. Davis
Cc: Nishanth Menon, Tero Kristo, Santosh Shilimkar, Philipp Zabel,
Rob Herring, Mark Rutland, Suman Anna,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <20161027214941.24641-4-afd-l0cyMroinI0@public.gmane.org>
On 27 October 2016 at 15:49, Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org> wrote:
> Some TI Keystone family of SoCs contain a system controller (like the
> Power Management Micro Controller (PMMC) on K2G SoCs) that manage the
> low-level device control (like clocks, resets etc) for the various
> hardware modules present on the SoC. These device control operations
> are provided to the host processor OS through a communication protocol
> called the TI System Control Interface (TI SCI) protocol.
>
> This patch adds a reset driver that communicates to the system
> controller over the TI SCI protocol for performing reset management
> of various devices present on the SoC. Various reset functionalities
> are achieved by the means of different TI SCI device operations
> provided by the TI SCI framework.
>
> Signed-off-by: Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>
> [s-anna-l0cyMroinI0@public.gmane.org: documentation changes, revised commit message]
> Signed-off-by: Suman Anna <s-anna-l0cyMroinI0@public.gmane.org>
> Signed-off-by: Nishanth Menon <nm-l0cyMroinI0@public.gmane.org>
> ---
> MAINTAINERS | 1 +
> drivers/reset/Kconfig | 9 ++
> drivers/reset/Makefile | 1 +
> drivers/reset/reset-ti-sci.c | 262 +++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 273 insertions(+)
> create mode 100644 drivers/reset/reset-ti-sci.c
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index accf991..b93d91a 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -11901,6 +11901,7 @@ F: include/dt-bindings/clock/k2g.h
> F: drivers/clk/keystone/sci-clk.c
> F: Documentation/devicetree/bindings/reset/ti,sci-reset.txt
> F: include/dt-bindings/reset/k2g.h
> +F: drivers/reset/reset-ti-sci.c
>
> THANKO'S RAREMONO AM/FM/SW RADIO RECEIVER USB DRIVER
> M: Hans Verkuil <hverkuil-qWit8jRvyhVmR6Xm/wNWPw@public.gmane.org>
> diff --git a/drivers/reset/Kconfig b/drivers/reset/Kconfig
> index 06d9fa2..4c21c9d 100644
> --- a/drivers/reset/Kconfig
> +++ b/drivers/reset/Kconfig
> @@ -66,6 +66,15 @@ config RESET_SUNXI
> help
> This enables the reset driver for Allwinner SoCs.
>
> +config RESET_TI_SCI
> + tristate "TI System Control Interface (TI-SCI) reset driver"
> + depends on RESET_CONTROLLER
> + depends on TI_SCI_PROTOCOL
> + help
> + This enables the reset driver support over TI System Control Interface
> + available on some new TI SoCs. If you wish to use reset resources
> + managed by the TI System Controller, say Y here. Otherwise, say N.
> +
> config TI_SYSCON_RESET
> tristate "TI SYSCON Reset Driver"
> depends on HAS_IOMEM
> diff --git a/drivers/reset/Makefile b/drivers/reset/Makefile
> index bbe7026..36321f2 100644
> --- a/drivers/reset/Makefile
> +++ b/drivers/reset/Makefile
> @@ -10,6 +10,7 @@ obj-$(CONFIG_RESET_PISTACHIO) += reset-pistachio.o
> obj-$(CONFIG_RESET_SOCFPGA) += reset-socfpga.o
> obj-$(CONFIG_RESET_STM32) += reset-stm32.o
> obj-$(CONFIG_RESET_SUNXI) += reset-sunxi.o
> +obj-$(CONFIG_RESET_TI_SCI) += reset-ti-sci.o
> obj-$(CONFIG_TI_SYSCON_RESET) += reset-ti-syscon.o
> obj-$(CONFIG_RESET_UNIPHIER) += reset-uniphier.o
> obj-$(CONFIG_RESET_ZYNQ) += reset-zynq.o
> diff --git a/drivers/reset/reset-ti-sci.c b/drivers/reset/reset-ti-sci.c
> new file mode 100644
> index 0000000..42ccf12
> --- /dev/null
> +++ b/drivers/reset/reset-ti-sci.c
> @@ -0,0 +1,262 @@
> +/*
> + * Texas Instrument's System Control Interface (TI-SCI) reset driver
> + *
> + * Copyright (C) 2015-2016 Texas Instruments Incorporated - http://www.ti.com/
> + * Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>
> + *
> + * 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.
> + *
> + * This program is distributed "as is" WITHOUT ANY WARRANTY of any
> + * kind, whether express or implied; without even the implied warranty
> + * of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + */
> +
> +#include <linux/idr.h>
> +#include <linux/module.h>
> +#include <linux/of.h>
> +#include <linux/platform_device.h>
> +#include <linux/reset-controller.h>
> +#include <linux/soc/ti/ti_sci_protocol.h>
> +
> +/**
> + * struct ti_sci_reset_control - reset control structure
> + * @dev_id: SoC-specific device identifier
> + * @reset_mask: reset mask to use for toggling reset
> + */
> +struct ti_sci_reset_control {
> + u32 dev_id;
> + u32 reset_mask;
> +};
> +
> +/**
> + * struct ti_sci_reset_data - reset controller information structure
> + * @rcdev: reset controller entity
> + * @dev: reset controller device pointer
> + * @sci: TI SCI handle used for communication with system controller
> + * @idr: idr structure for mapping ids to reset control structures
> + */
> +struct ti_sci_reset_data {
> + struct reset_controller_dev rcdev;
> + struct device *dev;
> + const struct ti_sci_handle *sci;
> + struct idr idr;
> +};
> +
> +#define to_ti_sci_reset_data(p) \
> + container_of((p), struct ti_sci_reset_data, rcdev)
> +
> +/**
> + * ti_sci_reset_set() - program a device's reset
> + * @rcdev: reset controller entity
> + * @id: ID of the reset to toggle
> + * @assert: boolean flag to indicate assert or deassert
> + *
> + * This is a common internal function used to assert or deassert a device's
> + * reset using the TI SCI protocol. The device's reset is asserted if the
> + * @assert argument is true, or deasserted if @assert argument is false.
> + * The mechanism itself is a read-modify-write procedure, the current device
> + * reset register is read using a TI SCI device operation, the new value is
> + * set or un-set using the reset's mask, and the new reset value written by
> + * using another TI SCI device operation.
> + *
> + * Return: 0 for successful request, else a corresponding error value
> + */
> +static int ti_sci_reset_set(struct reset_controller_dev *rcdev,
> + unsigned long id, bool assert)
> +{
> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
> + const struct ti_sci_handle *sci = data->sci;
> + const struct ti_sci_dev_ops *dev_ops = &sci->ops.dev_ops;
> + struct ti_sci_reset_control *control;
> + u32 reset_state;
> + int ret;
> +
> + control = idr_find(&data->idr, id);
> + if (!control)
> + return -EINVAL;
> +
> + ret = dev_ops->get_device_resets(sci, control->dev_id,
> + &reset_state);
> + if (ret)
> + return ret;
> +
> + if (assert)
> + reset_state |= control->reset_mask;
> + else
> + reset_state &= ~control->reset_mask;
> +
> + return dev_ops->set_device_resets(sci, control->dev_id,
> + reset_state);
> +}
> +
> +/**
> + * ti_sci_reset_assert() - assert device reset
> + * @rcdev: reset controller entity
> + * @id: ID of the reset to be asserted
> + *
> + * This function implements the reset driver op to assert a device's reset
> + * using the TI SCI protocol. This invokes the function ti_sci_reset_set()
> + * with the corresponding parameters as passed in, but with the @assert
> + * argument set to true for asserting the reset.
> + *
> + * Return: 0 for successful request, else a corresponding error value
> + */
> +static int ti_sci_reset_assert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return ti_sci_reset_set(rcdev, id, true);
> +}
> +
> +/**
> + * ti_sci_reset_deassert() - deassert device reset
> + * @rcdev: reset controller entity
> + * @id: ID of the reset to be deasserted
> + *
> + * This function implements the reset driver op to deassert a device's reset
> + * using the TI SCI protocol. This invokes the function ti_sci_reset_set()
> + * with the corresponding parameters as passed in, but with the @assert
> + * argument set to false for deasserting the reset.
> + *
> + * Return: 0 for successful request, else a corresponding error value
> + */
> +static int ti_sci_reset_deassert(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + return ti_sci_reset_set(rcdev, id, false);
> +}
> +
> +/**
> + * ti_sci_reset_status() - check device reset status
> + * @rcdev: reset controller entity
> + * @id: ID of reset to be checked
> + *
> + * This function implements the reset driver op to return the status of a
> + * device's reset using the TI SCI protocol. The reset register value is read
> + * by invoking the TI SCI device opertation .get_device_resets(), and the
> + * status of the specific reset is extracted and returned using this reset's
> + * reset mask.
> + *
> + * Return: 0 if reset is deasserted, or a non-zero value if reset is asserted
> + */
> +static int ti_sci_reset_status(struct reset_controller_dev *rcdev,
> + unsigned long id)
> +{
> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
> + const struct ti_sci_handle *sci = data->sci;
> + const struct ti_sci_dev_ops *dev_ops = &sci->ops.dev_ops;
> + struct ti_sci_reset_control *control;
> + u32 reset_state;
> + int ret;
> +
> + control = idr_find(&data->idr, id);
> + if (!control)
> + return -EINVAL;
> +
> + ret = dev_ops->get_device_resets(sci, control->dev_id,
> + &reset_state);
> + if (ret)
> + return ret;
> +
> + return reset_state & control->reset_mask;
> +}
> +
> +static struct reset_control_ops ti_sci_reset_ops = {
> + .assert = ti_sci_reset_assert,
> + .deassert = ti_sci_reset_deassert,
> + .status = ti_sci_reset_status,
> +};
> +
> +/**
> + * ti_sci_reset_of_xlate() - translate a set of OF arguments to a reset ID
> + * @rcdev: reset controller entity
> + * @reset_spec: OF reset argument specifier
> + *
> + * This function performs the translation of the reset argument specifier
> + * values defined in a reset consumer device node. The function allocates a
> + * reset control structure for that device reset, and will be used by the
> + * driver for performing any reset functions on that reset. An idr structure
> + * is allocated and used to map to the reset control structure. This idr
> + * is used by the driver to do reset lookups.
> + *
> + * Return: 0 for successful request, else a corresponding error value
> + */
> +static int ti_sci_reset_of_xlate(struct reset_controller_dev *rcdev,
> + const struct of_phandle_args *reset_spec)
> +{
> + struct ti_sci_reset_data *data = to_ti_sci_reset_data(rcdev);
> + struct ti_sci_reset_control *control;
> +
> + if (WARN_ON(reset_spec->args_count != rcdev->of_reset_n_cells))
> + return -EINVAL;
> +
> + control = devm_kzalloc(data->dev, sizeof(*control), GFP_KERNEL);
> + if (!control)
> + return -ENOMEM;
> +
> + control->dev_id = reset_spec->args[0];
> + control->reset_mask = reset_spec->args[1];
> +
> + return idr_alloc(&data->idr, control, 0, 0, GFP_KERNEL);
> +}
> +
> +static const struct of_device_id ti_sci_reset_of_match[] = {
> + { .compatible = "ti,sci-reset", },
> + { /* sentinel */ },
> +};
> +MODULE_DEVICE_TABLE(of, ti_sci_reset_of_match);
> +
> +static int ti_sci_reset_probe(struct platform_device *pdev)
> +{
> + struct ti_sci_reset_data *data;
> +
> + if (!pdev->dev.of_node)
> + return -ENODEV;
> +
> + data = devm_kzalloc(&pdev->dev, sizeof(*data), GFP_KERNEL);
> + if (!data)
> + return -ENOMEM;
> +
> + data->sci = devm_ti_sci_get_handle(&pdev->dev);
> + if (IS_ERR(data->sci))
> + return PTR_ERR(data->sci);
> +
> + data->rcdev.ops = &ti_sci_reset_ops;
> + data->rcdev.owner = THIS_MODULE;
> + data->rcdev.of_node = pdev->dev.of_node;
> + data->rcdev.of_reset_n_cells = 2;
> + data->rcdev.of_xlate = ti_sci_reset_of_xlate;
> + data->dev = &pdev->dev;
> + idr_init(&data->idr);
Hello Andrew,
For my own education, is there a specific reason to use a struct idr
as opposed to keeping a pointer to a struct ti_sci_reset_control in
truct ti_sci_reset_data? I'm not opposed to the way you've done
things but simply keeping a pointer sound more intuitive to me.
Thanks,
Mathieu
> +
> + platform_set_drvdata(pdev, data);
> +
> + return reset_controller_register(&data->rcdev);
> +}
> +
> +static int ti_sci_reset_remove(struct platform_device *pdev)
> +{
> + struct ti_sci_reset_data *data = platform_get_drvdata(pdev);
> +
> + reset_controller_unregister(&data->rcdev);
> +
> + idr_destroy(&data->idr);
> +
> + return 0;
> +}
> +
> +static struct platform_driver ti_sci_reset_driver = {
> + .probe = ti_sci_reset_probe,
> + .remove = ti_sci_reset_remove,
> + .driver = {
> + .name = "ti-sci-reset",
> + .of_match_table = ti_sci_reset_of_match,
> + },
> +};
> +module_platform_driver(ti_sci_reset_driver);
> +
> +MODULE_AUTHOR("Andrew F. Davis <afd-l0cyMroinI0@public.gmane.org>");
> +MODULE_DESCRIPTION("TI System Control Interface (TI SCI) Reset driver");
> +MODULE_LICENSE("GPL v2");
> --
> 2.10.1
>
--
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 v2 5/5] fpga manager: cyclone-ps-spi: make delay variable
From: Moritz Fischer @ 2016-10-28 17:41 UTC (permalink / raw)
To: Joshua Clayton
Cc: Alan Tull, Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer,
Fabio Estevam, Russell King, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <2239ffc8aba7d053825d2bea122a3c16cdd9f6e1.1477669745.git.stillcompiling@gmail.com>
Hi Joshua,
looks good to me; however, I think since you're adding initial support,
I'd squash this together with [3/5].
On Fri, Oct 28, 2016 at 09:56:42AM -0700, Joshua Clayton wrote:
> The status pin may not show ready in the time described in the
> Altetera manual. check the value several times before giving up
s/Altetera/Altera
> For the hardware I am working on, the status pin takes 250 us,
> 5 times as long as described by Altera.
>
> Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
> ---
> drivers/fpga/cyclone-ps-spi.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/fpga/cyclone-ps-spi.c b/drivers/fpga/cyclone-ps-spi.c
> index 4b70d5c..c368223 100644
> --- a/drivers/fpga/cyclone-ps-spi.c
> +++ b/drivers/fpga/cyclone-ps-spi.c
> @@ -20,6 +20,7 @@
>
> #define FPGA_RESET_TIME 50 /* time in usecs to trigger FPGA config */
> -#define FPGA_MIN_DELAY 250 /* min usecs to wait for config status */
> +#define FPGA_MIN_DELAY 50 /* min usecs to wait for config status */
> +#define FPGA_MAX_DELAY 1000 /* max usecs to wait for config status */
>
> struct cyclonespi_conf {
> struct gpio_desc *config;
> @@ -42,6 +43,7 @@ static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
> const char *buf, size_t count)
> {
> struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
> + int i;
>
> if (flags & FPGA_MGR_PARTIAL_RECONFIG) {
> dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
> @@ -56,13 +58,14 @@ static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
> }
>
> gpiod_set_value(conf->config, 1);
> - usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
> - if (gpiod_get_value(conf->status) == 0) {
> - dev_err(&mgr->dev, "Status pin not ready.\n");
> - return -EIO;
> + for (i = 0; i < (FPGA_MAX_DELAY / FPGA_MIN_DELAY); i++) {
> + usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
> + if (gpiod_get_value(conf->status))
> + return 0;
> }
>
> - return 0;
> + dev_err(&mgr->dev, "Status pin not ready.\n");
> + return -EIO;
> }
>
> static void rev_buf(void *buf, size_t len)
> --
> 2.7.4
>
Cheers,
Moritz
^ permalink raw reply
* Re: [PATCH v5 1/7] drm: sunxi: Add a basic DRM driver for Allwinner DE2
From: Jean-Francois Moine @ 2016-10-28 17:34 UTC (permalink / raw)
To: Maxime Ripard
Cc: Dave Airlie, Liam Girdwood, Mark Brown, Rob Herring,
alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw,
devicetree-u79uwXL29TY76Z2rM5mHXA,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-sunxi-/JYPxA39Uh5TLH3MbocFFw
In-Reply-To: <20161027220316.y7u3h4qsfftryrmp@lukather>
On Fri, 28 Oct 2016 00:03:16 +0200
Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org> wrote:
> On Tue, Oct 25, 2016 at 04:14:41PM +0200, Jean-Francois Moine wrote:
> > > > +Display controller
> > > > +==================
> > > > +
> > > > +Required properties:
> > > > +
> > > > +- compatible: value should be one of the following
> > > > + "allwinner,sun8i-a83t-display-engine"
> > > > + "allwinner,sun8i-h3-display-engine"
> > > > +
> > > > +- clocks: must include clock specifiers corresponding to entries in the
> > > > + clock-names property.
> > > > +
> > > > +- clock-names: must contain
> > > > + "gate": for DE activation
> > > > + "clock": DE clock
> > >
> > > We've been calling them bus and mod.
> >
> > I can understand "bus" (which is better than "apb"), but why "mod"?
>
> Allwinner has been calling the clocks that are supposed to generate
> the external signals (depending on where you were looking) module or
> mod clocks (which is also why we have mod in the clock
> compatibles). The module 1 clocks being used for the audio and the
> module 0 for the rest (SPI, MMC, NAND, display, etc.)
I did not find any 'module' in the H3 documentation.
So, is it really a good name?
> > > > +
> > > > +- resets: phandle to the reset of the device
> > > > +
> > > > +- ports: phandle's to the LCD ports
> > >
> > > Please use the OF graph.
> >
> > These ports are references to the graph of nodes. See
> > http://www.kernelhub.org/?msg=911825&p=2
>
> In an OF-graph, your phandle to the LCD controller would be replaced
> by an output endpoint.
This is the DE controller. There is no endpoint link at this level.
The Device Engine just handles the planes of the LCDs, but, indeed,
the LCDs must know about the DE and the DE must know about the LCDs.
There are 2 ways to realize this knowledge in the DT:
1) either the DE has one or two phandle's to the LCDs,
2) or the LCDs have a phandle to the DE.
I chose the 1st way, the DE ports pointing to the endpoint of the LCDs
which is part of the video link (OF-graph LCD <-> connector).
It would be possible to have phandles to the LCDs themselves, but this
asks for more code.
The second way is also possible, but it also complexifies a bit the
exchanges DE <-> LCD.
> > [snip]
> > > > +struct tcon {
> > > > + u32 gctl;
> > > > +#define TCON_GCTL_TCON_En BIT(31)
[snip]
> > > > + u32 fill_ctl; /* 0x300 */
> > > > + u32 fill_start0;
> > > > + u32 fill_end0;
> > > > + u32 fill_data0;
> > > > +};
> > >
> > > Please use defines instead of the structures.
> >
> > I think that structures are more readable.
>
> That's not really the point. No one in the kernel uses it (and even
> you use defines for registers offset in some places of that
> patch). And then you have André arguments.
I am not convinced, but I'll do as you said.
> > > > +void de2_disable_vblank(struct drm_device *drm, unsigned crtc)
> > > > +{
> > > > + struct priv *priv = drm->dev_private;
> > > > + struct lcd *lcd = priv->lcds[crtc];
> > > > +
> > > > + tcon_write(lcd->mmio, gint0,
> > > > + tcon_read(lcd->mmio, gint0) &
> > > > + ~TCON_GINT0_TCON1_Vb_Int_En);
> > > > +}
> > > > +
> > > > +/* panel functions */
> > >
> > > Panel functions? In the CRTC driver?
> >
> > Yes, dumb panel.
>
> What do you mean by that? Using a Parallel/RGB interface?
Sorry, I though this was a well-known name. The 'dump panel' was used
in the documentation of my previous ARM machine as the video frame sent
to the HDMI controller. 'video_frame' is OK for you?
[snip]
> > > > + ret = clk_prepare_enable(lcd->clk);
> > > > + if (ret)
> > > > + goto err2;
> > >
> > > Is there any reason not to do that in the enable / disable? Leaving
> > > clocks running while the device has no guarantee that it's going to be
> > > used seems like a waste of resources.
> >
> > If the machine does not need video (network server, router..), it is simpler
> > to prevent the video driver to be loaded (DT, module black list...).
>
> You might not have control on any of it, or you might just have no
> monitor attached for example. Recompiling the kernel or updating the
> DT when you want to plug an HDMI monitor seems like a poor UX :)
OK, I will check if this works.
> > > > +static const struct {
> > > > + char chan;
> > > > + char layer;
> > > > + char pipe;
> > > > +} plane2layer[DE2_N_PLANES] = {
> > > > + [DE2_PRIMARY_PLANE] = {0, 0, 0},
> > > > + [DE2_CURSOR_PLANE] = {1, 0, 1},
> > > > + [DE2_VI_PLANE] = {0, 1, 0},
> > > > +};
> > >
> > > Comments?
> >
> > This
> > primary plane is channel 0 (VI), layer 0, pipe 0
> > cursor plane is channel 1 (UI), layer 0, pipe 1
> > overlay plane is channel 0 (VI), layer 1, pipe 0
> > or the full explanation:
> > Constraints:
> > The VI channels can do RGB or YUV, while UI channels can do RGB
> > only.
> > The LCD 0 has 1 VI channel and 4 UI channels, while
> > LCD 1 has only 1 VI channel and 1 UI channel.
> > The cursor must go to a channel bigger than the primary channel,
> > otherwise it is not transparent.
> > First try:
> > Letting the primary plane (usually RGB) in the 2nd channel (UI),
> > as this is done in the legacy driver, asks for the cursor to go
> > to the next channel (UI), but this one does not exist in LCD1.
> > Retained layout:
> > So, we must use only 2 channels for the same behaviour on LCD0
> > (H3) and LCD1 (A83T)
> > The retained combination is:
> > - primary plane in the first channel (VI),
> > - cursor plane inthe 2nd channel (UI), and
> > - overlay plane in the 1st channel (VI).
> >
> > Note that there could be 3 overlay planes (a channel has 4
> > layers), but I am not sure that the A83T or the H3 could
> > support 3 simultaneous video streams...
>
> Do you know if the pipe works in the old display engine?
>
> Especially about the two-steps composition that wouldn't allow you to
> have alpha on all the planes?
>
> If it is similar, I think hardcoding the pipe number is pretty bad,
> because that would restrict the combination of planes and formats,
> while some other might have worked.
>From what I understood about the DE2, the pipes just define the priority
of the overlay channels (one pipe for one channel).
With the cursor constraint, there must be at least 2 channels in
order (primary, cursor). Then, with these 2 channels/pipes, there can be
6 so-called overlay planes (3 RGB/YUV and 3 RGB only).
Enabling the pipes 2 and 3 (LCD 0 only) would offer 8 more planes, but
RGB only. Then, it might be useful to have dynamic pipes.
[snip]
> > > > +void de2_de_plane_update(struct priv *priv,
> > > > + int lcd_num, int plane_ix,
> > > > + struct drm_plane_state *state,
> > > > + struct drm_plane_state *old_state)
> > > > +{
> > > > + struct drm_framebuffer *fb = state->fb;
> > > > + struct drm_gem_cma_object *gem;
> > > > + void __iomem *mux_o = priv->mmio;
> > > > + void __iomem *chan_o;
> > > > + u32 size = WH(state->crtc_w, state->crtc_h);
> > > > + u32 coord;
> > > > + u32 screen_size;
> > > > + u32 data, fcolor;
> > > > + u32 ui_sel, alpha_glob;
> > > > + int chan, layer, x, y;
> > > > + unsigned fmt;
> > > > + unsigned long flags;
> > > > +
> > > > + chan = plane2layer[plane_ix].chan;
> > > > + layer = plane2layer[plane_ix].layer;
> > > > +
> > > > + mux_o += (lcd_num == 0) ? DE_MUX0_BASE : DE_MUX1_BASE;
> > > > + chan_o = mux_o;
> > > > + chan_o += DE_MUX_CHAN_REGS + DE_MUX_CHAN_SZ * chan;
> > > > +
> > > > + x = state->crtc_x >= 0 ? state->crtc_x : 0;
> > > > + y = state->crtc_y >= 0 ? state->crtc_y : 0;
> > > > + coord = XY(x, y);
> > > > +
> > > > + /* handle the cursor move */
> > > > + if (plane_ix == DE2_CURSOR_PLANE
> > > > + && fb == old_state->fb) {
> > > > + spin_lock_irqsave(&de_lock, flags);
> > > > + de_lcd_select(priv, lcd_num, mux_o);
> > > > + if (chan == 0)
> > > > + vi_write(chan_o, cfg[layer].coord, coord);
> > > > + else
> > > > + ui_write(chan_o, cfg[layer].coord, coord);
> > > > + spin_unlock_irqrestore(&de_lock, flags);
> > > > + return;
> > > > + }
> > > > +
> > > > + gem = drm_fb_cma_get_gem_obj(fb, 0);
> > > > +
> > > > + ui_sel = alpha_glob = 0;
> > > > + switch (fb->pixel_format) {
> > > > + case DRM_FORMAT_ARGB8888:
> > > > + fmt = DE2_FORMAT_ARGB_8888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + break;
> > > > + case DRM_FORMAT_BGRA8888:
> > > > + fmt = DE2_FORMAT_BGRA_8888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + break;
> > > > + case DRM_FORMAT_XRGB8888:
> > > > + fmt = DE2_FORMAT_XRGB_8888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + alpha_glob = (1 << UI_CFG_ATTR_alpmod_SHIFT) |
> > > > + (0xff << UI_CFG_ATTR_alpha_SHIFT);
> > > > + break;
> > > > + case DRM_FORMAT_RGB888:
> > > > + fmt = DE2_FORMAT_RGB_888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + break;
> > > > + case DRM_FORMAT_BGR888:
> > > > + fmt = DE2_FORMAT_BGR_888;
> > > > + ui_sel = VI_CFG_ATTR_ui_sel;
> > > > + break;
> > > > + case DRM_FORMAT_YUYV:
> > > > + fmt = DE2_FORMAT_YUV422_I_YUYV;
> > > > + break;
> > > > + case DRM_FORMAT_YVYU:
> > > > + fmt = DE2_FORMAT_YUV422_I_YVYU;
> > > > + break;
> > > > + case DRM_FORMAT_YUV422:
> > > > + fmt = DE2_FORMAT_YUV422_P;
> > > > + break;
> > > > + case DRM_FORMAT_YUV420:
> > > > + fmt = DE2_FORMAT_YUV420_P;
> > > > + break;
> > > > + case DRM_FORMAT_UYVY:
> > > > + fmt = DE2_FORMAT_YUV422_I_UYVY;
> > > > + break;
> > > > + default:
> > > > + pr_err("format %.4s not yet treated\n",
> > > > + (char *) &fb->pixel_format);
> > > > + return;
> > > > + }
> > > > +
> > > > + spin_lock_irqsave(&de_lock, flags);
> > > > +
> > > > + screen_size = plane_ix == DE2_PRIMARY_PLANE ?
> > > > + size :
> > > > + glb_read(mux_o + DE_MUX_GLB_REGS, size);
> > > > +
> > > > + /* prepare the activation of alpha blending (1 bit per plane) */
> > > > + fcolor = bld_read(mux_o + DE_MUX_BLD_REGS, fcolor_ctl)
> > > > + | (0x100 << plane2layer[plane_ix].pipe);
> > > > +
> > > > + de_lcd_select(priv, lcd_num, mux_o);
> > > > +
> > > > + if (chan == 0) { /* VI channel */
> > > > + int i;
> > > > +
> > > > + data = VI_CFG_ATTR_en | (fmt << VI_CFG_ATTR_fmt_SHIFT) |
> > > > + ui_sel;
> > > > + vi_write(chan_o, cfg[layer].attr, data);
> > > > + vi_write(chan_o, cfg[layer].size, size);
> > > > + vi_write(chan_o, cfg[layer].coord, coord);
> > > > + for (i = 0; i < VI_N_PLANES; i++) {
> > > > + vi_write(chan_o, cfg[layer].pitch[i],
> > > > + fb->pitches[i] ? fb->pitches[i] :
> > > > + fb->pitches[0]);
> > > > + vi_write(chan_o, cfg[layer].top_laddr[i],
> > > > + gem->paddr + fb->offsets[i]);
> > > > + vi_write(chan_o, fcolor[layer], 0xff000000);
> > > > + }
> > > > + if (layer == 0)
> > > > + vi_write(chan_o, ovl_size[0], screen_size);
> > > > +
> > > > + } else { /* UI channel */
> > > > + data = UI_CFG_ATTR_en | (fmt << UI_CFG_ATTR_fmt_SHIFT) |
> > > > + alpha_glob;
> > > > + ui_write(chan_o, cfg[layer].attr, data);
> > > > + ui_write(chan_o, cfg[layer].size, size);
> > > > + ui_write(chan_o, cfg[layer].coord, coord);
> > > > + ui_write(chan_o, cfg[layer].pitch, fb->pitches[0]);
> > > > + ui_write(chan_o, cfg[layer].top_laddr,
> > > > + gem->paddr + fb->offsets[0]);
> > > > + if (layer == 0)
> > > > + ui_write(chan_o, ovl_size, screen_size);
> > > > + }
> > > > + bld_write(mux_o + DE_MUX_BLD_REGS, fcolor_ctl, fcolor);
> > > > +
> > > > + spin_unlock_irqrestore(&de_lock, flags);
> > > > +}
> > >
> > > Splitting that into functions would make it a bit more trivial and
> > > readable.
> >
> > Not sure: there is a lot of common data and different I/O accesses.
>
> You could still have different ones to set the buffers, formats and
> coordinates for example.
I will have a look...
[snip]
> > > > +static int __init de2_drm_init(void)
> > > > +{
> > > > + int ret;
> > > > +
> > > > +/* uncomment to activate the drm traces at startup time */
> > > > +/* drm_debug = DRM_UT_CORE | DRM_UT_DRIVER | DRM_UT_KMS |
> > > > + DRM_UT_PRIME | DRM_UT_ATOMIC; */
> > >
> > > That's useless.
> >
> > Right, but it seems that some people don't know how to debug a DRM
> > driver. This is only a reminder.
> >
> > > > + DRM_DEBUG_DRIVER("\n");
> > > > +
> > > > + ret = platform_driver_register(&de2_lcd_platform_driver);
> > > > + if (ret < 0)
> > > > + return ret;
> > > > +
> > > > + ret = platform_driver_register(&de2_drm_platform_driver);
> > > > + if (ret < 0)
> > > > + platform_driver_unregister(&de2_lcd_platform_driver);
> > > > +
> > > > + return ret;
> > > > +}
> > >
> > > And that really shouldn't be done that way.
> >
> > May you explain?
>
> This goes against the whole idea of the device and driver
> model. Drivers should only register themselves, device should be
> created by buses (or by using some external components if the bus
> can't: DT, ACPI, etc.). If there's a match, you get probed.
>
> A driver that creates its own device just to probe itself violates
> that.
In this function (module init), there is no driver yet.
The module contains 2 drivers: the DE (planes) and the LCD (CRTC),
and there is no macro to handle such modules.
> > > > +int de2_plane_init(struct drm_device *drm, struct lcd *lcd)
> > > > +{
> > > > + int ret, possible_crtcs = 1 << lcd->crtc_idx;
> > > > +
> > > > + ret = de2_one_plane_init(drm, &lcd->planes[DE2_PRIMARY_PLANE],
> > > > + DRM_PLANE_TYPE_PRIMARY, possible_crtcs,
> > > > + ui_formats, ARRAY_SIZE(ui_formats));
> > > > + if (ret >= 0)
> > > > + ret = de2_one_plane_init(drm, &lcd->planes[DE2_CURSOR_PLANE],
> > > > + DRM_PLANE_TYPE_CURSOR, possible_crtcs,
> > > > + ui_formats, ARRAY_SIZE(ui_formats));
> > >
> > > Nothing looks really special about that cursor plane. Any reasion not
> > > to make it an overlay?
> >
> > As explained above (channel/layer/pipe plane definitions), the cursor
> > cannot go in a channel lower or equal to the one of the primary plane.
> > Then, it must be known and, so, have an explicit plane.
>
> If you were to make it a plane, you could use atomic_check to check
> this and make sure this doesn't happen. And you would gain a generic
> plane that can be used for other purposes if needed.
The function drm_crtc_init_with_planes() offers a cursor plane for free.
On the other side, having 6 overlay planes is more than the SoCs can
support.
--
Ken ar c'hentañ | ** Breizh ha Linux atav! **
Jef | http://moinejf.free.fr/
--
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 v6 4/5] ARM: DTS: da850: Add cfgchip syscon node
From: David Lechner @ 2016-10-28 17:24 UTC (permalink / raw)
To: Kevin Hilman, Sekhar Nori
Cc: Rob Herring, Mark Rutland, Axel Haslam, Sergei Shtylyov,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <m2fungcses.fsf-rdvid1DuHRBWk0Htik3J/w@public.gmane.org>
On 10/28/2016 12:08 PM, Kevin Hilman wrote:
> Sekhar Nori <nsekhar-l0cyMroinI0@public.gmane.org> writes:
>
>> On Wednesday 26 October 2016 09:38 PM, David Lechner wrote:
>>> On 10/25/2016 10:06 PM, David Lechner wrote:
>>>> Add a syscon node for the SoC CFGCHIPn registers. This is needed for
>>>> the new usb phy driver.
>>>>
>>>> Signed-off-by: David Lechner <david-nq/r/kbU++upp/zk7JDF2g@public.gmane.org>
>>>> ---
>>>> arch/arm/boot/dts/da850.dtsi | 4 ++++
>>>> 1 file changed, 4 insertions(+)
>>>>
>>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>>> index f79e1b9..6bbf20d 100644
>>>> --- a/arch/arm/boot/dts/da850.dtsi
>>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>>> @@ -188,6 +188,10 @@
>>>> };
>>>>
>>>> };
>>>> + cfgchip: cfgchip@1417c {
>>>
>>> I wonder if there is a more generic name instead of cfgchip@. Is there a
>>> preferred generic name for syscon nodes?
>>
>> I did not find anything in ePAPR, but chip-controller might be more
>> appropriate.
>>
>>>
>>>> + compatible = "ti,da830-cfgchip", "syscon";
>>
>> Looks like we need "simple-mfd" too in the compatible list?
>>
>> I think we can also fold patch 5/5 into this patch and add the cfgchip
>> along with USB phy child node included.
>>
>> If you respin the patch, I can drop 4/5 and 5/5 that I have queued and
>> included the updated patch instead.
>
> Sekhar, what's your opinion of having this syscon just for CFGCHIP* vs
> a single syscon for the whole SYSCFG0 region.
>
> The drivers/bus driver from Bartosz is also using SYSCFG0 registers, and
> proposing a sysconf ro this region, but it will need to exclude the
> CFGCHIPn registers if we also have this syscon.
What about the pinmux registers, which are already being used separately
too?
>
> I tend to think we should just have one for the whole SYSCFG0 which
> this series could use.
>
> Unfortunately, the PHY driver is already merged and it references the
> syscon by compatible. The PHY driver should probably be fixed to find
> its syscon by phandle, and then maybe we could move to a single syscon
> for SYSCFG0?
I agree that this should be change, but I was thinking we should use
syscon_node_to_regmap(np->parent) since the phy node should be a child
of the syscon node.
>
> Let us know your preference, I don't have a very strong feeling either
> way, but since we're already part way down the path of the CFGCHIP
> syscon, we should keep it and later migrate it to one for all of
> SYSCFG0.
>
> Kevin
>
--
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 v6 4/5] ARM: DTS: da850: Add cfgchip syscon node
From: Kevin Hilman @ 2016-10-28 17:08 UTC (permalink / raw)
To: Sekhar Nori
Cc: David Lechner, Rob Herring, Mark Rutland, Axel Haslam,
Sergei Shtylyov, devicetree, linux-arm-kernel, linux-kernel
In-Reply-To: <09eb61b9-d66e-838e-2411-d787fda65096@ti.com>
Sekhar Nori <nsekhar@ti.com> writes:
> On Wednesday 26 October 2016 09:38 PM, David Lechner wrote:
>> On 10/25/2016 10:06 PM, David Lechner wrote:
>>> Add a syscon node for the SoC CFGCHIPn registers. This is needed for
>>> the new usb phy driver.
>>>
>>> Signed-off-by: David Lechner <david@lechnology.com>
>>> ---
>>> arch/arm/boot/dts/da850.dtsi | 4 ++++
>>> 1 file changed, 4 insertions(+)
>>>
>>> diff --git a/arch/arm/boot/dts/da850.dtsi b/arch/arm/boot/dts/da850.dtsi
>>> index f79e1b9..6bbf20d 100644
>>> --- a/arch/arm/boot/dts/da850.dtsi
>>> +++ b/arch/arm/boot/dts/da850.dtsi
>>> @@ -188,6 +188,10 @@
>>> };
>>>
>>> };
>>> + cfgchip: cfgchip@1417c {
>>
>> I wonder if there is a more generic name instead of cfgchip@. Is there a
>> preferred generic name for syscon nodes?
>
> I did not find anything in ePAPR, but chip-controller might be more
> appropriate.
>
>>
>>> + compatible = "ti,da830-cfgchip", "syscon";
>
> Looks like we need "simple-mfd" too in the compatible list?
>
> I think we can also fold patch 5/5 into this patch and add the cfgchip
> along with USB phy child node included.
>
> If you respin the patch, I can drop 4/5 and 5/5 that I have queued and
> included the updated patch instead.
Sekhar, what's your opinion of having this syscon just for CFGCHIP* vs
a single syscon for the whole SYSCFG0 region.
The drivers/bus driver from Bartosz is also using SYSCFG0 registers, and
proposing a sysconf ro this region, but it will need to exclude the
CFGCHIPn registers if we also have this syscon.
I tend to think we should just have one for the whole SYSCFG0 which
this series could use.
Unfortunately, the PHY driver is already merged and it references the
syscon by compatible. The PHY driver should probably be fixed to find
its syscon by phandle, and then maybe we could move to a single syscon
for SYSCFG0?
Let us know your preference, I don't have a very strong feeling either
way, but since we're already part way down the path of the CFGCHIP
syscon, we should keep it and later migrate it to one for all of
SYSCFG0.
Kevin
^ permalink raw reply
* Re: [PATCH 1/2] of, numa: Add function to disable of_node_to_nid().
From: David Daney @ 2016-10-28 17:02 UTC (permalink / raw)
To: Will Deacon
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA, Rob Herring, Frank Rowand,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Catalin Marinas, Robert Richter, Hanjun Guo, Ganapatrao Kulkarni,
Gilbert Netzer, David Daney
In-Reply-To: <20161028101905.GA6343-5wv7dgnIgG8@public.gmane.org>
On 10/28/2016 03:19 AM, Will Deacon wrote:
> On Tue, Oct 25, 2016 at 02:31:00PM -0700, David Daney wrote:
>> From: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>>
>> On arm64 NUMA kernels we can pass "numa=off" on the command line to
>> disable NUMA. A side effect of this is that kmalloc_node() calls to
>> non-zero nodes will crash the system with an OOPS:
>>
>> [ 0.000000] [<fffffc00081bba84>] __alloc_pages_nodemask+0xa4/0xe68
>> [ 0.000000] [<fffffc00082163a8>] new_slab+0xd0/0x57c
>> [ 0.000000] [<fffffc000821879c>] ___slab_alloc+0x2e4/0x514
>> [ 0.000000] [<fffffc000823882c>] __slab_alloc+0x48/0x58
>> [ 0.000000] [<fffffc00082195a0>] __kmalloc_node+0xd0/0x2e0
>> [ 0.000000] [<fffffc00081119b8>] __irq_domain_add+0x7c/0x164
>> [ 0.000000] [<fffffc0008b75d30>] its_probe+0x784/0x81c
>> [ 0.000000] [<fffffc0008b75e10>] its_init+0x48/0x1b0
>> .
>> .
>> .
>>
>> This is caused by code like this in kernel/irq/irqdomain.c
>>
>> domain = kzalloc_node(sizeof(*domain) + (sizeof(unsigned int) * size),
>> GFP_KERNEL, of_node_to_nid(of_node));
>>
>> When NUMA is disabled, the concept of a node is really undefined, so
>> of_node_to_nid() should unconditionally return NUMA_NO_NODE.
>>
>> Add __of_force_no_numa() to allow of_node_to_nid() to be forced to
>> return NUMA_NO_NODE.
>>
>> The follow on patch will call this new function from the arm64 numa
>> code.
>>
>> Reported-by: Gilbert Netzer <noname-QVTQ4TwcpgDLoDKTGw+V6w@public.gmane.org>
>> Signed-off-by: David Daney <david.daney-YGCgFSpz5w/QT0dZR+AlfA@public.gmane.org>
>> ---
>> drivers/of/of_numa.c | 15 +++++++++++++++
>> include/linux/of.h | 2 ++
>> 2 files changed, 17 insertions(+)
>>
>> diff --git a/drivers/of/of_numa.c b/drivers/of/of_numa.c
>> index f63d4b0d..2212299 100644
>> --- a/drivers/of/of_numa.c
>> +++ b/drivers/of/of_numa.c
>> @@ -150,12 +150,27 @@ static int __init of_numa_parse_distance_map(void)
>> return ret;
>> }
>>
>> +static bool of_force_no_numa;
>> +
>> +void __of_force_no_numa(void)
>> +{
>> + of_force_no_numa = true;
>> +}
>> +
>> int of_node_to_nid(struct device_node *device)
>> {
>> struct device_node *np;
>> u32 nid;
>> int r = -ENODATA;
>>
>> + /*
>> + * If NUMA forced off, nodes are meaningless. Return
>> + * NUMA_NO_NODE so that any node specific memory allocations
>> + * can succeed from the default pool.
>> + */
>> + if (of_force_no_numa)
>> + return NUMA_NO_NODE;
>
> Why don't you just check if the nid you get back from the device is set in
> numa_nodes_parsed and return NUMA_NO_NODE if not?
numa_nodes_parsed is __initdata. Perhaps node_possible_map would be better.
I will try that.
David.
>
> Will
>
--
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 v2 5/5] fpga manager: cyclone-ps-spi: make delay variable
From: Joshua Clayton @ 2016-10-28 16:56 UTC (permalink / raw)
To: Alan Tull, Moritz Fischer
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam,
Russell King, Joshua Clayton, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <cover.1477669744.git.stillcompiling@gmail.com>
The status pin may not show ready in the time described in the
Altetera manual. check the value several times before giving up
For the hardware I am working on, the status pin takes 250 us,
5 times as long as described by Altera.
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
drivers/fpga/cyclone-ps-spi.c | 13 ++++++++-----
1 file changed, 8 insertions(+), 5 deletions(-)
diff --git a/drivers/fpga/cyclone-ps-spi.c b/drivers/fpga/cyclone-ps-spi.c
index 4b70d5c..c368223 100644
--- a/drivers/fpga/cyclone-ps-spi.c
+++ b/drivers/fpga/cyclone-ps-spi.c
@@ -20,6 +20,7 @@
#define FPGA_RESET_TIME 50 /* time in usecs to trigger FPGA config */
-#define FPGA_MIN_DELAY 250 /* min usecs to wait for config status */
+#define FPGA_MIN_DELAY 50 /* min usecs to wait for config status */
+#define FPGA_MAX_DELAY 1000 /* max usecs to wait for config status */
struct cyclonespi_conf {
struct gpio_desc *config;
@@ -42,6 +43,7 @@ static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
const char *buf, size_t count)
{
struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+ int i;
if (flags & FPGA_MGR_PARTIAL_RECONFIG) {
dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
@@ -56,13 +58,14 @@ static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
}
gpiod_set_value(conf->config, 1);
- usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
- if (gpiod_get_value(conf->status) == 0) {
- dev_err(&mgr->dev, "Status pin not ready.\n");
- return -EIO;
+ for (i = 0; i < (FPGA_MAX_DELAY / FPGA_MIN_DELAY); i++) {
+ usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
+ if (gpiod_get_value(conf->status))
+ return 0;
}
- return 0;
+ dev_err(&mgr->dev, "Status pin not ready.\n");
+ return -EIO;
}
static void rev_buf(void *buf, size_t len)
--
2.7.4
^ permalink raw reply related
* [PATCH v2 4/5] ARM: dts: imx6q-evi: support cyclonespi
From: Joshua Clayton @ 2016-10-28 16:56 UTC (permalink / raw)
To: Alan Tull, Moritz Fischer
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam,
Russell King, Joshua Clayton, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <cover.1477669744.git.stillcompiling@gmail.com>
Add support for Altera cyclone V FPGA connected to an spi port
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
arch/arm/boot/dts/imx6q-evi.dts | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-evi.dts b/arch/arm/boot/dts/imx6q-evi.dts
index 6de21ff..bd0b85c 100644
--- a/arch/arm/boot/dts/imx6q-evi.dts
+++ b/arch/arm/boot/dts/imx6q-evi.dts
@@ -95,6 +95,15 @@
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_ecspi1 &pinctrl_ecspi1cs>;
status = "okay";
+
+ fpga_spi: cyclonespi@0 {
+ compatible = "altr,cyclone-ps-spi-fpga-mgr";
+ spi-max-frequency = <20000000>;
+ reg = <0>;
+ pinctrl-0 = <&pinctrl_fpgaspi>;
+ config-gpio = <&gpio4 9 GPIO_ACTIVE_HIGH>;
+ status-gpio = <&gpio4 11 GPIO_ACTIVE_HIGH>;
+ };
};
&ecspi3 {
@@ -325,6 +334,13 @@
>;
};
+ pinctrl_fpgaspi: fpgaspigrp {
+ fsl,pins = <
+ MX6QDL_PAD_KEY_ROW1__GPIO4_IO09 0x1b0b0
+ MX6QDL_PAD_KEY_ROW2__GPIO4_IO11 0x1b0b0
+ >;
+ };
+
pinctrl_gpminand: gpminandgrp {
fsl,pins = <
MX6QDL_PAD_NANDF_CLE__NAND_CLE 0xb0b1
--
2.7.4
^ permalink raw reply related
* [PATCH v2 3/5] fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
From: Joshua Clayton @ 2016-10-28 16:56 UTC (permalink / raw)
To: Alan Tull, Moritz Fischer
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam,
Russell King, Joshua Clayton, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <cover.1477669744.git.stillcompiling@gmail.com>
cyclone-ps-spi loads FPGA firmware over spi, using the "passive serial"
interface on Altera Cyclone FPGAS.
This is one of the simpler ways to set up an FPGA at runtime.
The signal interface is close to unidirectional spi with lsb first.
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
drivers/fpga/Kconfig | 7 ++
drivers/fpga/Makefile | 1 +
drivers/fpga/cyclone-ps-spi.c | 172 ++++++++++++++++++++++++++++++++++++++++++
3 files changed, 180 insertions(+)
create mode 100644 drivers/fpga/cyclone-ps-spi.c
diff --git a/drivers/fpga/Kconfig b/drivers/fpga/Kconfig
index cd84934..2462707 100644
--- a/drivers/fpga/Kconfig
+++ b/drivers/fpga/Kconfig
@@ -13,6 +13,13 @@ config FPGA
if FPGA
+config FPGA_MGR_CYCLONE_PS_SPI
+ tristate "Altera Cyclone FPGA Passive Serial over SPI"
+ depends on SPI
+ help
+ FPGA manager driver support for Altera Cyclone using the
+ passive serial interface over SPI
+
config FPGA_MGR_SOCFPGA
tristate "Altera SOCFPGA FPGA Manager"
depends on ARCH_SOCFPGA
diff --git a/drivers/fpga/Makefile b/drivers/fpga/Makefile
index 8d83fc6..8f93930 100644
--- a/drivers/fpga/Makefile
+++ b/drivers/fpga/Makefile
@@ -6,5 +6,6 @@
obj-$(CONFIG_FPGA) += fpga-mgr.o
# FPGA Manager Drivers
+obj-$(CONFIG_FPGA_MGR_CYCLONE_PS_SPI) += cyclone-ps-spi.o
obj-$(CONFIG_FPGA_MGR_SOCFPGA) += socfpga.o
obj-$(CONFIG_FPGA_MGR_ZYNQ_FPGA) += zynq-fpga.o
diff --git a/drivers/fpga/cyclone-ps-spi.c b/drivers/fpga/cyclone-ps-spi.c
new file mode 100644
index 0000000..4b70d5c
--- /dev/null
+++ b/drivers/fpga/cyclone-ps-spi.c
@@ -0,0 +1,172 @@
+/**
+ * Copyright (c) 2015 United Western Technologies, Corporation
+ *
+ * Joshua Clayton <stillcompiling@gmail.com>
+ *
+ * Manage Altera fpga firmware that is loaded over spi.
+ * Firmware must be in binary "rbf" format.
+ * Works on Cyclone V. Should work on cyclone series.
+ * May work on other Altera fpgas.
+ *
+ */
+
+#include <linux/bitrev.h>
+#include <linux/delay.h>
+#include <linux/fpga/fpga-mgr.h>
+#include <linux/gpio/consumer.h>
+#include <linux/module.h>
+#include <linux/of_gpio.h>
+#include <linux/spi/spi.h>
+
+#define FPGA_RESET_TIME 50 /* time in usecs to trigger FPGA config */
+#define FPGA_MIN_DELAY 250 /* min usecs to wait for config status */
+
+struct cyclonespi_conf {
+ struct gpio_desc *config;
+ struct gpio_desc *status;
+ struct spi_device *spi;
+};
+
+static const struct of_device_id of_ef_match[] = {
+ { .compatible = "altr,cyclone-ps-spi-fpga-mgr", },
+ {}
+};
+MODULE_DEVICE_TABLE(of, of_ef_match);
+
+static enum fpga_mgr_states cyclonespi_state(struct fpga_manager *mgr)
+{
+ return mgr->state;
+}
+
+static int cyclonespi_write_init(struct fpga_manager *mgr, u32 flags,
+ const char *buf, size_t count)
+{
+ struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+
+ if (flags & FPGA_MGR_PARTIAL_RECONFIG) {
+ dev_err(&mgr->dev, "Partial reconfiguration not supported.\n");
+ return -EINVAL;
+ }
+
+ gpiod_set_value(conf->config, 0);
+ usleep_range(FPGA_RESET_TIME, FPGA_RESET_TIME + 20);
+ if (gpiod_get_value(conf->status) == 1) {
+ dev_err(&mgr->dev, "Status pin should be low.\n");
+ return -EIO;
+ }
+
+ gpiod_set_value(conf->config, 1);
+ usleep_range(FPGA_MIN_DELAY, FPGA_MIN_DELAY + 20);
+ if (gpiod_get_value(conf->status) == 0) {
+ dev_err(&mgr->dev, "Status pin not ready.\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static void rev_buf(void *buf, size_t len)
+{
+ u32 *fw32 = (u32 *)buf;
+ const u32 *fw_end = (u32 *)(buf + len);
+
+ /* set buffer to lsb first */
+ while (fw32 < fw_end) {
+ *fw32 = bitrev8x4(*fw32);
+ fw32++;
+ }
+}
+
+static int cyclonespi_write(struct fpga_manager *mgr, const char *buf,
+ size_t count)
+{
+ struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+ const char *fw_data = buf;
+ const char *fw_data_end = fw_data + count;
+
+ while (fw_data < fw_data_end) {
+ int ret;
+ size_t stride = min(fw_data_end - fw_data, SZ_4K);
+
+ rev_buf((void *)fw_data, stride);
+ ret = spi_write(conf->spi, fw_data, stride);
+ if (ret) {
+ dev_err(&mgr->dev, "spi error in firmware write: %d\n",
+ ret);
+ return ret;
+ }
+ fw_data += stride;
+ }
+
+ return 0;
+}
+
+static int cyclonespi_write_complete(struct fpga_manager *mgr, u32 flags)
+{
+ struct cyclonespi_conf *conf = (struct cyclonespi_conf *)mgr->priv;
+
+ if (gpiod_get_value(conf->status) == 0) {
+ dev_err(&mgr->dev, "Error during configuration.\n");
+ return -EIO;
+ }
+
+ return 0;
+}
+
+static const struct fpga_manager_ops cyclonespi_ops = {
+ .state = cyclonespi_state,
+ .write_init = cyclonespi_write_init,
+ .write = cyclonespi_write,
+ .write_complete = cyclonespi_write_complete,
+};
+
+static int cyclonespi_probe(struct spi_device *spi)
+{
+ struct cyclonespi_conf *conf = devm_kzalloc(&spi->dev, sizeof(*conf),
+ GFP_KERNEL);
+
+ if (!conf)
+ return -ENOMEM;
+
+ conf->spi = spi;
+ conf->config = devm_gpiod_get(&spi->dev, "config", GPIOD_OUT_LOW);
+ if (IS_ERR(conf->config)) {
+ dev_err(&spi->dev, "Failed to get config gpio: %ld\n",
+ PTR_ERR(conf->config));
+ return PTR_ERR(conf->config);
+ }
+
+ conf->status = devm_gpiod_get(&spi->dev, "status", GPIOD_IN);
+ if (IS_ERR(conf->status)) {
+ dev_err(&spi->dev, "Failed to get status gpio: %ld\n",
+ PTR_ERR(conf->status));
+ return PTR_ERR(conf->status);
+ }
+
+ return fpga_mgr_register(&spi->dev,
+ "Altera Cyclone PS SPI FPGA Manager",
+ &cyclonespi_ops, conf);
+}
+
+static int cyclonespi_remove(struct spi_device *spi)
+{
+ fpga_mgr_unregister(&spi->dev);
+
+ return 0;
+}
+
+static struct spi_driver cyclonespi_driver = {
+ .driver = {
+ .name = "cyclone-ps-spi",
+ .owner = THIS_MODULE,
+ .of_match_table = of_match_ptr(of_ef_match),
+ },
+ .probe = cyclonespi_probe,
+ .remove = cyclonespi_remove,
+};
+
+module_spi_driver(cyclonespi_driver)
+
+MODULE_LICENSE("GPL");
+MODULE_AUTHOR("Joshua Clayton <stillcompiling@gmail.com>");
+MODULE_DESCRIPTION("Module to load Altera FPGA firmware over spi");
--
2.7.4
^ permalink raw reply related
* [PATCH v2 2/5] doc: dt: add cyclone-spi binding document
From: Joshua Clayton @ 2016-10-28 16:56 UTC (permalink / raw)
To: Alan Tull, Moritz Fischer
Cc: Mark Rutland, devicetree, Joshua Clayton, Russell King,
linux-kernel, Rob Herring, Sascha Hauer, Fabio Estevam, Shawn Guo,
linux-arm-kernel
In-Reply-To: <cover.1477669744.git.stillcompiling@gmail.com>
Describe a cyclonei-ps-spi devicetree entry, required features
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
.../bindings/fpga/cyclone-ps-spi-fpga-mgr.txt | 23 ++++++++++++++++++++++
1 file changed, 23 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
diff --git a/Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt b/Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
new file mode 100644
index 0000000..c942281
--- /dev/null
+++ b/Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
@@ -0,0 +1,23 @@
+Altera Cyclone Passive Serial SPI FPGA Manager
+
+Altera Cyclone FPGAs support a method of loading the bitstream over what is
+referred to as "passive serial".
+The passive serial link is not technically spi, and might require extra
+circuits in order to play nicely with other spi slaves on the same bus.
+
+See https://www.altera.com/literature/hb/cyc/cyc_c51013.pdf
+
+Required properties:
+- compatible : should contain "altr,cyclone-ps-spi-fpga-mgr"
+- reg : spi slave id of the fpga
+- config-gpio : config pin (referred to as nCONFIG in the cyclone manual)
+- status-gpio : status pin (referred to as nSTATUS in the cyclone manual)
+
+Example:
+ fpga_spi: evi-fpga-spi@0 {
+ compatible = "altr,cyclone-ps-spi-fpga-mgr";
+ spi-max-frequency = <20000000>;
+ reg = <0>;
+ config-gpio = <&gpio4 9 GPIO_ACTIVE_HIGH>;
+ status-gpio = <&gpio4 11 GPIO_ACTIVE_HIGH>;
+ };
--
2.7.4
^ permalink raw reply related
* [PATCH v2 1/5] lib: add bitrev8x4()
From: Joshua Clayton @ 2016-10-28 16:56 UTC (permalink / raw)
To: Alan Tull, Moritz Fischer
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam,
Russell King, Joshua Clayton, devicetree, linux-kernel,
linux-arm-kernel
In-Reply-To: <cover.1477669744.git.stillcompiling@gmail.com>
Add a function to reverse bytes within a 32 bit word.
This function is more efficient than using the 8 bit version when
iterating over an array
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
arch/arm/include/asm/bitrev.h | 5 +++++
include/linux/bitrev.h | 26 ++++++++++++++++++++++++++
2 files changed, 31 insertions(+)
diff --git a/arch/arm/include/asm/bitrev.h b/arch/arm/include/asm/bitrev.h
index ec291c3..6d2e9ca 100644
--- a/arch/arm/include/asm/bitrev.h
+++ b/arch/arm/include/asm/bitrev.h
@@ -17,4 +17,9 @@ static __always_inline __attribute_const__ u8 __arch_bitrev8(u8 x)
return __arch_bitrev32((u32)x) >> 24;
}
+static __always_inline __attribute_const__ u32 __arch_bitrev8x4(u32 x)
+{
+ __asm__ ("rbit %0, %1; rev %0, %0" : "=r" (x) : "r" (x));
+}
+
#endif
diff --git a/include/linux/bitrev.h b/include/linux/bitrev.h
index fb790b8..b1cfa1a 100644
--- a/include/linux/bitrev.h
+++ b/include/linux/bitrev.h
@@ -9,6 +9,7 @@
#define __bitrev32 __arch_bitrev32
#define __bitrev16 __arch_bitrev16
#define __bitrev8 __arch_bitrev8
+#define __bitrev8x4 __arch_bitrev8x4
#else
extern u8 const byte_rev_table[256];
@@ -27,6 +28,14 @@ static inline u32 __bitrev32(u32 x)
return (__bitrev16(x & 0xffff) << 16) | __bitrev16(x >> 16);
}
+static inline u32 __bitrev8x4(u32 x)
+{
+ return(__bitrev8(x & 0xff) |
+ (__bitrev8((x >> 8) & 0xff) << 8) |
+ (__bitrev8((x >> 16) & 0xff) << 16) |
+ (__bitrev8((x >> 24) & 0xff) << 24));
+}
+
#endif /* CONFIG_HAVE_ARCH_BITREVERSE */
#define __constant_bitrev32(x) \
@@ -50,6 +59,15 @@ static inline u32 __bitrev32(u32 x)
__x; \
})
+#define __constant_bitrev8x4(x) \
+({ \
+ u32 __x = x; \
+ __x = ((__x & (u32)0xF0F0F0F0UL) >> 4) | ((__x & (u32)0x0F0F0F0FUL) << 4); \
+ __x = ((__x & (u32)0xCCCCCCCCUL) >> 2) | ((__x & (u32)0x33333333UL) << 2); \
+ __x = ((__x & (u32)0xAAAAAAAAUL) >> 1) | ((__x & (u32)0x55555555UL) << 1); \
+ __x; \
+})
+
#define __constant_bitrev8(x) \
({ \
u8 __x = x; \
@@ -75,6 +93,14 @@ static inline u32 __bitrev32(u32 x)
__bitrev16(__x); \
})
+#define bitrev8x4(x) \
+({ \
+ u32 __x = x; \
+ __builtin_constant_p(__x) ? \
+ __constant_bitrev8x4(__x) : \
+ __bitrev8x4(__x); \
+})
+
#define bitrev8(x) \
({ \
u8 __x = x; \
--
2.7.4
^ permalink raw reply related
* [PATCH v2 0/4] Altera Cyclone Passive Serial SPI FPGA Manager
From: Joshua Clayton @ 2016-10-28 16:56 UTC (permalink / raw)
To: Alan Tull, Moritz Fischer
Cc: Rob Herring, Mark Rutland, Shawn Guo, Sascha Hauer, Fabio Estevam,
Russell King, Joshua Clayton, devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
This series adds an FPGA manager for Altera cyclone FPGAs
that can program them using an spi port and a couple of gpios, using
Alteras passive serial protocol.
Changes from v1:
- Changed the name from cyclone-spi-fpga-mgr to cyclone-ps-spi-fpga-mgr
This name change was requested by Alan Tull, to be specific about which
programming method is being employed on the fpga.
- Changed the name of the reset-gpio to config-gpio to closer match the
way the pins are described in the Altera manual
- Moved MODULE_LICENCE, _AUTHOR, and _DESCRIPTION to the bottom
- Added a bitrev8x4() function to the bitrev headers and implemented ARM
const, runtime, and ARM specific faster versions (This may end up
needing to be a standalone patch)
- Moved the bitswapping into cyclonespi_write(), as requested.
This falls short of my desired generic lsb first spi support, but is a step
in that direction.
- Fixed whitespace problems introduced during refactoring
- Replaced magic number for initial delay with a descriptive macro
- Poll the fpga to see when it is ready rather than a fixed 1 ms sleep
Joshua Clayton (5):
lib: add bitrev8x4()
doc: dt: add cyclone-spi binding document
fpga manager: Add cyclone-ps-spi driver for Altera FPGAs
ARM: dts: imx6q-evi: support cyclonespi
fpga manager: cyclone-ps-spi: make delay variable
.../bindings/fpga/cyclone-ps-spi-fpga-mgr.txt | 23 +++
arch/arm/boot/dts/imx6q-evi.dts | 16 ++
arch/arm/include/asm/bitrev.h | 5 +
drivers/fpga/Kconfig | 7 +
drivers/fpga/Makefile | 1 +
drivers/fpga/cyclone-ps-spi.c | 175 +++++++++++++++++++++
include/linux/bitrev.h | 26 +++
7 files changed, 253 insertions(+)
create mode 100644 Documentation/devicetree/bindings/fpga/cyclone-ps-spi-fpga-mgr.txt
create mode 100644 drivers/fpga/cyclone-ps-spi.c
--
2.7.4
--
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: [RESEND/PATCH v6 3/3] clk: qcom: Add A53 clock driver
From: Georgi Djakov @ 2016-10-28 16:55 UTC (permalink / raw)
To: Stephen Boyd
Cc: mturquette, linux-clk, linux-kernel, linux-arm-msm, devicetree,
Rob Herring
In-Reply-To: <20161028015438.GG16026@codeaurora.org>
On 10/28/2016 04:54 AM, Stephen Boyd wrote:
> On 10/19, Georgi Djakov wrote:
>> Add a driver for the A53 Clock Controller. It is a hardware block that
>> implements a combined mux and half integer divider functionality. It can
>> choose between a fixed-rate clock or the dedicated A53 PLL. The source
>> and the divider can be set both at the same time.
>>
>> This is required for enabling CPU frequency scaling on platforms like
>> MSM8916.
>>
>
> Please Cc DT reviewers.
>
Ok, will do.
>> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
>> ---
>> .../devicetree/bindings/clock/qcom,a53cc.txt | 22 +++
>> drivers/clk/qcom/Kconfig | 8 ++
>> drivers/clk/qcom/Makefile | 1 +
>> drivers/clk/qcom/a53cc.c | 155 +++++++++++++++++++++
>> 4 files changed, 186 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53cc.txt
>> create mode 100644 drivers/clk/qcom/a53cc.c
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,a53cc.txt b/Documentation/devicetree/bindings/clock/qcom,a53cc.txt
>> new file mode 100644
>> index 000000000000..a025f062f177
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,a53cc.txt
>> @@ -0,0 +1,22 @@
>> +Qualcomm A53 CPU Clock Controller Binding
>> +------------------------------------------------
>> +The A53 CPU Clock Controller is hardware, which provides a combined
>> +mux and divider functionality for the CPU clocks. It can choose between
>> +a fixed rate clock and the dedicated A53 PLL.
>> +
>> +Required properties :
>> +- compatible : shall contain:
>> +
>> + "qcom,a53cc"
>> +
>> +- reg : shall contain base register location and length
>> + of the APCS region
>> +- #clock-cells : shall contain 1
>> +
>> +Example:
>> +
>> + apcs: syscon@b011000 {
>> + compatible = "qcom,a53cc", "syscon";
>
> Why is it a syscon? Is that part used?
It is not used in this patchset. I will remove the syscon part from the
example and will change the compatible to qcom,a53cc-msm8916, as this
also seems to be SoC specific.
>
>> + reg = <0x0b011000 0x1000>;
>> + #clock-cells = <1>;
>> + };
>> diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
>> index a889f0b14b54..59dfcdc340e4 100644
>> --- a/drivers/clk/qcom/Kconfig
>> +++ b/drivers/clk/qcom/Kconfig
>> @@ -159,3 +159,11 @@ config QCOM_A53PLL
>> support for CPU frequencies above 1GHz.
>> Say Y if you want to support CPU frequency scaling on devices
>> such as MSM8916.
>> +
>> +config QCOM_A53CC
>> + bool "A53 Clock Controller"
>
> Can't these configs be tristate? Same applies to A53PLL.
>
I am using __clk_lookup() in this patch below, which is not exported.
The A53PLL could be a tristate, but i just made them both builtins for
consistency.
>> + depends on COMMON_CLK_QCOM && QCOM_A53PLL
>> + help
>> + Support for the A53 clock controller on some Qualcomm devices.
>> + Say Y if you want to support CPU frequency scaling on devices
>> + such as MSM8916.
>> diff --git a/drivers/clk/qcom/a53cc.c b/drivers/clk/qcom/a53cc.c
>> new file mode 100644
>> index 000000000000..4d20db9da407
>> --- /dev/null
>> +++ b/drivers/clk/qcom/a53cc.c
>> @@ -0,0 +1,155 @@
>> +/*
>> + * Copyright (c) 2016, Linaro Limited
>> + * Copyright (c) 2014, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/clk.h>
>> +#include <linux/clk-provider.h>
>> +#include <linux/cpu.h>
>
> Is this include used?
It isn't. Removed!
>
>> +#include <linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>> +
>> +#include "clk-regmap.h"
>> +#include "clk-regmap-mux-div.h"
>> +
>> +enum {
>> + P_GPLL0,
>> + P_A53PLL,
>> +};
>> +
>> +static const struct parent_map gpll0_a53cc_map[] = {
>> + { P_GPLL0, 4 },
>> + { P_A53PLL, 5 },
>> +};
>> +
>> +static const char * const gpll0_a53cc[] = {
>> + "gpll0_vote",
>> + "a53pll",
>> +};
>> +
>> +static const struct regmap_config a53cc_regmap_config = {
>> + .reg_bits = 32,
>> + .reg_stride = 4,
>> + .val_bits = 32,
>> + .max_register = 0x1000,
>> + .fast_io = true,
>> + .val_format_endian = REGMAP_ENDIAN_LITTLE,
>> +};
>> +
>> +static const struct of_device_id qcom_a53cc_match_table[] = {
>> + { .compatible = "qcom,a53cc" },
>> + { }
>> +};
>
> Can you move this down next to the driver please?
>
Ok, sure.
>> +
>> +/*
>> + * We use the notifier function for switching to a temporary safe configuration
>> + * (mux and divider), while the a53 pll is reconfigured.
>> + */
>> +static int a53cc_notifier_cb(struct notifier_block *nb, unsigned long event,
>> + void *data)
>> +{
>> + int ret = 0;
>> + struct clk_regmap_mux_div *md = container_of(nb,
>> + struct clk_regmap_mux_div,
>> + clk_nb);
>> +
>> + if (event == PRE_RATE_CHANGE)
>> + ret = __mux_div_set_src_div(md, md->safe_src, md->safe_div);
>> +
>> + return notifier_from_errno(ret);
>> +}
>> +
>> +static int qcom_a53cc_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct clk_regmap_mux_div *a53cc;
>> + struct resource *res;
>> + void __iomem *base;
>> + struct clk *pclk;
>> + struct regmap *regmap;
>> + struct clk_init_data init;
>
> = { } for safety?
>
Yes, thanks!
>> + int ret;
>> +
>> + a53cc = devm_kzalloc(dev, sizeof(*a53cc), GFP_KERNEL);
>> + if (!a53cc)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + a53cc->reg_offset = 0x50,
>> + a53cc->hid_width = 5,
>> + a53cc->hid_shift = 0,
>> + a53cc->src_width = 3,
>> + a53cc->src_shift = 8,
>> + a53cc->safe_src = 4,
>> + a53cc->safe_div = 3,
>
> Replace commas with semicolons please.
>
> Also do we need the safe things to be part of the struct? The
> notifier is here so we could just as easily hard code these
> things in the notifier.
>
Ok, will hardcode it with some comment.
Thanks again for reviewing!
BR,
Georgi
>> + a53cc->parent_map = gpll0_a53cc_map,
>> +
>> + init.name = "a53mux",
>> + init.parent_names = gpll0_a53cc,
>> + init.num_parents = 2,
>> + init.ops = &clk_regmap_mux_div_ops,
>> + init.flags = CLK_SET_RATE_PARENT;
>> + a53cc->clkr.hw.init = &init;
>> +
>> + pclk = __clk_lookup(gpll0_a53cc[1]);
>> + if (!pclk)
>> + return -EPROBE_DEFER;
>> +
>> + a53cc->clk_nb.notifier_call = a53cc_notifier_cb;
>> + ret = clk_notifier_register(pclk, &a53cc->clk_nb);
>> + if (ret) {
>> + dev_err(dev, "failed to register clock notifier: %d\n", ret);
>> + return ret;
>> + }
>
^ permalink raw reply
* Re: [PATCH 2/4] pinctrl: single: Use generic parser and #pinctrl-cells for pinctrl-single,pins
From: Tony Lindgren @ 2016-10-28 16:55 UTC (permalink / raw)
To: Linus Walleij
Cc: Jon Hunter, Mark Rutland, Rob Herring, Grygorii Strashko,
Nishanth Menon, linux-gpio, devicetree, linux-kernel, linux-omap
In-Reply-To: <20161026141628.x42rbn5o2ekdoluk@atomide.com>
* Tony Lindgren <tony@atomide.com> [161026 07:17]:
> * Tony Lindgren <tony@atomide.com> [161025 09:51]:
> > We can now use generic parser. To support the legacy binding without
> > #pinctrl-cells, add pcs_quirk_missing_pinctrl_cells() and warn about
> > missing #pinctrl-cells.
> ...
>
> > +/**
> > + * pcs_quirk_missing_pinctrl_cells - handle legacy binding
> > + * @pcs: pinctrl driver instance
> > + * @np: device tree node
> > + * @cells: number of cells
> > + *
> > + * Handle legacy binding with no #pinctrl-cells. This should be
> > + * always two pinctrl-single,bit-per-mux and one for others.
> > + * At some point we may want to consider removing this.
> > + */
> > +static int pcs_quirk_missing_pinctrl_cells(struct pcs_device *pcs,
> > + struct device_node *np,
> > + int cells)
> > +{
> > + struct property *p;
> > + const char *name = "#pinctrl-cells";
> > + int error;
> > + u32 val;
> > +
> > + error = of_property_read_u32(np, name, &val);
> > + if (!error)
> > + return 0;
> > +
> > + dev_warn(pcs->dev, "please update dts to use %s = <%i>\n",
> > + name, cells);
> > +
> > + p = devm_kzalloc(pcs->dev, sizeof(*p), GFP_KERNEL);
> > + if (!p)
> > + return -ENOMEM;
> > +
> > + p->length = sizeof(__be32);
> > + p->value = devm_kzalloc(pcs->dev, sizeof(__be32), GFP_KERNEL);
> > + if (!p->value)
> > + return -ENOMEM;
> > + *(__be32 *)p->value = cpu_to_be32(cells);
> > +
> > + p->name = devm_kstrdup(pcs->dev, name, GFP_KERNEL);
> > + if (!p->name)
> > + return -ENOMEM;
> > +
> > + pcs->missing_nr_pinctrl_cells = p;
> > +
> > + return of_add_property(np, pcs->missing_nr_pinctrl_cells);
> > +}
>
> Looking at some make randconfig results, looks like we don't have
> of_add_property() and of_remove_property() exported. Is there some
> reason not to export them?
I'll only do of_add_property() and of_remove_property() if we have
IS_BUILTIN(CONFIG_PINCTRL_SINGLE). I bet I'm the only one using it
as a loadable module right now :) Then we can remove those if we
decide to export of_add_property() and of_remove_property().
Regards,
Tony
^ permalink raw reply
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
From: Tony Lindgren @ 2016-10-28 16:53 UTC (permalink / raw)
To: Linus Walleij
Cc: Rob Herring, linux-kernel@vger.kernel.org, Jon Hunter,
Mark Rutland, Grygorii Strashko, Nishanth Menon,
linux-gpio@vger.kernel.org, devicetree@vger.kernel.org,
Linux-OMAP
In-Reply-To: <20161027141120.jqvul6q7iz5fjsmb@atomide.com>
* Tony Lindgren <tony@atomide.com> [161027 07:59]:
> * Linus Walleij <linus.walleij@linaro.org> [161027 00:57]:
> > On Tue, Oct 25, 2016 at 6:45 PM, Tony Lindgren <tony@atomide.com> wrote:
> > > +/*
> > > + * For pinctrl binding, typically #pinctrl-cells is for the pin controller
> > > + * device, so either parent or grandparent. See pinctrl-bindings.txt.
> > > + */
> > > +static int pinctrl_find_cells_size(const struct device_node *np,
> > > + const char *cells_name)
> > > +{
> > > + int cells_size, error;
> > > +
> > > + error = of_property_read_u32(np->parent, cells_name, &cells_size);
> > > + if (error) {
> > > + error = of_property_read_u32(np->parent->parent,
> > > + cells_name, &cells_size);
> > > + if (error)
> > > + return -ENOENT;
> > > + }
> > > +
> > > + return cells_size;
> > > +}
> >
> > Can't we just hardcode this to "#pinctrl-cells" and skip the cells_name
> > parameter? We can parametrize it the day we need it instead.
>
> Sure we can do that.
>
> > The rest of the helpers look nice and clean.
>
> OK cool thanks,
Below is an updated version of this patch with documentation updated
and cells_name removed. I'll repost the whole series once the DT
binding has been reviewed.
Regards,
Tony
8< -------------------------
>From tony Mon Sep 17 00:00:00 2001
From: Tony Lindgren <tony@atomide.com>
Date: Tue, 25 Oct 2016 08:33:34 -0700
Subject: [PATCH] pinctrl: Introduce generic #pinctrl-cells and
pinctrl_parse_index_with_args
Introduce #pinctrl-cells helper binding and generic helper functions
pinctrl_count_index_with_args() and pinctrl_parse_index_with_args().
Signed-off-by: Tony Lindgren <tony@atomide.com>
---
.../bindings/pinctrl/pinctrl-bindings.txt | 44 ++++++-
drivers/pinctrl/devicetree.c | 144 +++++++++++++++++++++
drivers/pinctrl/devicetree.h | 21 +++
3 files changed, 208 insertions(+), 1 deletion(-)
diff --git a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
--- a/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
+++ b/Documentation/devicetree/bindings/pinctrl/pinctrl-bindings.txt
@@ -97,6 +97,11 @@ For example:
};
== Pin controller devices ==
+Required properties: See the pin controller driver specific documentation
+
+Optional properties:
+#pinctrl-cells: Number of pin control cells in addition to the index within the
+ pin controller device instance
Pin controller devices should contain the pin configuration nodes that client
devices reference.
@@ -119,7 +124,8 @@ For example:
The contents of each of those pin configuration child nodes is defined
entirely by the binding for the individual pin controller device. There
-exists no common standard for this content.
+exists no common standard for this content. The pinctrl framework only
+provides generic helper bindings that the pin controller driver can use.
The pin configuration nodes need not be direct children of the pin controller
device; they may be grandchildren, for example. Whether this is legal, and
@@ -156,6 +162,42 @@ state_2_node_a {
pins = "mfio29", "mfio30";
};
+Optionally an altenative binding can be used if more suitable depending on the
+pin controller hardware. For hardaware where there is a large number of identical
+pin controller instances, naming each pin and function can easily become
+unmaintainable. This is especially the case if the same controller is used for
+different pins and functions depending on the SoC revision and packaging.
+
+For cases like this, the pin controller driver may use pinctrl-pin-array helper
+binding with a hardware based index and a number of pin configuration values:
+
+pincontroller {
+ ... /* Standard DT properties for the device itself elided */
+ #pinctrl-cells = <2>;
+
+ state_0_node_a {
+ pinctrl-pin-array = <
+ 0 A_DELAY_PS(0) G_DELAY_PS(120)
+ 4 A_DELAY_PS(0) G_DELAY_PS(360)
+ ...
+ >;
+ };
+ ...
+};
+
+Above #pinctrl-cells specifies the number of value cells in addition to the
+index of the registers. This is similar to the interrupts-extended binding with
+one exception. There is no need to specify the phandle for each entry as that
+is already known as the defined pins are always children of the pin controller
+node. Further having the phandle pointing to another pin controller would not
+currently work as the pinctrl framework uses named modes to group pins for each
+pin control device.
+
+The index for pinctrl-pin-array must relate to the hardware for the pinctrl
+registers, and must not be a virtual index of pin instances. The reason for
+this is to avoid mapping of the index in the dts files and the pin controller
+driver as it can change.
+
== Generic pin configuration node content ==
Many data items that are represented in a pin configuration node are common
diff --git a/drivers/pinctrl/devicetree.c b/drivers/pinctrl/devicetree.c
--- a/drivers/pinctrl/devicetree.c
+++ b/drivers/pinctrl/devicetree.c
@@ -253,3 +253,147 @@ int pinctrl_dt_to_map(struct pinctrl *p)
pinctrl_dt_free_maps(p);
return ret;
}
+
+/*
+ * For pinctrl binding, typically #pinctrl-cells is for the pin controller
+ * device, so either parent or grandparent. See pinctrl-bindings.txt.
+ */
+static int pinctrl_find_cells_size(const struct device_node *np)
+{
+ const char *cells_name = "#pinctrl-cells";
+ int cells_size, error;
+
+ error = of_property_read_u32(np->parent, cells_name, &cells_size);
+ if (error) {
+ error = of_property_read_u32(np->parent->parent,
+ cells_name, &cells_size);
+ if (error)
+ return -ENOENT;
+ }
+
+ return cells_size;
+}
+
+/**
+ * pinctrl_get_list_and_count - Gets the list and it's cell size and number
+ * @np: pointer to device node with the property
+ * @list_name: property that contains the list
+ * @list: pointer for the list found
+ * @cells_size: pointer for the cell size found
+ * @nr_elements: pointer for the number of elements found
+ *
+ * Typically np is a single pinctrl entry containing the list.
+ */
+static int pinctrl_get_list_and_count(const struct device_node *np,
+ const char *list_name,
+ const __be32 **list,
+ int *cells_size,
+ int *nr_elements)
+{
+ int size;
+
+ *cells_size = 0;
+ *nr_elements = 0;
+
+ *list = of_get_property(np, list_name, &size);
+ if (!*list)
+ return -ENOENT;
+
+ *cells_size = pinctrl_find_cells_size(np);
+ if (*cells_size < 0)
+ return -ENOENT;
+
+ /* First element is always the index within the pinctrl device */
+ *nr_elements = (size / sizeof(**list)) / (*cells_size + 1);
+
+ return 0;
+}
+
+/**
+ * pinctrl_count_index_with_args - Count number of elements in a pinctrl entry
+ * @np: pointer to device node with the property
+ * @list_name: property that contains the list
+ *
+ * Counts the number of elements in a pinctrl array consisting of an index
+ * within the controller and a number of u32 entries specified for each
+ * entry. Note that device_node is always for the parent pin controller device.
+ */
+int pinctrl_count_index_with_args(const struct device_node *np,
+ const char *list_name)
+{
+ const __be32 *list;
+ int size, nr_cells, error;
+
+ error = pinctrl_get_list_and_count(np, list_name, &list,
+ &nr_cells, &size);
+ if (error)
+ return error;
+
+ return size;
+}
+EXPORT_SYMBOL_GPL(pinctrl_count_index_with_args);
+
+/**
+ * pinctrl_copy_args - Populates of_phandle_args based on index
+ * @np: pointer to device node with the property
+ * @list: pointer to a list with the elements
+ * @index: entry within the list of elements
+ * @nr_cells: number of cells in the list
+ * @nr_elem: number of elements for each entry in the list
+ * @out_args: returned values
+ *
+ * Populates the of_phandle_args based on the index in the list.
+ */
+static int pinctrl_copy_args(const struct device_node *np,
+ const __be32 *list,
+ int index, int nr_cells, int nr_elem,
+ struct of_phandle_args *out_args)
+{
+ int i;
+
+ memset(out_args, 0, sizeof(*out_args));
+ out_args->np = (struct device_node *)np;
+ out_args->args_count = nr_cells + 1;
+
+ if (index >= nr_elem)
+ return -EINVAL;
+
+ list += index * (nr_cells + 1);
+
+ for (i = 0; i < nr_cells + 1; i++)
+ out_args->args[i] = be32_to_cpup(list++);
+
+ return 0;
+}
+
+/**
+ * pinctrl_parse_index_with_args - Find a node pointed by index in a list
+ * @np: pointer to device node with the property
+ * @list_name: property that contains the list
+ * @index: index within the list
+ * @out_arts: entries in the list pointed by index
+ *
+ * Finds the selected element in a pinctrl array consisting of an index
+ * within the controller and a number of u32 entries specified for each
+ * entry. Note that device_node is always for the parent pin controller device.
+ */
+int pinctrl_parse_index_with_args(const struct device_node *np,
+ const char *list_name, int index,
+ struct of_phandle_args *out_args)
+{
+ const __be32 *list;
+ int nr_elem, nr_cells, error;
+
+ error = pinctrl_get_list_and_count(np, list_name, &list,
+ &nr_cells, &nr_elem);
+ if (error || !nr_cells)
+ return error;
+
+ error = pinctrl_copy_args(np, list, index, nr_cells, nr_elem,
+ out_args);
+ if (error)
+ return error;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(pinctrl_parse_index_with_args);
diff --git a/drivers/pinctrl/devicetree.h b/drivers/pinctrl/devicetree.h
--- a/drivers/pinctrl/devicetree.h
+++ b/drivers/pinctrl/devicetree.h
@@ -21,6 +21,13 @@
void pinctrl_dt_free_maps(struct pinctrl *p);
int pinctrl_dt_to_map(struct pinctrl *p);
+int pinctrl_count_index_with_args(const struct device_node *np,
+ const char *list_name);
+
+int pinctrl_parse_index_with_args(const struct device_node *np,
+ const char *list_name, int index,
+ struct of_phandle_args *out_args);
+
#else
static inline int pinctrl_dt_to_map(struct pinctrl *p)
@@ -32,4 +39,18 @@ static inline void pinctrl_dt_free_maps(struct pinctrl *p)
{
}
+static inline int pinctrl_count_index_with_args(const struct device_node *np,
+ const char *list_name)
+{
+ return -ENODEV;
+}
+
+static inline int
+pinctrl_parse_index_with_args(const struct device_node *np,
+ const char *list_name, int index,
+ struct of_phandle_args *out_args)
+{
+ return -ENODEV;
+}
+
#endif
--
2.9.3
^ permalink raw reply
* Re: [RESEND/PATCH v6 1/3] clk: qcom: Add A53 PLL support
From: Georgi Djakov @ 2016-10-28 16:47 UTC (permalink / raw)
To: Stephen Boyd
Cc: mturquette, linux-clk, linux-kernel, linux-arm-msm, devicetree,
Rob Herring
In-Reply-To: <20161028014914.GF16026@codeaurora.org>
Hi Stephen, thanks for reviewing!
On 10/28/2016 04:49 AM, Stephen Boyd wrote:
> On 10/19, Georgi Djakov wrote:
>> Add support for the PLL, which generates the higher range of CPU
>> frequencies on MSM8916 platforms.
>>
>> Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
>
> Please Cc dt reviewers.
Ok, sure!
>
>> ---
>> .../devicetree/bindings/clock/qcom,a53-pll.txt | 17 ++++
>> drivers/clk/qcom/Kconfig | 9 +++
>> drivers/clk/qcom/Makefile | 1 +
>> drivers/clk/qcom/a53-pll.c | 94 ++++++++++++++++++++++
>> 4 files changed, 121 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/clock/qcom,a53-pll.txt
>> create mode 100644 drivers/clk/qcom/a53-pll.c
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,a53-pll.txt b/Documentation/devicetree/bindings/clock/qcom,a53-pll.txt
>> new file mode 100644
>> index 000000000000..50e71e4e13a6
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/clock/qcom,a53-pll.txt
>> @@ -0,0 +1,17 @@
>> +A53 PLL Binding
>> +---------------
>> +The A53 PLL is the main CPU PLL used for frequencies above 1GHz.
>
> Perhaps add which SoC(s) its in?
Ok, done!
>
>> +
>> +Required properties :
>> +- compatible : Shall contain only one of the following:
>> +
>> + "qcom,a53-pll"
>
> And this could be something more SoC specific too?
I will make it qcom,a53pll-msm8916
>
>> +
>> +- reg : shall contain base register location and length
>> +
>> +Example:
>> +
>> + a53pll: a53pll@0b016000 {
>
> Drop the leading 0
Ok, done!
>
>> + compatible = "qcom,a53-pll";
>> + reg = <0x0b016000 0x40>;
>
> Should probably have #clock-cells = <0> just in case.
>
Ok, done!
>> + };
>> diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
>> index 1fb1f5476cb0..7d27f47f0c92 100644
>> --- a/drivers/clk/qcom/Makefile
>> +++ b/drivers/clk/qcom/Makefile
>> @@ -29,3 +29,4 @@ obj-$(CONFIG_MSM_LCC_8960) += lcc-msm8960.o
>> obj-$(CONFIG_MSM_MMCC_8960) += mmcc-msm8960.o
>> obj-$(CONFIG_MSM_MMCC_8974) += mmcc-msm8974.o
>> obj-$(CONFIG_MSM_MMCC_8996) += mmcc-msm8996.o
>> +obj-$(CONFIG_QCOM_A53PLL) += a53-pll.o
>> diff --git a/drivers/clk/qcom/a53-pll.c b/drivers/clk/qcom/a53-pll.c
>> new file mode 100644
>> index 000000000000..43902080f34b
>> --- /dev/null
>> +++ b/drivers/clk/qcom/a53-pll.c
>> @@ -0,0 +1,94 @@
>> +/*
>> + * Copyright (c) 2016, Linaro Limited
>> + * Copyright (c) 2014, The Linux Foundation. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/kernel.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/regmap.h>
>
> #include <linux/clk-provider.h>?
>
Ok.
>> +
>> +#include "clk-pll.h"
>> +#include "clk-regmap.h"
>> +
>> +static struct pll_freq_tbl a53pll_freq[] = {
>
> Can this be const?
>
Yes, done!
>> + { 998400000, 52, 0x0, 0x1, 0 },
>> + { 1094400000, 57, 0x0, 0x1, 0 },
>> + { 1152000000, 62, 0x0, 0x1, 0 },
>> + { 1209600000, 65, 0x0, 0x1, 0 },
>> + { 1401600000, 73, 0x0, 0x1, 0 },
>> +};
>> +
>> +static const struct regmap_config a53pll_regmap_config = {
>> + .reg_bits = 32,
>> + .reg_stride = 4,
>> + .val_bits = 32,
>> + .max_register = 0x40,
>> + .fast_io = true,
>> + .val_format_endian = REGMAP_ENDIAN_LITTLE,
>> +};
>> +
>> +static const struct of_device_id qcom_a53pll_match_table[] = {
>> + { .compatible = "qcom,a53-pll" },
>> + { }
>> +};
>> +
>> +static int qcom_a53pll_probe(struct platform_device *pdev)
>> +{
>> + struct device *dev = &pdev->dev;
>> + struct device_node *node = dev->of_node;
>> + struct clk_pll *pll;
>> + struct resource *res;
>> + void __iomem *base;
>> + struct regmap *regmap;
>> + struct clk_init_data init;
>
> Initialize to { } for safety?
>
Ok!
>> +
>> + pll = devm_kzalloc(dev, sizeof(*pll), GFP_KERNEL);
>> + if (!pll)
>> + return -ENOMEM;
>> +
>> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>> + base = devm_ioremap_resource(dev, res);
>> + if (IS_ERR(base))
>> + return PTR_ERR(base);
>> +
>> + regmap = devm_regmap_init_mmio(dev, base, &a53pll_regmap_config);
>> + if (IS_ERR(regmap))
>> + return PTR_ERR(regmap);
>> +
>> + pll->l_reg = 0x04,
>> + pll->m_reg = 0x08,
>> + pll->n_reg = 0x0c,
>> + pll->config_reg = 0x14,
>> + pll->mode_reg = 0x00,
>> + pll->status_reg = 0x1c,
>> + pll->status_bit = 16,
>> + pll->freq_tbl = a53pll_freq,
>
> Replace commas with semicolon please.
>
Ah, right! Done!
>> +
>> + init.name = node->name;
>
> Please just hardcode the string name for now. Best to not get
> tied down to DT node names if we don't need to.
>
Ok, will hardcode it as a53pll.
>> + init.parent_names = (const char *[]){ "xo" },
>> + init.num_parents = 1,
>> + init.ops = &clk_pll_sr2_ops,
>> + init.flags = CLK_IS_CRITICAL;
>> + pll->clkr.hw.init = &init;
>> +
>> + return devm_clk_register_regmap(dev, &pll->clkr);
>> +}
>> +
>> +static struct platform_driver qcom_a53pll_driver = {
>> + .probe = qcom_a53pll_probe,
>> + .driver = {
>> + .name = "qcom-a53pll",
>> + .of_match_table = qcom_a53pll_match_table,
>> + },
>> +};
>> +
>> +builtin_platform_driver(qcom_a53pll_driver);
>
^ permalink raw reply
* Re: specifying order of /dev/mmcblk devices via device-tree?
From: Tim Harvey @ 2016-10-28 16:45 UTC (permalink / raw)
To: Mark Rutland, Javier Martinez Canillas, Fabio Estevam
Cc: devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org
In-Reply-To: <20161028153755.GL5806@leverpostej>
On Fri, Oct 28, 2016 at 8:37 AM, Mark Rutland <mark.rutland-5wv7dgnIgG8@public.gmane.org> wrote:
> On Fri, Oct 28, 2016 at 08:23:04AM -0700, Tim Harvey wrote:
>> Greetings,
>>
>> I have an IMX6 board that has the following:
>> sdhc1: mmc0: sdio radio
>> sdhc2: mmc1: /dev/mmcblk1: microSD connector
>> sdhc3: mmc2: /dev/mmcblk2: on-board eMMC
>>
>> I would like to have sdhc3 registered as /dev/mmcblk0 and sdhc2
>> registered as /dev/mmcblk1 so that permanent storage is the first
>> mmcblk device as I think this is more intuitive however currently
>> these get instanced in the order they appear in the imx6qdl.dtsi
>> device-tree configuration and are not able to be mapped the way I want
>> them in my dts file.
>>
>> Is there a way, or if not is there a desire for a way, to specify the
>> order of /dev/mmcblk devices via device-tree?
>
> As with many other devices, there is no standard way of controlling the
> Linux enumeration (and given the ID space is shared with other dynamic
> devices it's not something that could generally work).
>
> These should be refererd to by UUID if possible.
>
> If not, we could cosider adding a by-dt-path or something like that.
>
> Thanks,
> Mark.
Mark / Javier/ Fabio,
Thanks - this is very useful.
Yes, I agree that using UUID's is the way to go and now I see how that
can be easily accessed via uboot 'part' command.
Regards,
Tim
--
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 v2 3/4] arm64: arch_timer: Work around Erratum Hisilicon-161601
From: Will Deacon @ 2016-10-28 16:00 UTC (permalink / raw)
To: Ding Tianhong
Cc: catalin.marinas-5wv7dgnIgG8, marc.zyngier-5wv7dgnIgG8,
mark.rutland-5wv7dgnIgG8, oss-fOR+EgIDQEHk1uMJSBkQmQ,
devicetree-u79uwXL29TY76Z2rM5mHXA,
shawnguo-DgEjT+Ai2ygdnm+yROfE0A, stuart.yoder-3arQi8VN3Tc,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linuxarm-hv44wF8Li93QT0dZR+AlfA
In-Reply-To: <1477553651-13428-3-git-send-email-dingtianhong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
Hi Ding,
On Thu, Oct 27, 2016 at 03:34:10PM +0800, Ding Tianhong wrote:
> Erratum Hisilicon-161601 says that the ARM generic timer counter "has the
> potential to contain an erroneous value when the timer value changes".
> Accesses to TVAL (both read and write) are also affected due to the implicit counter
> read. Accesses to CVAL are not affected.
>
> The workaround is to reread the system count registers until the value of the second
> read is larger than the first one by less than 32, the system counter can be guaranteed
> not to return wrong value twice by back-to-back read and the error value is always larger
> than the correct one by 32. Writes to TVAL are replaced with an equivalent write to CVAL.
>
> The workaround is enabled if the hisilicon,erratum-161601 property is found in
> the timer node in the device tree. This can be overridden with the
> clocksource.arm_arch_timer.hisilicon-161601 boot parameter, which allows KVM
> users to enable the workaround until a mechanism is implemented to
> automatically communicate this information.
>
> Fix some description for fsl erratum a008585.
>
> v2: Significant rework based on feedback, including seperate the fsl erratum a008585
> to another patch, update the erratum name and remove unwanted code.
>
> Signed-off-by: Ding Tianhong <dingtianhong-hv44wF8Li93QT0dZR+AlfA@public.gmane.org>
> ---
[...]
> diff --git a/drivers/clocksource/Kconfig b/drivers/clocksource/Kconfig
> index 8a753fd..4aafb6a 100644
> --- a/drivers/clocksource/Kconfig
> +++ b/drivers/clocksource/Kconfig
> @@ -312,8 +312,20 @@ config FSL_ERRATUM_A008585
> help
> This option enables a workaround for Freescale/NXP Erratum
> A-008585 ("ARM generic timer may contain an erroneous
> - value"). The workaround will only be active if the
> + value"). The workaround will be active if the
> fsl,erratum-a008585 property is found in the timer node.
> + This can be overridden with the clocksource.arm_arch_timer.fsl-a008585
> + boot parameter.
> +
> +config HISILICON_ERRATUM_161601
> + bool "Workaround for Hisilicon Erratum 161601"
> + default y
> + depends on ARM_ARCH_TIMER && ARM64
> + help
> + This option enables a workaround for Hisilicon Erratum
> + 161601. The workaround will be active if the hisilicon,erratum-161601
> + property is found in the timer node. This can be overridden with
> + the clocksource.arm_arch_timer.hisilicon-161601 boot parameter.
I'm really not keen on having a kernel commandline parameter for this.
It's not something we've done for other, similar errata (e.g. CNTFRQ
reporting the wrong value) and I think it's a slippery slope to having
more of these workarounds controlled at boot-time. If you have a board
that is affected by this, it's always going to need the workaround. Why
would you turn it off?
Will
--
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
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