* Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled
From: Thomas Bogendoerfer @ 2020-05-20 18:40 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Greg Kroah-Hartman, Arnd Bergmann, Rob Herring, linux-pm,
devicetree, Vincenzo Frascino, Thomas Gleixner, linux-mips,
linux-kernel
In-Reply-To: <20200520115926.lk6ycke75flwzcd2@mobilestation>
On Wed, May 20, 2020 at 02:59:26PM +0300, Serge Semin wrote:
> I think there is a misunderstanding here. In this patch I am not enabling
you are right, I've missed the fact, that this also needs to be enabled
in TLB entries. Strange that MIPS added the enable bit while R10k simply
do uncached acclerated, whenever TLB entry selects it.
> If there is no misunderstanding and you said what you said, that even enabling
> the feature for utilization might be dangerous, let's at least leave the
> MIPS_CONF_MM, MIPS_CONF_MM_FULL and MIPS_CONF_MM_SYS_SYSAD fields
> definition in the "arch/mips/include/asm/mipsregs.h" header. I'll use
> them to enable the write-merge in my platform code.
>
> What do you think?
I withdraw my concerns and will apply the patch as is.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH v2 18/20] mips: csrc-r4k: Decrease r4k-clocksource rating if CPU_FREQ enabled
From: Thomas Bogendoerfer @ 2020-05-20 18:30 UTC (permalink / raw)
To: Serge Semin
Cc: Serge Semin, Alexey Malahov, Paul Burton, Ralf Baechle,
Greg Kroah-Hartman, Arnd Bergmann, Rob Herring, linux-pm,
devicetree, Vincenzo Frascino, Thomas Gleixner, linux-mips,
linux-kernel
In-Reply-To: <20200520134826.pc6si3k6boaexp4i@mobilestation>
On Wed, May 20, 2020 at 04:48:26PM +0300, Serge Semin wrote:
> On Wed, May 20, 2020 at 03:38:27PM +0200, Thomas Bogendoerfer wrote:
> > On Wed, May 20, 2020 at 03:12:01PM +0300, Serge Semin wrote:
> > > Since you don't like the way I initially fixed it, suppose there we don't have
> > > another way but to introduce something like CONFIG_MIPS_CPS_NS16550_WIDTH
> > > parameter to select a proper accessors, like sw in our case, and sb by defaul).
> > > Right?
> >
> > to be on the safe side it's probably the best thing. But I don't know
> > enough about CPS_NS16550 to judge whether shift value correlates with
> > possible access width.
>
> The base address passed to the _mips_cps_putc() leaf is UART-base address. It
> has nothing to do with CPS. See:
ok, I'm confused. So this isn't an uart inside CPS hardware, but an uart used
by CPS code for debug output, right ?
To solve the issued please add CONFIG_MIPS_CPS_NS16550_WIDTH to select the
correct access width.
Thomas.
--
Crap can work. Given enough thrust pigs will fly, but it's not necessarily a
good idea. [ RFC1925, 2.3 ]
^ permalink raw reply
* Re: [PATCH] dt-bindings: input: touchscreen: edt-ft5x06: change reg property
From: Heiko Stübner @ 2020-05-20 18:41 UTC (permalink / raw)
To: Dmitry Torokhov
Cc: Johan Jonker, robh+dt, linux-input, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel
In-Reply-To: <20200520171324.GS89269@dtor-ws>
Hi Dmitry,
Am Mittwoch, 20. Mai 2020, 19:13:24 CEST schrieb Dmitry Torokhov:
> Hi Johan,
>
> On Wed, May 20, 2020 at 09:33:27AM +0200, Johan Jonker wrote:
> > A test with the command below gives this error:
> >
> > arch/arm/boot/dts/rk3188-bqedison2qc.dt.yaml:
> > touchscreen@3e: reg:0:0: 56 was expected
> >
> > The touchscreen chip on 'rk3188-bqedison2qc' and other BQ models
> > was shipped with different addresses then the binding currently allows.
> > Change the reg property that any address will pass.
> >
> > make ARCH=arm dtbs_check
> > DT_SCHEMA_FILES=Documentation/devicetree/bindings/input/touchscreen/
> > edt-ft5x06.yaml
> >
> > Signed-off-by: Johan Jonker <jbx6244@gmail.com>
> > ---
> > Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> > index 383d64a91..baa8e8f7e 100644
> > --- a/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> > +++ b/Documentation/devicetree/bindings/input/touchscreen/edt-ft5x06.yaml
> > @@ -42,7 +42,7 @@ properties:
> > - focaltech,ft6236
> >
> > reg:
> > - const: 0x38
> > + maxItems: 1
>
> Should we have a list of valid addresses instead of allowing any
> address? Controllers usually have only a couple of addresses that they
> support.
from what I've read, the fdt touchscreen controllers are just a generic
cpu with device-specific (or better panel-specific) firmware, which seems
to include the address as well - so it looks to be variable.
But of course that is only 2nd hand knowledge for me ;-)
But also, the i2c address is something you cannot really mess up,
either it is correct and your touchscreen works, or it isn't and and
adding entries to this list every time a new address variant pops up
feels clumsy.
Heiko
^ permalink raw reply
* Re: [PATCH v8 09/10] dt-bindings: interconnect: Add interconnect-tags bindings
From: Sibi Sankar @ 2020-05-20 18:51 UTC (permalink / raw)
To: Saravana Kannan
Cc: Rob Herring, Georgi Djakov, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki, Matthias Kaehlcke,
Rajendra Nayak, Bjorn Andersson, Vincent Guittot, Jordan Crouse,
Evan Green, Linux PM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
In-Reply-To: <CAGETcx8+NZYT863ySLf6XvgLBm8PM_4euue2=zbDscgmDFh+7g@mail.gmail.com>
On 2020-05-20 01:27, Saravana Kannan wrote:
> On Tue, May 19, 2020 at 11:58 AM Rob Herring <robh@kernel.org> wrote:
>>
>> On Tue, May 12, 2020 at 03:53:26PM +0300, Georgi Djakov wrote:
>> > From: Sibi Sankar <sibis@codeaurora.org>
>> >
>> > Add interconnect-tags bindings to enable passing of optional
>> > tag information to the interconnect framework.
>> >
>> > Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
>> > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
>> > ---
>> > v8:
>> > * New patch, picked from here:
>> > https://lore.kernel.org/r/20200504202243.5476-10-sibis@codeaurora.org
>> >
>> > .../devicetree/bindings/interconnect/interconnect.txt | 5 +++++
>> > 1 file changed, 5 insertions(+)
>> >
>> > diff --git a/Documentation/devicetree/bindings/interconnect/interconnect.txt b/Documentation/devicetree/bindings/interconnect/interconnect.txt
>> > index 6f5d23a605b7..c1a226a934e5 100644
>> > --- a/Documentation/devicetree/bindings/interconnect/interconnect.txt
>> > +++ b/Documentation/devicetree/bindings/interconnect/interconnect.txt
>> > @@ -55,6 +55,11 @@ interconnect-names : List of interconnect path name strings sorted in the same
>> > * dma-mem: Path from the device to the main memory of
>> > the system
>> >
>> > +interconnect-tags : List of interconnect path tags sorted in the same order as the
>> > + interconnects property. Consumers can append a specific tag to
>> > + the path and pass this information to the interconnect framework
>> > + to do aggregation based on the attached tag.
>>
>> Why isn't this information in the 'interconnect' arg cells?
>>
>> We have 'interconnect-names' because strings don't mix with cells. An
>> expanding list of 'interconnect-.*' is not a good pattern IMO.
Rob,
Currently the interconnect paths
assume a default tag and only few
icc paths require tags that differ
from the default ones. Encoding the
tags in the interconnect arg cells
would force all paths to specify
the tags. I guess that's okay.
>
> Also, is there an example for interconnect-tags that I missed? Is it a
> list of strings, numbers, etc?
Saravana,
https://patchwork.kernel.org/patch/11527589/
^^ is an example of interconnect-tag useage.
>
> -Saravana
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project.
^ permalink raw reply
* [PATCH v3 0/2] mfd: Add ENE KB3930 Embedded Controller driver
From: Lubomir Rintel @ 2020-05-20 18:59 UTC (permalink / raw)
To: Lee Jones; +Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel
Hi,
please consider applying the patches chained to this message. It's the
third version of the driver for the ENE KB3930 Embedded Controller.
This version attempts to address the issues pointed out in review of v2.
A more detailed change log it in the patch description of patch 2/2.
Thanks,
Lubo
^ permalink raw reply
* Re: [PATCH v8 09/10] dt-bindings: interconnect: Add interconnect-tags bindings
From: Saravana Kannan @ 2020-05-20 19:13 UTC (permalink / raw)
To: Sibi Sankar
Cc: Rob Herring, Georgi Djakov, Viresh Kumar, Nishanth Menon,
Stephen Boyd, Rafael J. Wysocki, Matthias Kaehlcke,
Rajendra Nayak, Bjorn Andersson, Vincent Guittot, Jordan Crouse,
Evan Green, Linux PM,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS, LKML
In-Reply-To: <3a392629be195fa6bebca18309efffab@codeaurora.org>
On Wed, May 20, 2020 at 11:51 AM Sibi Sankar <sibis@codeaurora.org> wrote:
>
> On 2020-05-20 01:27, Saravana Kannan wrote:
> > On Tue, May 19, 2020 at 11:58 AM Rob Herring <robh@kernel.org> wrote:
> >>
> >> On Tue, May 12, 2020 at 03:53:26PM +0300, Georgi Djakov wrote:
> >> > From: Sibi Sankar <sibis@codeaurora.org>
> >> >
> >> > Add interconnect-tags bindings to enable passing of optional
> >> > tag information to the interconnect framework.
> >> >
> >> > Signed-off-by: Sibi Sankar <sibis@codeaurora.org>
> >> > Signed-off-by: Georgi Djakov <georgi.djakov@linaro.org>
> >> > ---
> >> > v8:
> >> > * New patch, picked from here:
> >> > https://lore.kernel.org/r/20200504202243.5476-10-sibis@codeaurora.org
> >> >
> >> > .../devicetree/bindings/interconnect/interconnect.txt | 5 +++++
> >> > 1 file changed, 5 insertions(+)
> >> >
> >> > diff --git a/Documentation/devicetree/bindings/interconnect/interconnect.txt b/Documentation/devicetree/bindings/interconnect/interconnect.txt
> >> > index 6f5d23a605b7..c1a226a934e5 100644
> >> > --- a/Documentation/devicetree/bindings/interconnect/interconnect.txt
> >> > +++ b/Documentation/devicetree/bindings/interconnect/interconnect.txt
> >> > @@ -55,6 +55,11 @@ interconnect-names : List of interconnect path name strings sorted in the same
> >> > * dma-mem: Path from the device to the main memory of
> >> > the system
> >> >
> >> > +interconnect-tags : List of interconnect path tags sorted in the same order as the
> >> > + interconnects property. Consumers can append a specific tag to
> >> > + the path and pass this information to the interconnect framework
> >> > + to do aggregation based on the attached tag.
> >>
> >> Why isn't this information in the 'interconnect' arg cells?
> >>
> >> We have 'interconnect-names' because strings don't mix with cells. An
> >> expanding list of 'interconnect-.*' is not a good pattern IMO.
>
> Rob,
> Currently the interconnect paths
> assume a default tag and only few
> icc paths require tags that differ
> from the default ones. Encoding the
> tags in the interconnect arg cells
> would force all paths to specify
> the tags. I guess that's okay.
I think that's the right thing. Those cells are meant to be "args" to
the provider.
> >
> > Also, is there an example for interconnect-tags that I missed? Is it a
> > list of strings, numbers, etc?
>
> Saravana,
> https://patchwork.kernel.org/patch/11527589/
> ^^ is an example of interconnect-tag useage.
If we actually merge interconnect-tags, I think the doc should be
updated. Instead of having to grep around.
-Saravana
^ permalink raw reply
* [PATCH v3 1/2] dt-bindings: mfd: Add ENE KB3930 Embedded Controller binding
From: Lubomir Rintel @ 2020-05-20 18:59 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
Lubomir Rintel
In-Reply-To: <20200520185955.2144252-1-lkundrak@v3.sk>
Add binding document for the ENE KB3930 Embedded Controller.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
Changes since v1:
- Addressed binding validation failure
.../devicetree/bindings/mfd/ene-kb3930.yaml | 55 +++++++++++++++++++
1 file changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/mfd/ene-kb3930.yaml
diff --git a/Documentation/devicetree/bindings/mfd/ene-kb3930.yaml b/Documentation/devicetree/bindings/mfd/ene-kb3930.yaml
new file mode 100644
index 000000000000..005f5cb59ab1
--- /dev/null
+++ b/Documentation/devicetree/bindings/mfd/ene-kb3930.yaml
@@ -0,0 +1,55 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/mfd/ene-kb3930.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ENE KB3930 Embedded Controller bindings
+
+description: |
+ This binding describes the ENE KB3930 Embedded Controller attached to a
+ I2C bus.
+
+maintainers:
+ - Lubomir Rintel <lkundrak@v3.sk>
+
+properties:
+ compatible:
+ items:
+ - enum:
+ - dell,wyse-ariel-ec # Dell Wyse Ariel board (3020)
+ - const: ene,kb3930
+ reg:
+ maxItems: 1
+
+ off-gpios:
+ description: GPIO used with the shutdown protocol on Ariel
+ maxItems: 2
+
+ system-power-controller: true
+
+required:
+ - compatible
+ - reg
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ embedded-controller@58 {
+ compatible = "dell,wyse-ariel-ec", "ene,kb3930";
+ reg = <0x58>;
+ system-power-controller;
+
+ off-gpios = <&gpio 126 GPIO_ACTIVE_HIGH>,
+ <&gpio 127 GPIO_ACTIVE_HIGH>;
+ };
+ };
+
+...
--
2.26.2
^ permalink raw reply related
* [PATCH v3 2/2] mfd: ene-kb3930: Add driver for ENE KB3930 Embedded Controller
From: Lubomir Rintel @ 2020-05-20 18:59 UTC (permalink / raw)
To: Lee Jones
Cc: Rob Herring, Mark Rutland, devicetree, linux-kernel,
Lubomir Rintel
In-Reply-To: <20200520185955.2144252-1-lkundrak@v3.sk>
This driver provides access to the EC RAM of said embedded controller
attached to the I2C bus as well as optionally supporting its slightly weird
power-off/restart protocol.
A particular implementation of the EC firmware can be identified by a
model byte. If this driver identifies the Dell Ariel platform, it
registers the appropriate cells.
Signed-off-by: Lubomir Rintel <lkundrak@v3.sk>
---
Changes since v2:
- Sort the includes
- s/EC_MODEL_ID/EC_MODEL/
- Add a couple of clarifying comments
- Use #defines for values used in poweroff routine
- Remove priority from a restart notifier block
- s/priv/ddata/
- s/ec_ram/ram_regmap/ for the regmap name
- Fix the error handling when getting off gpios was not successful
- Remove a useless dev_info at the end of probe()
- Use i2c probe_new() callback, drop i2c_device_id
- Modify the logic in checking the model ID
drivers/mfd/Kconfig | 10 ++
drivers/mfd/Makefile | 1 +
drivers/mfd/ene-kb3930.c | 215 +++++++++++++++++++++++++++++++++++++++
3 files changed, 226 insertions(+)
create mode 100644 drivers/mfd/ene-kb3930.c
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index 0a59249198d3..dae18a2beab5 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -398,6 +398,16 @@ config MFD_DLN2
etc. must be enabled in order to use the functionality of
the device.
+config MFD_ENE_KB3930
+ tristate "ENE KB3930 Embedded Controller support"
+ depends on I2C
+ depends on MACH_MMP3_DT || COMPILE_TEST
+ select MFD_CORE
+ help
+ This adds support for accessing the registers on ENE KB3930, Embedded
+ Controller. Additional drivers such as LEDS_ARIEL must be enabled in
+ order to use the functionality of the device.
+
config MFD_EXYNOS_LPASS
tristate "Samsung Exynos SoC Low Power Audio Subsystem"
depends on ARCH_EXYNOS || COMPILE_TEST
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index f935d10cbf0f..2d2f5bc12841 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -14,6 +14,7 @@ obj-$(CONFIG_ARCH_BCM2835) += bcm2835-pm.o
obj-$(CONFIG_MFD_BCM590XX) += bcm590xx.o
obj-$(CONFIG_MFD_BD9571MWV) += bd9571mwv.o
obj-$(CONFIG_MFD_CROS_EC_DEV) += cros_ec_dev.o
+obj-$(CONFIG_MFD_ENE_KB3930) += ene-kb3930.o
obj-$(CONFIG_MFD_EXYNOS_LPASS) += exynos-lpass.o
obj-$(CONFIG_HTC_PASIC3) += htc-pasic3.o
diff --git a/drivers/mfd/ene-kb3930.c b/drivers/mfd/ene-kb3930.c
new file mode 100644
index 000000000000..0d44c4c524f0
--- /dev/null
+++ b/drivers/mfd/ene-kb3930.c
@@ -0,0 +1,215 @@
+// SPDX-License-Identifier: BSD-2-Clause OR GPL-2.0-or-later
+/*
+ * ENE KB3930 Embedded Controller Driver
+ *
+ * Copyright (C) 2020 Lubomir Rintel
+ */
+
+#include <linux/delay.h>
+#include <linux/gpio/consumer.h>
+#include <linux/i2c.h>
+#include <linux/mfd/core.h>
+#include <linux/module.h>
+#include <linux/reboot.h>
+#include <linux/regmap.h>
+
+/* I2C registers that are multiplexing access to the EC RAM. */
+enum {
+ EC_DATA_IN = 0x00,
+ EC_RAM_OUT = 0x80,
+ EC_RAM_IN = 0x81,
+};
+
+/* EC RAM registers. */
+enum {
+ EC_MODEL = 0x30,
+ EC_VERSION_MAJ = 0x31,
+ EC_VERSION_MIN = 0x32,
+};
+
+struct kb3930 {
+ struct i2c_client *client;
+ struct regmap *ram_regmap;
+ struct gpio_descs *off_gpios;
+};
+
+struct kb3930 *global_kb3930;
+
+#define EC_GPIO_WAVE 0
+#define EC_GPIO_OFF_MODE 1
+
+#define EC_OFF_MODE_REBOOT 0
+#define EC_OFF_MODE_POWER 1
+
+static void kb3930_off(struct kb3930 *ddata, int off_mode)
+{
+ gpiod_direction_output(ddata->off_gpios->desc[EC_GPIO_OFF_MODE],
+ off_mode);
+
+ /*
+ * The EC initiates a shutdown when it detects a 10 MHz wave, inspecting the
+ * other GPIO pin to decide whether it's supposed to turn the power off or
+ * reset the board.
+ */
+ while (1) {
+ mdelay(50);
+ gpiod_direction_output(ddata->off_gpios->desc[EC_GPIO_WAVE], 0);
+ mdelay(50);
+ gpiod_direction_output(ddata->off_gpios->desc[EC_GPIO_WAVE], 1);
+ }
+}
+
+static int kb3930_restart(struct notifier_block *this,
+ unsigned long mode, void *cmd)
+{
+ kb3930_off(global_kb3930, EC_OFF_MODE_REBOOT);
+ return NOTIFY_DONE;
+}
+
+static void kb3930_power_off(void)
+{
+ kb3930_off(global_kb3930, EC_OFF_MODE_POWER);
+}
+
+static struct notifier_block kb3930_restart_nb = {
+ .notifier_call = kb3930_restart,
+};
+
+static const struct mfd_cell ariel_ec_cells[] = {
+ { .name = "dell-wyse-ariel-led", },
+ { .name = "dell-wyse-ariel-power", },
+};
+
+static int kb3930_ec_ram_reg_write(void *context, unsigned int reg,
+ unsigned int val)
+{
+ struct kb3930 *ddata = context;
+
+ return i2c_smbus_write_word_data(ddata->client, EC_RAM_OUT,
+ (val << 8) | reg);
+}
+
+static int kb3930_ec_ram_reg_read(void *context, unsigned int reg,
+ unsigned int *val)
+{
+ struct kb3930 *ddata = context;
+ int ret;
+
+ ret = i2c_smbus_write_word_data(ddata->client, EC_RAM_IN, reg);
+ if (ret < 0)
+ return ret;
+
+ ret = i2c_smbus_read_word_data(ddata->client, EC_DATA_IN);
+ if (ret < 0)
+ return ret;
+
+ *val = ret >> 8;
+ return 0;
+}
+
+static const struct regmap_config kb3930_ram_regmap_config = {
+ .name = "ec_ram",
+ .reg_bits = 8,
+ .val_bits = 8,
+ .reg_stride = 1,
+ .max_register = 0xff,
+ .reg_write = kb3930_ec_ram_reg_write,
+ .reg_read = kb3930_ec_ram_reg_read,
+ .fast_io = false,
+};
+
+static int kb3930_probe(struct i2c_client *client)
+{
+ struct device *dev = &client->dev;
+ struct device_node *np = dev->of_node;
+ struct kb3930 *ddata;
+ unsigned int model;
+ int ret;
+
+ if (global_kb3930)
+ return -EEXIST;
+
+ ddata = devm_kzalloc(dev, sizeof(*ddata), GFP_KERNEL);
+ if (!ddata)
+ return -ENOMEM;
+
+ global_kb3930 = ddata;
+ ddata->client = client;
+ i2c_set_clientdata(client, ddata);
+
+ ddata->ram_regmap = devm_regmap_init(dev, NULL, ddata,
+ &kb3930_ram_regmap_config);
+ if (IS_ERR(ddata->ram_regmap))
+ return PTR_ERR(ddata->ram_regmap);
+
+ ret = regmap_read(ddata->ram_regmap, EC_MODEL, &model);
+ if (ret < 0)
+ return ret;
+
+ /* Currently we only support the cells present on Dell Ariel model. */
+ if (model != 'J') {
+ dev_err(dev, "unknown board model: %02x\n", model);
+ return -ENODEV;
+ }
+
+ /* These are the cells valid for model == 'J' only. */
+ ret = devm_mfd_add_devices(dev, PLATFORM_DEVID_AUTO,
+ ariel_ec_cells,
+ ARRAY_SIZE(ariel_ec_cells),
+ NULL, 0, NULL);
+ if (ret < 0)
+ return ret;
+
+ if (of_property_read_bool(np, "system-power-controller")) {
+ ddata->off_gpios =
+ devm_gpiod_get_array_optional(dev, "off", GPIOD_IN);
+ if (IS_ERR(ddata->off_gpios))
+ return PTR_ERR(ddata->off_gpios);
+ if (ddata->off_gpios->ndescs < 2) {
+ dev_err(dev, "invalid off-gpios property\n");
+ return -EINVAL;
+ }
+ }
+ if (ddata->off_gpios) {
+ register_restart_handler(&kb3930_restart_nb);
+ if (pm_power_off == NULL)
+ pm_power_off = kb3930_power_off;
+ }
+
+ return 0;
+}
+
+static int kb3930_remove(struct i2c_client *client)
+{
+ struct kb3930 *ddata = i2c_get_clientdata(client);
+
+ if (ddata->off_gpios) {
+ if (pm_power_off == kb3930_power_off)
+ pm_power_off = NULL;
+ unregister_restart_handler(&kb3930_restart_nb);
+ }
+ global_kb3930 = NULL;
+
+ return 0;
+}
+
+static const struct of_device_id kb3930_dt_ids[] = {
+ { .compatible = "ene,kb3930" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, kb3930_dt_ids);
+
+static struct i2c_driver kb3930_driver = {
+ .probe_new = kb3930_probe,
+ .remove = kb3930_remove,
+ .driver = {
+ .name = "ene-kb3930",
+ .of_match_table = of_match_ptr(kb3930_dt_ids),
+ },
+};
+
+module_i2c_driver(kb3930_driver);
+
+MODULE_AUTHOR("Lubomir Rintel <lkundrak@v3.sk>");
+MODULE_DESCRIPTION("ENE KB3930 Embedded Controller Driver");
+MODULE_LICENSE("Dual BSD/GPL");
--
2.26.2
^ permalink raw reply related
* Re: [PATCH net-next v2 3/4] dt-bindings: net: Add RGMII internal delay for DP83869
From: Andrew Lunn @ 2020-05-20 19:27 UTC (permalink / raw)
To: Dan Murphy
Cc: Florian Fainelli, hkallweit1, davem, netdev, linux-kernel,
devicetree
In-Reply-To: <7e117c01-fa6e-45f3-05b7-4efe7a3c1943@ti.com>
Hi Dan
> UGH I think I just got volunteered to do make them common.
There is code you can copy from PHY drivers. :-)
What would be kind of nice is if the validate was in the core as
well. Pass a list of possible delays in pS, and it will do a
phydev_err() if what is in DT does not match one of the listed
delays. Take a look around at what current drivers do and see if you
can find a nice abstraction which will work for a few drivers. We
cannot easily convert existing drivers without breaking DT, but a
design which works in theory for what we currently have has a good
chance of working for any new PHY driver.
Andrew
^ permalink raw reply
* Re: [PATCH v7 11/11] media: platform: Add jpeg dec/enc feature
From: Tomasz Figa @ 2020-05-20 19:40 UTC (permalink / raw)
To: Xia Jiang
Cc: Hans Verkuil, Mauro Carvalho Chehab, Rob Herring,
Matthias Brugger, Rick Chang, linux-media, devicetree,
linux-kernel, linux-arm-kernel, linux-mediatek, Marek Szyprowski,
srv_heupstream
In-Reply-To: <1589020095.24163.150.camel@mhfsdcap03>
Hi Xia,
On Sat, May 09, 2020 at 06:28:15PM +0800, Xia Jiang wrote:
> On Fri, 2020-05-01 at 17:37 +0000, Tomasz Figa wrote:
> > Hi Xia,
> >
> > On Thu, Apr 16, 2020 at 12:03:15PM +0800, Xia Jiang wrote:
> > > On Fri, 2020-03-06 at 20:23 +0900, Tomasz Figa wrote:
> > > > Hi Xia,
> > > >
> > > > On Tue, Mar 03, 2020 at 08:34:46PM +0800, Xia Jiang wrote:
> > > > > Add mtk jpeg encode v4l2 driver based on jpeg decode, because that jpeg
> > > > > decode and encode have great similarities with function operation.
> > > >
> > > > Thank you for the patch. Please see my comments inline.
> > >
> > > Dear Tomasz,
> > >
> > > Thank you for your reply. I have followed your advice and submited v8
> > > version patch.
> > >
> > > Please check my reply below.
> Dear Tomasz,
> I have some confuse about your advice, please check my reply below.
Sorry for the late reply again. Please see my reply inline.
> > [snip]
> > > >
> > > > >
> > > > > - switch (s->target) {
> > > > > - case V4L2_SEL_TGT_COMPOSE:
> > > > > - s->r.left = 0;
> > > > > - s->r.top = 0;
> > > > > - ctx->out_q.w = s->r.width;
> > > > > - ctx->out_q.h = s->r.height;
> > > > > - break;
> > > > > - default:
> > > > > - return -EINVAL;
> > > > > + switch (s->target) {
> > > > > + case V4L2_SEL_TGT_CROP:
> > > > > + s->r.left = 0;
> > > > > + s->r.top = 0;
> > > > > + ctx->out_q.w = s->r.width;
> > > > > + ctx->out_q.h = s->r.height;
> > > >
> > > > What happens if the userspace provides a value bigger than current format?
> > > we need get the min value of userspace value and current value,changed
> > > it like this:
> > > ctx->out_q.w = min(s->r.width, ctx->out_q.w);
> > > ctx->out_q.h = min(s->r.height,ctx->out_q.h);
> >
> > Since ctx->out_q is modified by this function, wouldn't that cause
> > problems if S_SELECTION was called two times, first with a smaller
> > rectangle and then with a bigger one? We should store the active crop
> > and format separately and use the latter for min().
> Add a member variable(struct v4l2_rect) in out_q structure for storing
> the active crop, like this:
> s->r.width = min(s->r.width, ctx->out_q.w);
> s->r.height = min(s->r.height,ctx->out_q.h);
> ctx->out_q.rect.width = s->r.width;
> ctx->out_q.rect.height = s->r.height;
> Is that ok?
Yes. I'd call it crop_rect and it can be simplified further into:
ct->out_q.crop_rect = s->r;
> >
> > [snip]
> > > > >
> > > > > while ((vb = mtk_jpeg_buf_remove(ctx, q->type)))
> > > > > v4l2_m2m_buf_done(vb, VB2_BUF_STATE_ERROR);
> > > > > @@ -772,6 +1011,45 @@ static int mtk_jpeg_set_dec_dst(struct mtk_jpeg_ctx *ctx,
> > > > > return 0;
> > > > > }
> > > > >
> > > > > +static void mtk_jpeg_set_enc_dst(struct mtk_jpeg_ctx *ctx, void __iomem *base,
> > > > > + struct vb2_buffer *dst_buf,
> > > > > + struct mtk_jpeg_enc_bs *bs)
> > > > > +{
> > > > > + bs->dma_addr = vb2_dma_contig_plane_dma_addr(dst_buf, 0);
> > > > > + bs->dma_addr_offset = ctx->enable_exif ? MTK_JPEG_DEFAULT_EXIF_SIZE : 0;
> > > >
> > > > Could you explain what is the meaning of the dma_addr_offset and where the
> > > > default EXIF size comes from? Also, how is the encoder output affected by
> > > > the enable_exif flag?
> > > If enabled the exif mode, the real output will be filled at the locaiton
> > > of dst_addr+ dma_addr_offset(exif size).The dma_addr_offset will be
> > > filled by the application.
> > > The default exif size is setted as constant value 64k according to the
> > > spec.(Exif metadata are restricted in size to 64kB in JPEG images
> > > because according to the specification this information must be
> > > contained within a signed JPEG APP1 segment)
> >
> > Okay, thanks. Then it sounds like MTK_JPEG_MAX_EXIF_SIZE could be a more
> > appropriate name.
> >
> > [snip]
> > > > > +}
> > > > > +
> > > > > static void mtk_jpeg_device_run(void *priv)
> > > > > {
> > > > > struct mtk_jpeg_ctx *ctx = priv;
> > > > > @@ -782,6 +1060,8 @@ static void mtk_jpeg_device_run(void *priv)
> > > > > struct mtk_jpeg_src_buf *jpeg_src_buf;
> > > > > struct mtk_jpeg_bs bs;
> > > > > struct mtk_jpeg_fb fb;
> > > > > + struct mtk_jpeg_enc_bs enc_bs;
> > > > > + struct mtk_jpeg_enc_fb enc_fb;
> > > > > int i;
> > > > >
> > > > > src_buf = v4l2_m2m_next_src_buf(ctx->fh.m2m_ctx);
> > > > > @@ -792,30 +1072,47 @@ static void mtk_jpeg_device_run(void *priv)
> > > > > for (i = 0; i < dst_buf->vb2_buf.num_planes; i++)
> > > > > vb2_set_plane_payload(&dst_buf->vb2_buf, i, 0);
> > > > > buf_state = VB2_BUF_STATE_DONE;
> > > >
> > > > About existing code, but we may want to explain this.
> > > > What is this last frame handling above for?
> > > if the user gives us a empty buffer(means it is the last frame),the
> > > driver will not encode and done the buffer to the user.
> > >
> >
> > An empty buffer is not a valid way of signaling a last frame in V4L2. In
> > general, I'm not sure there is such a thing in JPEG, because all frames
> > are separate from each other and we always expect 1 input buffer and 1
> > output buffer for one frame. We might want to remove the special
> > handling in a follow up patch.
> How does application to end jpeg operation in motion jpeg if we remove
> this? I tryed to end with the condition that the input number equals
> output number in UT, and is ok.
That's correct. The operation ends when the number of CAPTURE buffers
dequeued is the same as the number of OUTPUT buffers queued.
> >
> > > > > - goto dec_end;
> > > > > + goto device_run_end;
> > > > > }
> > > > >
> > > > > - if (mtk_jpeg_check_resolution_change(ctx, &jpeg_src_buf->dec_param)) {
> > > > > - mtk_jpeg_queue_src_chg_event(ctx);
> > > > > - ctx->state = MTK_JPEG_SOURCE_CHANGE;
> > > > > - v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > > > > - return;
> > > > > - }
> > > > > + if (jpeg->mode == MTK_JPEG_ENC) {
> > > > > + spin_lock_irqsave(&jpeg->hw_lock, flags);
> > > > > + mtk_jpeg_enc_reset(jpeg->reg_base);
> > > >
> > > > Why do we need to reset every frame?
> > > We do this operation is to ensure that all registers are cleared.
> > > It's safer from the hardware point of view.
> >
> > Wouldn't this only waste power? If we reset the hardware after powering
> > up, the only registers that could change would be changed by the driver
> > itself. The driver should program all registers properly when starting
> > next frame anyway, so such a reset shouldn't be necessary.
> I confirmed with hardware designer again that we need to reset every
> frame. If we do not do like this, unexpected mistakes may occur.
Okay, thanks for double checking. Please add a comment to the code that it
is a hardware requirement.
> >
> > > >
> > > > > +
> > > > > + mtk_jpeg_set_enc_dst(ctx, jpeg->reg_base, &dst_buf->vb2_buf,
> > > > > + &enc_bs);
> > > > > + mtk_jpeg_set_enc_src(ctx, jpeg->reg_base, &src_buf->vb2_buf,
> > > > > + &enc_fb);
> > > > > + mtk_jpeg_enc_set_ctrl_cfg(jpeg->reg_base, ctx->enable_exif,
> > > > > + ctx->enc_quality,
> > > > > + ctx->restart_interval);
> > > > > +
> > > > > + mtk_jpeg_enc_start(jpeg->reg_base);
> > > > > + } else {
> > > > > + if (mtk_jpeg_check_resolution_change
> > > > > + (ctx, &jpeg_src_buf->dec_param)) {
> > > > > + mtk_jpeg_queue_src_chg_event(ctx);
> > > > > + ctx->state = MTK_JPEG_SOURCE_CHANGE;
> > > > > + v4l2_m2m_job_finish(jpeg->m2m_dev, ctx->fh.m2m_ctx);
> > > >
> > > > This is a bit strange. Resolution change should be signaled when the
> > > > hardware attempted to decode a frame and detected a different resolution
> > > > than current. It shouldn't be necessary for the userspace to queue a pair
> > > > of buffers to signal it, as with the current code.
> > > If the the resolution is bigger than current, the current buffer will
> > > not be enough for the changed resolution.Shouldn't it tell the userspace
> > > to queue new buffer and stream on again?
> >
> > The V4L2 decode flow is as follows:
> > - application configures and starts only the OUTPUT queue,
> > - application queues an OUTPUT buffer with a frame worth of bitstream,
> > - decoder parses the bitstream headers, detects CAPTURE format and
> > signals the source change event,
> > - application reads CAPTURE format and configures and starts the
> > CAPTURE queue,
> > - application queues a CAPTURE buffer,
> > - decoder decodes the image to the queued buffer.
> >
> > In case of subsequent (dynamic) resolution change:
> > - application queues an OUTPUT buffer and a CAPTURE buffer,
> > - decoder parses the bitstream, notices resolution change, updates
> > CAPTURE format and signals the source change event, refusing to
> > continue the decoding until the application acknowledges it,
> > - application either reallocates its CAPTURE buffers or confirms that
> > the existing buffers are fine and acknowledges resolution change,
> > - decoding continues.
> >
> > For more details, please check the interface specification:
> > https://www.kernel.org/doc/html/latest/media/uapi/v4l/dev-decoder.html
> >
> I tryed to move this operation from device_run() to
> mtk_jpeg_dec_buf_queue(),but have a problem in motion jpeg.For example,I
> queued three buffers continuously,the third buffer has resolution
> change(bigger than the second buffer),but the capture buffer used in
> device run didn't changed.
> How do we handle this case?
Sorry, I think I misread the driver code. It looks like there is a code
that parses the JPEG header from the source buffer called from
mtk_jpeg_dec_buf_queue() and that is the moment the driver detects the new
resolution. Then it only signals the event once all the previously queued
frames have been decoded, i.e. when the first new resolution frame gets to
device_run(). I think the current code should be fine then. Sorry for
confusion again!
> > [snip]
> > > > > - ret = video_register_device(jpeg->dec_vdev, VFL_TYPE_GRABBER, 3);
> > > > > + ret = video_register_device(jpeg->vfd_jpeg, VFL_TYPE_GRABBER, -1);
> > > >
> > > > FYI the type changed to VFL_TYPE_VIDEO recently.
> > > I changed VFL_TYPE_GRABBER to VFL_TYPE_VIDEO,but builded fail.
> >
> > What kernel version are you building with?
> I build it with the latest kernel 5.7,but builed fail again.
That's strange. There is no VFL_TYPE_GRABBER in 5.7 anymore:
https://elixir.bootlin.com/linux/v5.7-rc6/source/include/media/v4l2-dev.h#L24
Best regards,
Tomasz
^ permalink raw reply
* [PATCH v3 0/5] iio: imu: bmi160: added regulator and mount-matrix support
From: Jonathan Albrieux @ 2020-05-20 19:46 UTC (permalink / raw)
To: linux-kernel
Cc: ~postmarketos/upstreaming, daniel.baluta, Jonathan Albrieux,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler
v3:
- separate typo fix into another patch
- clean up of documentation
- clean up of patch messages
v2:
- fixed missing description for iio: imu: bmi160: added regulator
support
https://lore.kernel.org/linux-iio/20200519075111.6356-1-jonathan.albrieux@gmail.com/
v1:
- initial patch submission
https://lore.kernel.org/linux-iio/20200518133358.18978-1-jonathan.albrieux@gmail.com/
Convert txt format documentation to yaml.
Add documentation about vdd-supply, vddio-supply and mount-matrix.
Add vdd-supply and vddio-supply support.
Add mount-matrix binding support. As chip could have different
orientations a mount matrix support is needed to correctly translate
these differences.
Jonathan Albrieux (5):
dt-bindings: iio: imu: bmi160: convert txt format to yaml
dt-bindings: iio: imu: bmi160: add regulators and mount-matrix
iio: imu: bmi160: fix typo
iio: imu: bmi160: added regulator support
iio: imu: bmi160: added mount-matrix support
.../devicetree/bindings/iio/imu/bmi160.txt | 37 --------
.../bindings/iio/imu/bosch,bmi160.yaml | 92 +++++++++++++++++++
drivers/iio/imu/bmi160/bmi160.h | 3 +
drivers/iio/imu/bmi160/bmi160_core.c | 46 +++++++++-
4 files changed, 140 insertions(+), 38 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
--
2.17.1
^ permalink raw reply
* [PATCH v3 1/5] dt-bindings: iio: imu: bmi160: convert txt format to yaml
From: Jonathan Albrieux @ 2020-05-20 19:46 UTC (permalink / raw)
To: linux-kernel
Cc: ~postmarketos/upstreaming, daniel.baluta, Jonathan Albrieux,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Jonathan Cameron, Rob Herring
In-Reply-To: <20200520194656.16218-1-jonathan.albrieux@gmail.com>
Converts documentation from txt format to yaml.
Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
.../devicetree/bindings/iio/imu/bmi160.txt | 37 ---------
.../bindings/iio/imu/bosch,bmi160.yaml | 76 +++++++++++++++++++
2 files changed, 76 insertions(+), 37 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/iio/imu/bmi160.txt
create mode 100644 Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
diff --git a/Documentation/devicetree/bindings/iio/imu/bmi160.txt b/Documentation/devicetree/bindings/iio/imu/bmi160.txt
deleted file mode 100644
index 900c169de00f..000000000000
--- a/Documentation/devicetree/bindings/iio/imu/bmi160.txt
+++ /dev/null
@@ -1,37 +0,0 @@
-Bosch BMI160 - Inertial Measurement Unit with Accelerometer, Gyroscope
-and externally connectable Magnetometer
-
-https://www.bosch-sensortec.com/bst/products/all_products/bmi160
-
-Required properties:
- - compatible : should be "bosch,bmi160"
- - reg : the I2C address or SPI chip select number of the sensor
- - spi-max-frequency : set maximum clock frequency (only for SPI)
-
-Optional properties:
- - interrupts : interrupt mapping for IRQ
- - interrupt-names : set to "INT1" if INT1 pin should be used as interrupt
- input, set to "INT2" if INT2 pin should be used instead
- - drive-open-drain : set if the specified interrupt pin should be configured as
- open drain. If not set, defaults to push-pull.
-
-Examples:
-
-bmi160@68 {
- compatible = "bosch,bmi160";
- reg = <0x68>;
-
- interrupt-parent = <&gpio4>;
- interrupts = <12 IRQ_TYPE_EDGE_RISING>;
- interrupt-names = "INT1";
-};
-
-bmi160@0 {
- compatible = "bosch,bmi160";
- reg = <0>;
- spi-max-frequency = <10000000>;
-
- interrupt-parent = <&gpio2>;
- interrupts = <12 IRQ_TYPE_LEVEL_LOW>;
- interrupt-names = "INT2";
-};
diff --git a/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml b/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
new file mode 100644
index 000000000000..46cb4fde1165
--- /dev/null
+++ b/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
@@ -0,0 +1,76 @@
+# SPDX-License-Identifier: (GPL-2.0 OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/iio/imu/bosch,bmi160.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Bosch BMI160
+
+maintainers:
+ - Daniel Baluta <daniel.baluta@nxp.com> (?)
+
+description: |
+ Inertial Measurement Unit with Accelerometer, Gyroscope and externally
+ connectable Magnetometer
+ https://www.bosch-sensortec.com/bst/products/all_products/bmi160
+
+properties:
+ compatible:
+ const: bosch,bmi160
+
+ reg:
+ maxItems: 1
+
+ spi-max-frequency:
+ maxItems: 1
+
+ interrupts:
+ maxItems: 1
+
+ interrupt-names:
+ enum:
+ - INT1
+ - INT2
+ description: |
+ set to "INT1" if INT1 pin should be used as interrupt input, set
+ to "INT2" if INT2 pin should be used instead
+
+ drive-open-drain:
+ description: |
+ set if the specified interrupt pin should be configured as
+ open drain. If not set, defaults to push-pull.
+
+required:
+ - compatible
+ - reg
+
+examples:
+ - |
+ // Example for I2C
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ bmi160@68 {
+ compatible = "bosch,bmi160";
+ reg = <0x68>;
+ interrupt-parent = <&gpio4>;
+ interrupts = <12 1>;
+ interrupt-names = "INT1";
+ };
+ };
+ - |
+ // Example for SPI
+ spi {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ bmi160@0 {
+ compatible = "bosch,bmi160";
+ reg = <0>;
+ spi-max-frequency = <10000000>;
+ interrupt-parent = <&gpio2>;
+ interrupts = <12 1>;
+ interrupt-names = "INT2";
+ };
+ };
--
2.17.1
^ permalink raw reply related
* [PATCH v3 2/5] dt-bindings: iio: imu: bmi160: add regulators and mount-matrix
From: Jonathan Albrieux @ 2020-05-20 19:46 UTC (permalink / raw)
To: linux-kernel
Cc: ~postmarketos/upstreaming, daniel.baluta, Jonathan Albrieux,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Jonathan Cameron, Rob Herring
In-Reply-To: <20200520194656.16218-1-jonathan.albrieux@gmail.com>
Add vdd-supply and vddio-supply support.
Add mount-matrix support.
Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
.../bindings/iio/imu/bosch,bmi160.yaml | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml b/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
index 46cb4fde1165..1a1b1a14aa2e 100644
--- a/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
+++ b/Documentation/devicetree/bindings/iio/imu/bosch,bmi160.yaml
@@ -40,6 +40,17 @@ properties:
set if the specified interrupt pin should be configured as
open drain. If not set, defaults to push-pull.
+ vdd-supply:
+ maxItems: 1
+ description: provide VDD power to the sensor.
+
+ vddio-supply:
+ maxItems: 1
+ description: provide VDD IO power to the sensor.
+
+ mount-matrix:
+ description: an optional 3x3 mounting rotation matrix
+
required:
- compatible
- reg
@@ -54,9 +65,14 @@ examples:
bmi160@68 {
compatible = "bosch,bmi160";
reg = <0x68>;
+ vdd-supply = <&pm8916_l17>;
+ vddio-supply = <&pm8916_l6>;
interrupt-parent = <&gpio4>;
interrupts = <12 1>;
interrupt-names = "INT1";
+ mount-matrix = "0", "1", "0",
+ "-1", "0", "0",
+ "0", "0", "1";
};
};
- |
--
2.17.1
^ permalink raw reply related
* [PATCH v3 3/5] iio: imu: bmi160: fix typo
From: Jonathan Albrieux @ 2020-05-20 19:46 UTC (permalink / raw)
To: linux-kernel
Cc: ~postmarketos/upstreaming, daniel.baluta, Jonathan Albrieux,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Jonathan Cameron
In-Reply-To: <20200520194656.16218-1-jonathan.albrieux@gmail.com>
Fix a typo in MODULE_AUTHOR() argument.
Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
drivers/iio/imu/bmi160/bmi160_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index 6af65d6f1d28..77b05bd4a2b2 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -853,6 +853,6 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
}
EXPORT_SYMBOL_GPL(bmi160_core_probe);
-MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com");
+MODULE_AUTHOR("Daniel Baluta <daniel.baluta@intel.com>");
MODULE_DESCRIPTION("Bosch BMI160 driver");
MODULE_LICENSE("GPL v2");
--
2.17.1
^ permalink raw reply related
* [PATCH v3 4/5] iio: imu: bmi160: added regulator support
From: Jonathan Albrieux @ 2020-05-20 19:46 UTC (permalink / raw)
To: linux-kernel
Cc: ~postmarketos/upstreaming, daniel.baluta, Jonathan Albrieux,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Jonathan Cameron
In-Reply-To: <20200520194656.16218-1-jonathan.albrieux@gmail.com>
Add vdd-supply and vddio-supply support.
Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
drivers/iio/imu/bmi160/bmi160.h | 2 ++
drivers/iio/imu/bmi160/bmi160_core.c | 24 ++++++++++++++++++++++++
2 files changed, 26 insertions(+)
diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h
index 621f5309d735..923c3b274fde 100644
--- a/drivers/iio/imu/bmi160/bmi160.h
+++ b/drivers/iio/imu/bmi160/bmi160.h
@@ -3,10 +3,12 @@
#define BMI160_H_
#include <linux/iio/iio.h>
+#include <linux/regulator/consumer.h>
struct bmi160_data {
struct regmap *regmap;
struct iio_trigger *trig;
+ struct regulator_bulk_data supplies[2];
};
extern const struct regmap_config bmi160_regmap_config;
diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index 77b05bd4a2b2..d3316ca02fbd 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -15,6 +15,7 @@
#include <linux/delay.h>
#include <linux/irq.h>
#include <linux/of_irq.h>
+#include <linux/regulator/consumer.h>
#include <linux/iio/iio.h>
#include <linux/iio/triggered_buffer.h>
@@ -709,6 +710,12 @@ static int bmi160_chip_init(struct bmi160_data *data, bool use_spi)
unsigned int val;
struct device *dev = regmap_get_device(data->regmap);
+ ret = regulator_bulk_enable(ARRAY_SIZE(data->supplies), data->supplies);
+ if (ret) {
+ dev_err(dev, "Failed to enable regulators: %d\n", ret);
+ return ret;
+ }
+
ret = regmap_write(data->regmap, BMI160_REG_CMD, BMI160_CMD_SOFTRESET);
if (ret)
return ret;
@@ -793,9 +800,16 @@ int bmi160_probe_trigger(struct iio_dev *indio_dev, int irq, u32 irq_type)
static void bmi160_chip_uninit(void *data)
{
struct bmi160_data *bmi_data = data;
+ struct device *dev = regmap_get_device(bmi_data->regmap);
+ int ret;
bmi160_set_mode(bmi_data, BMI160_GYRO, false);
bmi160_set_mode(bmi_data, BMI160_ACCEL, false);
+
+ ret = regulator_bulk_disable(ARRAY_SIZE(bmi_data->supplies),
+ bmi_data->supplies);
+ if (ret)
+ dev_err(dev, "Failed to disable regulators: %d\n", ret);
}
int bmi160_core_probe(struct device *dev, struct regmap *regmap,
@@ -815,6 +829,16 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
dev_set_drvdata(dev, indio_dev);
data->regmap = regmap;
+ data->supplies[0].supply = "vdd";
+ data->supplies[1].supply = "vddio";
+ ret = devm_regulator_bulk_get(dev,
+ ARRAY_SIZE(data->supplies),
+ data->supplies);
+ if (ret) {
+ dev_err(dev, "Failed to get regulators: %d\n", ret);
+ return ret;
+ }
+
ret = bmi160_chip_init(data, use_spi);
if (ret)
return ret;
--
2.17.1
^ permalink raw reply related
* [PATCH v3 5/5] iio: imu: bmi160: added mount-matrix support
From: Jonathan Albrieux @ 2020-05-20 19:46 UTC (permalink / raw)
To: linux-kernel
Cc: ~postmarketos/upstreaming, daniel.baluta, Jonathan Albrieux,
open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS,
Hartmut Knaack, Lars-Peter Clausen,
open list:IIO SUBSYSTEM AND DRIVERS, Peter Meerwald-Stadler,
Jonathan Cameron
In-Reply-To: <20200520194656.16218-1-jonathan.albrieux@gmail.com>
Add mount-matrix binding support. As chip could have different orientations
a mount matrix support is needed to correctly translate these differences.
Signed-off-by: Jonathan Albrieux <jonathan.albrieux@gmail.com>
---
drivers/iio/imu/bmi160/bmi160.h | 1 +
drivers/iio/imu/bmi160/bmi160_core.c | 20 ++++++++++++++++++++
2 files changed, 21 insertions(+)
diff --git a/drivers/iio/imu/bmi160/bmi160.h b/drivers/iio/imu/bmi160/bmi160.h
index 923c3b274fde..a82e040bd109 100644
--- a/drivers/iio/imu/bmi160/bmi160.h
+++ b/drivers/iio/imu/bmi160/bmi160.h
@@ -9,6 +9,7 @@ struct bmi160_data {
struct regmap *regmap;
struct iio_trigger *trig;
struct regulator_bulk_data supplies[2];
+ struct iio_mount_matrix orientation;
};
extern const struct regmap_config bmi160_regmap_config;
diff --git a/drivers/iio/imu/bmi160/bmi160_core.c b/drivers/iio/imu/bmi160/bmi160_core.c
index d3316ca02fbd..26d586daee26 100644
--- a/drivers/iio/imu/bmi160/bmi160_core.c
+++ b/drivers/iio/imu/bmi160/bmi160_core.c
@@ -110,6 +110,7 @@
.storagebits = 16, \
.endianness = IIO_LE, \
}, \
+ .ext_info = bmi160_ext_info, \
}
/* scan indexes follow DATA register order */
@@ -265,6 +266,20 @@ static const struct bmi160_odr_item bmi160_odr_table[] = {
},
};
+static const struct iio_mount_matrix *
+bmi160_get_mount_matrix(const struct iio_dev *indio_dev,
+ const struct iio_chan_spec *chan)
+{
+ struct bmi160_data *data = iio_priv(indio_dev);
+
+ return &data->orientation;
+}
+
+static const struct iio_chan_spec_ext_info bmi160_ext_info[] = {
+ IIO_MOUNT_MATRIX(IIO_SHARED_BY_DIR, bmi160_get_mount_matrix),
+ { }
+};
+
static const struct iio_chan_spec bmi160_channels[] = {
BMI160_CHANNEL(IIO_ACCEL, X, BMI160_SCAN_ACCEL_X),
BMI160_CHANNEL(IIO_ACCEL, Y, BMI160_SCAN_ACCEL_Y),
@@ -839,6 +854,11 @@ int bmi160_core_probe(struct device *dev, struct regmap *regmap,
return ret;
}
+ ret = iio_read_mount_matrix(dev, "mount-matrix",
+ &data->orientation);
+ if (ret)
+ return ret;
+
ret = bmi160_chip_init(data, use_spi);
if (ret)
return ret;
--
2.17.1
^ permalink raw reply related
* Re: [PATCH net-next v2 3/4] dt-bindings: net: Add RGMII internal delay for DP83869
From: Dan Murphy @ 2020-05-20 20:02 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, hkallweit1, davem, netdev, linux-kernel,
devicetree
In-Reply-To: <20200520192719.GK652285@lunn.ch>
Andrew
On 5/20/20 2:27 PM, Andrew Lunn wrote:
> Hi Dan
>
>> UGH I think I just got volunteered to do make them common.
> There is code you can copy from PHY drivers. :-)
>
> What would be kind of nice is if the validate was in the core as
> well. Pass a list of possible delays in pS, and it will do a
> phydev_err() if what is in DT does not match one of the listed
> delays. Take a look around at what current drivers do and see if you
> can find a nice abstraction which will work for a few drivers. We
> cannot easily convert existing drivers without breaking DT, but a
> design which works in theory for what we currently have has a good
> chance of working for any new PHY driver.
I think adding it in the core would be a bit of a challenge. I think
each PHY driver needs to handle parsing and validating this property on
its own (like fifo-depth). It is a PHY specific setting.
Take the DP83867/9 and the ADIN1200/ADIN1300.
The 8386X devices has a delta granularity of 250pS and the AD devices is
200pS per each setting
And the 867/9 has 3x more values (15) vs only 5 for the AD PHY.
And the Atheros AR803x PHY does use rgmii-id in the yaml, which I guess
is what you were pointing out, that if set the PHY uses a default 2nS
delay and it is not configurable.
Same with the Broadcomm.
Ack to not changing already existing drivers which is only 2 the AD PHY
and the DP83867 PHY. But I can update the yaml for the 83867 and mark
the TI specific properties as deprecated in favor of the new properties
like I did with fifo-depth.
Dan
> Andrew
^ permalink raw reply
* Re: [PATCH v6 01/11] PCI: designware-ep: Add multiple PFs support for DWC
From: Rob Herring @ 2020-05-20 20:32 UTC (permalink / raw)
To: Xiaowei Bao
Cc: roy.zang, robh+dt, linux-kernel, Zhiqiang.Hou, jingoohan1,
devicetree, gustavo.pimentel, Minghuan.Lian, mingkai.hu,
linux-arm-kernel, leoyang.li, shawnguo, andrew.murray, linux-pci,
bhelgaas, lorenzo.pieralisi, kishon, linuxppc-dev, amurray
In-Reply-To: <20200314033038.24844-2-xiaowei.bao@nxp.com>
On Sat, 14 Mar 2020 11:30:28 +0800, Xiaowei Bao wrote:
> Add multiple PFs support for DWC, due to different PF have different
> config space, we use func_conf_select callback function to access
> the different PF's config space, the different chip company need to
> implement this callback function when use the DWC IP core and intend
> to support multiple PFs feature.
>
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> Acked-by: Gustavo Pimentel <gustavo.pimentel@synopsys.com>
> ---
> v2:
> - Remove duplicate redundant code.
> - Reimplement the PF config space access way.
> v3:
> - Integrate duplicate code for func_select.
> - Move PCIE_ATU_FUNC_NUM(pf) (pf << 20) to ((pf) << 20).
> - Add the comments for func_conf_select function.
> v4:
> - Correct the commit message.
> v5:
> - No change.
> v6:
> - No change.
>
> drivers/pci/controller/dwc/pcie-designware-ep.c | 123 ++++++++++++++++--------
> drivers/pci/controller/dwc/pcie-designware.c | 59 ++++++++----
> drivers/pci/controller/dwc/pcie-designware.h | 18 +++-
> 3 files changed, 142 insertions(+), 58 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] dt-bindings: net: Add RGMII internal delay for DP83869
From: Andrew Lunn @ 2020-05-20 20:44 UTC (permalink / raw)
To: Dan Murphy
Cc: Florian Fainelli, hkallweit1, davem, netdev, linux-kernel,
devicetree
In-Reply-To: <0bba1378-0847-491f-8f21-ac939ac48820@ti.com>
> I think adding it in the core would be a bit of a challenge. I think each
> PHY driver needs to handle parsing and validating this property on its own
> (like fifo-depth). It is a PHY specific setting.
fifo-depth yes. But some delays follow a common pattern.
e.g.
Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
Device Tree Value Delay Pad Skew Register Value
-----------------------------------------------------
0 -840ps 0000
200 -720ps 0001
400 -600ps 0010
600 -480ps 0011
800 -360ps 0100
1000 -240ps 0101
1200 -120ps 0110
1400 0ps 0111
1600 120ps 1000
1800 240ps 1001
2000 360ps 1010
2200 480ps 1011
2400 600ps 1100
2600 720ps 1101
2800 840ps 1110
3000 960ps 1111
Documentation/devicetree/bindings/net/adi,adin.yaml
adi,rx-internal-delay-ps:
description: |
RGMII RX Clock Delay used only when PHY operates in RGMII mode with
internal delay (phy-mode is 'rgmii-id' or 'rgmii-rxid') in pico-seconds.
enum: [ 1600, 1800, 2000, 2200, 2400 ]
default: 2000
adi,tx-internal-delay-ps:
description: |
RGMII TX Clock Delay used only when PHY operates in RGMII mode with
internal delay (phy-mode is 'rgmii-id' or 'rgmii-txid') in pico-seconds.
enum: [ 1600, 1800, 2000, 2200, 2400 ]
default: 2000
Documentation/devicetree/bindings/net/apm-xgene-enet.txt
- tx-delay: Delay value for RGMII bridge TX clock.
Valid values are between 0 to 7, that maps to
417, 717, 1020, 1321, 1611, 1913, 2215, 2514 ps
Default value is 4, which corresponds to 1611 ps
- rx-delay: Delay value for RGMII bridge RX clock.
Valid values are between 0 to 7, that maps to
273, 589, 899, 1222, 1480, 1806, 2147, 2464 ps
Default value is 2, which corresponds to 899 ps
You could implement checking against a table of valid values, which is
something you need for your PHY. You could even consider making it a
2D table, and return the register value, not the delay?
Andrew
^ permalink raw reply
* Re: [PATCH v6 04/11] PCI: designware-ep: Modify MSI and MSIX CAP way of finding
From: Rob Herring @ 2020-05-20 20:45 UTC (permalink / raw)
To: Xiaowei Bao
Cc: Zhiqiang.Hou, Minghuan.Lian, mingkai.hu, bhelgaas, shawnguo,
leoyang.li, kishon, lorenzo.pieralisi, roy.zang, amurray,
jingoohan1, gustavo.pimentel, andrew.murray, linux-pci,
devicetree, linux-kernel, linux-arm-kernel, linuxppc-dev
In-Reply-To: <20200314033038.24844-5-xiaowei.bao@nxp.com>
On Sat, Mar 14, 2020 at 11:30:31AM +0800, Xiaowei Bao wrote:
> Each PF of EP device should have it's own MSI or MSIX capabitily
s/it's/its/
> struct, so create a dw_pcie_ep_func struct and remove the msi_cap
> and msix_cap to this struct from dw_pcie_ep, and manage the PFs
> with a list.
>
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> ---
> v3:
> - This is a new patch, to fix the issue of MSI and MSIX CAP way of
> finding.
> v4:
> - Correct some word of commit message.
> v5:
> - No change.
> v6:
> - Fix up the compile error.
>
> drivers/pci/controller/dwc/pcie-designware-ep.c | 135 +++++++++++++++++++++---
> drivers/pci/controller/dwc/pcie-designware.h | 18 +++-
> 2 files changed, 134 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/pci/controller/dwc/pcie-designware-ep.c b/drivers/pci/controller/dwc/pcie-designware-ep.c
> index 933bb89..fb915f2 100644
> --- a/drivers/pci/controller/dwc/pcie-designware-ep.c
> +++ b/drivers/pci/controller/dwc/pcie-designware-ep.c
> @@ -19,6 +19,19 @@ void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> pci_epc_linkup(epc);
> }
>
> +struct dw_pcie_ep_func *
> +dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
> +{
> + struct dw_pcie_ep_func *ep_func;
> +
> + list_for_each_entry(ep_func, &ep->func_list, list) {
> + if (ep_func->func_no == func_no)
> + return ep_func;
> + }
> +
> + return NULL;
> +}
> +
> static unsigned int dw_pcie_ep_func_select(struct dw_pcie_ep *ep, u8 func_no)
> {
> unsigned int func_offset = 0;
> @@ -59,6 +72,47 @@ void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> __dw_pcie_ep_reset_bar(pci, func_no, bar, 0);
> }
>
> +static u8 __dw_pcie_ep_find_next_cap(struct dw_pcie_ep *ep, u8 func_no,
> + u8 cap_ptr, u8 cap)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + unsigned int func_offset = 0;
> + u8 cap_id, next_cap_ptr;
> + u16 reg;
> +
> + if (!cap_ptr)
> + return 0;
> +
> + func_offset = dw_pcie_ep_func_select(ep, func_no);
> +
> + reg = dw_pcie_readw_dbi(pci, func_offset + cap_ptr);
> + cap_id = (reg & 0x00ff);
> +
> + if (cap_id > PCI_CAP_ID_MAX)
> + return 0;
> +
> + if (cap_id == cap)
> + return cap_ptr;
> +
> + next_cap_ptr = (reg & 0xff00) >> 8;
> + return __dw_pcie_ep_find_next_cap(ep, func_no, next_cap_ptr, cap);
> +}
> +
> +static u8 dw_pcie_ep_find_capability(struct dw_pcie_ep *ep, u8 func_no, u8 cap)
> +{
> + struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + unsigned int func_offset = 0;
> + u8 next_cap_ptr;
> + u16 reg;
> +
> + func_offset = dw_pcie_ep_func_select(ep, func_no);
> +
> + reg = dw_pcie_readw_dbi(pci, func_offset + PCI_CAPABILITY_LIST);
> + next_cap_ptr = (reg & 0x00ff);
> +
> + return __dw_pcie_ep_find_next_cap(ep, func_no, next_cap_ptr, cap);
> +}
> +
> static int dw_pcie_ep_write_header(struct pci_epc *epc, u8 func_no,
> struct pci_epf_header *hdr)
> {
> @@ -246,13 +300,18 @@ static int dw_pcie_ep_get_msi(struct pci_epc *epc, u8 func_no)
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> u32 val, reg;
> unsigned int func_offset = 0;
> + struct dw_pcie_ep_func *ep_func;
>
> - if (!ep->msi_cap)
> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> + if (!ep_func)
> + return -EINVAL;
> +
> + if (!ep_func->msi_cap)
> return -EINVAL;
if (!ep_func || !ep_func->msi_cap)
return -EINVAL;
>
> func_offset = dw_pcie_ep_func_select(ep, func_no);
>
> - reg = ep->msi_cap + func_offset + PCI_MSI_FLAGS;
> + reg = ep_func->msi_cap + func_offset + PCI_MSI_FLAGS;
> val = dw_pcie_readw_dbi(pci, reg);
> if (!(val & PCI_MSI_FLAGS_ENABLE))
> return -EINVAL;
> @@ -268,13 +327,18 @@ static int dw_pcie_ep_set_msi(struct pci_epc *epc, u8 func_no, u8 interrupts)
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> u32 val, reg;
> unsigned int func_offset = 0;
> + struct dw_pcie_ep_func *ep_func;
> +
> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> + if (!ep_func)
> + return -EINVAL;
>
> - if (!ep->msi_cap)
> + if (!ep_func->msi_cap)
> return -EINVAL;
Same here.
>
> func_offset = dw_pcie_ep_func_select(ep, func_no);
>
> - reg = ep->msi_cap + func_offset + PCI_MSI_FLAGS;
> + reg = ep_func->msi_cap + func_offset + PCI_MSI_FLAGS;
> val = dw_pcie_readw_dbi(pci, reg);
> val &= ~PCI_MSI_FLAGS_QMASK;
> val |= (interrupts << 1) & PCI_MSI_FLAGS_QMASK;
> @@ -291,13 +355,18 @@ static int dw_pcie_ep_get_msix(struct pci_epc *epc, u8 func_no)
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> u32 val, reg;
> unsigned int func_offset = 0;
> + struct dw_pcie_ep_func *ep_func;
> +
> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> + if (!ep_func)
> + return -EINVAL;
>
> - if (!ep->msix_cap)
> + if (!ep_func->msix_cap)
> return -EINVAL;
Same here for msix.
>
> func_offset = dw_pcie_ep_func_select(ep, func_no);
>
> - reg = ep->msix_cap + func_offset + PCI_MSIX_FLAGS;
> + reg = ep_func->msix_cap + func_offset + PCI_MSIX_FLAGS;
> val = dw_pcie_readw_dbi(pci, reg);
> if (!(val & PCI_MSIX_FLAGS_ENABLE))
> return -EINVAL;
> @@ -313,13 +382,18 @@ static int dw_pcie_ep_set_msix(struct pci_epc *epc, u8 func_no, u16 interrupts)
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> u32 val, reg;
> unsigned int func_offset = 0;
> + struct dw_pcie_ep_func *ep_func;
>
> - if (!ep->msix_cap)
> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> + if (!ep_func)
> + return -EINVAL;
> +
> + if (!ep_func->msix_cap)
> return -EINVAL;
And here.
>
> func_offset = dw_pcie_ep_func_select(ep, func_no);
>
> - reg = ep->msix_cap + func_offset + PCI_MSIX_FLAGS;
> + reg = ep_func->msix_cap + func_offset + PCI_MSIX_FLAGS;
> val = dw_pcie_readw_dbi(pci, reg);
> val &= ~PCI_MSIX_FLAGS_QSIZE;
> val |= interrupts;
> @@ -404,6 +478,7 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> u8 interrupt_num)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct dw_pcie_ep_func *ep_func;
> struct pci_epc *epc = ep->epc;
> unsigned int aligned_offset;
> unsigned int func_offset = 0;
> @@ -413,25 +488,29 @@ int dw_pcie_ep_raise_msi_irq(struct dw_pcie_ep *ep, u8 func_no,
> bool has_upper;
> int ret;
>
> - if (!ep->msi_cap)
> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> + if (!ep_func)
> + return -EINVAL;
> +
> + if (!ep_func->msi_cap)
> return -EINVAL;
And here.
>
> func_offset = dw_pcie_ep_func_select(ep, func_no);
>
> /* Raise MSI per the PCI Local Bus Specification Revision 3.0, 6.8.1. */
> - reg = ep->msi_cap + func_offset + PCI_MSI_FLAGS;
> + reg = ep_func->msi_cap + func_offset + PCI_MSI_FLAGS;
> msg_ctrl = dw_pcie_readw_dbi(pci, reg);
> has_upper = !!(msg_ctrl & PCI_MSI_FLAGS_64BIT);
> - reg = ep->msi_cap + func_offset + PCI_MSI_ADDRESS_LO;
> + reg = ep_func->msi_cap + func_offset + PCI_MSI_ADDRESS_LO;
> msg_addr_lower = dw_pcie_readl_dbi(pci, reg);
> if (has_upper) {
> - reg = ep->msi_cap + func_offset + PCI_MSI_ADDRESS_HI;
> + reg = ep_func->msi_cap + func_offset + PCI_MSI_ADDRESS_HI;
> msg_addr_upper = dw_pcie_readl_dbi(pci, reg);
> - reg = ep->msi_cap + func_offset + PCI_MSI_DATA_64;
> + reg = ep_func->msi_cap + func_offset + PCI_MSI_DATA_64;
> msg_data = dw_pcie_readw_dbi(pci, reg);
> } else {
> msg_addr_upper = 0;
> - reg = ep->msi_cap + func_offset + PCI_MSI_DATA_32;
> + reg = ep_func->msi_cap + func_offset + PCI_MSI_DATA_32;
> msg_data = dw_pcie_readw_dbi(pci, reg);
> }
> aligned_offset = msg_addr_lower & (epc->mem->page_size - 1);
> @@ -467,6 +546,7 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> u16 interrupt_num)
> {
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> + struct dw_pcie_ep_func *ep_func;
> struct pci_epc *epc = ep->epc;
> u16 tbl_offset, bir;
> unsigned int func_offset = 0;
> @@ -477,9 +557,16 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> void __iomem *msix_tbl;
> int ret;
>
> + ep_func = dw_pcie_ep_get_func_from_ep(ep, func_no);
> + if (!ep_func)
> + return -EINVAL;
> +
> + if (!ep_func->msix_cap)
> + return -EINVAL;
And here.
> +
> func_offset = dw_pcie_ep_func_select(ep, func_no);
>
> - reg = ep->msix_cap + func_offset + PCI_MSIX_TABLE;
> + reg = ep_func->msix_cap + func_offset + PCI_MSIX_TABLE;
> tbl_offset = dw_pcie_readl_dbi(pci, reg);
> bir = (tbl_offset & PCI_MSIX_TABLE_BIR);
> tbl_offset &= PCI_MSIX_TABLE_OFFSET;
> @@ -558,6 +645,7 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> int i;
> int ret;
> u32 reg;
> + u8 func_no;
> void *addr;
> u8 hdr_type;
> unsigned int nbars;
> @@ -566,6 +654,9 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> struct dw_pcie *pci = to_dw_pcie_from_ep(ep);
> struct device *dev = pci->dev;
> struct device_node *np = dev->of_node;
> + struct dw_pcie_ep_func *ep_func;
> +
> + INIT_LIST_HEAD(&ep->func_list);
>
> if (!pci->dbi_base || !pci->dbi_base2) {
> dev_err(dev, "dbi_base/dbi_base2 is not populated\n");
> @@ -632,9 +723,19 @@ int dw_pcie_ep_init(struct dw_pcie_ep *ep)
> if (ret < 0)
> epc->max_functions = 1;
>
> - ep->msi_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSI);
> + for (func_no = 0; func_no < epc->max_functions; func_no++) {
> + ep_func = devm_kzalloc(dev, sizeof(*ep_func), GFP_KERNEL);
Why do you need a list if you allocate all the functions at once? You
could just do an array. Or do the allocations as needed and keep the
list. I assume all functions aren't always used.
> + if (!ep_func)
> + return -ENOMEM;
>
> - ep->msix_cap = dw_pcie_find_capability(pci, PCI_CAP_ID_MSIX);
> + ep_func->func_no = func_no;
> + ep_func->msi_cap = dw_pcie_ep_find_capability(ep, func_no,
> + PCI_CAP_ID_MSI);
> + ep_func->msix_cap = dw_pcie_ep_find_capability(ep, func_no,
> + PCI_CAP_ID_MSIX);
> +
> + list_add_tail(&ep_func->list, &ep->func_list);
> + }
>
> if (ep->ops->ep_init)
> ep->ops->ep_init(ep);
> diff --git a/drivers/pci/controller/dwc/pcie-designware.h b/drivers/pci/controller/dwc/pcie-designware.h
> index cb32afa..dd9b7b4 100644
> --- a/drivers/pci/controller/dwc/pcie-designware.h
> +++ b/drivers/pci/controller/dwc/pcie-designware.h
> @@ -230,8 +230,16 @@ struct dw_pcie_ep_ops {
> unsigned int (*func_conf_select)(struct dw_pcie_ep *ep, u8 func_no);
> };
>
> +struct dw_pcie_ep_func {
> + struct list_head list;
> + u8 func_no;
> + u8 msi_cap; /* MSI capability offset */
> + u8 msix_cap; /* MSI-X capability offset */
> +};
> +
> struct dw_pcie_ep {
> struct pci_epc *epc;
> + struct list_head func_list;
> const struct dw_pcie_ep_ops *ops;
> phys_addr_t phys_base;
> size_t addr_size;
> @@ -244,8 +252,6 @@ struct dw_pcie_ep {
> u32 num_ob_windows;
> void __iomem *msi_mem;
> phys_addr_t msi_mem_phys;
> - u8 msi_cap; /* MSI capability offset */
> - u8 msix_cap; /* MSI-X capability offset */
> };
>
> struct dw_pcie_ops {
> @@ -437,6 +443,8 @@ int dw_pcie_ep_raise_msix_irq(struct dw_pcie_ep *ep, u8 func_no,
> int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep, u8 func_no,
> u16 interrupt_num);
> void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar);
> +struct dw_pcie_ep_func *
> +dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no);
> #else
> static inline void dw_pcie_ep_linkup(struct dw_pcie_ep *ep)
> {
> @@ -478,5 +486,11 @@ static inline int dw_pcie_ep_raise_msix_irq_doorbell(struct dw_pcie_ep *ep,
> static inline void dw_pcie_ep_reset_bar(struct dw_pcie *pci, enum pci_barno bar)
> {
> }
> +
> +static inline struct dw_pcie_ep_func *
> +dw_pcie_ep_get_func_from_ep(struct dw_pcie_ep *ep, u8 func_no)
> +{
> + return NULL;
> +}
> #endif
> #endif /* _PCIE_DESIGNWARE_H */
> --
> 2.9.5
>
^ permalink raw reply
* Re: [PATCH v6 07/11] PCI: layerscape: Modify the way of getting capability with different PEX
From: Rob Herring @ 2020-05-20 20:45 UTC (permalink / raw)
To: Xiaowei Bao
Cc: gustavo.pimentel, linuxppc-dev, kishon, amurray, shawnguo,
linux-kernel, leoyang.li, bhelgaas, lorenzo.pieralisi, devicetree,
roy.zang, linux-pci, linux-arm-kernel, robh+dt, Zhiqiang.Hou,
jingoohan1, andrew.murray, mingkai.hu, Minghuan.Lian
In-Reply-To: <20200314033038.24844-8-xiaowei.bao@nxp.com>
On Sat, 14 Mar 2020 11:30:34 +0800, Xiaowei Bao wrote:
> The different PCIe controller in one board may be have different
> capability of MSI or MSIX, so change the way of getting the MSI
> capability, make it more flexible.
>
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> ---
> v2:
> - Remove the repeated assignment code.
> v3:
> - Use ep_func msi_cap and msix_cap to decide the msi_capable and
> msix_capable of pci_epc_features struct.
> v4:
> - No change.
> v5:
> - No change.
> v6:
> - No change.
>
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 31 +++++++++++++++++++-------
> 1 file changed, 23 insertions(+), 8 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v6 09/11] PCI: layerscape: Add EP mode support for ls1088a and ls2088a
From: Rob Herring @ 2020-05-20 20:50 UTC (permalink / raw)
To: Xiaowei Bao
Cc: lorenzo.pieralisi, linux-kernel, Minghuan.Lian, linuxppc-dev,
linux-arm-kernel, Zhiqiang.Hou, linux-pci, devicetree, shawnguo,
robh+dt, mingkai.hu, gustavo.pimentel, jingoohan1, andrew.murray,
bhelgaas, kishon, roy.zang, amurray, leoyang.li
In-Reply-To: <20200314033038.24844-10-xiaowei.bao@nxp.com>
On Sat, 14 Mar 2020 11:30:36 +0800, Xiaowei Bao wrote:
> Add PCIe EP mode support for ls1088a and ls2088a, there are some
> difference between LS1 and LS2 platform, so refactor the code of
> the EP driver.
>
> Signed-off-by: Xiaowei Bao <xiaowei.bao@nxp.com>
> ---
> v2:
> - This is a new patch for supporting the ls1088a and ls2088a platform.
> v3:
> - Adjust the some struct assignment order in probe function.
> v4:
> - No change.
> v5:
> - No change.
> v6:
> - No change.
>
> drivers/pci/controller/dwc/pci-layerscape-ep.c | 72 +++++++++++++++++++-------
> 1 file changed, 53 insertions(+), 19 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH net-next v2 3/4] dt-bindings: net: Add RGMII internal delay for DP83869
From: Dan Murphy @ 2020-05-20 20:55 UTC (permalink / raw)
To: Andrew Lunn
Cc: Florian Fainelli, hkallweit1, davem, netdev, linux-kernel,
devicetree
In-Reply-To: <20200520204423.GA677363@lunn.ch>
Andrew
On 5/20/20 3:44 PM, Andrew Lunn wrote:
>> I think adding it in the core would be a bit of a challenge. I think each
>> PHY driver needs to handle parsing and validating this property on its own
>> (like fifo-depth). It is a PHY specific setting.
> fifo-depth yes. But some delays follow a common pattern.
>
> e.g.
> Documentation/devicetree/bindings/net/micrel-ksz90x1.txt
>
> Device Tree Value Delay Pad Skew Register Value
> -----------------------------------------------------
> 0 -840ps 0000
> 200 -720ps 0001
> 400 -600ps 0010
> 600 -480ps 0011
> 800 -360ps 0100
> 1000 -240ps 0101
> 1200 -120ps 0110
> 1400 0ps 0111
> 1600 120ps 1000
> 1800 240ps 1001
> 2000 360ps 1010
> 2200 480ps 1011
> 2400 600ps 1100
> 2600 720ps 1101
> 2800 840ps 1110
> 3000 960ps 1111
>
> Documentation/devicetree/bindings/net/adi,adin.yaml
>
> adi,rx-internal-delay-ps:
> description: |
> RGMII RX Clock Delay used only when PHY operates in RGMII mode with
> internal delay (phy-mode is 'rgmii-id' or 'rgmii-rxid') in pico-seconds.
> enum: [ 1600, 1800, 2000, 2200, 2400 ]
> default: 2000
>
> adi,tx-internal-delay-ps:
> description: |
> RGMII TX Clock Delay used only when PHY operates in RGMII mode with
> internal delay (phy-mode is 'rgmii-id' or 'rgmii-txid') in pico-seconds.
> enum: [ 1600, 1800, 2000, 2200, 2400 ]
> default: 2000
>
> Documentation/devicetree/bindings/net/apm-xgene-enet.txt
>
> - tx-delay: Delay value for RGMII bridge TX clock.
> Valid values are between 0 to 7, that maps to
> 417, 717, 1020, 1321, 1611, 1913, 2215, 2514 ps
> Default value is 4, which corresponds to 1611 ps
> - rx-delay: Delay value for RGMII bridge RX clock.
> Valid values are between 0 to 7, that maps to
> 273, 589, 899, 1222, 1480, 1806, 2147, 2464 ps
> Default value is 2, which corresponds to 899 ps
>
> You could implement checking against a table of valid values, which is
> something you need for your PHY. You could even consider making it a
> 2D table, and return the register value, not the delay?
So provide a helper function that will just basically parse an array and
return the indexed value.
The outlier here is the AD device since the index to value is not 1-1
mapping. Not sure we need a 2D table like the AD driver.
I actually implemented this in the dp83869 a bit ago and have done this
in a few other non-PHY drivers.
I guess I can look at making it a utility function in the networking space.
Dan
>
> Andrew
^ permalink raw reply
* Re: [PATCH v7 1/5] PCI: Don't disable decoding when mmio_always_on is set
From: Rob Herring @ 2020-05-20 20:55 UTC (permalink / raw)
To: Jiaxun Yang
Cc: Huacai Chen, linux-mips, linux-kernel, devicetree,
Thomas Bogendoerfer, Lorenzo Pieralisi, linux-pci, Rob Herring,
Paul Burton, Bjorn Helgaas
In-Reply-To: <20200428011429.1852081-2-jiaxun.yang@flygoat.com>
On Tue, 28 Apr 2020 09:14:16 +0800, Jiaxun Yang wrote:
> Don't disable MEM/IO decoding when a device have both non_compliant_bars
> and mmio_always_on.
>
> That would allow us quirk devices with junk in BARs but can't disable
> their decoding.
>
> Signed-off-by: Jiaxun Yang <jiaxun.yang@flygoat.com>
> ---
> drivers/pci/probe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
Acked-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 01/14] PCI: cadence: Fix cdns_pcie_{host|ep}_setup() error path
From: Rob Herring @ 2020-05-20 20:59 UTC (permalink / raw)
To: Kishon Vijay Abraham I
Cc: devicetree, linux-arm-kernel, Tom Joseph, Arnd Bergmann,
Bjorn Helgaas, Greg Kroah-Hartman, linux-omap, Lorenzo Pieralisi,
Rob Herring, linux-pci, linux-kernel
In-Reply-To: <20200506151429.12255-2-kishon@ti.com>
On Wed, 6 May 2020 20:44:16 +0530, Kishon Vijay Abraham I wrote:
> commit bd22885aa188 ("PCI: cadence: Refactor driver to use as a core
> library") while refactoring the Cadence PCIe driver to be used as
> library, removed pm_runtime_get_sync() from cdns_pcie_ep_setup()
> and cdns_pcie_host_setup() but missed to remove the corresponding
> pm_runtime_put_sync() in the error path. Fix it here.
>
> Fixes: bd22885aa188 ("PCI: cadence: Refactor driver to use as a core library")
> Signed-off-by: Kishon Vijay Abraham I <kishon@ti.com>
> ---
> drivers/pci/controller/cadence/pcie-cadence-ep.c | 9 ++-------
> drivers/pci/controller/cadence/pcie-cadence-host.c | 6 +-----
> 2 files changed, 3 insertions(+), 12 deletions(-)
>
Reviewed-by: Rob Herring <robh@kernel.org>
^ 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