* Re: [PATCH] soc: qcom: Add SoC info driver
From: Arnd Bergmann @ 2016-10-27 13:41 UTC (permalink / raw)
To: Imran Khan
Cc: andy.gross, David Brown, Rob Herring, Mark Rutland,
open list:ARM/QUALCOMM SUPPORT, open list:ARM/QUALCOMM SUPPORT,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
open list
In-Reply-To: <fd34e299-a3c2-e981-a92e-b61638a98e5f@codeaurora.org>
On Thursday, October 27, 2016 6:40:27 PM CEST Imran Khan wrote:
> On 10/26/2016 8:16 PM, Arnd Bergmann wrote:
> > On Wednesday, October 26, 2016 7:42:08 PM CEST Imran Khan wrote:
> >> On 10/26/2016 7:35 PM, Arnd Bergmann wrote:
> >>>>> As we are talking about generic soc_device_attribute fields, I was hoping that
> >>>>> having a vendor field would be helpful as along with family it would provide
> >>>>> a more thorough information. Also as more than one foundries may be used for
> >>>>> a soc, can we have a field say foundry_id to provide this information.
> >>> My first feeling is that this 'vendor' information can should be
> >>> derived from the family. It's also not clear what would happen
> >>> to this when a company gets bought. E.g. the Oxnas product family
> >>> was subsequently owned by Oxford, PLX, Avago and Broadcom, and the
> >>> mxs family was Sigmatel, Freescale, now NXP and might soon be
> >>> Qualcomm. What would you put in there in this case?
> >>
> >> Okay, not having vendor field is fine for me. Could you also suggest
> >> something about the foundry_id field.
> >
> > This one seems more well-defined, so it's probably ok to add. What
> > would be the use case of reading this? Would you want to read it
> > just from user space or also from the kernel?
> >
>
> As of now the use case I can think of, only involve reading this from user
> space. For example for the same soc, coming from different foundries with
> different manufacturing process, we may have a situation where some inconsistent
> h/w behavior is being observed only on parts received from a certain foundry
> and in those cases this information may help in segregation of problematic socs
> and may also be used in testing these socs under a different set of settings like
> voltage, frequency etc.
>
> > Maybe this can be combined with a manufacturing process, which probably
> > falls into a similar category, so we could have something like
> > "TSMC 28ULP" as a string in there.
> >
>
> Yes. Having a manufacturing process as part of foundry-id can provide a more
> thorough information.
Ok, sounds good. Let's do it like this. We can always add support for
in-kernel matching of this string if needed later.
Arnd
^ permalink raw reply
* Re: [RFC PATCH 0/5] Add an overlay manager to handle board capes
From: Rob Herring @ 2016-10-27 13:41 UTC (permalink / raw)
To: Antoine Tenart
Cc: Mark Rutland, Thomas Petazzoni, Dmitry Shmidt,
devicetree@vger.kernel.org, Pantelis Antoniou, Stephen Boyd,
linux-kernel@vger.kernel.org, Hans de Goede, Maxime Ripard,
Frank Rowand, linux-arm-kernel@lists.infradead.org
In-Reply-To: <20161026145756.21689-1-antoine.tenart@free-electrons.com>
Please Cc the maintainers of drivers/of/.
+ Frank R, Hans, Dmitry S
On Wed, Oct 26, 2016 at 9:57 AM, Antoine Tenart
<antoine.tenart@free-electrons.com> wrote:
> Hi all,
>
> Many boards now come with dips and compatible capes; among others the
> C.H.I.P, or Beaglebones. All these boards have a kernel implementing an
> out-of-tree "cape manager" which is used to detected capes, retrieve
> their description and apply a corresponding overlay. This series is an
> attempt to start a discussion, with an implementation of such a manager
> which is somehow generic (i.e. formats or cape detectors can be added).
> Other use cases could make use of this manager to dynamically load dt
> overlays based on some input / hw presence.
I'd like to see an input source be the kernel command line and/or a DT
chosen property. Another overlay manager was proposed not to long
ago[1] as well. There's also the Allwinner tablet use case from Hans
where i2c devices are probed and detected. That's not using overlays
currently, but maybe could.
Another thing to consider is different sources of overlays. Besides in
the filesystem, overlays could be built into the kernel (already
supported), embedded in the dtb (as the other overlay mgr did) or we
could extend FDT format to append them.
> The proposed design is a library which can be used by detector drivers
> to parse headers and load the corresponding overlay. Helpers are
> provided for this purpose. The whole thing is divided into 3 entities:
>
> - The parser which is project-specific (to allow supporting headers
> already into the wild). It registers a function parsing an header's
> data and filling one or more strings which will be used to find
> matching dtbo on the fs.
>
> - The overlay manager helpers allowing to parse a header to retrieve
> the previously mentioned strings and to load a compatible overlay.
>
> - The detectors which are used to detect capes and get their description
> (to be parsed).
What about things like power has to be turned on first to detect
boards and read their ID? I think this needs to be tied into the
driver model. Though, don't go sticking cape mgr nodes into DT. Maybe
a driver gets bound to a connector node, but we've got to sort out
connector bindings first.
> An example of parser and detector is given, compatible with what's done
> for the C.H.I.P. As the w1 framework is really bad (and we should
> probably do something about that) the detector code is far from being
> perfect; but that's not related to what we try to achieve here.
>
> The actual implementation has a limitation: the detectors cannot be
> built-in the kernel image as they would likely detect capes at boot time
> but will fail to get their corresponding dt overlays as the fs isn't
> mounted yet. The only case this can work is when dt overlays are
> built-in firmwares. This isn't an issue for the C.H.I.P. use case right
> now. There was a discussion about making an helper to wait for the
> rootfs to be mount but the answer was "this is the driver's problem".
I thought there are firmware loading calls that will wait. I think
this all needs to work asynchronously both for firmware loading and
because w1 is really slow.
> I'd like to get comments, specifically from people using custom cape
> managers, to see if this could fill their needs (with I guess some
> modifications).
Having 2 would certainly give a better sense this is generic.
Rob
[1] https://patchwork.ozlabs.org/patch/667805/
^ permalink raw reply
* Re: [PATCH v12 0/5] Goodix touchscreen enhancements
From: Bastien Nocera @ 2016-10-27 13:44 UTC (permalink / raw)
To: Irina Tirdea, linux-input
Cc: Dmitry Torokhov, Aleksei Mamlin, Karsten Merker, Mark Rutland,
Rob Herring, Octavian Purdila, linux-kernel, devicetree
In-Reply-To: <1473530257-7495-1-git-send-email-irina.tirdea@intel.com>
On Sat, 2016-09-10 at 20:57 +0300, Irina Tirdea wrote:
> This is an update for a previous set of patches [1] with enhancements
> for the Goodix touchscreens. The first 3 patches are leftovers from
> the previous patchset (with minor changes due to rebase), while the
> last 2 patches are new.
>
> Thanks,
> Irina
>
> [1] https://lkml.org/lkml/2015/11/19/241
>
> Changes in v12:
> - add documentation for configuration firmware update
> - minor changes due to rebase
> - add 2 new patches to fix reset sequence and add support for gt9157
>
> Previous changes:
> https://lkml.org/lkml/2015/11/19/241
>
> Irina Tirdea (5):
> Input: goodix - add sysfs interface to dump config
> Input: goodix - add support for ESD
> Input: goodix - add runtime power management support
I tested those 3 patches in their v11, but my test tablet's software is
currently broken. Until I find the time to regenerate install images,
I'll just do code reviews.
I assume the first 3 patches don't break anything on the device I
tested this on, but I'd like to make sure it was at least tested on
ACPI-based systems.
Can you mention somewhere on which devices those patches were tested?
Cheers
> Input: goodix - fix reset sequence
> Input: goodix - add support for gt9157
>
> .../bindings/input/touchscreen/goodix.txt | 5 +
> Documentation/input/goodix.txt | 84 +++++
> drivers/input/touchscreen/goodix.c | 346
> +++++++++++++++++++--
> 3 files changed, 415 insertions(+), 20 deletions(-)
> create mode 100644 Documentation/input/goodix.txt
>
^ permalink raw reply
* Re: [PATCH v12 1/5] Input: goodix - add sysfs interface to dump config
From: Bastien Nocera @ 2016-10-27 13:44 UTC (permalink / raw)
To: Irina Tirdea, linux-input
Cc: Dmitry Torokhov, Aleksei Mamlin, Karsten Merker, Mark Rutland,
Rob Herring, Octavian Purdila, linux-kernel, devicetree
In-Reply-To: <1473530257-7495-2-git-send-email-irina.tirdea@intel.com>
On Sat, 2016-09-10 at 20:57 +0300, Irina Tirdea wrote:
> Goodix devices have a configuration information register area that
> specifies various parameters for the device. The configuration
> information
> has a specific format described in the Goodix datasheet. It includes
> X/Y
> resolution, maximum supported touch points, interrupt flags, various
> sesitivity factors and settings for advanced features (like gesture
> recognition).
>
> Export a sysfs interface that would allow reading the configuration
> information. The default device configuration can be used as a
> starting
> point for creating a valid configuration firmware used by the device
> at
> init time to update its configuration.
>
> This sysfs interface will be exported only if the gpio pins are
> properly
> initialized from ACPI/DT.
Reviewed-by: Bastien Nocera <hadess@hadess.net>
Only thing I don't like is the overly complicated bash script. I'd
really rather the code was in C, making it easier to debug, and not
rely on a fair number of external utilities.
>
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> ---
> Documentation/input/goodix.txt | 84
> ++++++++++++++++++++++++++++++++++++++
> drivers/input/touchscreen/goodix.c | 64 ++++++++++++++++++++++++++
> ---
> 2 files changed, 143 insertions(+), 5 deletions(-)
> create mode 100644 Documentation/input/goodix.txt
>
> diff --git a/Documentation/input/goodix.txt
> b/Documentation/input/goodix.txt
> new file mode 100644
> index 0000000..f9be1e2
> --- /dev/null
> +++ b/Documentation/input/goodix.txt
> @@ -0,0 +1,84 @@
> +Goodix touchscreen driver
> +=====================================
> +
> +How to update configuration firmware
> +=====================================
> +
> +Goodix touchscreen devices have a set of registers that specify
> configuration
> +information for the device. The configuration information has a
> specific format
> +described in the Goodix datasheet. It includes X/Y resolution,
> maximum
> +supported touch points, interrupt flags, various sesitivity factors
> and
> +settings for advanced features (like gesture recognition).
> +
> +The devices have an initial default configuration that can be read
> through
> +the sysfs interface (/sys/class/input/inputX/device/dump_config).
> This default
> +configuration can be used as a starting point for creating a new
> configuration
> +firmware file. At init, the driver will read the configuration
> firmware file
> +and update the device configuration.
> +
> +This configuration can be accesed only if both interrupt and reset
> gpio pins
> +are connected and properly configured through ACPI _DSD/DT
> properties.
> +
> +Below are instructions on how to generate a valid configuration
> starting from
> +the device default configuration.
> +
> +1. Dump the default configuration of the device to a file:
> + $ cat /sys/class/input/inputX/device/dump_config >
> goodix_<model>_cfg
> +
> +2. Make the needed changes to the configuration (e.g. change
> resolution of
> +x/y axes, maximum reported touch points, switch X,Y axes, etc.). For
> more
> +details check the Goodix datasheet for format of Configuration
> Registers.
> +
> +3. Generate a valid configuration starting from goodix_<model>_cfg.
> +After making changes, you need to recompute the checksum of the
> entire
> +configuration data, set Config_Fresh to 1 and generate the binary
> config
> +firmware image. This can be done using a helper script similar to
> the
> +one below:
> +
> +#!/bin/bash
> +
> +if [[ $# -lt 1 ]]; then
> + echo "$0 fw_filename"
> + exit 1
> +fi
> +
> +file_in="$1"
> +file_out_bin=${file_in}.bin
> +
> +print_val ()
> +{
> + val="$1"
> + printf "0x%.2x" "$val" | xxd -r -p >> ${file_out_bin}
> +}
> +
> +rm -f ${file_out_bin}
> +
> +size=`cat ${file_in} | wc -w`
> +
> +checksum=0
> +i=1
> +for val in `cat ${file_in}`; do
> + val="0x$val"
> + if [[ $i == $size ]]; then
> + # Config_Fresh
> + print_val 0x01
> + elif [[ $i == $((size-1)) ]]; then
> + # Config_Chksum
> + checksum=$(( (~ checksum + 1) & 0xFF))
> + print_val $checksum
> + else
> + checksum=$((checksum + val))
> + print_val $val
> + fi
> + i=$((i+1))
> +done
> +
> +echo "Wrote ${file_out_bin}"
> +
> +4. Copy the binary config firmware in the appropriate location
> +(e.g. /lib/firmware), using the name goodix_<model>_cfg.bin (e.g.
> for gt911,
> +use goodix_911_cfg.bin).
> +
> +5. Check that the new firmware was successfully written to the
> device
> +after reboot. Config_Fresh is reset to 0 after a successful update
> of the
> +configuration.
> diff --git a/drivers/input/touchscreen/goodix.c
> b/drivers/input/touchscreen/goodix.c
> index 240b16f..2447b73 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -430,6 +430,40 @@ static int goodix_reset(struct goodix_ts_data
> *ts)
> return 0;
> }
>
> +static ssize_t goodix_dump_config_show(struct device *dev,
> + struct device_attribute
> *attr, char *buf)
> +{
> + struct goodix_ts_data *ts = dev_get_drvdata(dev);
> + u8 config[GOODIX_CONFIG_MAX_LENGTH];
> + int error, count = 0, i;
> +
> + wait_for_completion(&ts->firmware_loading_complete);
> +
> + error = goodix_i2c_read(ts->client, GOODIX_REG_CONFIG_DATA,
> + config, ts->cfg_len);
> + if (error) {
> + dev_warn(&ts->client->dev,
> + "Error reading config (%d)\n", error);
> + return error;
> + }
> +
> + for (i = 0; i < ts->cfg_len; i++)
> + count += scnprintf(buf + count, PAGE_SIZE - count,
> "%02x ",
> + config[i]);
> + return count;
> +}
> +
> +static DEVICE_ATTR(dump_config, S_IRUGO, goodix_dump_config_show,
> NULL);
> +
> +static struct attribute *goodix_attrs[] = {
> + &dev_attr_dump_config.attr,
> + NULL
> +};
> +
> +static const struct attribute_group goodix_attr_group = {
> + .attrs = goodix_attrs,
> +};
> +
> /**
> * goodix_get_gpio_config - Get GPIO config from ACPI/DT
> *
> @@ -735,11 +769,22 @@ static int goodix_ts_probe(struct i2c_client
> *client,
> ts->cfg_len = goodix_get_cfg_len(ts->id);
>
> if (ts->gpiod_int && ts->gpiod_rst) {
> + error = sysfs_create_group(&client->dev.kobj,
> + &goodix_attr_group);
> + if (error) {
> + dev_err(&client->dev,
> + "Failed to create sysfs group:
> %d\n",
> + error);
> + return error;
> + }
> +
> /* update device config */
> ts->cfg_name = devm_kasprintf(&client->dev,
> GFP_KERNEL,
> "goodix_%d_cfg.bin",
> ts->id);
> - if (!ts->cfg_name)
> - return -ENOMEM;
> + if (!ts->cfg_name) {
> + error = -ENOMEM;
> + goto err_sysfs_remove_group;
> + }
>
> error = request_firmware_nowait(THIS_MODULE, true,
> ts->cfg_name,
> &client->dev,
> GFP_KERNEL, ts,
> @@ -748,7 +793,7 @@ static int goodix_ts_probe(struct i2c_client
> *client,
> dev_err(&client->dev,
> "Failed to invoke firmware loader:
> %d\n",
> error);
> - return error;
> + goto err_sysfs_remove_group;
> }
>
> return 0;
> @@ -759,14 +804,23 @@ static int goodix_ts_probe(struct i2c_client
> *client,
> }
>
> return 0;
> +
> +err_sysfs_remove_group:
> + if (ts->gpiod_int && ts->gpiod_rst)
> + sysfs_remove_group(&client->dev.kobj,
> &goodix_attr_group);
> + return error;
> }
>
> static int goodix_ts_remove(struct i2c_client *client)
> {
> struct goodix_ts_data *ts = i2c_get_clientdata(client);
>
> - if (ts->gpiod_int && ts->gpiod_rst)
> - wait_for_completion(&ts->firmware_loading_complete);
> + if (!ts->gpiod_int || !ts->gpiod_rst)
> + return 0;
> +
> + wait_for_completion(&ts->firmware_loading_complete);
> +
> + sysfs_remove_group(&client->dev.kobj, &goodix_attr_group);
>
> return 0;
> }
^ permalink raw reply
* Re: [PATCH v12 2/5] Input: goodix - add support for ESD
From: Bastien Nocera @ 2016-10-27 13:44 UTC (permalink / raw)
To: Irina Tirdea, linux-input
Cc: Dmitry Torokhov, Aleksei Mamlin, Karsten Merker, Mark Rutland,
Rob Herring, Octavian Purdila, linux-kernel, devicetree
In-Reply-To: <1473530257-7495-3-git-send-email-irina.tirdea@intel.com>
On Sat, 2016-09-10 at 20:57 +0300, Irina Tirdea wrote:
> Add ESD (Electrostatic Discharge) protection mechanism.
>
> The driver enables ESD protection in HW and checks a register
> to determine if ESD occurred. If ESD is signalled by the HW,
> the driver will reset the device.
>
> The ESD poll time (in ms) can be set through the sysfs property
> esd_timeout. If it is set to 0, ESD protection is disabled.
> Recommended value is 2000 ms.
What's the default value though?
> The initial value for ESD timeout
> can be set through esd-recovery-timeout-ms ACPI/DT property.
> If there is no such property defined, ESD protection is disabled.
> For ACPI 5.1, the property can be specified using _DSD properties:
> Device (STAC)
> {
> Name (_HID, "GDIX1001")
> ...
>
> Name (_DSD, Package ()
> {
> ToUUID("daffd814-6eba-4d8c-8a91-bc9bbf4aa301"),
> Package ()
> {
> Package (2) { "esd-recovery-timeout-ms", Package(1) {
> 2000 }},
> ...
> }
> })
> }
Are there any devices which ship with that information? What do the
Windows drivers use as a default for ESD, and how is it declared in
ACPI?
> The ESD protection mechanism is only available if the gpio pins
> are properly initialized from ACPI/DT.
>
> This is based on Goodix datasheets for GT911 and GT9271 and on Goodix
> driver gt9xx.c for Android (publicly available in Android kernel
> trees for various devices).
Could you link to a "close to upstream" tree that includes that
support? Do they use the same timeout, or is it set to a default
hardcoded value instead?
Cheers
> Signed-off-by: Irina Tirdea <irina.tirdea@intel.com>
> Acked-by: Rob Herring <robh@kernel.org>
> ---
> .../bindings/input/touchscreen/goodix.txt | 4 +
> drivers/input/touchscreen/goodix.c | 130
> ++++++++++++++++++++-
> 2 files changed, 132 insertions(+), 2 deletions(-)
>
> diff --git
> a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> index c98757a..ef5f42d 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> @@ -18,6 +18,10 @@ Optional properties:
> - irq-gpios : GPIO pin used for IRQ. The driver uses
> the
> interrupt gpio pin as output to reset the
> device.
> - reset-gpios : GPIO pin used for reset
> + - esd-recovery-timeout-ms : ESD poll time (in milli seconds) for
> the driver to
> + check if ESD occurred and in that case
> reset the
> + device. ESD is disabled if this property
> is not set
> + or is set to 0.
>
> - touchscreen-inverted-x : X axis is inverted (boolean)
> - touchscreen-inverted-y : Y axis is inverted (boolean)
> diff --git a/drivers/input/touchscreen/goodix.c
> b/drivers/input/touchscreen/goodix.c
> index 2447b73..cf39dc4 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -49,10 +49,13 @@ struct goodix_ts_data {
> const char *cfg_name;
> struct completion firmware_loading_complete;
> unsigned long irq_flags;
> + atomic_t esd_timeout;
> + struct delayed_work esd_work;
> };
>
> #define GOODIX_GPIO_INT_NAME "irq"
> #define GOODIX_GPIO_RST_NAME "reset"
> +#define GOODIX_DEVICE_ESD_TIMEOUT_PROPERTY "esd-recovery-
> timeout-ms"
>
> #define GOODIX_MAX_HEIGHT 4096
> #define GOODIX_MAX_WIDTH 4096
> @@ -67,6 +70,8 @@ struct goodix_ts_data {
> /* Register defines */
> #define GOODIX_REG_COMMAND 0x8040
> #define GOODIX_CMD_SCREEN_OFF 0x05
> +#define GOODIX_CMD_ESD_ENABLED 0xAA
> +#define GOODIX_REG_ESD_CHECK 0x8041
>
> #define GOODIX_READ_COOR_ADDR 0x814E
> #define GOODIX_REG_CONFIG_DATA 0x8047
> @@ -453,10 +458,114 @@ static ssize_t goodix_dump_config_show(struct
> device *dev,
> return count;
> }
>
> +static void goodix_disable_esd(struct goodix_ts_data *ts)
> +{
> + if (!atomic_read(&ts->esd_timeout))
> + return;
> + cancel_delayed_work_sync(&ts->esd_work);
> +}
> +
> +static int goodix_enable_esd(struct goodix_ts_data *ts)
> +{
> + int error, esd_timeout;
> +
> + esd_timeout = atomic_read(&ts->esd_timeout);
> + if (!esd_timeout)
> + return 0;
> +
> + error = goodix_i2c_write_u8(ts->client,
> GOODIX_REG_ESD_CHECK,
> + GOODIX_CMD_ESD_ENABLED);
> + if (error) {
> + dev_err(&ts->client->dev, "Failed to enable ESD:
> %d\n", error);
> + return error;
> + }
> +
> + schedule_delayed_work(&ts->esd_work, round_jiffies_relative(
> + msecs_to_jiffies(esd_timeout)));
> + return 0;
> +}
> +
> +static void goodix_esd_work(struct work_struct *work)
> +{
> + struct goodix_ts_data *ts = container_of(work, struct
> goodix_ts_data,
> + esd_work.work);
> + int retries = 3, error;
> + u8 esd_data[2];
> + const struct firmware *cfg = NULL;
> +
> + wait_for_completion(&ts->firmware_loading_complete);
> +
> + while (--retries) {
> + error = goodix_i2c_read(ts->client,
> GOODIX_REG_COMMAND,
> + esd_data, sizeof(esd_data));
> + if (error)
> + continue;
> + if (esd_data[0] != GOODIX_CMD_ESD_ENABLED &&
> + esd_data[1] == GOODIX_CMD_ESD_ENABLED) {
> + /* feed the watchdog */
> + goodix_i2c_write_u8(ts->client,
> + GOODIX_REG_COMMAND,
> + GOODIX_CMD_ESD_ENABLED);
> + break;
> + }
> + }
> +
> + if (!retries) {
> + dev_dbg(&ts->client->dev, "Performing ESD
> recovery.\n");
> + goodix_free_irq(ts);
> + goodix_reset(ts);
> + error = request_firmware(&cfg, ts->cfg_name, &ts-
> >client->dev);
> + if (!error) {
> + goodix_send_cfg(ts, cfg);
> + release_firmware(cfg);
> + }
> + goodix_request_irq(ts);
> + goodix_enable_esd(ts);
> + return;
> + }
> +
> + schedule_delayed_work(&ts->esd_work, round_jiffies_relative(
> + msecs_to_jiffies(atomic_read(&ts-
> >esd_timeout))));
> +}
> +
> +static ssize_t goodix_esd_timeout_show(struct device *dev,
> + struct device_attribute
> *attr, char *buf)
> +{
> + struct goodix_ts_data *ts = dev_get_drvdata(dev);
> +
> + return scnprintf(buf, PAGE_SIZE, "%d\n", atomic_read(&ts-
> >esd_timeout));
> +}
> +
> +static ssize_t goodix_esd_timeout_store(struct device *dev,
> + struct device_attribute
> *attr,
> + const char *buf, size_t
> count)
> +{
> + struct goodix_ts_data *ts = dev_get_drvdata(dev);
> + int error, esd_timeout, new_esd_timeout;
> +
> + error = kstrtouint(buf, 10, &new_esd_timeout);
> + if (error)
> + return error;
> +
> + esd_timeout = atomic_read(&ts->esd_timeout);
> + if (esd_timeout && !new_esd_timeout)
> + goodix_disable_esd(ts);
> +
> + atomic_set(&ts->esd_timeout, new_esd_timeout);
> + if (!esd_timeout && new_esd_timeout)
> + goodix_enable_esd(ts);
> +
> + return count;
> +}
> +
> static DEVICE_ATTR(dump_config, S_IRUGO, goodix_dump_config_show,
> NULL);
> +/* ESD timeout in ms. Default disabled (0). Recommended 2000 ms. */
> +static DEVICE_ATTR(esd_timeout, S_IRUGO | S_IWUSR,
> goodix_esd_timeout_show,
> + goodix_esd_timeout_store);
>
> static struct attribute *goodix_attrs[] = {
> &dev_attr_dump_config.attr,
> + &dev_attr_esd_timeout.attr,
> NULL
> };
>
> @@ -713,7 +822,11 @@ static void goodix_config_cb(const struct
> firmware *cfg, void *ctx)
> goto err_release_cfg;
> }
>
> - goodix_configure_dev(ts);
> + error = goodix_configure_dev(ts);
> + if (error)
> + goto err_release_cfg;
> +
> + goodix_enable_esd(ts);
>
> err_release_cfg:
> release_firmware(cfg);
> @@ -724,7 +837,7 @@ static int goodix_ts_probe(struct i2c_client
> *client,
> const struct i2c_device_id *id)
> {
> struct goodix_ts_data *ts;
> - int error;
> + int error, esd_timeout;
>
> dev_dbg(&client->dev, "I2C Address: 0x%02x\n", client-
> >addr);
>
> @@ -740,6 +853,7 @@ static int goodix_ts_probe(struct i2c_client
> *client,
> ts->client = client;
> i2c_set_clientdata(client, ts);
> init_completion(&ts->firmware_loading_complete);
> + INIT_DELAYED_WORK(&ts->esd_work, goodix_esd_work);
>
> error = goodix_get_gpio_config(ts);
> if (error)
> @@ -769,6 +883,12 @@ static int goodix_ts_probe(struct i2c_client
> *client,
> ts->cfg_len = goodix_get_cfg_len(ts->id);
>
> if (ts->gpiod_int && ts->gpiod_rst) {
> + error = device_property_read_u32(&ts->client->dev,
> + GOODIX_DEVICE_ESD_TIMEOUT_PR
> OPERTY,
> + &esd_timeout);
> + if (!error)
> + atomic_set(&ts->esd_timeout, esd_timeout);
> +
> error = sysfs_create_group(&client->dev.kobj,
> &goodix_attr_group);
> if (error) {
> @@ -821,6 +941,7 @@ static int goodix_ts_remove(struct i2c_client
> *client)
> wait_for_completion(&ts->firmware_loading_complete);
>
> sysfs_remove_group(&client->dev.kobj, &goodix_attr_group);
> + goodix_disable_esd(ts);
>
> return 0;
> }
> @@ -836,6 +957,7 @@ static int __maybe_unused goodix_suspend(struct
> device *dev)
> return 0;
>
> wait_for_completion(&ts->firmware_loading_complete);
> + goodix_disable_esd(ts);
>
> /* Free IRQ as IRQ pin is used as output in the suspend
> sequence */
> goodix_free_irq(ts);
> @@ -894,6 +1016,10 @@ static int __maybe_unused goodix_resume(struct
> device *dev)
> if (error)
> return error;
>
> + error = goodix_enable_esd(ts);
> + if (error)
> + return error;
> +
> return 0;
> }
>
^ permalink raw reply
* Re: [PATCH v12 3/5] Input: goodix - add runtime power management support
From: Bastien Nocera @ 2016-10-27 13:44 UTC (permalink / raw)
To: Irina Tirdea, linux-input-u79uwXL29TY76Z2rM5mHXA
Cc: Dmitry Torokhov, Aleksei Mamlin, Karsten Merker, Mark Rutland,
Rob Herring, Octavian Purdila,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1473530257-7495-4-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On Sat, 2016-09-10 at 20:57 +0300, Irina Tirdea wrote:
> Add support for runtime power management so that the device is
> turned off when not used (when the userspace holds no open
> handles of the input device). The device uses autosuspend with a
> default delay of 2 seconds, so the device will suspend if no
> handles to it are open for 2 seconds.
>
> The runtime management support is only available if the gpio pins
> are properly initialized from ACPI/DT.
>
> Signed-off-by: Irina Tirdea <irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>
<snip>
> +static int goodix_set_power_state(struct goodix_ts_data *ts, bool
> on)
I don't like having booleans for this sort of thing[1], as it's never
clear whether "on" is "power state on" or "device is on".
I'd rather you defined a 2 member enum, with meaningful names, so that
the effect is clear when we're reading the caller.
[1]: https://blog.ometer.com/2011/01/20/boolean-parameters-are-wrong/
> +{
> + int error;
> +
> + if (on) {
> + error = pm_runtime_get_sync(&ts->client->dev);
> + } else {
> + pm_runtime_mark_last_busy(&ts->client->dev);
> + error = pm_runtime_put_autosuspend(&ts->client-
> >dev);
> + }
> +
> + if (error < 0) {
> + dev_err(&ts->client->dev,
> + "failed to change power state to %d\n", on);
> + if (on)
> + pm_runtime_put_noidle(&ts->client->dev);
> +
> + return error;
> + }
> +
> + return 0;
> +}
> +
> static int goodix_ts_read_input_report(struct goodix_ts_data *ts, u8
> *data)
> {
> int touch_num;
> @@ -444,6 +474,10 @@ static ssize_t goodix_dump_config_show(struct
> device *dev,
>
> wait_for_completion(&ts->firmware_loading_complete);
>
> + error = goodix_set_power_state(ts, true);
> + if (error)
> + return error;
> +
> error = goodix_i2c_read(ts->client, GOODIX_REG_CONFIG_DATA,
> config, ts->cfg_len);
> if (error) {
> @@ -452,6 +486,8 @@ static ssize_t goodix_dump_config_show(struct
> device *dev,
> return error;
> }
>
> + goodix_set_power_state(ts, false);
> +
> for (i = 0; i < ts->cfg_len; i++)
> count += scnprintf(buf + count, PAGE_SIZE - count,
> "%02x ",
> config[i]);
> @@ -548,11 +584,13 @@ static ssize_t goodix_esd_timeout_store(struct
> device *dev,
> return error;
>
> esd_timeout = atomic_read(&ts->esd_timeout);
> - if (esd_timeout && !new_esd_timeout)
> + if (esd_timeout && !new_esd_timeout &&
> + pm_runtime_active(&ts->client->dev))
> goodix_disable_esd(ts);
>
> atomic_set(&ts->esd_timeout, new_esd_timeout);
> - if (!esd_timeout && new_esd_timeout)
> + if (!esd_timeout && new_esd_timeout &&
> + pm_runtime_active(&ts->client->dev))
> goodix_enable_esd(ts);
>
> return count;
> @@ -573,6 +611,34 @@ static const struct attribute_group
> goodix_attr_group = {
> .attrs = goodix_attrs,
> };
>
> +static int goodix_open(struct input_dev *input_dev)
> +{
> + struct goodix_ts_data *ts = input_get_drvdata(input_dev);
> + int error;
> +
> + if (!ts->gpiod_int || !ts->gpiod_rst)
> + return 0;
> +
> + wait_for_completion(&ts->firmware_loading_complete);
> +
> + error = goodix_set_power_state(ts, true);
> + if (error)
> + return error;
> + atomic_inc(&ts->open_count);
> + return 0;
> +}
> +
> +static void goodix_close(struct input_dev *input_dev)
> +{
> + struct goodix_ts_data *ts = input_get_drvdata(input_dev);
> +
> + if (!ts->gpiod_int || !ts->gpiod_rst)
> + return;
> +
> + goodix_set_power_state(ts, false);
> + atomic_dec(&ts->open_count);
> +}
> +
> /**
> * goodix_get_gpio_config - Get GPIO config from ACPI/DT
> *
> @@ -754,6 +820,9 @@ static int goodix_request_input_dev(struct
> goodix_ts_data *ts)
> ts->input_dev->id.vendor = 0x0416;
> ts->input_dev->id.product = ts->id;
> ts->input_dev->id.version = ts->version;
> + ts->input_dev->open = goodix_open;
> + ts->input_dev->close = goodix_close;
> + input_set_drvdata(ts->input_dev, ts);
>
> error = input_register_device(ts->input_dev);
> if (error) {
> @@ -808,7 +877,8 @@ static int goodix_configure_dev(struct
> goodix_ts_data *ts)
> * @ts: our goodix_ts_data pointer
> *
> * request_firmware_wait callback that finishes
> - * initialization of the device.
> + * initialization of the device. This will only be called
> + * when ts->gpiod_int and ts->gpiod_rst are properly initialized.
> */
> static void goodix_config_cb(const struct firmware *cfg, void *ctx)
> {
> @@ -828,6 +898,19 @@ static void goodix_config_cb(const struct
> firmware *cfg, void *ctx)
>
> goodix_enable_esd(ts);
>
> + pm_runtime_set_autosuspend_delay(&ts->client->dev,
> + GOODIX_AUTOSUSPEND_DELAY_MS
> );
> + pm_runtime_use_autosuspend(&ts->client->dev);
> + error = pm_runtime_set_active(&ts->client->dev);
> + if (error) {
> + dev_err(&ts->client->dev, "failed to set active:
> %d\n", error);
> + goto err_release_cfg;
> + }
> + pm_runtime_enable(&ts->client->dev);
> + /* Must not suspend immediately after device initialization
> */
> + pm_runtime_mark_last_busy(&ts->client->dev);
> + pm_request_autosuspend(&ts->client->dev);
> +
> err_release_cfg:
> release_firmware(cfg);
> complete_all(&ts->firmware_loading_complete);
> @@ -854,6 +937,7 @@ static int goodix_ts_probe(struct i2c_client
> *client,
> i2c_set_clientdata(client, ts);
> init_completion(&ts->firmware_loading_complete);
> INIT_DELAYED_WORK(&ts->esd_work, goodix_esd_work);
> + mutex_init(&ts->mutex);
>
> error = goodix_get_gpio_config(ts);
> if (error)
> @@ -940,23 +1024,33 @@ static int goodix_ts_remove(struct i2c_client
> *client)
>
> wait_for_completion(&ts->firmware_loading_complete);
>
> + pm_runtime_disable(&client->dev);
> + pm_runtime_set_suspended(&client->dev);
> + pm_runtime_put_noidle(&client->dev);
> +
> sysfs_remove_group(&client->dev.kobj, &goodix_attr_group);
> goodix_disable_esd(ts);
>
> return 0;
> }
>
> -static int __maybe_unused goodix_suspend(struct device *dev)
> +static int __maybe_unused goodix_sleep(struct device *dev)
> {
> struct i2c_client *client = to_i2c_client(dev);
> struct goodix_ts_data *ts = i2c_get_clientdata(client);
> - int error;
> + int error = 0;
>
> /* We need gpio pins to suspend/resume */
> if (!ts->gpiod_int || !ts->gpiod_rst)
> return 0;
>
> wait_for_completion(&ts->firmware_loading_complete);
> +
> + mutex_lock(&ts->mutex);
> +
> + if (ts->suspended)
> + goto out_error;
> +
> goodix_disable_esd(ts);
>
> /* Free IRQ as IRQ pin is used as output in the suspend
> sequence */
> @@ -966,7 +1060,7 @@ static int __maybe_unused goodix_suspend(struct
> device *dev)
> error = gpiod_direction_output(ts->gpiod_int, 0);
> if (error) {
> goodix_request_irq(ts);
> - return error;
> + goto out_error;
> }
>
> usleep_range(5000, 6000);
> @@ -977,7 +1071,8 @@ static int __maybe_unused goodix_suspend(struct
> device *dev)
> dev_err(&ts->client->dev, "Screen off command
> failed\n");
> gpiod_direction_input(ts->gpiod_int);
> goodix_request_irq(ts);
> - return -EAGAIN;
> + error = -EAGAIN;
> + goto out_error;
> }
>
> /*
> @@ -986,44 +1081,77 @@ static int __maybe_unused
> goodix_suspend(struct device *dev)
> * sooner, delay 58ms here.
> */
> msleep(58);
> + ts->suspended = true;
> + mutex_unlock(&ts->mutex);
> +
> return 0;
> +
> +out_error:
> + mutex_unlock(&ts->mutex);
> + return error;
> }
>
> -static int __maybe_unused goodix_resume(struct device *dev)
> +static int __maybe_unused goodix_wakeup(struct device *dev)
> {
> struct i2c_client *client = to_i2c_client(dev);
> struct goodix_ts_data *ts = i2c_get_clientdata(client);
> - int error;
> + int error = 0;
>
> if (!ts->gpiod_int || !ts->gpiod_rst)
> return 0;
>
> + mutex_lock(&ts->mutex);
> +
> + if (!ts->suspended)
> + goto out_error;
> +
> /*
> * Exit sleep mode by outputting HIGH level to INT pin
> * for 2ms~5ms.
> */
> error = gpiod_direction_output(ts->gpiod_int, 1);
> if (error)
> - return error;
> + goto out_error;
>
> usleep_range(2000, 5000);
>
> error = goodix_int_sync(ts);
> if (error)
> - return error;
> + goto out_error;
>
> error = goodix_request_irq(ts);
> if (error)
> - return error;
> + goto out_error;
>
> error = goodix_enable_esd(ts);
> if (error)
> - return error;
> + goto out_error;
> +
> + ts->suspended = false;
> + mutex_unlock(&ts->mutex);
>
> return 0;
> +
> +out_error:
> + mutex_unlock(&ts->mutex);
> + return error;
> +}
> +
> +static int __maybe_unused goodix_resume(struct device *dev)
> +{
> + struct i2c_client *client = to_i2c_client(dev);
> + struct goodix_ts_data *ts = i2c_get_clientdata(client);
> +
> + if (!atomic_read(&ts->open_count))
> + return 0;
> +
> + return goodix_wakeup(dev);
> }
>
> -static SIMPLE_DEV_PM_OPS(goodix_pm_ops, goodix_suspend,
> goodix_resume);
> +static const struct dev_pm_ops goodix_pm_ops = {
> + SET_SYSTEM_SLEEP_PM_OPS(goodix_sleep, goodix_resume)
> + SET_RUNTIME_PM_OPS(goodix_sleep, goodix_wakeup, NULL)
> +};
>
> static const struct i2c_device_id goodix_ts_id[] = {
> { "GDIX1001:00", 0 },
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v12 4/5] Input: goodix - fix reset sequence
From: Bastien Nocera @ 2016-10-27 13:44 UTC (permalink / raw)
To: Irina Tirdea, linux-input-u79uwXL29TY76Z2rM5mHXA
Cc: Dmitry Torokhov, Aleksei Mamlin, Karsten Merker, Mark Rutland,
Rob Herring, Octavian Purdila,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1473530257-7495-5-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On Sat, 2016-09-10 at 20:57 +0300, Irina Tirdea wrote:
> According to the Goodix datasheet, the reset sequence will leave
> the reset line set to output high. To end the selection of the I2C
> address, we just need to set the input line to low for at least
> 50 ms and then set it to input floating (already implemented by
> goodix_int_sync).
>
> Remove setting the reset line to input from the reset sequence,
> since that is not reflected in the datasheet.
>
> This is based on Goodix datasheets for GT911 and GT9271 and on Goodix
> driver gt9xx.c for Android (publicly available in Android kernel
> trees for various devices).
>
> Suggested-by: Troy Kisky <troy.kisky-Q5RJGjKts06CY9SHAMCTRUEOCMrvLtNR@public.gmane.org>
> Signed-off-by: Irina Tirdea <irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Code looks fine. Again, would be nice to know on which devices this was
tested.
> ---
> drivers/input/touchscreen/goodix.c | 4 ----
> 1 file changed, 4 deletions(-)
>
> diff --git a/drivers/input/touchscreen/goodix.c
> b/drivers/input/touchscreen/goodix.c
> index 182ff9c..6fba804 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -454,10 +454,6 @@ static int goodix_reset(struct goodix_ts_data
> *ts)
> usleep_range(6000, 10000); /* T4: > 5ms */
>
> /* end select I2C slave addr */
> - error = gpiod_direction_input(ts->gpiod_rst);
> - if (error)
> - return error;
> -
> error = goodix_int_sync(ts);
> if (error)
> return error;
--
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 5/5] Input: goodix - add support for gt9157
From: Bastien Nocera @ 2016-10-27 13:44 UTC (permalink / raw)
To: Irina Tirdea, linux-input-u79uwXL29TY76Z2rM5mHXA
Cc: Dmitry Torokhov, Aleksei Mamlin, Karsten Merker, Mark Rutland,
Rob Herring, Octavian Purdila,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1473530257-7495-6-git-send-email-irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
On Sat, 2016-09-10 at 20:57 +0300, Irina Tirdea wrote:
> Goodix touchscreen GT9157 has the same basic functionality
> as GT911 touchscreen. This is based on Goodix datasheets
> for GT911 and GT9157 and on Goodix driver gt9xx.c for
> Android (publicly available in Android kernel trees for
> various devices).
>
> Signed-off-by: Irina Tirdea <irina.tirdea-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
Looks obviously correct.
Reviewed-by: Bastien Nocera <hadess-0MeiytkfxGOsTnJN9+BGXg@public.gmane.org>
> ---
> Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 1 +
> drivers/input/touchscreen/goodix.c | 2
> ++
> 2 files changed, 3 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> index ef5f42d..421b7d5 100644
> --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt
> @@ -5,6 +5,7 @@ Required properties:
> - compatible : Should be "goodix,gt911"
> or "goodix,gt9110"
> or "goodix,gt912"
> + or "goodix,gt9157"
> or "goodix,gt927"
> or "goodix,gt9271"
> or "goodix,gt928"
> diff --git a/drivers/input/touchscreen/goodix.c
> b/drivers/input/touchscreen/goodix.c
> index 6fba804..bd4dd4b 100644
> --- a/drivers/input/touchscreen/goodix.c
> +++ b/drivers/input/touchscreen/goodix.c
> @@ -192,6 +192,7 @@ static int goodix_get_cfg_len(u16 id)
> case 911:
> case 9271:
> case 9110:
> + case 9157:
> case 927:
> case 928:
> return GOODIX_CONFIG_911_LENGTH;
> @@ -1168,6 +1169,7 @@ static const struct of_device_id
> goodix_of_match[] = {
> { .compatible = "goodix,gt911" },
> { .compatible = "goodix,gt9110" },
> { .compatible = "goodix,gt912" },
> + { .compatible = "goodix,gt9157" },
> { .compatible = "goodix,gt927" },
> { .compatible = "goodix,gt9271" },
> { .compatible = "goodix,gt928" },
--
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: [RFC PATCH 00/13] of: Make drivers/of/resolver.c more readable
From: Pantelis Antoniou @ 2016-10-27 13:46 UTC (permalink / raw)
To: Frank Rowand; +Cc: Rob Herring, devicetree, linux-kernel
In-Reply-To: <580FC874.4000401@gmail.com>
Hi Frank,
> On Oct 26, 2016, at 00:02 , Frank Rowand <frowand.list@gmail.com> wrote:
>
> On 10/25/16 13:58, frowand.list@gmail.com wrote:
>> From: Frank Rowand <frank.rowand@am.sony.com>
>>
>> drivers/of/resolve.c is a bit difficult to read. Clean it up so
>> that review of future overlay related patches will be easier.
>
> < snip >
>
> Hi Pantelis,
>
> Can you test this patch series on some real hardware?
>
Sure, I’ll give it whirl today. Been swamped after ELCE but now
I have a little bit of time.
> Thanks,
>
> Frank
Regards
— Pantelis
^ permalink raw reply
* Re: [RESEND PATCH v2] clk: hisilicon: add CRG driver for Hi3798CV200 SoC
From: Rob Herring @ 2016-10-27 13:48 UTC (permalink / raw)
To: Jiancheng Xue
Cc: mturquette-rdvid1DuHRBWk0Htik3J/w, sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
mark.rutland-5wv7dgnIgG8, linux-clk-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
bin.chen-QSEj5FYQhm4dnm+yROfE0A, elder-QSEj5FYQhm4dnm+yROfE0A,
hermit.wangheming-C8/M+/jPZTeaMJb+Lgu22Q,
yanhaifeng-C8/M+/jPZTeaMJb+Lgu22Q, wenpan-C8/M+/jPZTeaMJb+Lgu22Q
In-Reply-To: <1477013830-11481-1-git-send-email-xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
On Fri, Oct 21, 2016 at 09:37:10AM +0800, Jiancheng Xue wrote:
> Add CRG driver for Hi3798CV200 SoC. CRG(Clock and Reset
> Generator) module generates clock and reset signals used
> by other module blocks on SoC.
>
> Signed-off-by: Jiancheng Xue <xuejiancheng-C8/M+/jPZTeaMJb+Lgu22Q@public.gmane.org>
> ---
> change log
> v2:
> - Fixed compiling error when compiled as a module.
> - Fixed issues pointed by Stephen Boyd.
> - Added prefix HISTB for clock index macro definitions.
What Stephen asked for is send this and the Hi3516CV300 series as one
series since there is a dependency.
>
> .../clock/{hi3519-crg.txt => hisi-crg.txt} | 11 +-
In any case, for the binding:
Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> drivers/clk/hisilicon/Kconfig | 8 +
> drivers/clk/hisilicon/Makefile | 1 +
> drivers/clk/hisilicon/crg-hi3798cv200.c | 305 +++++++++++++++++++++
> drivers/clk/hisilicon/crg.h | 34 +++
> include/dt-bindings/clock/histb-clock.h | 64 +++++
> 6 files changed, 419 insertions(+), 4 deletions(-)
> rename Documentation/devicetree/bindings/clock/{hi3519-crg.txt => hisi-crg.txt} (82%)
> create mode 100644 drivers/clk/hisilicon/crg-hi3798cv200.c
> create mode 100644 drivers/clk/hisilicon/crg.h
> create mode 100644 include/dt-bindings/clock/histb-clock.h
--
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: [RFC PATCH 02/13] of: Remove excessive printks to reduce clutter
From: Pantelis Antoniou @ 2016-10-27 13:51 UTC (permalink / raw)
To: Rob Herring
Cc: Frank Rowand, devicetree@vger.kernel.org,
linux-kernel@vger.kernel.org
In-Reply-To: <CAL_Jsq+TWZw+VcbYZp_xmFR-yitY8bn+gqe1Q2N42UFZAYsZKg@mail.gmail.com>
Hi Rob, Frank,
> On Oct 27, 2016, at 15:21 , Rob Herring <robh+dt@kernel.org> wrote:
>
> On Tue, Oct 25, 2016 at 3:58 PM, <frowand.list@gmail.com> wrote:
>> From: Frank Rowand <frank.rowand@am.sony.com>
>
> Maybe some should be debug?
>
Yes, please do not get rid of them completely.
Leave them at least as debug level so that if there’s a problem
there’s a way to figure out why something happened.
>> Signed-off-by: Frank Rowand <frank.rowand@am.sony.com>
>> ---
>> drivers/of/resolver.c | 28 ----------------------------
>> 1 file changed, 28 deletions(-)
>>
>> diff --git a/drivers/of/resolver.c b/drivers/of/resolver.c
>> index 4ff0220d7aa2..93a7ca0bf98c 100644
>> --- a/drivers/of/resolver.c
>> +++ b/drivers/of/resolver.c
>> @@ -116,8 +116,6 @@ static int __of_adjust_phandle_ref(struct device_node *node,
>>
>> propval = kmalloc(rprop->length, GFP_KERNEL);
>> if (!propval) {
>> - pr_err("%s: Could not copy value of '%s'\n",
>> - __func__, rprop->name);
>> return -ENOMEM;
>> }
>
> I would remove the brackets in this patch rather than separately.
>
>> memcpy(propval, rprop->value, rprop->length);
Regards
— Pantelis
^ permalink raw reply
* Re: [RFC PATCH 5/5] of: overlay-mgr: add a detector for headers stored on a ds2431 eeprom over w1
From: Antoine Tenart @ 2016-10-27 13:55 UTC (permalink / raw)
To: Matthias Brugger
Cc: mark.rutland, thomas.petazzoni, devicetree, pantelis.antoniou,
Antoine Tenart, sboyd, linux-kernel, maxime.ripard,
linux-arm-kernel
In-Reply-To: <e839e92f-127b-efc3-b10f-37477d1c5d9a@suse.com>
[-- Attachment #1.1: Type: text/plain, Size: 551 bytes --]
Hello Matthias,
On Thu, Oct 27, 2016 at 11:19:14AM +0200, Matthias Brugger wrote:
> On 10/26/2016 04:57 PM, Antoine Tenart wrote:
> > Signed-off-by: Antoine Tenart <antoine.tenart@free-electrons.com>
> > ---
>
> Please provide a commit message.
Sure. There are other modifications I'd like to do in the series if it
happens to be an use case for people. This patch is given as an example
of how we could implement this.
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 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 v2 2/2] arm64: dts: hi6220: add resets property into dwmmc nodes
From: Vincent Guittot @ 2016-10-27 13:56 UTC (permalink / raw)
To: Guodong Xu
Cc: Mark Rutland, rob.herring, devicetree@vger.kernel.org,
xinliang.liu, Catalin Marinas, Will Deacon, linux-kernel, xuwei5,
jh80.chung, Rob Herring, John Stultz, Fathi Boudra, LAK
In-Reply-To: <1472094041-5357-2-git-send-email-guodong.xu@linaro.org>
Hi,
My hikey board failed to detect and mount sdcard with v4.9-rc1 and i
have bisected the issue to this patch. Once reverted, the sdcard is
detected again.
Regards,
Vincent
On 25 August 2016 at 05:00, Guodong Xu <guodong.xu@linaro.org> wrote:
> Add resets property into dwmmc_0, dwmmc_1 and dwmmc_2 for hi6220
>
> Code and documentation to this property were confirmed by maintainers.
> See:
> [1] https://patchwork.kernel.org/patch/9276607/
> [2] https://patchwork.kernel.org/patch/8487151/
> [3] https://lkml.org/lkml/2016/8/12/91
>
> cc: Jaehoon Chung <jh80.chung@samsung.com>
> cc: Rob Herring <robh+dt@kernel.org>
> Signed-off-by: Guodong Xu <guodong.xu@linaro.org>
> ---
> arch/arm64/boot/dts/hisilicon/hi6220.dtsi | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> index d0b887a..63608e9 100644
> --- a/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> +++ b/arch/arm64/boot/dts/hisilicon/hi6220.dtsi
> @@ -771,6 +771,7 @@
> interrupts = <0x0 0x48 0x4>;
> clocks = <&sys_ctrl 2>, <&sys_ctrl 1>;
> clock-names = "ciu", "biu";
> + resets = <&sys_ctrl PERIPH_RSTDIS0_MMC0>;
> bus-width = <0x8>;
> vmmc-supply = <&ldo19>;
> pinctrl-names = "default";
> @@ -790,6 +791,7 @@
> #size-cells = <0x0>;
> clocks = <&sys_ctrl 4>, <&sys_ctrl 3>;
> clock-names = "ciu", "biu";
> + resets = <&sys_ctrl PERIPH_RSTDIS0_MMC1>;
> vqmmc-supply = <&ldo7>;
> vmmc-supply = <&ldo10>;
> bus-width = <0x4>;
> @@ -807,6 +809,7 @@
> interrupts = <0x0 0x4a 0x4>;
> clocks = <&sys_ctrl HI6220_MMC2_CIUCLK>, <&sys_ctrl HI6220_MMC2_CLK>;
> clock-names = "ciu", "biu";
> + resets = <&sys_ctrl PERIPH_RSTDIS0_MMC2>;
> bus-width = <0x4>;
> broken-cd;
> pinctrl-names = "default", "idle";
> --
> 1.9.1
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH V3 0/6] Add support for Tegra GMI bus controller
From: Mirza Krak @ 2016-10-27 14:01 UTC (permalink / raw)
To: swarren, thierry.reding, jonathanh
Cc: mark.rutland, gnurou, Mirza Krak, pgaikwad, linux-clk, devicetree,
pdeschrijver, sboyd, linux, linux-kernel, robh+dt, linux-tegra,
mturquette, linux-arm-kernel
From: Mirza Krak <mirza.krak@gmail.com.com>
Hi.
This patch series adds support for the Tegra GMI bus controller.
I have tested this series on a Tegra30 using a Colibri T30 SOM on a custom
carrier board which has multiple CAN controllers (SJA1000) connected to the
GMI bus.
I have re-based on top of latest tegra/for-next in V3. Also see individual
patches for changes in V3.
I have picked up all the comments and suggestions from V2, but I still do not
have an ACK from Rob on the bindings and discussions have halted for some time
now and I hope that this re-send could be the basis for new discussions.
See below links for previous discussions.
Comments on RFC:
https://marc.info/?l=linux-clk&m=146893557629903&w=2
https://marc.info/?l=linux-tegra&m=146893541829801&w=2
https://marc.info/?l=linux-tegra&m=146893542429814&w=2
Comments on V1:
https://marc.info/?l=linux-arm-kernel&m=147051551821122&w=2
https://marc.info/?l=linux-arm-kernel&m=147051553121150&w=2
https://marc.info/?l=linux-arm-kernel&m=147194856600627&w=2
https://marc.info/?l=linux-arm-kernel&m=147072742432211&w=2
Comments on V2:
https://marc.info/?l=devicetree&m=147522253920226&w=2
https://marc.info/?l=linux-tegra&m=147204588027687&w=2
https://marc.info/?l=linux-tegra&m=147204588027687&w=2
https://marc.info/?l=devicetree&m=147256931318922&w=2
Mirza Krak (6):
clk: tegra: add TEGRA20_CLK_NOR to init table
clk: tegra: add TEGRA30_CLK_NOR to init table
dt/bindings: Add bindings for Tegra GMI controller
ARM: tegra: Add Tegra30 GMI support
ARM: tegra: Add Tegra20 GMI support
bus: Add support for Tegra Generic Memory Interface
.../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132 ++++++++++
arch/arm/boot/dts/tegra20.dtsi | 16 +-
arch/arm/boot/dts/tegra30.dtsi | 13 +
drivers/bus/Kconfig | 8 +
drivers/bus/Makefile | 1 +
drivers/bus/tegra-gmi.c | 267 +++++++++++++++++++++
drivers/clk/tegra/clk-tegra20.c | 1 +
drivers/clk/tegra/clk-tegra30.c | 1 +
8 files changed, 438 insertions(+), 1 deletion(-)
create mode 100644 Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
create mode 100644 drivers/bus/tegra-gmi.c
--
2.1.4
^ permalink raw reply
* [PATCH V3 1/6] clk: tegra: add TEGRA20_CLK_NOR to init table
From: Mirza Krak @ 2016-10-27 14:01 UTC (permalink / raw)
To: swarren, thierry.reding, jonathanh
Cc: mark.rutland, gnurou, pgaikwad, linux-clk, devicetree,
pdeschrijver, Mirza Krak, sboyd, linux, linux-kernel, robh+dt,
linux-tegra, mturquette, linux-arm-kernel
In-Reply-To: <1477576872-2665-1-git-send-email-mirza.krak@gmail.com>
From: Mirza Krak <mirza.krak@gmail.com>
Add TEGRA20_CLK_NOR to init table and set default rate to 92 MHz which
is max rate.
The maximum rate value of 92 MHz is pulled from the downstream L4T
kernel.
Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
Tested-on: Colibri T20/T30 on EvalBoard V3.x and GMI-Memory Board
---
Changes in v2:
- no changes
Changes in v3:
- Added comment in commit message where I got the maximum rates from.
drivers/clk/tegra/clk-tegra20.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/tegra/clk-tegra20.c b/drivers/clk/tegra/clk-tegra20.c
index 837e5cb..13d3b5a 100644
--- a/drivers/clk/tegra/clk-tegra20.c
+++ b/drivers/clk/tegra/clk-tegra20.c
@@ -1047,6 +1047,7 @@ static struct tegra_clk_init_table init_table[] __initdata = {
{ TEGRA20_CLK_SDMMC3, TEGRA20_CLK_PLL_P, 48000000, 0 },
{ TEGRA20_CLK_SDMMC4, TEGRA20_CLK_PLL_P, 48000000, 0 },
{ TEGRA20_CLK_SPI, TEGRA20_CLK_PLL_P, 20000000, 0 },
+ { TEGRA20_CLK_NOR, TEGRA20_CLK_PLL_P, 92000000, 0 },
{ TEGRA20_CLK_SBC1, TEGRA20_CLK_PLL_P, 100000000, 0 },
{ TEGRA20_CLK_SBC2, TEGRA20_CLK_PLL_P, 100000000, 0 },
{ TEGRA20_CLK_SBC3, TEGRA20_CLK_PLL_P, 100000000, 0 },
--
2.1.4
^ permalink raw reply related
* [PATCH V3 2/6] clk: tegra: add TEGRA30_CLK_NOR to init table
From: Mirza Krak @ 2016-10-27 14:01 UTC (permalink / raw)
To: swarren, thierry.reding, jonathanh
Cc: gnurou, linux, pdeschrijver, pgaikwad, mturquette, sboyd, robh+dt,
mark.rutland, devicetree, linux-tegra, linux-kernel,
linux-arm-kernel, linux-clk, Mirza Krak
In-Reply-To: <1477576872-2665-1-git-send-email-mirza.krak@gmail.com>
From: Mirza Krak <mirza.krak@gmail.com>
Add TEGRA30_CLK_NOR to init table and set default rate to 127 MHz which
is max rate.
The maximum rate value of 127 MHz is pulled from the downstream L4T
kernel.
Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
Tested-on: Colibri T20/T30 on EvalBoard V3.x and GMI-Memory Board
---
Changes in v2:
- no changes
Changes in v3:
- Added comment in commit message where I got the maximum rates from.
drivers/clk/tegra/clk-tegra30.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/clk/tegra/clk-tegra30.c b/drivers/clk/tegra/clk-tegra30.c
index 8e2db5e..67f1677 100644
--- a/drivers/clk/tegra/clk-tegra30.c
+++ b/drivers/clk/tegra/clk-tegra30.c
@@ -1252,6 +1252,7 @@ static struct tegra_clk_init_table init_table[] __initdata = {
{ TEGRA30_CLK_SDMMC1, TEGRA30_CLK_PLL_P, 48000000, 0 },
{ TEGRA30_CLK_SDMMC2, TEGRA30_CLK_PLL_P, 48000000, 0 },
{ TEGRA30_CLK_SDMMC3, TEGRA30_CLK_PLL_P, 48000000, 0 },
+ { TEGRA30_CLK_NOR, TEGRA30_CLK_PLL_P, 127000000, 0 },
{ TEGRA30_CLK_PLL_M, TEGRA30_CLK_CLK_MAX, 0, 1 },
{ TEGRA30_CLK_PCLK, TEGRA30_CLK_CLK_MAX, 0, 1 },
{ TEGRA30_CLK_CSITE, TEGRA30_CLK_CLK_MAX, 0, 1 },
--
2.1.4
^ permalink raw reply related
* [PATCH V3 3/6] dt/bindings: Add bindings for Tegra GMI controller
From: Mirza Krak @ 2016-10-27 14:01 UTC (permalink / raw)
To: swarren, thierry.reding, jonathanh
Cc: mark.rutland, gnurou, pgaikwad, linux-clk, devicetree,
pdeschrijver, Mirza Krak, sboyd, linux, linux-kernel, robh+dt,
linux-tegra, mturquette, linux-arm-kernel
In-Reply-To: <1477576872-2665-1-git-send-email-mirza.krak@gmail.com>
From: Mirza Krak <mirza.krak@gmail.com>
Document the devicetree bindings for the Generic Memory Interface (GMI)
bus driver found on Tegra SOCs.
Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
Tested-on: Colibri T20/T30 on EvalBoard V3.x and GMI-Memory Board
---
Changes in v2:
- Updated examples and some information based on comments from Jon Hunter.
Changes in v3:
- Updates ranges description based on comments from Rob Herring
.../devicetree/bindings/bus/nvidia,tegra20-gmi.txt | 132 +++++++++++++++++++++
1 file changed, 132 insertions(+)
create mode 100644 Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
diff --git a/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
new file mode 100644
index 0000000..49bda2f
--- /dev/null
+++ b/Documentation/devicetree/bindings/bus/nvidia,tegra20-gmi.txt
@@ -0,0 +1,132 @@
+Device tree bindings for NVIDIA Tegra Generic Memory Interface bus
+
+The Generic Memory Interface bus enables memory transfers between internal and
+external memory. Can be used to attach various high speed devices such as
+synchronous/asynchronous NOR, FPGA, UARTS and more.
+
+The actual devices are instantiated from the child nodes of a GMI node.
+
+Required properties:
+ - compatible : Should contain one of the following:
+ For Tegra20 must contain "nvidia,tegra20-gmi".
+ For Tegra30 must contain "nvidia,tegra30-gmi".
+ - reg: Should contain GMI controller registers location and length.
+ - clocks: Must contain an entry for each entry in clock-names.
+ - clock-names: Must include the following entries: "gmi"
+ - resets : Must contain an entry for each entry in reset-names.
+ - reset-names : Must include the following entries: "gmi"
+ - #address-cells: The number of cells used to represent physical base
+ addresses in the GMI address space. Should be 2.
+ - #size-cells: The number of cells used to represent the size of an address
+ range in the GMI address space. Should be 1.
+ - ranges: Must be set up to reflect the memory layout with three integer values
+ for each chip-select line in use (only one entry is supported, see below
+ comments):
+ <cs-number> <offset> <physical address of mapping> <size>
+
+Note that the GMI controller does not have any internal chip-select address
+decoding, because of that chip-selects either need to be managed via software
+or by employing external chip-select decoding logic.
+
+If external chip-select logic is used to support multiple devices it is assumed
+that the devices use the same timing and so are probably the same type. It also
+assumes that they can fit in the 256MB address range. In this case only one
+child device is supported which represents the active chip-select line, see
+examples for more insight.
+
+The chip-select number is decoded from the child nodes second address cell of
+'ranges' property, if 'ranges' property is not present or empty chip-select will
+then be decoded from the first cell of the 'reg' property.
+
+Optional child cs node properties:
+
+ - nvidia,snor-data-width-32bit: Use 32bit data-bus, default is 16bit.
+ - nvidia,snor-mux-mode: Enable address/data MUX mode.
+ - nvidia,snor-rdy-active-before-data: Assert RDY signal one cycle before data.
+ If omitted it will be asserted with data.
+ - nvidia,snor-rdy-inv: RDY signal is active high
+ - nvidia,snor-adv-inv: ADV signal is active high
+ - nvidia,snor-oe-inv: WE/OE signal is active high
+ - nvidia,snor-cs-inv: CS signal is active high
+
+ Note that there is some special handling for the timing values.
+ From Tegra TRM:
+ Programming 0 means 1 clock cycle: actual cycle = programmed cycle + 1
+
+ - nvidia,snor-muxed-width: Number of cycles MUX address/data asserted on the
+ bus. Valid values are 0-15, default is 1
+ - nvidia,snor-hold-width: Number of cycles CE stays asserted after the
+ de-assertion of WR_N (in case of SLAVE/MASTER Request) or OE_N
+ (in case of MASTER Request). Valid values are 0-15, default is 1
+ - nvidia,snor-adv-width: Number of cycles during which ADV stays asserted.
+ Valid values are 0-15, default is 1.
+ - nvidia,snor-ce-width: Number of cycles before CE is asserted.
+ Valid values are 0-15, default is 4
+ - nvidia,snor-we-width: Number of cycles during which WE stays asserted.
+ Valid values are 0-15, default is 1
+ - nvidia,snor-oe-width: Number of cycles during which OE stays asserted.
+ Valid values are 0-255, default is 1
+ - nvidia,snor-wait-width: Number of cycles before READY is asserted.
+ Valid values are 0-255, default is 3
+
+Example with two SJA1000 CAN controllers connected to the GMI bus. We wrap the
+controllers with a simple-bus node since they are all connected to the same
+chip-select (CS4), in this example external address decoding is provided:
+
+gmi@70090000 {
+ compatible = "nvidia,tegra20-gmi";
+ reg = <0x70009000 0x1000>;
+ #address-cells = <2>;
+ #size-cells = <1>;
+ clocks = <&tegra_car TEGRA20_CLK_NOR>;
+ clock-names = "gmi";
+ resets = <&tegra_car 42>;
+ reset-names = "gmi";
+ ranges = <4 0 0xd0000000 0xfffffff>;
+
+ status = "okay";
+
+ bus@4,0 {
+ compatible = "simple-bus";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0 4 0 0x40100>;
+
+ nvidia,snor-mux-mode;
+ nvidia,snor-adv-inv;
+
+ can@0 {
+ reg = <0 0x100>;
+ ...
+ };
+
+ can@40000 {
+ reg = <0x40000 0x100>;
+ ...
+ };
+ };
+};
+
+Example with one SJA1000 CAN controller connected to the GMI bus
+on CS4:
+
+gmi@70090000 {
+ compatible = "nvidia,tegra20-gmi";
+ reg = <0x70009000 0x1000>;
+ #address-cells = <2>;
+ #size-cells = <1>;
+ clocks = <&tegra_car TEGRA20_CLK_NOR>;
+ clock-names = "gmi";
+ resets = <&tegra_car 42>;
+ reset-names = "gmi";
+ ranges = <4 0 0xd0000000 0xfffffff>;
+
+ status = "okay";
+
+ can@4,0 {
+ reg = <4 0 0x100>;
+ nvidia,snor-mux-mode;
+ nvidia,snor-adv-inv;
+ ...
+ };
+};
--
2.1.4
^ permalink raw reply related
* [PATCH V3 4/6] ARM: tegra: Add Tegra30 GMI support
From: Mirza Krak @ 2016-10-27 14:01 UTC (permalink / raw)
To: swarren, thierry.reding, jonathanh
Cc: mark.rutland, gnurou, pgaikwad, linux-clk, devicetree,
pdeschrijver, Mirza Krak, sboyd, linux, linux-kernel, robh+dt,
linux-tegra, mturquette, linux-arm-kernel
In-Reply-To: <1477576872-2665-1-git-send-email-mirza.krak@gmail.com>
From: Mirza Krak <mirza.krak@gmail.com>
Add a device node for the GMI controller found on Tegra30.
Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
Tested-on: Colibri T20/T30 on EvalBoard V3.x and GMI-Memory Board
---
Changes in v2:
- added address-cells, size-cells and ranges properties
Changes in v3:
- no changes
arch/arm/boot/dts/tegra30.dtsi | 13 +++++++++++++
1 file changed, 13 insertions(+)
diff --git a/arch/arm/boot/dts/tegra30.dtsi b/arch/arm/boot/dts/tegra30.dtsi
index 5030065..bbb1c00 100644
--- a/arch/arm/boot/dts/tegra30.dtsi
+++ b/arch/arm/boot/dts/tegra30.dtsi
@@ -439,6 +439,19 @@
status = "disabled";
};
+ gmi@70009000 {
+ compatible = "nvidia,tegra30-gmi";
+ reg = <0x70009000 0x1000>;
+ #address-cells = <2>;
+ #size-cells = <1>;
+ ranges = <0 0 0x48000000 0x7ffffff>;
+ clocks = <&tegra_car TEGRA30_CLK_NOR>;
+ clock-names = "gmi";
+ resets = <&tegra_car 42>;
+ reset-names = "gmi";
+ status = "disabled";
+ };
+
pwm: pwm@7000a000 {
compatible = "nvidia,tegra30-pwm", "nvidia,tegra20-pwm";
reg = <0x7000a000 0x100>;
--
2.1.4
^ permalink raw reply related
* [PATCH V3 5/6] ARM: tegra: Add Tegra20 GMI support
From: Mirza Krak @ 2016-10-27 14:01 UTC (permalink / raw)
To: swarren, thierry.reding, jonathanh
Cc: mark.rutland, gnurou, pgaikwad, linux-clk, devicetree,
pdeschrijver, Mirza Krak, sboyd, linux, linux-kernel, robh+dt,
linux-tegra, mturquette, linux-arm-kernel
In-Reply-To: <1477576872-2665-1-git-send-email-mirza.krak@gmail.com>
From: Mirza Krak <mirza.krak@gmail.com>
Add a device node for the GMI controller found on Tegra20.
Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
Tested-on: Colibri T20/T30 on EvalBoard V3.x and GMI-Memory Board
---
Changes in v2:
- added address-cells, size-cells and ranges properties
Changes in v3:
- fixed range address which is not the same as Tegra30.
arch/arm/boot/dts/tegra20.dtsi | 16 +++++++++++++++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/arm/boot/dts/tegra20.dtsi b/arch/arm/boot/dts/tegra20.dtsi
index 2207c08..b22cddb 100644
--- a/arch/arm/boot/dts/tegra20.dtsi
+++ b/arch/arm/boot/dts/tegra20.dtsi
@@ -1,4 +1,4 @@
-#include <dt-bindings/clock/tegra20-car.h>
+include <dt-bindings/clock/tegra20-car.h>
#include <dt-bindings/gpio/tegra-gpio.h>
#include <dt-bindings/pinctrl/pinctrl-tegra.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
@@ -376,6 +376,20 @@
status = "disabled";
};
+
+ gmi@70009000 {
+ compatible = "nvidia,tegra20-gmi";
+ reg = <0x70009000 0x1000>;
+ #address-cells = <2>;
+ #size-cells = <1>;
+ ranges = <0 0 0xd0000000 0xfffffff>;
+ clocks = <&tegra_car TEGRA20_CLK_NOR>;
+ clock-names = "gmi";
+ resets = < &tegra_car 42>;
+ reset-names = "gmi";
+ status = "disabled";
+ };
+
pwm: pwm@7000a000 {
compatible = "nvidia,tegra20-pwm";
reg = <0x7000a000 0x100>;
--
2.1.4
^ permalink raw reply related
* [PATCH V3 6/6] bus: Add support for Tegra Generic Memory Interface
From: Mirza Krak @ 2016-10-27 14:01 UTC (permalink / raw)
To: swarren, thierry.reding, jonathanh
Cc: mark.rutland, gnurou, pgaikwad, linux-clk, devicetree,
pdeschrijver, Mirza Krak, sboyd, linux, linux-kernel, robh+dt,
linux-tegra, mturquette, linux-arm-kernel
In-Reply-To: <1477576872-2665-1-git-send-email-mirza.krak@gmail.com>
From: Mirza Krak <mirza.krak@gmail.com>
The Generic Memory Interface bus can be used to connect high-speed
devices such as NOR flash, FPGAs, DSPs...
Signed-off-by: Mirza Krak <mirza.krak@gmail.com>
Tested-by: Marcel Ziswiler <marcel.ziswiler@toradex.com>
Tested-on: Colibri T20/T30 on EvalBoard V3.x and GMI-Memory Board
---
Changes in v2:
- Fixed some checkpatch errors
- Re-ordered probe to get rid of local variables
- Moved of_platform_default_populate call to the end of probe
- Use the timing and configuration properties from the child device
- Added warning if more then 1 child device exist
Changes in v3:
- added helper function to disable the controller which is used in remove and
on error.
- Added logic to parse CS# from "ranges" property with fallback to "reg"
property
drivers/bus/Kconfig | 8 ++
drivers/bus/Makefile | 1 +
drivers/bus/tegra-gmi.c | 267 ++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 276 insertions(+)
create mode 100644 drivers/bus/tegra-gmi.c
diff --git a/drivers/bus/Kconfig b/drivers/bus/Kconfig
index 4ed7d26..2e75a7f 100644
--- a/drivers/bus/Kconfig
+++ b/drivers/bus/Kconfig
@@ -141,6 +141,14 @@ config TEGRA_ACONNECT
Driver for the Tegra ACONNECT bus which is used to interface with
the devices inside the Audio Processing Engine (APE) for Tegra210.
+config TEGRA_GMI
+ tristate "Tegra Generic Memory Interface bus driver"
+ depends on ARCH_TEGRA
+ help
+ Driver for the Tegra Generic Memory Interface bus which can be used
+ to attach devices such as NOR, UART, FPGA and more.
+
+
config UNIPHIER_SYSTEM_BUS
tristate "UniPhier System Bus driver"
depends on ARCH_UNIPHIER && OF
diff --git a/drivers/bus/Makefile b/drivers/bus/Makefile
index ac84cc4..34e2bab 100644
--- a/drivers/bus/Makefile
+++ b/drivers/bus/Makefile
@@ -18,5 +18,6 @@ obj-$(CONFIG_OMAP_OCP2SCP) += omap-ocp2scp.o
obj-$(CONFIG_SUNXI_RSB) += sunxi-rsb.o
obj-$(CONFIG_SIMPLE_PM_BUS) += simple-pm-bus.o
obj-$(CONFIG_TEGRA_ACONNECT) += tegra-aconnect.o
+obj-$(CONFIG_TEGRA_GMI) += tegra-gmi.o
obj-$(CONFIG_UNIPHIER_SYSTEM_BUS) += uniphier-system-bus.o
obj-$(CONFIG_VEXPRESS_CONFIG) += vexpress-config.o
diff --git a/drivers/bus/tegra-gmi.c b/drivers/bus/tegra-gmi.c
new file mode 100644
index 0000000..dd9623e
--- /dev/null
+++ b/drivers/bus/tegra-gmi.c
@@ -0,0 +1,267 @@
+/*
+ * Driver for NVIDIA Generic Memory Interface
+ *
+ * Copyright (C) 2016 Host Mobility AB. All rights reserved.
+ *
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+#include <linux/clk.h>
+#include <linux/delay.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of_device.h>
+#include <linux/reset.h>
+
+#define TEGRA_GMI_CONFIG 0x00
+#define TEGRA_GMI_CONFIG_GO BIT(31)
+#define TEGRA_GMI_BUS_WIDTH_32BIT BIT(30)
+#define TEGRA_GMI_MUX_MODE BIT(28)
+#define TEGRA_GMI_RDY_BEFORE_DATA BIT(24)
+#define TEGRA_GMI_RDY_ACTIVE_HIGH BIT(23)
+#define TEGRA_GMI_ADV_ACTIVE_HIGH BIT(22)
+#define TEGRA_GMI_OE_ACTIVE_HIGH BIT(21)
+#define TEGRA_GMI_CS_ACTIVE_HIGH BIT(20)
+#define TEGRA_GMI_CS_SELECT(x) ((x & 0x7) << 4)
+
+#define TEGRA_GMI_TIMING0 0x10
+#define TEGRA_GMI_MUXED_WIDTH(x) ((x & 0xf) << 12)
+#define TEGRA_GMI_HOLD_WIDTH(x) ((x & 0xf) << 8)
+#define TEGRA_GMI_ADV_WIDTH(x) ((x & 0xf) << 4)
+#define TEGRA_GMI_CE_WIDTH(x) (x & 0xf)
+
+#define TEGRA_GMI_TIMING1 0x14
+#define TEGRA_GMI_WE_WIDTH(x) ((x & 0xff) << 16)
+#define TEGRA_GMI_OE_WIDTH(x) ((x & 0xff) << 8)
+#define TEGRA_GMI_WAIT_WIDTH(x) (x & 0xff)
+
+struct tegra_gmi_priv {
+ void __iomem *base;
+ struct reset_control *rst;
+ struct clk *clk;
+
+ u32 snor_config;
+ u32 snor_timing0;
+ u32 snor_timing1;
+};
+
+static void tegra_gmi_disable(struct tegra_gmi_priv *priv)
+{
+ u32 config;
+
+ /* stop GMI operation */
+ config = readl(priv->base + TEGRA_GMI_CONFIG);
+ config &= ~TEGRA_GMI_CONFIG_GO;
+ writel(config, priv->base + TEGRA_GMI_CONFIG);
+
+ reset_control_assert(priv->rst);
+ clk_disable_unprepare(priv->clk);
+}
+
+static void tegra_gmi_init(struct device *dev, struct tegra_gmi_priv *priv)
+{
+ writel(priv->snor_timing0, priv->base + TEGRA_GMI_TIMING0);
+ writel(priv->snor_timing1, priv->base + TEGRA_GMI_TIMING1);
+
+ priv->snor_config |= TEGRA_GMI_CONFIG_GO;
+ writel(priv->snor_config, priv->base + TEGRA_GMI_CONFIG);
+}
+
+static int tegra_gmi_parse_dt(struct device *dev, struct tegra_gmi_priv *priv)
+{
+ struct device_node *child = of_get_next_available_child(dev->of_node,
+ NULL);
+ u32 property, ranges[4];
+ int ret;
+
+ if (!child) {
+ dev_warn(dev, "no child nodes found\n");
+ return 0;
+ }
+
+ /*
+ * We currently only support one child device due to lack of
+ * chip-select address decoding. Which means that we only have one
+ * chip-select line from the GMI controller.
+ */
+ if (of_get_child_count(dev->of_node) > 1)
+ dev_warn(dev, "only one child device is supported.");
+
+ if (of_property_read_bool(child, "nvidia,snor-data-width-32bit"))
+ priv->snor_config |= TEGRA_GMI_BUS_WIDTH_32BIT;
+
+ if (of_property_read_bool(child, "nvidia,snor-mux-mode"))
+ priv->snor_config |= TEGRA_GMI_MUX_MODE;
+
+ if (of_property_read_bool(child, "nvidia,snor-rdy-active-before-data"))
+ priv->snor_config |= TEGRA_GMI_RDY_BEFORE_DATA;
+
+ if (of_property_read_bool(child, "nvidia,snor-rdy-inv"))
+ priv->snor_config |= TEGRA_GMI_RDY_ACTIVE_HIGH;
+
+ if (of_property_read_bool(child, "nvidia,snor-adv-inv"))
+ priv->snor_config |= TEGRA_GMI_ADV_ACTIVE_HIGH;
+
+ if (of_property_read_bool(child, "nvidia,snor-oe-inv"))
+ priv->snor_config |= TEGRA_GMI_OE_ACTIVE_HIGH;
+
+ if (of_property_read_bool(child, "nvidia,snor-cs-inv"))
+ priv->snor_config |= TEGRA_GMI_CS_ACTIVE_HIGH;
+
+ /* Decode the CS# */
+ ret = of_property_read_u32_array(child, "ranges", ranges, 4);
+ if (ret < 0) {
+ /* Invalid binding */
+ if (ret == -EOVERFLOW) {
+ dev_err(dev, "invalid ranges length\n");
+ goto error_cs_decode;
+ }
+
+ /*
+ * If we reach here it means that the child node has an empty
+ * ranges or it does not exist at all. Attempt to decode the
+ * CS# from the reg property instead.
+ */
+ ret = of_property_read_u32(child, "reg", &property);
+ if (ret < 0) {
+ dev_err(dev, "no reg property found\n");
+ goto error_cs_decode;
+ }
+ } else {
+ property = ranges[1];
+ }
+
+ priv->snor_config |= TEGRA_GMI_CS_SELECT(property);
+
+ /* The default values that are provided below are reset values */
+ if (!of_property_read_u32(child, "nvidia,snor-muxed-width", &property))
+ priv->snor_timing0 |= TEGRA_GMI_MUXED_WIDTH(property);
+ else
+ priv->snor_timing0 |= TEGRA_GMI_MUXED_WIDTH(1);
+
+ if (!of_property_read_u32(child, "nvidia,snor-hold-width", &property))
+ priv->snor_timing0 |= TEGRA_GMI_HOLD_WIDTH(property);
+ else
+ priv->snor_timing0 |= TEGRA_GMI_HOLD_WIDTH(1);
+
+ if (!of_property_read_u32(child, "nvidia,snor-adv-width", &property))
+ priv->snor_timing0 |= TEGRA_GMI_ADV_WIDTH(property);
+ else
+ priv->snor_timing0 |= TEGRA_GMI_ADV_WIDTH(1);
+
+ if (!of_property_read_u32(child, "nvidia,snor-ce-width", &property))
+ priv->snor_timing0 |= TEGRA_GMI_CE_WIDTH(property);
+ else
+ priv->snor_timing0 |= TEGRA_GMI_CE_WIDTH(4);
+
+ if (!of_property_read_u32(child, "nvidia,snor-we-width", &property))
+ priv->snor_timing1 |= TEGRA_GMI_WE_WIDTH(property);
+ else
+ priv->snor_timing1 |= TEGRA_GMI_WE_WIDTH(1);
+
+ if (!of_property_read_u32(child, "nvidia,snor-oe-width", &property))
+ priv->snor_timing1 |= TEGRA_GMI_OE_WIDTH(property);
+ else
+ priv->snor_timing1 |= TEGRA_GMI_OE_WIDTH(1);
+
+ if (!of_property_read_u32(child, "nvidia,snor-wait-width", &property))
+ priv->snor_timing1 |= TEGRA_GMI_WAIT_WIDTH(property);
+ else
+ priv->snor_timing1 |= TEGRA_GMI_WAIT_WIDTH(3);
+
+error_cs_decode:
+ if (ret < 0)
+ dev_err(dev, "failed to decode chip-select number\n");
+
+ of_node_put(child);
+ return ret;
+}
+
+static int tegra_gmi_probe(struct platform_device *pdev)
+{
+ struct resource *res;
+ struct device *dev = &pdev->dev;
+ struct tegra_gmi_priv *priv;
+ int ret;
+
+ priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
+ if (!priv)
+ return -ENOMEM;
+
+ res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ priv->base = devm_ioremap_resource(dev, res);
+ if (IS_ERR(priv->base))
+ return PTR_ERR(priv->base);
+
+ priv->clk = devm_clk_get(dev, "gmi");
+ if (IS_ERR(priv->clk)) {
+ dev_err(dev, "can not get clock\n");
+ return PTR_ERR(priv->clk);
+ }
+
+ priv->rst = devm_reset_control_get(dev, "gmi");
+ if (IS_ERR(priv->rst)) {
+ dev_err(dev, "can not get reset\n");
+ return PTR_ERR(priv->rst);
+ }
+
+ ret = tegra_gmi_parse_dt(dev, priv);
+ if (ret)
+ return ret;
+
+ ret = clk_prepare_enable(priv->clk);
+ if (ret) {
+ dev_err(dev, "fail to enable clock.\n");
+ return ret;
+ }
+
+ reset_control_assert(priv->rst);
+ udelay(2);
+ reset_control_deassert(priv->rst);
+
+ tegra_gmi_init(dev, priv);
+
+ ret = of_platform_default_populate(dev->of_node, NULL, dev);
+ if (ret < 0) {
+ dev_err(dev, "fail to create devices.\n");
+ tegra_gmi_disable(priv);
+ return ret;
+ }
+
+ dev_set_drvdata(dev, priv);
+
+ return 0;
+}
+
+static int tegra_gmi_remove(struct platform_device *pdev)
+{
+ struct tegra_gmi_priv *priv = dev_get_drvdata(&pdev->dev);
+
+ of_platform_depopulate(&pdev->dev);
+
+ tegra_gmi_disable(priv);
+
+ return 0;
+}
+
+static const struct of_device_id tegra_gmi_id_table[] = {
+ { .compatible = "nvidia,tegra20-gmi", },
+ { .compatible = "nvidia,tegra30-gmi", },
+ { }
+};
+MODULE_DEVICE_TABLE(of, tegra_gmi_id_table);
+
+static struct platform_driver tegra_gmi_driver = {
+ .probe = tegra_gmi_probe,
+ .remove = tegra_gmi_remove,
+ .driver = {
+ .name = "tegra-gmi",
+ .of_match_table = tegra_gmi_id_table,
+ },
+};
+module_platform_driver(tegra_gmi_driver);
+
+MODULE_AUTHOR("Mirza Krak <mirza.krak@gmail.com");
+MODULE_DESCRIPTION("NVIDIA Tegra GMI Bus Driver");
+MODULE_LICENSE("GPL v2");
--
2.1.4
^ permalink raw reply related
* Re: [PATCH 02/12] ASoC: dapm: Implement stereo mixer control support
From: Chen-Yu Tsai @ 2016-10-27 14:02 UTC (permalink / raw)
To: Mark Brown
Cc: Chen-Yu Tsai, Liam Girdwood, Jaroslav Kysela, Takashi Iwai,
Rob Herring, Mark Rutland, Russell King, Maxime Ripard,
Linux-ALSA, linux-arm-kernel, linux-kernel, devicetree,
linux-sunxi
In-Reply-To: <20161026175052.GE25322-GFdadSzt00ze9xe1eoZjHA@public.gmane.org>
On Thu, Oct 27, 2016 at 1:50 AM, Mark Brown <broonie-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> On Mon, Oct 03, 2016 at 07:07:54PM +0800, Chen-Yu Tsai wrote:
>
>> /* find dapm widget path assoc with kcontrol */
>> dapm_kcontrol_for_each_path(path, kcontrol) {
>> + /*
>> + * If status for the second channel was given ( >= 0 ),
>> + * consider the second and later paths as the second
>> + * channel.
>> + */
>> + if (found && rconnect >= 0)
>> + soc_dapm_connect_path(path, rconnect, "mixer update");
>> + else
>> + soc_dapm_connect_path(path, connect, "mixer update");
>> found = 1;
>> - soc_dapm_connect_path(path, connect, "mixer update");
>
> This really only works for two channels with the current inteface - the
> comment makes it sound like it'll work for more but we can only pass in
> two (and there's only support for specifying two everywhere).
I could rework it to pass a list of connected status' and the number
of elements instead, but it wouldn't work well for situations where
the number of channels on the kcontrol != the number of paths. I'm not
sure if that's even a valid setup, but it does work with the current
core code.
On the other hand, are there kcontrols that are multi-channel
(> 2 channels)?
I'm inclined to just fixup the comment to make it clear that the
implementation supports stereo, i.e. 2 channels, only.
>
>> - change = dapm_kcontrol_set_value(kcontrol, val);
>> + /* This assumes field width < (bits in unsigned int / 2) */
>> + change = dapm_kcontrol_set_value(kcontrol, val | (rval << width));
>
> That seems like a bit of an assumption in cases where we've got a single
> control for both power and volume? They're very rare though, I'm not
> even sure any exist. It'd be good to have a check in the code just in
> case it does come up, it'll likely be a nightmare to debug if someone
> does run into it.
Agreed. I'll put a check and warning before it.
>
> Otherwise I think this makes sense.
Thanks for the review!
Regards
ChenYu
^ permalink raw reply
* Re: [RFC PATCH 1/5] of: introduce the overlay manager
From: Antoine Tenart @ 2016-10-27 14:03 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Mark Rutland, Thomas Petazzoni, devicetree@vger.kernel.org,
pantelis.antoniou, Antoine Tenart, sboyd,
linux-kernel@vger.kernel.org, Maxime Ripard,
linux-arm-kernel@lists.infradead.org
In-Reply-To: <CANLsYkyszsn2TLYhUjHMt2ZnEUmE9eSc71W2KWBL0fWhh3PPBw@mail.gmail.com>
[-- Attachment #1.1: Type: text/plain, Size: 3670 bytes --]
Hello Mathieu,
On Wed, Oct 26, 2016 at 10:29:59AM -0600, Mathieu Poirier wrote:
>
> Please find my comments below.
Thanks for the comments. I expected more distant reviews, on the overall
architecture to know if this could fit the needs of others. But anyway
your comments are helpful if we ever decide to go with an overlay
manager like this one.
> On 26 October 2016 at 08:57, Antoine Tenart
> <antoine.tenart@free-electrons.com> wrote:
> > +
> > +/*
> > + * overlay_mgr_register_format()
> > + *
> > + * Adds a new format candidate to the list of supported formats. The registered
> > + * formats are used to parse the headers stored on the dips.
> > + */
> > +int overlay_mgr_register_format(struct overlay_mgr_format *candidate)
> > +{
> > + struct overlay_mgr_format *format;
> > + int err = 0;
> > +
> > + spin_lock(&overlay_mgr_format_lock);
> > +
> > + /* Check if the format is already registered */
> > + list_for_each_entry(format, &overlay_mgr_formats, list) {
> > + if (!strcpy(format->name, candidate->name)) {
>
> This function is public to the rest of the kernel - limiting the
> lenght of ->name and using strncpy() is probably a good idea.
I totally agree. This was in fact something I wanted to do.
> > +
> > +/*
> > + * overlay_mgr_parse()
> > + *
> > + * Parse raw data with registered format parsers. Fills the candidate string if
> > + * one parser understood the raw data format.
> > + */
> > +int overlay_mgr_parse(struct device *dev, void *data, char ***candidates,
>
> I'm pretty sure there is another way to proceed than using 3 levels of
> references. It makes the code hard to read and a prime candidate for
> errors.
Sure. I guess we could allocate an array of fixed-length strings which
would be less flexible but I don't think we need something that flexible
here.
>
> > +
> > + format->parse(dev, data, candidates, n);
>
> ->parse() returns an error code. It is probably a good idea to check
> it. If it isn't needed then a comment explaining why it is the case
> would be appreciated.
So the point of the parse function is to determine if the data read from
a source is a compatible header of a given format. Returning an error
doesn't mean the header won't be recognized by another one.
We could maybe handle this better, by returning an error iif different
that -EINVAL. Or we could have one function to check the compatibility
and one to parse it, if compatible.
> > +static int _overlay_mgr_apply(struct device *dev, char *candidate)
> > +{
> > + struct overlay_mgr_overlay *overlay;
> > + struct device_node *node;
> > + const struct firmware *firmware;
> > + char *firmware_name;
> > + int err = 0;
> > +
> > + spin_lock(&overlay_mgr_lock);
> > +
> > + list_for_each_entry(overlay, &overlay_mgr_overlays, list) {
> > + if (!strcmp(overlay->name, candidate)) {
> > + dev_err(dev, "overlay already loaded\n");
> > + err = -EEXIST;
> > + goto err_lock;
> > + }
> > + }
> > +
> > + overlay = devm_kzalloc(dev, sizeof(*overlay), GFP_KERNEL);
>
> Function devm_kzalloc() can sleep but you're holding a spinlock - I'm
> surprised the kernel didn't complain here. Allocate the memory before
> holding the lock. If the overly is already loaded simply free it on
> the error path.
Right.
Thanks,
Antoine
--
Antoine Ténart, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 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 v2 2/4] dt-bindings: Add TI SCI PM Domains
From: Dave Gerlach @ 2016-10-27 14:07 UTC (permalink / raw)
To: Tero Kristo, Rob Herring
Cc: Nishanth Menon, devicetree, Ulf Hansson, Sudeep Holla,
Kevin Hilman, Keerthy, linux-pm, Rafael J . Wysocki, linux-kernel,
Russell King, Santosh Shilimkar, linux-arm-kernel
In-Reply-To: <8724b41d-158a-f52c-8df6-4e8913b4094a@ti.com>
On 10/27/2016 04:02 AM, Tero Kristo wrote:
> On 27/10/16 01:04, Rob Herring wrote:
>> On Wed, Oct 19, 2016 at 03:33:45PM -0500, Dave Gerlach wrote:
>>> Add a generic power domain implementation, TI SCI PM Domains, that
>>> will hook into the genpd framework and allow the TI SCI protocol to
>>> control device power states.
>>>
>>> Also, provide macros representing each device index as understood
>>> by TI SCI to be used in the device node power-domain references.
>>> These are identifiers for the K2G devices managed by the PMMC.
>>>
>>> Signed-off-by: Nishanth Menon <nm@ti.com>
>>> Signed-off-by: Dave Gerlach <d-gerlach@ti.com>
>>> ---
>>> .../devicetree/bindings/soc/ti/sci-pm-domain.txt | 54 +++++++++++++
>>> MAINTAINERS | 2 +
>>> include/dt-bindings/genpd/k2g.h | 90 ++++++++++++++++++++++
>>> 3 files changed, 146 insertions(+)
>>> create mode 100644 Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>> create mode 100644 include/dt-bindings/genpd/k2g.h
>>>
>>> diff --git a/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>> b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>> new file mode 100644
>>> index 000000000000..32f38a349656
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/soc/ti/sci-pm-domain.txt
>>> @@ -0,0 +1,54 @@
>>> +Texas Instruments TI-SCI Generic Power Domain
>>> +---------------------------------------------
>>> +
>>> +Some TI SoCs contain a system controller (like the PMMC, etc...) that is
>>> +responsible for controlling the state of the IPs that are present.
>>> +Communication between the host processor running an OS and the system
>>> +controller happens through a protocol known as TI-SCI [1]. This pm domain
>>> +implementation plugs into the generic pm domain framework and makes use of
>>> +the TI SCI protocol power on and off each device when needed.
>>> +
>>> +[1] Documentation/devicetree/bindings/arm/keystone/ti,sci.txt
>>> +
>>> +PM Domain Node
>>> +==============
>>> +The PM domain node represents the global PM domain managed by the PMMC,
>>> +which in this case is the single implementation as documented by the generic
>>> +PM domain bindings in Documentation/devicetree/bindings/power/power_domain.txt.
>>> +
>>> +Required Properties:
>>> +--------------------
>>> +- compatible: should be "ti,sci-pm-domain"
>>> +- #power-domain-cells: Must be 0.
>>> +- ti,sci: Phandle to the TI SCI device to use for managing the devices.
>>> +
>>> +Example:
>>> +--------------------
>>> +k2g_pds: k2g_pds {
>>> + compatible = "ti,sci-pm-domain";
>>> + #power-domain-cells = <0>;
>>> + ti,sci = <&pmmc>;
>>> +};
>>
>> Why not just make the PMMC node be the power-domain provider itself? If
>> not that, then make this a child node of it. The same comment applies to
>> all the SCI functions, but I guess I've already acked some of them.
>
> This seems to be a bug in this documentation actually. ti,sci handle is no
> longer supported, and all the sci stuff must be under the parent sci node.
>
>>
>> I really don't like reviewing all these TI SCI bindings one by one. Each
>> one on its own seems fine, but I don't see the full picture.
>
> The full picture is represented under the documentation for the main protocol
> support itself. See this patch:
>
> https://patchwork.kernel.org/patch/9383281/
>
> Copy pasted here as ref:
>
> Example (K2G):
> -------------
> pmmc: pmmc {
> compatible = "ti,k2g-sci";
> ...
>
> my_clk_node: clk_node {
> ...
> ...
> };
>
> my_pd_node: pd_node {
> ...
> ...
> };
> };
>
>
Yes my bad I will fix this in V3 once we straighten out the ID portion of the
binding.
Regards,
Dave
^ permalink raw reply
* Re: [PATCH 1/4] pinctrl: Introduce generic #pinctrl-cells and pinctrl_parse_index_with_args
From: Tony Lindgren @ 2016-10-27 14:11 UTC (permalink / raw)
To: Linus Walleij
Cc: Rob Herring, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Jon Hunter, Mark Rutland, Grygorii Strashko, Nishanth Menon,
linux-gpio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux-OMAP
In-Reply-To: <CACRpkdYWB4aa2XvW-yT39cKqy3XChSwCiFRy1mVWhbhse63=3A-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
* Linus Walleij <linus.walleij-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org> [161027 00:57]:
> On Tue, Oct 25, 2016 at 6:45 PM, Tony Lindgren <tony-4v6yS6AI5VpBDgjK7y7TUQ@public.gmane.org> wrote:
>
> I need some DT person to take a look at this binding and ACK it.
>
> > +For pin controller hardware with a large number of identical registers naming
> > +each bit both can be unmaintainable. Further there can be a large number of similar
> > +pinctrl hardware using the same registers for different purposes depending on the
> > +packaging. For cases like this, the pinctrl driver may use pinctrl-pin-array helper
> > +binding using a hardware based index and a number of configuration values:
>
> Maybe we can reword it a bit so that it is clear that this is an
> either-or approach
> for the pin controller, either they use the pins/groups/functions scheme
> or they use this scheme.
Sure, this is just an optional helper.
> > +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)
> > + ...
> > + >;
> > + };
> > + ...
> > +};
>
> Looks all right to me. Sad to add to the binding mess, but on the other
> hand, in the overall picture this nicely consolidates and structure
> pinctrl-single.
>
> > +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.
>
> OK
>
> > And we want to avoid another case of interrupt
> > +numbering with pinctrl numbering.
>
> Maybe this file is not a good place for making technical arguments,
> more describing what we agreed on, so cut that sentence IMO.
Sure :)
> > +/*
> > + * 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,
Tony
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI
From: Mark Rutland @ 2016-10-27 14:18 UTC (permalink / raw)
To: Leo Li
Cc: devicetree@vger.kernel.org, Marc Zyngier, Stuart Yoder,
linux-kernel@vger.kernel.org, M.H. Lian, Mingkai Hu, Scott Wood,
Shawn Guo, linux-arm-kernel@lists.infradead.org
In-Reply-To: <DB4PR04MB07813D4DA5D5567229394A7D8FAB0@DB4PR04MB0781.eurprd04.prod.outlook.com>
On Wed, Oct 26, 2016 at 10:09:07PM +0000, Leo Li wrote:
>
> > -----Original Message-----
> > From: Mark Rutland [mailto:mark.rutland@arm.com]
> > Sent: Wednesday, October 26, 2016 5:31 AM
> > To: M.H. Lian <minghuan.lian@nxp.com>
> > Cc: linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > devicetree@vger.kernel.org; Marc Zyngier <marc.zyngier@arm.com>; Stuart
> > Yoder <stuart.yoder@nxp.com>; Leo Li <leoyang.li@nxp.com>; Scott Wood
> > <scott.wood@nxp.com>; Shawn Guo <shawnguo@kernel.org>; Mingkai Hu
> > <mingkai.hu@nxp.com>
> > Subject: Re: [PATCH 1/6] dt/bindings: adjust bindings for Layerscape SCFG MSI
> >
> > On Tue, Oct 25, 2016 at 08:35:40PM +0800, Minghuan Lian wrote:
> > > -- compatible: should be "fsl,<soc-name>-msi" to identify
> > > - Layerscape PCIe MSI controller block such as:
> > > - "fsl,1s1021a-msi"
> > > - "fsl,1s1043a-msi"
> > > +- compatible: should be "fsl,ls-scfg-msi"
> >
> > This breaks old DTBs, and throws away information which you describe above as
> > valuable. So another NAK for that.
>
> I agree with you that we should maintain the backward compatibility.
> But on the other hand, I just found that there is a silly typo in the
> original binding that "ls" is wrongly spelled as "1s" and they look
> too close to be noticed in previous patch reviews. :(
Sure, that's annoying, but we're stuck with it.
> The driver and all the DTSes used the binding with the typo which
> covered up the problem. So even if we want to keep the
> "fsl,<soc-name>-msi" binding, we probably want to fix the typo, right?
> And that breaks the backward compatibility too.
Regardless of what we do, we should *not* break compatibility. The old
strings must remain.
However, we can *add* correctly-spelt variants, and mark the existing
strings as deprecated (in both the binding and driver). The in-kernel
dts can be updated to use the correctly-spelt strings.
Thanks,
Mark.
^ 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