Re: [ANNOUNCE] 5.10.214-rt106

[Pavel Machek]

Hi!

> The tag is there in the repository:
> 
> 
> https://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git/commit/?h=v5.10.214-rt106-rebase
> 
> When I released 5.10.215-rt107 (and its -rebase counterpart), 
> 5.10.214-rt106-rebase
> was no longer pointing to a commit inside that  branch, probably why your git
> update didn't get the tag. You could try a
> 
> git fetch --tags 

Aha, thanks for the pointer and sorry for the noise.

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: PGP signature

Re: [ANNOUNCE] 5.10.214-rt106

[Pavel Machek]

Hi!

> I'm pleased to announce the 5.10.214-rt106 stable release.
> 
> This release is simply an update to the new stable 5.10.214 version and no
> RT-specific changes have been performed.
> 
> You can get this release via the git tree at:
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/rt/linux-stable-rt.git
> 
>   branch: v5.10-rt
>   Head SHA1: 3d208061796d4addeb543c78e0a4ec769b6ce6b2

Thank you.

Could you also push v5.10.214-rt106-rebase tag to the repository for
consistency?

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: PGP signature

Re: [ANNOUNCE] 5.10.213-rt105

[Pavel Machek]

Hi!

> I'm pleased to announce the 5.10.213-rt105 stable release.
> 
> This release is an update to the new stable 5.10.213 version and no extra
> changes have been performed.

Thanks for release.

I see v5.10.214-rt106-rc1 is out there and now v5.10.215-rt107-rc1,
but I don't see v5.10.214-rt106 (which would be useful to me).

Is it a mistake or should I plan for v5.10.215-rt107?

Thanks and best regards,
Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

Re: [PATCHv3 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge

[Pavel Machek]

Hi!

> > This is driver for ANX7688 USB-C HDMI, with flashing and debugging
> > features removed. ANX7688 is rather criticial piece on PinePhone,
> > there's no display and no battery charging without it.
> > 
> > There's likely more work to be done here, but having basic support
> > in mainline is needed to be able to work on the other stuff
> > (networking, cameras, power management).
> > 
> > Signed-off-by: Ondrej Jirman 
> > Co-developed-by: Martijn Braam 
> > Co-developed-by: Samuel Holland 
> > Signed-off-by: Pavel Machek 
> 
> Just couple of quick comments below - I did not have time to go over
> this very thoroughly, but I think you need to make a new version in
> any case because of comments in 1/2.

Yes, there will be new version.

There is ton of sleep primitives, and existing ones are okay. I can
change everything to fdelay or whatever primitive-of-the-day is, but
I'd rather not do pointless changes.

You can ask for major changes, or complain about extra newlines, but
doing both in the same email does not make sense.

> Btw, Co-developed-by comes before Signed-off-by I think.

I believe this order is fine, too.

> > +++ b/drivers/usb/typec/anx7688.c
> > @@ -0,0 +1,1830 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * ANX7688 USB-C HDMI bridge/PD driver
> > + *
> > + * Warning, this driver is somewhat PinePhone specific.
> 
> So why not split this into core part and PinePhone specific glue
> part?

Potentially a lot of work I can't really test and would rather not do.

> > +   struct delayed_work work;
> > +   struct timer_list work_timer;
> > +
> > +   struct mutex lock;
> 
> Undocumented lock.

This is simple driver. How do you expect me to document it? Protects
this data structure, not exactly uncommon.

> > +
> > +   /* wait for power to stabilize and release reset */
> > +   msleep(10);
> > +   gpiod_set_value(anx7688->gpio_reset, 0);
> > +   usleep_range(2, 4);
> 
> Why not just use usleep_range() in both cases.

It does not really make code any better. Can do if you insist.

> > +static int anx7688_connect(struct anx7688 *anx7688)
> > +{
> > +   struct typec_partner_desc desc = {};
> > +   int ret, i;
> > +   u8 fw[2];
> > +   const u8 dp_snk_identity[16] = {
> > +   0x00, 0x00, 0x00, 0xec, /* id header */
> > +   0x00, 0x00, 0x00, 0x00, /* cert stat */
> > +   0x00, 0x00, 0x00, 0x00, /* product type */
> > +   0x39, 0x00, 0x00, 0x51  /* alt mode adapter */
> > +   };
> > +   const u8 svid[4] = {
> > +   0x00, 0x00, 0x01, 0xff,
> > +   };
> 
> Why not get those from DT?

Are you sure it belongs to the DT (and that DT people will agree)?

> > +   u32 caps[8];
> > +
> > +   dev_dbg(anx7688->dev, "cable inserted\n");
> > +
> > +   anx7688->last_status = -1;
> > +   anx7688->last_cc_status = -1;
> > +   anx7688->last_dp_state = -1;
> > +
> > +   msleep(10);
> 
> Please make a comment here why you have to wait, and use
> usleep_range().

/* Dunno because working code does that and waiting for hardware to
settle down after cable insertion kind of looks like a good thing */

I did not write the driver, and there's no good documentation for this
chip. I can try to invent something, but...

> > +   i = 0;
> > +   while (1) {
> > +   ret = anx7688_reg_read(anx7688, 
> > ANX7688_REG_EEPROM_LOAD_STATUS0);
> > +
> > +   if (ret >= 0 && (ret & ANX7688_EEPROM_FW_LOADED) == 
> > ANX7688_EEPROM_FW_LOADED) {
> > +   dev_dbg(anx7688->dev, "eeprom0 = 0x%02x\n", ret);
> > +   dev_dbg(anx7688->dev, "fw loaded after %d ms\n", i * 
> > 10);
> > +   break;
> > +   }
> > +
> > +   if (i > 99) {
> > +   set_bit(ANX7688_F_FW_FAILED, anx7688->flags);
> > +   dev_err(anx7688->dev,
> > +   "boot firmware load failed (you may need to 
> > flash FW to anx7688 first)\n");
> > +   ret = -ETIMEDOUT;
> > +   goto err_vconoff;
> > +   }
> > +   msleep(5);
> > +   i++;
> > +   }
> 
> You need to use something like time_is_after_jiffies() here instead of
> a counter.

Well, this works as well, but yes, I agree here.

> > +   ret = i2c_smbus_read_i2c_block_data(anx7688->client,
> > +   ANX7688_REG_FW_VERSION1, 2, fw);
> >

Re: [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document

[Pavel Machek]

Hi!

> > Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
> > but I did best I could.
> > 
> > Signed-off-by: Pavel Machek 
> 
> ...
> 
> > +  cabledet-gpios:
> > +maxItems: 1
> > +description: GPIO controlling CABLE_DET (C3) pin.
> > +
> > +  avdd10-supply:
> > +description: 1.0V power supply going to AVDD10 (A4, ...) pins
> > +
> > +  dvdd10-supply:
> > +description: 1.0V power supply going to DVDD10 (D6, ...) pins
> > +
> > +  avdd18-supply:
> > +description: 1.8V power supply going to AVDD18 (E3, ...) pins
> > +
> > +  dvdd18-supply:
> > +description: 1.8V power supply going to DVDD18 (G4, ...) pins
> > +
> > +  avdd33-supply:
> > +description: 3.3V power supply going to AVDD33 (C4, ...) pins
> > +
> > +  i2c-supply: true
> > +  vconn-supply: true
> 
> There are no such supplies like i2c and vconn on the schematics.
> 
> I think this represents some other part of component which was added
> here only for convenience.

Can you give me pointer to documentation you are looking at?

Best regards,
Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

[PATCHv3 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge

[Pavel Machek]

From: Ondrej Jirman 

This is driver for ANX7688 USB-C HDMI, with flashing and debugging
features removed. ANX7688 is rather criticial piece on PinePhone,
there's no display and no battery charging without it.

There's likely more work to be done here, but having basic support
in mainline is needed to be able to work on the other stuff
(networking, cameras, power management).

Signed-off-by: Ondrej Jirman 
Co-developed-by: Martijn Braam 
Co-developed-by: Samuel Holland 
Signed-off-by: Pavel Machek 

---

v2: Fix checkpatch stuff. Some cleanups, adapt to dts format in 1/2.
v3: Turn down debugging, as requested during review.

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 2f80c2792dbd..c9043ae61546 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -64,6 +64,17 @@ config TYPEC_ANX7411
  If you choose to build this driver as a dynamically linked module, the
  module will be called anx7411.ko.
 
+config TYPEC_ANX7688
+   tristate "Analogix ANX7688 Type-C DRP Port controller and mux driver"
+   depends on I2C
+   depends on USB_ROLE_SWITCH
+   help
+ Say Y or M here if your system has Analogix ANX7688 Type-C Bridge
+ controller driver.
+
+ If you choose to build this driver as a dynamically linked module, the
+ module will be called anx7688.ko.
+
 config TYPEC_RT1719
tristate "Richtek RT1719 Sink Only Type-C controller driver"
depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 7a368fea61bc..3f8ff94ad294 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_TYPEC_TCPM)+= tcpm/
 obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
 obj-$(CONFIG_TYPEC_TPS6598X)   += tipd/
 obj-$(CONFIG_TYPEC_ANX7411)+= anx7411.o
+obj-$(CONFIG_TYPEC_ANX7688)+= anx7688.o
 obj-$(CONFIG_TYPEC_HD3SS3220)  += hd3ss3220.o
 obj-$(CONFIG_TYPEC_STUSB160X)  += stusb160x.o
 obj-$(CONFIG_TYPEC_RT1719) += rt1719.o
diff --git a/drivers/usb/typec/anx7688.c b/drivers/usb/typec/anx7688.c
new file mode 100644
index ..407cbd5fedba
--- /dev/null
+++ b/drivers/usb/typec/anx7688.c
@@ -0,0 +1,1830 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ANX7688 USB-C HDMI bridge/PD driver
+ *
+ * Warning, this driver is somewhat PinePhone specific.
+ *
+ * How this works:
+ * - this driver allows to program firmware into ANX7688 EEPROM, and
+ *   initialize it
+ * - it then communicates with the firmware running on the OCM (on-chip
+ *   microcontroller)
+ * - it detects whether there is cable plugged in or not and powers
+ *   up or down the ANX7688 based on that
+ * - when the cable is connected the firmware on the OCM will handle
+ *   the detection of the nature of the device on the other end
+ *   of the USB-C cable
+ * - this driver then communicates with the USB phy to let it swap
+ *   data roles accordingly
+ * - it also enables VBUS and VCONN regulators as appropriate
+ * - USB phy driver (Allwinner) needs to know whether to switch to
+ *   device or host mode, or whether to turn off
+ * - when the firmware detects SRC.1.5A or SRC.3.0A via CC pins
+ *   or something else via PD, it notifies this driver via software
+ *   interrupt and this driver will determine how to update the TypeC
+ *   port status and what input current limit is appropriate
+ * - input current limit determination happens 500ms after cable
+ *   insertion or hard reset (delay is necessary to determine whether
+ *   the remote end is PD capable or not)
+ * - this driver tells to the PMIC driver that the input current limit
+ *   needs to be changed
+ * - this driver also monitors PMIC status and re-sets the input current
+ *   limit if it changes for some reason (due to PMIC internal decision
+ *   making) (this is disabled for now)
+ *
+ * ANX7688 FW behavior as observed:
+ *
+ * - DO NOT SET MORE THAN 1 SINK CAPABILITY! Firmware will ignore what
+ *   you set and send hardcoded PDO_BATT 5-21V 30W message!
+ *
+ * Product brief:
+ * 
https://www.analogix.com/en/system/files/AA-002281-PB-6-ANX7688_Product_Brief_0.pdf
+ * Development notes:
+ * https://xnux.eu/devices/feature/anx7688.html
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* firmware regs */
+
+#define ANX7688_REG_VBUS_OFF_DELAY_TIME 0x22
+#define ANX7688_REG_FEATURE_CTRL0x27
+#define ANX7688_REG_EEPROM_LOAD_STATUS1 0x11
+#define ANX7688_REG_EEPROM_LOAD_STATUS0 0x12
+#define ANX7688_REG_FW_VERSION1 0x15
+#define ANX7688_REG_FW_VERSION0 0x16
+
+#define ANX7688_EEPROM_FW_LOADED   0x01
+
+#define ANX7688_REG_STATUS_INT_MASK 0x17
+#define ANX7688_REG_STATUS_INT  0x28
+#define ANX7688_IRQS_RECEIVED_MSG   BIT(0)
+#define ANX7688_IRQS_RECEIVED_ACK  

[PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document

[Pavel Machek]

Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
but I did best I could.

Signed-off-by: Pavel Machek 

---

v2: implement review feedback
v3: fix single character pointed by robot

diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml 
b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
new file mode 100644
index ..48b9ae936cb5
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
@@ -0,0 +1,127 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+# Pin names can be deduced from
+# 
https://files.pine64.org/doc/PinePhone/PinePhone%20v1.2b%20Released%20Schematic.pdf
+
+title: Analogix ANX7688 Type-C controller
+
+maintainers:
+  - Pavel Machek 
+
+properties:
+  compatible:
+enum:
+  - analogix,anx7688
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+
+  reset-gpios:
+maxItems: 1
+description: GPIO controlling RESET_N (B7) pin.
+
+  enable-gpios:
+maxItems: 1
+description: GPIO controlling POWER_EN (D2) pin.
+
+  cabledet-gpios:
+maxItems: 1
+description: GPIO controlling CABLE_DET (C3) pin.
+
+  avdd10-supply:
+description: 1.0V power supply going to AVDD10 (A4, ...) pins
+
+  dvdd10-supply:
+description: 1.0V power supply going to DVDD10 (D6, ...) pins
+
+  avdd18-supply:
+description: 1.8V power supply going to AVDD18 (E3, ...) pins
+
+  dvdd18-supply:
+description: 1.8V power supply going to DVDD18 (G4, ...) pins
+
+  avdd33-supply:
+description: 3.3V power supply going to AVDD33 (C4, ...) pins
+
+  i2c-supply: true
+  vconn-supply: true
+  hdmi-vt-supply: true
+  vbus-supply: true
+  vbus-in-supply: true
+
+  connector:
+type: object
+$ref: /schemas/connector/usb-connector.yaml
+
+description:
+  Properties for usb c connector.
+
+properties:
+  compatible:
+const: usb-c-connector
+
+required:
+  - compatible
+  - reg
+  - connector
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+#include 
+
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+typec@2c {
+compatible = "analogix,anx7688";
+reg = <0x2c>;
+interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+interrupt-parent = <>;
+
+enable-gpios = < 3 10 GPIO_ACTIVE_LOW>; /* PD10 */
+reset-gpios = < 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
+cabledet-gpios = <_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
+
+avdd10-supply = <_anx1v0>;
+dvdd10-supply = <_anx1v0>;
+avdd18-supply = <_ldo_io1>;
+dvdd18-supply = <_ldo_io1>;
+avdd33-supply = <_dcdc1>;
+i2c-supply = <_ldo_io0>;
+vconn-supply = <_vconn5v0>;
+hdmi-vt-supply = <_dldo1>;
+
+vbus-supply = <_usb_5v>;
+vbus-in-supply = <_power_supply>;
+
+typec_con: connector {
+compatible = "usb-c-connector";
+power-role = "dual";
+data-role = "dual";
+try-power-role = "source";
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+port@0 {
+reg = <0>;
+typec_con_ep: endpoint {
+remote-endpoint = <_hs_ep>;
+};
+};
+};
+};
+};
+};
+...

-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

Re: [PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge

[Pavel Machek]

Hi!

> I'm sorry to keep you waiting.

Thanks for comments.

> > +   struct gpio_desc *gpio_reset;
> > +   struct gpio_desc *gpio_cabledet;
> > +
> > +   uint32_t src_caps[8];
> 
> Use u32 instead of uint32_t.

Will replace globally.

> > +static int anx7688_reg_read(struct anx7688 *anx7688, u8 reg_addr)
> > +{
> > +   int ret;
> > +
> > +   ret = i2c_smbus_read_byte_data(anx7688->client, reg_addr);
> > +   if (ret < 0)
> > +   dev_err(anx7688->dev, "i2c read failed at 0x%x (%d)\n",
> > +   reg_addr, ret);
> 
> dev_dbg instead.

I'd prefer not to. i2c functions should not really fail, and if they
do, we want the error log. This is not debugging, this is i2c failing.

> > +static void anx7688_power_enable(struct anx7688 *anx7688)
> > +{
> > +   gpiod_set_value(anx7688->gpio_reset, 1);
> > +   gpiod_set_value(anx7688->gpio_enable, 1);
> > +
> > +   /* wait for power to stabilize and release reset */
> > +   msleep(10);
> 
> So is it okay that the sleep may take up to 20ms?

I don't see how that would be a problem.

> > +   gpiod_set_value(anx7688->gpio_reset, 0);
> > +   udelay(2);
> 
> Use usleep_range() instead.

Can do, but it makes no difference.

> > +   gpiod_set_value(anx7688->gpio_reset, 1);
> > +   msleep(5);
> 
> The same question here, is it a problem if the sleep ends up taking
> 20ms?

Again, I expect that to be ok.

> > +   ret = anx7688_tcpc_reg_read(anx7688, ANX7688_TCPC_REG_INTERFACE_SEND);
> > +   if (ret) {
> > +   dev_err(anx7688->dev,
> > +   "failed to send pd packet (tx buffer full)\n");
> 
> One line should be enought for that one.

That makes it go over 80 columns, but yes, can be one line.

> > +   // wait until the message is processed (30ms max)
> > +   for (i = 0; i < 300; i++) {
> > +   ret = anx7688_tcpc_reg_read(anx7688, 
> > ANX7688_TCPC_REG_INTERFACE_SEND);
> > +   if (ret <= 0)
> > +   return ret;
> > +
> > +   udelay(100);
> > +   }
> > +
> > +   dev_err(anx7688->dev, "timeout waiting for the message queue flush\n");
> 
> Maybe dev_dbg for this too.

Let's not hide these. If they happen, we can downgrade them, but they
should not.

> > +   /* wait till the firmware is loaded (typically ~30ms) */
> > +   for (i = 0; i < 100; i++) {
> > +   ret = anx7688_reg_read(anx7688, 
> > ANX7688_REG_EEPROM_LOAD_STATUS0);
> > +
> > +   if (ret >= 0 && (ret & ANX7688_EEPROM_FW_LOADED) == 
> > ANX7688_EEPROM_FW_LOADED) {
> > +   dev_dbg(anx7688->dev, "eeprom0 = 0x%02x\n", ret);
> > +   dev_info(anx7688->dev, "fw loaded after %d ms\n", i * 
> > 10);
> 
> Debugging information. Use dev_dbg.

Ok.

> > +   set_bit(ANX7688_F_FW_FAILED, anx7688->flags);
> > +   dev_err(anx7688->dev, "boot firmware load failed (you may need to flash 
> > FW to anx7688 first)\n");
> > +   ret = -ETIMEDOUT;
> > +   goto err_vconoff;
> > +
> > +fw_loaded:
> 
> This label looks a bit messy to me. You could also move that firmware
> loading wait to its own function.

Ok, let me try to refactor this.

> > +static int anx7688_handle_pd_message_response(struct anx7688 *anx7688,
> > + u8 to_cmd, u8 resp)
> > +{
...
> > +   return 0;
> > +}
> 
> Noise. Drop this whole function. If you need this kind of information,
> then please consider trace points, or just use some debugfs trick like
> what we have in drivers/usb/typec/tcpm/tcpm.c and few other drivers.

Ok.

> > +   switch (cmd) {
> > +   case ANX7688_OCM_MSG_PWR_SRC_CAP:
> > +   dev_info(anx7688->dev, "received SRC_CAP\n");
> 
> Noise.

Ok, let me convert these to dev_dbg.

> > +
> > +   if (len % 4 != 0) {
> > +   dev_warn(anx7688->dev, "received invalid sized PDO 
> > array\n");
> > +   break;
> > +   }
> > +
> > +   /* the partner is PD capable */
> > +   anx7688->pd_capable = true;
> > +
> > +   for (i = 0; i < len / 4; i++) {
> > +   pdo = le32_to_cpu(pdos[i]);
> > +
> > +   if (pdo_type(pdo) == PDO_TYPE_FIXED) {
> > +   unsigned int voltage = pdo_fixed_voltage(pdo);
> > +   unsigned int max_curr = pdo_max_current(pdo);
> > +
> > +   dev_info(anx7688->dev, "SRC_CAP PDO_FIXED (%umV 
> > %umA)\n", voltage, max_curr);
> 
> Noise.
> 
> > +   } else if (pdo_type(pdo) == PDO_TYPE_BATT) {
> > +   unsigned int min_volt = pdo_min_voltage(pdo);
> > +   unsigned int max_volt = pdo_max_voltage(pdo);
> > +   unsigned int max_pow = pdo_max_power(pdo);
> > +
> > +   dev_info(anx7688->dev, "SRC_CAP PDO_BATT 
> > (%umV-%umV %umW)\n", min_volt, max_volt, max_pow);
> 
> Noise. That line also really should be split in two.
> 
> I'm stopping my review here. This driver is too noisy. All dev_info
> calls need to be 

Re: [PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge

[Pavel Machek]

Hi!

> From: Ondrej Jirman 
> 
> This is driver for ANX7688 USB-C HDMI, with flashing and debugging
> features removed. ANX7688 is rather criticial piece on PinePhone,
> there's no display and no battery charging without it.
> 
> There's likely more work to be done here, but having basic support
> in mainline is needed to be able to work on the other stuff
> (networking, cameras, power management).
> 
> Signed-off-by: Ondrej Jirman 
> Co-developed-by: Martijn Braam 
> Co-developed-by: Samuel Holland 
> Signed-off-by: Pavel Machek 

Any comments here?

Best regards,
Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

Re: [PATCHv2 1/2] dt-bindings: usb: typec: anx7688: start a binding document

[Pavel Machek]

Hi!

> Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
> but I did best I could.
> 
> Signed-off-by: Pavel Machek 

Any more comments here? Automatic system told me I need to replace one
character...

Best regards,
Pavel

> ---
> 
> v2: implement review feedback
> 
> diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml 
> b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> new file mode 100644
> index ..9e887eafb5fc
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> @@ -0,0 +1,127 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +# Pin names can be deduced from
> +# 
> https://files.pine64.org/doc/PinePhone/PinePhone%20v1.2b%20Released%20Schematic.pdf
> +
> +title: Analogix ANX7688 Type-C controller
> +
> +maintainers:
> +  - Pavel Machek 
> +
> +properties:
> +  compatible:
> +enum:
> +  - analogix,anx7688
> +
> +  reg:
> +maxItems: 1
> +
> +  interrupts:
> +maxItems: 1
> +
> +  reset-gpios:
> +maxItems: 1
> +description: GPIO controlling RESET_N (B7) pin.
> +
> +  enable-gpios:
> +maxItems: 1
> +description: GPIO controlling POWER_EN (D2) pin.
> +
> +  cabledet-gpios:
> +maxItems: 1
> +description: GPIO controlling CABLE_DET (C3) pin.
> +
> +  avdd10-supply:
> +description: 1.0V power supply going to AVDD10 (A4, ...) pins
> +
> +  dvdd10-supply:
> +description: 1.0V power supply going to DVDD10 (D6, ...) pins
> +
> +  avdd18-supply:
> +description: 1.8V power supply going to AVDD18 (E3, ...) pins
> +
> +  dvdd18-supply:
> +description: 1.8V power supply going to DVDD18 (G4, ...) pins
> +
> +  avdd33-supply:
> +description: 3.3V power supply going to AVDD33 (C4, ...) pins
> +
> +  i2c-supply: true
> +  vconn-supply: true
> +  hdmi-vt-supply: true
> +  vbus-supply: true
> +  vbus-in-supply: true
> +
> +  connector:
> +type: object
> +$ref: /schemas/connector/usb-connector.yaml
> +
> +description:
> +  Properties for usb c connector.
> +
> +properties:
> +  compatible:
> +const: usb-c-connector
> +
> +required:
> +  - compatible
> +  - reg
> +  - connector
> +
> +additionalProperties: false
> +
> +examples:
> +  - |
> +#include 
> +#include 
> +
> +i2c {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +
> +typec@2c {
> +compatible = "analogix,anx7688";
> +reg = <0x2c>;
> +interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
> +interrupt-parent = <>;
> +
> +enable-gpios = < 3 10 GPIO_ACTIVE_LOW>; /* PD10 */
> +reset-gpios = < 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
> +cabledet-gpios = <_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
> +
> +avdd10-supply = <_anx1v0>;
> +dvdd10-supply = <_anx1v0>;
> +avdd18-supply = <_ldo_io1>;
> +dvdd18-supply = <_ldo_io1>;
> +avdd33-supply = <_dcdc1>;
> +i2c-supply = <_ldo_io0>;
> +vconn-supply = <_vconn5v0>;
> +hdmi_vt-supply = <_dldo1>;
> +
> +vbus-supply = <_usb_5v>;
> +vbus-in-supply = <_power_supply>;
> +
> +typec_con: connector {
> +compatible = "usb-c-connector";
> +power-role = "dual";
> +data-role = "dual";
> +try-power-role = "source";
> +
> +ports {
> +#address-cells = <1>;
> +#size-cells = <0>;
> +port@0 {
> +reg = <0>;
> +typec_con_ep: endpoint {
> +remote-endpoint = <_hs_ep>;
> +};
> +};
> +};
> +};
> +};
> +};
> +...
> 



-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

[PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge

[Pavel Machek]

From: Ondrej Jirman 

This is driver for ANX7688 USB-C HDMI, with flashing and debugging
features removed. ANX7688 is rather criticial piece on PinePhone,
there's no display and no battery charging without it.

There's likely more work to be done here, but having basic support
in mainline is needed to be able to work on the other stuff
(networking, cameras, power management).

Signed-off-by: Ondrej Jirman 
Co-developed-by: Martijn Braam 
Co-developed-by: Samuel Holland 
Signed-off-by: Pavel Machek 

---

v2: Fix checkpatch stuff. Some cleanups, adapt to dts format in 1/2.

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 2f80c2792dbd..c9043ae61546 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -64,6 +64,17 @@ config TYPEC_ANX7411
  If you choose to build this driver as a dynamically linked module, the
  module will be called anx7411.ko.
 
+config TYPEC_ANX7688
+   tristate "Analogix ANX7688 Type-C DRP Port controller and mux driver"
+   depends on I2C
+   depends on USB_ROLE_SWITCH
+   help
+ Say Y or M here if your system has Analogix ANX7688 Type-C Bridge
+ controller driver.
+
+ If you choose to build this driver as a dynamically linked module, the
+ module will be called anx7688.ko.
+
 config TYPEC_RT1719
tristate "Richtek RT1719 Sink Only Type-C controller driver"
depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 7a368fea61bc..3f8ff94ad294 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_TYPEC_TCPM)+= tcpm/
 obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
 obj-$(CONFIG_TYPEC_TPS6598X)   += tipd/
 obj-$(CONFIG_TYPEC_ANX7411)+= anx7411.o
+obj-$(CONFIG_TYPEC_ANX7688)+= anx7688.o
 obj-$(CONFIG_TYPEC_HD3SS3220)  += hd3ss3220.o
 obj-$(CONFIG_TYPEC_STUSB160X)  += stusb160x.o
 obj-$(CONFIG_TYPEC_RT1719) += rt1719.o
diff --git a/drivers/usb/typec/anx7688.c b/drivers/usb/typec/anx7688.c
new file mode 100644
index ..14d033bbc0c7
--- /dev/null
+++ b/drivers/usb/typec/anx7688.c
@@ -0,0 +1,1927 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * ANX7688 USB-C HDMI bridge/PD driver
+ *
+ * Warning, this driver is somewhat PinePhone specific.
+ *
+ * How this works:
+ * - this driver allows to program firmware into ANX7688 EEPROM, and
+ *   initialize it
+ * - it then communicates with the firmware running on the OCM (on-chip
+ *   microcontroller)
+ * - it detects whether there is cable plugged in or not and powers
+ *   up or down the ANX7688 based on that
+ * - when the cable is connected the firmware on the OCM will handle
+ *   the detection of the nature of the device on the other end
+ *   of the USB-C cable
+ * - this driver then communicates with the USB phy to let it swap
+ *   data roles accordingly
+ * - it also enables VBUS and VCONN regulators as appropriate
+ * - USB phy driver (Allwinner) needs to know whether to switch to
+ *   device or host mode, or whether to turn off
+ * - when the firmware detects SRC.1.5A or SRC.3.0A via CC pins
+ *   or something else via PD, it notifies this driver via software
+ *   interrupt and this driver will determine how to update the TypeC
+ *   port status and what input current limit is appropriate
+ * - input current limit determination happens 500ms after cable
+ *   insertion or hard reset (delay is necessary to determine whether
+ *   the remote end is PD capable or not)
+ * - this driver tells to the PMIC driver that the input current limit
+ *   needs to be changed
+ * - this driver also monitors PMIC status and re-sets the input current
+ *   limit if it changes for some reason (due to PMIC internal decision
+ *   making) (this is disabled for now)
+ *
+ * ANX7688 FW behavior as observed:
+ *
+ * - DO NOT SET MORE THAN 1 SINK CAPABILITY! Firmware will ignore what
+ *   you set and send hardcoded PDO_BATT 5-21V 30W message!
+ *
+ * Product brief:
+ * 
https://www.analogix.com/en/system/files/AA-002281-PB-6-ANX7688_Product_Brief_0.pdf
+ * Development notes:
+ * https://xnux.eu/devices/feature/anx7688.html
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* firmware regs */
+
+#define ANX7688_REG_VBUS_OFF_DELAY_TIME 0x22
+#define ANX7688_REG_FEATURE_CTRL0x27
+#define ANX7688_REG_EEPROM_LOAD_STATUS1 0x11
+#define ANX7688_REG_EEPROM_LOAD_STATUS0 0x12
+#define ANX7688_REG_FW_VERSION1 0x15
+#define ANX7688_REG_FW_VERSION0 0x16
+
+#define ANX7688_EEPROM_FW_LOADED   0x01
+
+#define ANX7688_REG_STATUS_INT_MASK 0x17
+#define ANX7688_REG_STATUS_INT  0x28
+#define ANX7688_IRQS_RECEIVED_MSG   BIT(0)
+#define ANX7688_IRQS_RECEIVED_ACK   BIT(1)
+#define ANX7688_IRQS_VCONN_CHANGE  

[PATCHv2 1/2] dt-bindings: usb: typec: anx7688: start a binding document

[Pavel Machek]

Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
but I did best I could.

Signed-off-by: Pavel Machek 

---

v2: implement review feedback

diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml 
b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
new file mode 100644
index ..9e887eafb5fc
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
@@ -0,0 +1,127 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+# Pin names can be deduced from
+# 
https://files.pine64.org/doc/PinePhone/PinePhone%20v1.2b%20Released%20Schematic.pdf
+
+title: Analogix ANX7688 Type-C controller
+
+maintainers:
+  - Pavel Machek 
+
+properties:
+  compatible:
+enum:
+  - analogix,anx7688
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+
+  reset-gpios:
+maxItems: 1
+description: GPIO controlling RESET_N (B7) pin.
+
+  enable-gpios:
+maxItems: 1
+description: GPIO controlling POWER_EN (D2) pin.
+
+  cabledet-gpios:
+maxItems: 1
+description: GPIO controlling CABLE_DET (C3) pin.
+
+  avdd10-supply:
+description: 1.0V power supply going to AVDD10 (A4, ...) pins
+
+  dvdd10-supply:
+description: 1.0V power supply going to DVDD10 (D6, ...) pins
+
+  avdd18-supply:
+description: 1.8V power supply going to AVDD18 (E3, ...) pins
+
+  dvdd18-supply:
+description: 1.8V power supply going to DVDD18 (G4, ...) pins
+
+  avdd33-supply:
+description: 3.3V power supply going to AVDD33 (C4, ...) pins
+
+  i2c-supply: true
+  vconn-supply: true
+  hdmi-vt-supply: true
+  vbus-supply: true
+  vbus-in-supply: true
+
+  connector:
+type: object
+$ref: /schemas/connector/usb-connector.yaml
+
+description:
+  Properties for usb c connector.
+
+properties:
+  compatible:
+const: usb-c-connector
+
+required:
+  - compatible
+  - reg
+  - connector
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+#include 
+
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+typec@2c {
+compatible = "analogix,anx7688";
+reg = <0x2c>;
+interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+interrupt-parent = <>;
+
+enable-gpios = < 3 10 GPIO_ACTIVE_LOW>; /* PD10 */
+reset-gpios = < 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
+cabledet-gpios = <_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
+
+avdd10-supply = <_anx1v0>;
+dvdd10-supply = <_anx1v0>;
+avdd18-supply = <_ldo_io1>;
+dvdd18-supply = <_ldo_io1>;
+avdd33-supply = <_dcdc1>;
+i2c-supply = <_ldo_io0>;
+vconn-supply = <_vconn5v0>;
+hdmi_vt-supply = <_dldo1>;
+
+vbus-supply = <_usb_5v>;
+vbus-in-supply = <_power_supply>;
+
+typec_con: connector {
+compatible = "usb-c-connector";
+power-role = "dual";
+data-role = "dual";
+try-power-role = "source";
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+port@0 {
+reg = <0>;
+typec_con_ep: endpoint {
+remote-endpoint = <_hs_ep>;
+};
+};
+};
+};
+};
+};
+...

-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

Re: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document

[Pavel Machek]

Hi!

> > +  reset-gpios:
> > +maxItems: 1
> 
> blank line
> 
> > +  avdd10-supply:
> > +description:
> > +  1.0V power supply
> 
> Keep description in one line and add a blank line. This code is not that
> readable.
> 
> > +  i2c-supply:
> > +description:
> > +  Power supply
> 
> Drop all useless descriptions (so : true)

Ok, fixed, I hope I got it right.

> > +  vbus-supply:
> > +description:
> > +  Power supply
> > +  vbus_in-supply:
> 
> No underscores in property names.

Ok.

> > +  connector:
> > +type: object
> > +$ref: ../connector/usb-connector.yaml
> 
> Full path, so /schemas/connector/..
> 
> > +unevaluatedProperties: false
> 
> Drop

> > +
> > +description:
> > +  Properties for usb c connector.
> 
> > +
> > +properties:
> > +  compatible:
> > +const: usb-c-connector
> > +
> > +  power-role: true
> > +
> > +  data-role: true
> > +
> > +  try-power-role: true
> 
> I don't understand why do you have here properties. Do you see any
> binding like this?

Well, it looks like I copied these mistakes from analogix,anx7411.yaml .

> > +
> > +required:
> > +  - compatible
> 
> Drop, why is it needed?

Again, copy from analogix,anx7411.yaml .

Should I try to fix up analogix,anx7411.yaml , and submit that, first,
or is it easier if you do that?

Thanks and best regards,
Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

Re: [PATCH] dt-bindings: usb: typec: anx7688: start a binding document

[Pavel Machek]

Hi!
> > Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
> > but I did best I could.
> > 
> > Signed-off-by: Pavel Machek 
> > 
> 
> You miss proper diffstat which makes reviewing difficult.

> Actually entire patch is corrupted and impossible to apply.

Sorry about that.

> Anyway, where is any user of this? Nothing in commit msg explains
>  this.

User being is worked on:

https://lore.kernel.org/lkml/2024020126-emote-unsubtly-3394@gregkh/T/

Thanks for comments. I'll go through them and try to improve things.

Best regards,
Pavel

> 
> > diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml 
> > b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> > new file mode 100644
> > index ..b9d60586937f
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
> > @@ -0,0 +1,140 @@
> > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Analogix ANX7688 Type-C controller
> > +
> > +maintainers:
> > +  - Pavel Machek 
> > +
> > +properties:
> > +  compatible:
> > +enum:
> > +  - analogix,anx7688
> > +
> > +  reg:
> > +maxItems: 1
> > +
> > +  interrupts:
> > +maxItems: 1
> > +
> > +  reset-gpios:
> > +maxItems: 1
> 
> blank line
> 
> > +  enable-gpios:
> > +maxItems: 1
> > +  cabledet-gpios:
> > +maxItems: 1
> > +
> > +  avdd10-supply:
> > +description:
> > +  1.0V power supply
> 
> Keep description in one line and add a blank line. This code is not that
> readable.
> 
> > +  dvdd10-supply:
> > +description:
> > +  1.0V power supply
> > +  avdd18-supply:
> > +description:
> > +  1.8V power supply
> > +  dvdd18-supply:
> > +description:
> > +  1.8V power supply
> > +  avdd33-supply:
> > +description:
> > +  3.3V power supply
> > +  i2c-supply:
> > +description:
> > +  Power supply
> 
> Drop all useless descriptions (so : true)
> 
> > +  vconn-supply:
> > +description:
> > +  Power supply
> > +  hdmi_vt-supply:
> > +description:
> > +  Power supply
> > +
> > +  vbus-supply:
> > +description:
> > +  Power supply
> > +  vbus_in-supply:
> 
> No underscores in property names.
> 
> > +description:
> > +  Power supply
> > +
> > +  connector:
> > +type: object
> > +$ref: ../connector/usb-connector.yaml
> 
> Full path, so /schemas/connector/..
> 
> > +unevaluatedProperties: false
> 
> Drop
> 
> > +
> > +description:
> > +  Properties for usb c connector.
> 
> > +
> > +properties:
> > +  compatible:
> > +const: usb-c-connector
> > +
> > +  power-role: true
> > +
> > +  data-role: true
> > +
> > +  try-power-role: true
> 
> I don't understand why do you have here properties. Do you see any
> binding like this?
> 
> > +
> > +required:
> > +  - compatible
> 
> Drop, why is it needed?
> 
> > +
> > +required:
> > +  - compatible
> > +  - reg
> > +  - connector
> > +
> > +additionalProperti
> 
> I don't know what's further but this patch is not a patch... Please read
> submitting-patches, organize your patchset correctly into manageable
> logical chunks and send them as proper patchset, not one junk.
> 
> b4 helps here a lot...
> 
> > 
> 
> Best regards,
> Krzysztof

-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

[PATCH] dt-bindings: usb: typec: anx7688: start a binding document

[Pavel Machek]

Add binding for anx7688 usb type-c bridge. I don't have a datasheet,
but I did best I could.

Signed-off-by: Pavel Machek 

diff --git a/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml 
b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
new file mode 100644
index ..b9d60586937f
--- /dev/null
+++ b/Documentation/devicetree/bindings/usb/analogix,anx7688.yaml
@@ -0,0 +1,140 @@
+# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/usb/analogix,anx7688.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Analogix ANX7688 Type-C controller
+
+maintainers:
+  - Pavel Machek 
+
+properties:
+  compatible:
+enum:
+  - analogix,anx7688
+
+  reg:
+maxItems: 1
+
+  interrupts:
+maxItems: 1
+
+  reset-gpios:
+maxItems: 1
+  enable-gpios:
+maxItems: 1
+  cabledet-gpios:
+maxItems: 1
+
+  avdd10-supply:
+description:
+  1.0V power supply
+  dvdd10-supply:
+description:
+  1.0V power supply
+  avdd18-supply:
+description:
+  1.8V power supply
+  dvdd18-supply:
+description:
+  1.8V power supply
+  avdd33-supply:
+description:
+  3.3V power supply
+  i2c-supply:
+description:
+  Power supply
+  vconn-supply:
+description:
+  Power supply
+  hdmi_vt-supply:
+description:
+  Power supply
+
+  vbus-supply:
+description:
+  Power supply
+  vbus_in-supply:
+description:
+  Power supply
+
+  connector:
+type: object
+$ref: ../connector/usb-connector.yaml
+unevaluatedProperties: false
+
+description:
+  Properties for usb c connector.
+
+properties:
+  compatible:
+const: usb-c-connector
+
+  power-role: true
+
+  data-role: true
+
+  try-power-role: true
+
+required:
+  - compatible
+
+required:
+  - compatible
+  - reg
+  - connector
+
+additionalProperties: false
+
+examples:
+  - |
+#include 
+#include 
+
+i2c {
+#address-cells = <1>;
+#size-cells = <0>;
+
+typec@2c {
+compatible = "analogix,anx7688";
+reg = <0x2c>;
+interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+interrupt-parent = <>;
+
+enable-gpios = < 3 10 GPIO_ACTIVE_LOW>; /* PD10 */
+reset-gpios = < 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
+cabledet-gpios = <_pio 0 8 GPIO_ACTIVE_HIGH>; /* PL8 */
+
+avdd10-supply = <_anx1v0>;
+dvdd10-supply = <_anx1v0>;
+avdd18-supply = <_ldo_io1>;
+dvdd18-supply = <_ldo_io1>;
+avdd33-supply = <_dcdc1>;
+i2c-supply = <_ldo_io0>;
+vconn-supply = <_vconn5v0>;
+hdmi_vt-supply = <_dldo1>;
+
+vbus-supply = <_usb_5v>;
+vbus_in-supply = <_power_supply>;
+
+typec_con: connector {
+compatible = "usb-c-connector";
+power-role = "dual";
+data-role = "dual";
+try-power-role = "source";
+
+ports {
+#address-cells = <1>;
+#size-cells = <0>;
+port@0 {
+reg = <0>;
+typec_con_ep: endpoint {
+remote-endpoint = <_hs_ep>;
+};
+};
+};
+};
+};
+};
+...
diff --git a/Documentation/devicetree/bindings/usb/generic-xhci.yaml 
b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
index 594ebb3ee432..6ceafa4af292 100644
--- a/Documentation/devicetree/bindings/usb/generic-xhci.yaml
+++ b/Documentation/devicetree/bindings/usb/generic-xhci.yaml
@@ -9,9 +9,6 @@ title: USB xHCI Controller
 maintainers:
   - Mathias Nyman 
 
-allOf:
-  - $ref: usb-xhci.yaml#
-
 properties:
   compatible:
 oneOf:
@@ -25,6 +22,11 @@ properties:
   - marvell,armada-380-xhci
   - marvell,armada-8k-xhci
   - const: generic-xhci
+  - description: Broadcom SoCs with power domains
+items:
+  - enum:
+  - brcm,bcm2711-xhci
+  - const: brcm,xhci-brcm-v2
   - description: Broadcom STB SoCs with xHCI
 enum:
   - brcm,xhci-brcm-v2
@@ -49,6 +51,9 @@ properties:
   - const: core
   - const: reg
 
+  power-domains:
+maxItems: 1
+
 unevaluatedProperties: false
 
 required:
@@ -56,6 +61,20 @@ required:
   - reg
   - interrupts
 
+allOf:
+  - $ref: usb-xhci.yaml#
+  - if:
+  properties:
+compatible:
+  contains:
+const: brcm,bcm2711-xhci
+then:
+  required:
+- power-domains
+else:
+  properties:
+power-domains: false
+
 examples:
   - |
 usb@

Re: [PATCH] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge

[Pavel Machek]

Hi!

> > From: Ondrej Jirman 
> > 
> > This is driver for ANX7688 USB-C HDMI, with flashing and debugging
> > features removed. ANX7688 is rather criticial piece on PinePhone,
> > there's no display and no battery charging without it.
> 
> Don't remove the flashing part. Some Pinephones come without the firmware
> in the past. Even recently, I've seen some people in the Pine chat
> asking how to flash the firmware on some old PinePhone.
> 
> It's a safe operation that can be done at any time and can only be done
> from the kernel driver.

I can re-add it later, but initial merge will be tricky enough as-is.

> > There's likely more work to be done here, but having basic support
> > in mainline is needed to be able to work on the other stuff
> > (networking, cameras, power management).
> > 
> > Signed-off-by: Ondrej Jirman 
> 
> I should be second in order of sign-offs. Martijn wrote a non-working skeleton
> https://megous.com/git/linux/commit/?h=pp-5.7=30e33cefd7956a2b49fb27008b4af9d868974e58
> driver. Then I picked it up and developed it over years to a working thing.
> Almost none of the skeleton remains.

I believe From: should match First signed-off. I guess Martijn should
be marked as co-developed-by or something like that?

> License is GPLv2.

Ok.

> > +   ret = of_property_read_variable_u32_array(dev->of_node, "sink-caps",
> > + anx7688->snk_caps,
> > + 1, 
> > ARRAY_SIZE(anx7688->snk_caps));
> > +   if (ret < 0) {
> > +   dev_err(dev, "failed to get sink-caps from DT\n");
> > +   return ret;
> > +   }
> 
> ^^^ The driver will need to follow usb-c-connector bindings and it will need
> a bindings documentation for itself.
> 
> That's one of the missing things that I did not implement, yet.

Yep, I fought with device trees for few days. I should have yaml
ready.

Best regards,
Pavel

-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

Re: [PATCH] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge

[Pavel Machek]

Hi!

> > --- /dev/null
> > +++ b/drivers/usb/typec/anx7688.c
> > @@ -0,0 +1,1866 @@
> > +/*
> > + * ANX7688 USB-C HDMI bridge/PD driver
> > + *
> 
> 
> 
> Did this pass checkpatch?  I need a spdx line for new files please,
> don't force us to go back and guess in the future, that isn't nice.

Thanks for the review. Fixing checkpatch issues is easy, but it looks
like closer look will be needed at the devicetree probing.

Also, what are the authors preferences about the license? I'd prefer
GPLv2+.

Best regards,
Pavel

[PATCH] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge

[Pavel Machek]

From: Ondrej Jirman 

This is driver for ANX7688 USB-C HDMI, with flashing and debugging
features removed. ANX7688 is rather criticial piece on PinePhone,
there's no display and no battery charging without it.

There's likely more work to be done here, but having basic support
in mainline is needed to be able to work on the other stuff
(networking, cameras, power management).

Signed-off-by: Ondrej Jirman 
Signed-off-by: Martijn Braam 
Signed-off-by: Samuel Holland 
Signed-off-by: Pavel Machek 

diff --git a/drivers/usb/typec/Kconfig b/drivers/usb/typec/Kconfig
index 2f80c2792dbd..982b7c444a1f 100644
--- a/drivers/usb/typec/Kconfig
+++ b/drivers/usb/typec/Kconfig
@@ -64,6 +65,17 @@ config TYPEC_ANX7411
  If you choose to build this driver as a dynamically linked module, the
  module will be called anx7411.ko.
 
+config TYPEC_ANX7688
+   tristate "Analogix ANX7688 Type-C DRP Port controller and mux driver"
+   depends on I2C
+   depends on USB_ROLE_SWITCH
+   help
+ Say Y or M here if your system has Analogix ANX7688 Type-C Bridge
+ controller driver.
+
+ If you choose to build this driver as a dynamically linked module, the
+ module will be called anx7688.ko.
+
 config TYPEC_RT1719
tristate "Richtek RT1719 Sink Only Type-C controller driver"
depends on USB_ROLE_SWITCH || !USB_ROLE_SWITCH
diff --git a/drivers/usb/typec/Makefile b/drivers/usb/typec/Makefile
index 7a368fea61bc..3f8ff94ad294 100644
--- a/drivers/usb/typec/Makefile
+++ b/drivers/usb/typec/Makefile
@@ -7,6 +7,7 @@ obj-$(CONFIG_TYPEC_TCPM)+= tcpm/
 obj-$(CONFIG_TYPEC_UCSI)   += ucsi/
 obj-$(CONFIG_TYPEC_TPS6598X)   += tipd/
 obj-$(CONFIG_TYPEC_ANX7411)+= anx7411.o
+obj-$(CONFIG_TYPEC_ANX7688)+= anx7688.o
 obj-$(CONFIG_TYPEC_HD3SS3220)  += hd3ss3220.o
 obj-$(CONFIG_TYPEC_STUSB160X)  += stusb160x.o
 obj-$(CONFIG_TYPEC_RT1719) += rt1719.o
diff --git a/drivers/usb/typec/anx7688.c b/drivers/usb/typec/anx7688.c
new file mode 100644
index ..9b693c9fd085
--- /dev/null
+++ b/drivers/usb/typec/anx7688.c
@@ -0,0 +1,1866 @@
+/*
+ * ANX7688 USB-C HDMI bridge/PD driver
+ *
+ * Warning, this driver is somewhat PinePhone specific.
+ *
+ * How this works:
+ * - this driver allows to program firmware into ANX7688 EEPROM, and
+ *   initialize it
+ * - it then communicates with the firmware running on the OCM (on-chip
+ *   microcontroller)
+ * - it detects whether there is cable plugged in or not and powers
+ *   up or down the ANX7688 based on that
+ * - when the cable is connected the firmware on the OCM will handle
+ *   the detection of the nature of the device on the other end
+ *   of the USB-C cable
+ * - this driver then communicates with the USB phy to let it swap
+ *   data roles accordingly
+ * - it also enables VBUS and VCONN regulators as appropriate
+ * - USB phy driver (Allwinner) needs to know whether to switch to
+ *   device or host mode, or whether to turn off
+ * - when the firmware detects SRC.1.5A or SRC.3.0A via CC pins
+ *   or something else via PD, it notifies this driver via software
+ *   interrupt and this driver will determine how to update the TypeC
+ *   port status and what input current limit is appropriate
+ * - input current limit determination happens 500ms after cable
+ *   insertion or hard reset (delay is necessary to determine whether
+ *   the remote end is PD capable or not)
+ * - this driver tells to the PMIC driver that the input current limit
+ *   needs to be changed
+ * - this driver also monitors PMIC status and re-sets the input current
+ *   limit if it changes for some reason (due to PMIC internal decision
+ *   making) (this is disabled for now)
+ *
+ * ANX7688 FW behavior as observed:
+ *
+ * - DO NOT SET MORE THAN 1 SINK CAPABILITY! Firmware will ignore what
+ *   you set and send hardcoded PDO_BATT 5-21V 30W message!
+ *
+ * Product brief:
+ * 
https://www.analogix.com/en/system/files/AA-002281-PB-6-ANX7688_Product_Brief_0.pdf
+ * Development notes:
+ * https://xnux.eu/devices/feature/anx7688.html
+ */
+
+#define DEBUG
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+/* firmware regs */
+
+#define ANX7688_REG_VBUS_OFF_DELAY_TIME 0x22
+#define ANX7688_REG_FEATURE_CTRL0x27
+#define ANX7688_REG_EEPROM_LOAD_STATUS1 0x11
+#define ANX7688_REG_EEPROM_LOAD_STATUS0 0x12
+#define ANX7688_REG_FW_VERSION1 0x15
+#define ANX7688_REG_FW_VERSION0 0x16
+
+#define ANX7688_EEPROM_FW_LOADED   0x01
+
+#define ANX7688_REG_STATUS_INT_MASK 0x17
+#define ANX7688_REG_STATUS_INT  0x28
+#define ANX7688_IRQS_RECEIVED_MSG   BIT(0)
+#define ANX7688_IRQS_RECEIVED_ACK   BIT(1)
+#define ANX7688_IRQS_VCONN_CHANGE   BIT(2)
+#define ANX7688_IRQS_VBUS_CHANGEBIT(3)
+#define ANX7688_IRQS_CC_STATUS_CHAN

Re: [ANNOUNCE] 5.10.204-rt100

[Pavel Machek]

Hi!

> > We (as in cip project), are trying to do -cip-rt releases
> > once a month. Are there any plans for 5.10-rt release any time soon?
> > That would help us ;-).
> 
> I already pushed v5.10-rt-next (containing v5.10.209-rt101-rc1) to
> kernel.org and kernelci should pick that up for comprehensive testing
> within the next hour. As soon as the testing is done I will perform the
> release dance.
> 
> My vacations started (abruptly) a few days before I planned and that lead
> to some delays. People volunteered to run the builds if anything critical
> popped up, but that was not the case.
> 
> Sorry for the inconvenience, I do hope a release tomorrow or Friday does
> not disrupt your workflow too much.

No problem, thanks for the information, and looking forward to the
release.

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: PGP signature

Re: [ANNOUNCE] 5.10.204-rt100

[Pavel Machek]

Hi!

We (as in cip project), are trying to do -cip-rt releases
once a month. Are there any plans for 5.10-rt release any time soon?
That would help us ;-).

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: PGP signature

Re: Front camera on pinephone

[Pavel Machek]

Hi!

> Isn't this simply the case of picking the gc2145 bits from Megis tree?
> 
> https://megous.com/git/linux/tree/arch/arm64/boot/dts/allwinner/sun50i-a64-pinephone.dtsi?h=orange-pi-5.10#n410

Well, that, adjusting dts bits to the new driver, testing and
submitting the patch. And it looks someone did the work and patches
are floating around. GOod :-).

I'm fighting with kexec crashing as soon as I pass dtb. Do I need to
modify the dtb somehow?

Best regards,
Pavel

Front camera on pinephone

[Pavel Machek]

Hi!

In 6.8-rc0, driver for gc2145 (front camera on pinephone) was merged,
but we don't have corresponding dts entries. Does anyone have setup
where they can fix it easily?

[If you have hints how to set-up pinephone for kernel development,
that would be welcome. Currently I have mobian with rather old 6.1
kernel on it, and lack of keyboard/ethernet makes things tricky.]

Best regards,
Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

Re: [PATCH v3 0/3] media: i2c: gc2145: GC2145 sensor support

[Pavel Machek]

Hi!

> This serie adds support for the GalaxyCore GC2145 sensor.
> Initialization is based on scripts provided by GalaxyCore,
> allowing 3 fixed configurations supported for the time being.

There is another version of the driver, from Ondrej Jirman, floating
around. See mail "gc2145 camera driver (front camera on
PinePhone)". I've added some cc-s in case people wanted to comment on
it.

Best regards,
Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

Re: [ANNOUNCE] 5.10.199-rt97

[Pavel Machek]

Hi!

> I'm pleased to announce the 5.10.199-rt97 stable release.
> 
> This release is an update to the new stable 5.10.199 version and no
> RT-specific changes were made, with the exception of a fix to a build
> failure due to upstream changes affecting only the PREEMPT_RT code.

Thanks for the release. Do you plan 5.10.200-based one by chance? .199
is buggy on hardware we care about, so it would make our job a bit
easier.

Thanks and best regards,
Pavel
-- 
DENX Software Engineering GmbH,Managing Director: Erika Unter
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: PGP signature

Re: [PATCH 0/2] arm64: dts: qcom: longcheer l8910 and l9100: Enable RGB LED

[Pavel Machek]

Hi!

> With the driver for ktd2026 recently applied to linux-leds[1], the LED
> can be enabled on longcheer l8910 and l9100.

Please make sure sysfs name is consistent with notification LED on
other phones, as documented by well-known-leds.txt.

Best regards,
Pavel
-- 

Re: droid4 -- weird behaviour when attempting to use usb host

[Pavel Machek]

Hi!

> > I'm having some fun with usb host. Good news is it works with
> > externally powered hub... after a while. I get some error messages
> > about inability to go host mode, but with enough patience it
> > eventually does enter host mode and I see my keyboard/mouse.
> > 
> > And usually in that process, one of my cpu cores disappear. top no
> > longer shows 2 cores, and I was wondering for a while if d4 is
> > single-core system. It is not, my two cores are back after reboot.
> > 
> > That's with 6.1.9 kernel from leste. Ideas how to debug this would be
> > welcome. (Do you use usb host?)
> 
> You are using a "proper" non-standard usb micro-b cable that grounds
> the id pin, right?

Yes.

> If not, try with one of those as it allows the hardware to do what it's
> supposed to do.
> 
> And presumably you don't have a hacked usb hub that feeds back the
> vbus to your phone, right?

Do have hacked hub. Or more precisely, have device that needs external
power (spinning rust), and hub passes it back to the device.

I'll retry with a keyboard... but I recall it behaved funny with that, too.

> If you have, that should not be used as the pmic can feed vbus.

Well, my plan was to use it as a desktop, and external power is useful
that as Droid battery is not that big.

Best regards,
Pavel
-- 
People of Russia, stop Putin before his war on Ukraine escalates.


signature.asc
Description: PGP signature

droid4 -- weird behaviour when attempting to use usb host

[Pavel Machek]

Hi!

I'm having some fun with usb host. Good news is it works with
externally powered hub... after a while. I get some error messages
about inability to go host mode, but with enough patience it
eventually does enter host mode and I see my keyboard/mouse.

And usually in that process, one of my cpu cores disappear. top no
longer shows 2 cores, and I was wondering for a while if d4 is
single-core system. It is not, my two cores are back after reboot.

That's with 6.1.9 kernel from leste. Ideas how to debug this would be
welcome. (Do you use usb host?)

Best regards,
Pavel
-- 

Re: [PATCH 5.11 007/122] dmaengine: idxd: Fix clobbering of SWERR overflow bit on writeback

[Pavel Machek]

Hi!

> Current code blindly writes over the SWERR and the OVERFLOW bits. Write
> back the bits actually read instead so the driver avoids clobbering the
> OVERFLOW bit that comes after the register is read.

I believe this is incorrect. Changelog explains that we need to
preserve bits in the register...

> diff --git a/drivers/dma/idxd/irq.c b/drivers/dma/idxd/irq.c
> index a60ca11a5784..f1463fc58112 100644
> --- a/drivers/dma/idxd/irq.c
> +++ b/drivers/dma/idxd/irq.c
> @@ -124,7 +124,9 @@ static int process_misc_interrupts(struct idxd_device 
> *idxd, u32 cause)
>   for (i = 0; i < 4; i++)
>   idxd->sw_err.bits[i] = ioread64(idxd->reg_base +
>   IDXD_SWERR_OFFSET + i * sizeof(u64));
> - iowrite64(IDXD_SWERR_ACK, idxd->reg_base + IDXD_SWERR_OFFSET);
> +
> + iowrite64(idxd->sw_err.bits[0] & IDXD_SWERR_ACK,
> +   idxd->reg_base + IDXD_SWERR_OFFSET);

...but that is not what the code does.

I suspect it should be 

> + iowrite64(idxd->sw_err.bits[0] | IDXD_SWERR_ACK,
> +   idxd->reg_base + IDXD_SWERR_OFFSET);

Best regards,
Pavel

-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH 5.10 043/103] net: tipc: Fix spelling errors in net/tipc module

[Pavel Machek]

Hi!

> [ Upstream commit a79ace4b312953c5835fafb12adc3cb6878b26bd ]
> 
> These patches fix a series of spelling errors in net/tipc module.

This should not be in -stable, it just cleans up comments.

Best regards,
Pavel

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 5.10 000/103] 5.10.32-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 5.10.32 release.
> There are 103 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.
> 
> Responses should be made by Wed, 21 Apr 2021 13:05:09 +.
> Anything received after that time might be too late.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-5.10.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 5.10 042/103] net/rds: Avoid potential use after free in rds_send_remove_from_sock

[Pavel Machek]

Hi!

> [ Upstream commit 0c85a7e87465f2d4cbc768e245f4f45b2f299b05 ]
> 
> In case of rs failure in rds_send_remove_from_sock(), the 'rm' resource
> is freed and later under spinlock, causing potential use-after-free.
> Set the free pointer to NULL to avoid undefined behavior.

This patch is crazy. Take a look at Message-ID:
<20210419084953.GA28564@amd>. Or just look at the patch :-).

Best regards,
Pavel
> +++ b/net/rds/message.c
> @@ -180,6 +180,7 @@ void rds_message_put(struct rds_message *rm)
>   rds_message_purge(rm);
>  
>   kfree(rm);
> + rm = NULL;
>   }
>  }
>  EXPORT_SYMBOL_GPL(rds_message_put);
> diff --git a/net/rds/send.c b/net/rds/send.c
> index 985d0b7713ac..fe5264b9d4b3 100644
> --- a/net/rds/send.c
> +++ b/net/rds/send.c
> @@ -665,7 +665,7 @@ static void rds_send_remove_from_sock(struct list_head 
> *messages, int status)
>  unlock_and_drop:
>   spin_unlock_irqrestore(>m_rs_lock, flags);
>   rds_message_put(rm);
> - if (was_on_sock)
> + if (was_on_sock && rm)
>   rds_message_put(rm);
>   }
>  

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH AUTOSEL 5.10 41/46] net/rds: Avoid potential use after free in rds_send_remove_from_sock

[Pavel Machek]

Hi!

> From: Aditya Pakki 
> 
> [ Upstream commit 0c85a7e87465f2d4cbc768e245f4f45b2f299b05 ]
> 
> In case of rs failure in rds_send_remove_from_sock(), the 'rm' resource
> is freed and later under spinlock, causing potential use-after-free.
> Set the free pointer to NULL to avoid undefined behavior.

This patch is crazy. It adds dead code.

> +++ b/net/rds/message.c
> @@ -180,6 +180,7 @@ void rds_message_put(struct rds_message *rm)
>   rds_message_purge(rm);
>  
>   kfree(rm);
> + rm = NULL;
>   }
>  }

We are already exiting function, changing local variable has no
effect.

> +++ b/net/rds/send.c
> @@ -665,7 +665,7 @@ static void rds_send_remove_from_sock(struct list_head 
> *messages, int status)
>  unlock_and_drop:
>   spin_unlock_irqrestore(>m_rs_lock, flags);
>   rds_message_put(rm);
> - if (was_on_sock)
> + if (was_on_sock && rm)
>   rds_message_put(rm);
>   }

If rm was non-NULL calling first rds_message_put (and it was,
otherwise we oopsed), it is still non-NULL in second test.

Best regards,
Pavel
-- 
'DENX Software Engineering GmbH,  Managing Director:Wolfgang Denk'
'HRB 165235 Munich, Office: Kirchenstr.5, D-82194   Groebenzell, Germany'



signature.asc
Description: Digital signature

Re: [PATCH] base: power: runtime.c: Remove a unnecessary space

[Pavel Machek]

On Sun 2021-04-18 09:11:43, Sebastian Fricke wrote:
> Hey Joe,
> 
> On 18.04.2021 00:09, Joe Perches wrote:
> >On Sun, 2021-04-18 at 06:08 +, Sebastian Fricke wrote:
> >>Remove a redundant space to improve the quality of the comment.
> >
> >I think this patch is not useful.
> >
> >It's not redundant.
> 
> Thank you, I actually found this pattern a few more times but I wanted
> to check first if this is a mistake or chosen consciously.

It is okay as it is. No need to remove extra space. No need to add it,
either.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH 4.19 00/13] 4.19.188-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 4.19.188 release.
> There are 13 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any kernel problems here: (some boards are
unavailable)

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.19.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 5.10 00/25] 5.10.31-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 5.10.31 release.
> There are 25 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any kernel problems here: (some boards
are unavailable)

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.4.y
https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.19.y
https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-5.10.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 4.4 00/38] 4.4.267-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 4.4.267 release.
> There are 38 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.4.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel


-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 5.10 000/188] 5.10.30-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 5.10.30 release.
> There are 188 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-5.10.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 4.19 00/66] 4.19.187-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 4.19.187 release.
> There are 66 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.19.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 4/4] rtc:abx80x: Enable xt digital calibration

[Pavel Machek]

Hi!

> The XT digital calibration feature allows to improve the RTC accuracy,
> using a Distributed Digital Calibration function.
> See ch. 5.9.1 of AB08XX Series Ultra Low Power RTC IC User's Guide
> https://abracon.com/realtimeclock/AB08XX-Application-Manual.pdf
> 
> Signed-off-by: Kirill Kapranov 
> ---
>  drivers/rtc/rtc-abx80x.c | 145 
> +++
>  1 file changed, 145 insertions(+)


>  
> +static const int xt_freq_nom = 32768000; //Nominal XT frequency 32 kHz in mHz

C-style comment?

is it 32 kHz or 32.768 kHz?

> +{
> + int retval;
> + long Adj;

Adj -> adj.

> +
> +static DEVICE_ATTR_WO(xt_frequency);

You are adding new user interface, you sould add documentation.

Best regards,
Pavel

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 5.10 06/41] net: pxa168_eth: Fix a potential data race in pxa168_eth_remove

[Pavel Machek]

Hi!

> [ Upstream commit 0571a753cb07982cc82f4a5115e0b321da89e1f3 ]
> 
> pxa168_eth_remove() firstly calls unregister_netdev(),
> then cancels a timeout work. unregister_netdev() shuts down a device
> interface and removes it from the kernel tables. If the timeout occurs
> in parallel, the timeout work (pxa168_eth_tx_timeout_task) performs stop
> and open of the device. It may lead to an inconsistent state and memory
> leaks.

AFAICT the timeout work does a lot of processing, including
pxa168_eth_open(), pxa168_init_phy() and phy_connect_direct(). We
probably don't want that to run with clock being disabled and DMA
being unmapped.

We certainly don't want phy_disconnect() being undone by
phy_connect_direct() running in the workqueue.

IOW this patch is not enough to fix the bugs, and at least fix below
is needed to get something reasonable.

Signed-off-by: Pavel Machek (CIP) 

Best regards,
Pavel

> +++ b/drivers/net/ethernet/marvell/pxa168_eth.c
> @@ -1544,8 +1544,8 @@ static int pxa168_eth_remove(struct platform_device 
> *pdev)
>   clk_disable_unprepare(pep->clk);
>   mdiobus_unregister(pep->smi_bus);
>   mdiobus_free(pep->smi_bus);
> - unregister_netdev(dev);
>   cancel_work_sync(>tx_timeout_task);
> + unregister_netdev(dev);
>   free_netdev(dev);
>   return 0;
>  }

diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c 
b/drivers/net/ethernet/marvell/pxa168_eth.c
index d1e4d42e497d..432be22a51be 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -1532,7 +1532,8 @@ static int pxa168_eth_remove(struct platform_device *pdev)
 {
struct net_device *dev = platform_get_drvdata(pdev);
struct pxa168_eth_private *pep = netdev_priv(dev);
-
+   
+   cancel_work_sync(>tx_timeout_task);
if (pep->htpr) {
dma_free_coherent(pep->dev->dev.parent, HASH_ADDR_TABLE_SIZE,
  pep->htpr, pep->htpr_dma);
@@ -1545,7 +1546,6 @@ static int pxa168_eth_remove(struct platform_device *pdev)
mdiobus_unregister(pep->smi_bus);
mdiobus_free(pep->smi_bus);
unregister_netdev(dev);
-   cancel_work_sync(>tx_timeout_task);
free_netdev(dev);
return 0;
 }

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCHv2] gnss: motmdm: Add support for Motorola Mapphone MDM6600 modem

[Pavel Machek]

Hi!

> > Could I get some comments on the gnss patch? It is fairly simple, and
> > I believe it is ready for merge.
> 
> Let's get the mux driver in shape first.

Well, gnss driver is now completely independent of that in the new
versions.

> > Here's new version of the serdev multiplexing patch for reference. And
> > yes, I'd like you to review the design here, too. Yes,
> > gsm_serdev_register_tty_port() needs to be cleaned up, but with ifdefs
> > you can see alternative approach I tried to work around "controller
> > busy" problem.
> 
> As I said before, if you're trying to work around the one
> client-per-port assumption of serdev, you're doing it wrong. You still
> have one client per *virtual* port.

Yes, I have one client per virtual port. But those virtual ports need
their parents.

We can have

serial@ -- motorola,mapphone-mdm6600-serial
(serdev here)
modem -- ts27010-mux
(here is the problem!)
serial@4 --  serial@1 -- 
(serdev here)(serdev here)
gnss -- motorola,mapphone-mdm6600-gnss   modem voice -- something

and I believe we agree on that. But serial@1 and serial@4 still need
some kind of parent, and that's where I'm getting the controller busy
problem.

> > +++ b/Documentation/devicetree/bindings/serdev/serdev-ngsm.yaml
> 
> > +  ttymask:
> > +$ref: /schemas/types.yaml#/definitions/uint64
> > +description: Mask of the TS 27.010 channel TTY interfaces to start (64 
> > bit)
> 
> How is this intended to be used? Looks too Linux-specific for a binding.

The mask says which virtual ports contain "something". We have kernel
clients for some, and we want to expose the rest of the used ones to
the userspace.

> So is this is probably the issue: you're skipping one level of the
> topology and do not describe the virtual ports.

Well, docs need to be updated.

> You should probably just replace the ttymask property with explicit
> nodes for the virtual ports to enable. With no client devices described
> in devicetree, these will then show up as regular tty devices under
> Linux.

Yes, that can be done.

> If you add a compatible property for the virtual ports (e.g.
> "ts27010-port") you might be able to just use of_platform_populate()
> instead of walking the child nodes in gsm_serdev_register_tty_port().

I have "gsmmux,port" for that. And yes, I can take a look at
of_platform_populate.

But as I explained above, there's still problem with the parent
devices.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH 4.4 00/28] 4.4.265-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 4.4.265 release.
> There are 28 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.4.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 4.19 00/56] 4.19.185-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 4.19.185 release.
> There are 56 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any kernel problems here: (Siemens boards
are unavailable)

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.19.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 5.10 000/126] 5.10.28-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 5.10.28 release.
> There are 126 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any kernel problems here: (Siemens boards
are unavailable)

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-5.10.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 5.10 079/126] drm/tegra: sor: Grab runtime PM reference across reset

[Pavel Machek]

Hi!

> > > However, these functions alone don't provide any guarantees at the
> > > system level. Drivers need to ensure that the only a single consumer has
> > > access to the reset at the same time. In order for the SOR to be able to
> > > exclusively access its reset, it must therefore ensure that the SOR
> > > power domain is not powered off by holding on to a runtime PM reference
> > > to that power domain across the reset assert/deassert operation.
> > 
> > Yeah, but it should not leak the PM reference in the error handling.
> 
> True. However the only reason why the code could realistically fail
> between pm_runtime_resume_and_get() and pm_runtime_put() is if we did
> not take the runtime PM reference (which would cause the call to
> reset_control_acquire() to fail).
> 
> So the chances of this actually leaking are practically zero, so I
> didn't want to bloat this bugfix with what's essentially dead code. I
> can queue up your fix below for v5.14, though, since it's obviously
> more correct from a theoretical point of view.

Yes, that sounds like good solution, thanks!

Best regards,
Pavel

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 5.10 079/126] drm/tegra: sor: Grab runtime PM reference across reset

[Pavel Machek]

Hi!

> However, these functions alone don't provide any guarantees at the
> system level. Drivers need to ensure that the only a single consumer has
> access to the reset at the same time. In order for the SOR to be able to
> exclusively access its reset, it must therefore ensure that the SOR
> power domain is not powered off by holding on to a runtime PM reference
> to that power domain across the reset assert/deassert operation.

Yeah, but it should not leak the PM reference in the error handling.

Signed-off-by: Pavel Machek (CIP) 

Best regards,
Pavel

diff --git a/drivers/gpu/drm/tegra/sor.c b/drivers/gpu/drm/tegra/sor.c
index 7b88261f57bb..b3b727b2a3c5 100644
--- a/drivers/gpu/drm/tegra/sor.c
+++ b/drivers/gpu/drm/tegra/sor.c
@@ -3125,21 +3125,21 @@ static int tegra_sor_init(struct host1x_client *client)
if (err < 0) {
dev_err(sor->dev, "failed to acquire SOR reset: %d\n",
err);
-   return err;
+   goto maybe_put;
}
 
err = reset_control_assert(sor->rst);
if (err < 0) {
dev_err(sor->dev, "failed to assert SOR reset: %d\n",
err);
-   return err;
+   goto maybe_put;
}
}
 
err = clk_prepare_enable(sor->clk);
if (err < 0) {
dev_err(sor->dev, "failed to enable clock: %d\n", err);
-   return err;
+   goto maybe_put;
}
 
usleep_range(1000, 3000);
@@ -3150,7 +3150,7 @@ static int tegra_sor_init(struct host1x_client *client)
dev_err(sor->dev, "failed to deassert SOR reset: %d\n",
err);
clk_disable_unprepare(sor->clk);
-   return err;
+   goto maybe_put;
}
 
reset_control_release(sor->rst);
@@ -3171,6 +3171,11 @@ static int tegra_sor_init(struct host1x_client *client)
}
 
return 0;
+
+ maybe_put:
+   if (sor->rst)
+   pm_runtime_put(sor->dev);
+   return err;
 }
 
 static int tegra_sor_exit(struct host1x_client *client)


-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 5.10 048/126] net: ethernet: aquantia: Handle error cleanup of start on open

[Pavel Machek]

Hi!

> From: Nathan Rossi 
> 
> [ Upstream commit 8a28af7a3e85ddf358f8c41e401a33002f7a9587 ]
> 
> The aq_nic_start function can fail in a variety of cases which leaves
> the device in broken state.
> 
> An example case where the start function fails is the
> request_threaded_irq which can be interrupted, resulting in a EINTR
> result. This can be manually triggered by bringing the link up (e.g. ip
> link set up) and triggering a SIGINT on the initiating process (e.g.
> Ctrl+C). This would put the device into a half configured state.
> Subsequently bringing the link up again would cause the napi_enable to
> BUG.
> 
> In order to correctly clean up the failed attempt to start a device call
> aq_nic_stop.

No.

> +++ b/drivers/net/ethernet/aquantia/atlantic/aq_main.c
> @@ -71,8 +71,10 @@ static int aq_ndev_open(struct net_device *ndev)
>   goto err_exit;
>  
>   err = aq_nic_start(aq_nic);
> - if (err < 0)
> + if (err < 0) {
> + aq_nic_stop(aq_nic);
>   goto err_exit;
> + }
>  
>  err_exit:
>   if (err < 0)

First, take a look at the goto. Does not need to be there.

Second check the crazy calling convention. If nic_start() fails, it
should clean up after itself.

Then, check the code. nic_stop() undoes initialization that was not
even done in the nic_start().

This introduces more problems than it solves.

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 5.10 047/126] ath10k: hold RCU lock when calling ieee80211_find_sta_by_ifaddr()

[Pavel Machek]

Hi!

> Fix ath10k_wmi_tlv_op_pull_peer_stats_info() to hold RCU lock before it
> calls ieee80211_find_sta_by_ifaddr() and release it when the resulting
> pointer is no longer needed.

It does that. But is also does the unlock even if it did not take the
lock:

> +++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
> @@ -576,13 +576,13 @@ static void ath10k_wmi_event_tdls_peer(struct ath10k 
> *ar, struct sk_buff *skb)
>   case WMI_TDLS_TEARDOWN_REASON_TX:
>   case WMI_TDLS_TEARDOWN_REASON_RSSI:
>   case WMI_TDLS_TEARDOWN_REASON_PTR_TIMEOUT:
> + rcu_read_lock();
>   station = ieee80211_find_sta_by_ifaddr(ar->hw,
>  ev->peer_macaddr.addr,
>  NULL);
>   if (!station) {
>   ath10k_warn(ar, "did not find station from tdls peer 
> event");
> - kfree(tb);
> - return;
> + goto exit;
>   }
>   arvif = ath10k_get_arvif(ar, __le32_to_cpu(ev->vdev_id));
>   ieee80211_tdls_oper_request(
> @@ -593,6 +593,9 @@ static void ath10k_wmi_event_tdls_peer(struct ath10k *ar, 
> struct sk_buff *skb)
>   );
>   break;
>   }
> +
> +exit:
> + rcu_read_unlock();
>   kfree(tb);
>  }

The switch only takes the lock in 3 branches, but it is released
unconditionally at the end.

Something like this?

Best regards,
Pavel

Signed-off-by: Pavel Machek (CIP) 

diff --git a/drivers/net/wireless/ath/ath10k/wmi-tlv.c 
b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
index e7072fc4f487..e03ff56d938b 100644
--- a/drivers/net/wireless/ath/ath10k/wmi-tlv.c
+++ b/drivers/net/wireless/ath/ath10k/wmi-tlv.c
@@ -582,20 +582,19 @@ static void ath10k_wmi_event_tdls_peer(struct ath10k *ar, 
struct sk_buff *skb)
   NULL);
if (!station) {
ath10k_warn(ar, "did not find station from tdls peer 
event");
-   goto exit;
-   }
-   arvif = ath10k_get_arvif(ar, __le32_to_cpu(ev->vdev_id));
-   ieee80211_tdls_oper_request(
+   } else {
+   arvif = ath10k_get_arvif(ar, 
__le32_to_cpu(ev->vdev_id));
+   ieee80211_tdls_oper_request(
arvif->vif, station->addr,
NL80211_TDLS_TEARDOWN,
WLAN_REASON_TDLS_TEARDOWN_UNREACHABLE,
GFP_ATOMIC
);
+   }
+   rcu_read_unlock();
break;
}
 
-exit:
-   rcu_read_unlock();
kfree(tb);
 }
 


-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH 5.10 039/126] can: dev: move driver related infrastructure into separate subdir

[Pavel Machek]

Hi!

> From: Marc Kleine-Budde 
> 
> [ Upstream commit 3e77f70e734584e0ad1038e459ed3fd2400f873a ]
> 
> This patch moves the CAN driver related infrastructure into a separate subdir.
> It will be split into more files in the coming patches.

I don't think this is suitable for stable. I don't think any of the
follow up patches depend on it...?

Best regards,
Pavel

>  drivers/net/can/Makefile   | 7 +--
>  drivers/net/can/dev/Makefile   | 7 +++
>  drivers/net/can/{ => dev}/dev.c| 0
>  drivers/net/can/{ => dev}/rx-offload.c | 0

-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH 5.10 004/126] ext4: shrink race window in ext4_should_retry_alloc()

[Pavel Machek]

Hi!

> From: Eric Whitney 

> A per filesystem percpu counter exported via sysfs is added to allow
> users or developers to track the number of times the retry limit is
> exceeded without resorting to debugging methods.  This should provide
> some insight into worst case retry behavior.

This adds new counter exported via sysfs, but no documentation of that
counter...

Best regards,
Pavel

> @@ -257,6 +259,7 @@ static struct attribute *ext4_attrs[] = {
>   ATTR_LIST(session_write_kbytes),
>   ATTR_LIST(lifetime_write_kbytes),
>   ATTR_LIST(reserved_clusters),
> + ATTR_LIST(sra_exceeded_retry_limit),
>   ATTR_LIST(inode_readahead_blks),
>   ATTR_LIST(inode_goal),
>   ATTR_LIST(mb_stats),

-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH] mmc: dw_mmc-k3: use the correct HiSilicon copyright

[Pavel Machek]

Hi!

> On Tue, 30 Mar 2021 at 08:43, Hao Fang  wrote:
> >
> > s/Hisilicon/HiSilicon/g.
> > It should use capital S, according to
> > https://www.hisilicon.com/en/terms-of-use.
> >
> > Signed-off-by: Hao Fang 

> >  /*
> >   * Copyright (c) 2013 Linaro Ltd.
> > - * Copyright (c) 2013 Hisilicon Limited.
> > + * Copyright (c) 2013 HiSilicon Limited.
> 
> This change looks really silly to me, but I am not a lawyer - so I
> can't tell if this makes sense or not.
> 
> In any case, "Hisilicon" is being used all over the kernel - do you
> intend to patch all places with similar changes like this one? Perhaps
> just send a big tree-wide-patch instead and see what people think?

Plus, I'd expect authors to submit changes to copyright notices.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH 4.4 00/33] 4.4.264-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 4.4.264 release.
> There are 33 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.4.y
https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.19.y
https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-5.10.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

[PATCH] ASoC: audio-graph-card: Add audio mixer for Motorola mdm6600

[Pavel Machek]

motmdm.c handles audio configuration on "gsmtty2"; it needs to know
whether we are in call or not; that part is in motmdm-state.c and
listens on "gsmtty1".

To configure Alsamixer for voice calls do for example:

Speaker Right -> Voice
Call Noise Cancellation -> Unmute
Call Output -> Speakerphone
Call -> 100
Mic2 -> 40
Left -> Mic 2
Voice -> 55
Mic2 -> 40
Left -> Mic 2

Tony wrote original version using custom interface to n_gsm, Pavel
switched it to plain serdev and split it to two drivers to be easier
to debug and understand. Credit is Tony's, bugs are probably Pavel's.

Signed-off-by: Pavel Machek 
Co-authored-by: Tony Lindgren 


---

v2: Fix compile error.
---
 sound/soc/codecs/Kconfig|   8 +
 sound/soc/codecs/Makefile   |   4 +
 sound/soc/codecs/motmdm-state.c | 177 
 sound/soc/codecs/motmdm.c   | 688 
 4 files changed, 877 insertions(+)
 create mode 100644 sound/soc/codecs/motmdm-state.c
 create mode 100644 sound/soc/codecs/motmdm.c

diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
index ba4eb54aafcb..94000ab3cf3e 100644
--- a/sound/soc/codecs/Kconfig
+++ b/sound/soc/codecs/Kconfig
@@ -932,6 +932,14 @@ config SND_SOC_MAX9860
depends on I2C
select REGMAP_I2C
 
+config SND_SOC_MOTMDM
+   tristate "Motorola Modem TS 27.010 Voice Call Codec"
+   depends on SERIAL_DEV_BUS
+   help
+ Enable support for Motorola TS 27.010 serdev voice
+ call codec driver for Motorola Mapphone series of
+ devices such as Droid 4.
+
 config SND_SOC_MSM8916_WCD_ANALOG
tristate "Qualcomm MSM8916 WCD Analog Codec"
depends on SPMI || COMPILE_TEST
diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
index d277f0366e09..f63022e0a292 100644
--- a/sound/soc/codecs/Makefile
+++ b/sound/soc/codecs/Makefile
@@ -128,6 +128,8 @@ snd-soc-max9850-objs := max9850.o
 snd-soc-max9860-objs := max9860.o
 snd-soc-mc13783-objs := mc13783.o
 snd-soc-ml26124-objs := ml26124.o
+snd-soc-motmdm-objs := motmdm.o
+snd-soc-motmdm-state-objs := motmdm-state.o
 snd-soc-msm8916-analog-objs := msm8916-wcd-analog.o
 snd-soc-msm8916-digital-objs := msm8916-wcd-digital.o
 snd-soc-mt6351-objs := mt6351.o
@@ -443,6 +445,8 @@ obj-$(CONFIG_SND_SOC_MAX9850)   += snd-soc-max9850.o
 obj-$(CONFIG_SND_SOC_MAX9860)  += snd-soc-max9860.o
 obj-$(CONFIG_SND_SOC_MC13783)  += snd-soc-mc13783.o
 obj-$(CONFIG_SND_SOC_ML26124)  += snd-soc-ml26124.o
+obj-$(CONFIG_SND_SOC_MOTMDM)   += snd-soc-motmdm.o
+obj-$(CONFIG_SND_SOC_MOTMDM)   += snd-soc-motmdm-state.o
 obj-$(CONFIG_SND_SOC_MSM8916_WCD_ANALOG) +=snd-soc-msm8916-analog.o
 obj-$(CONFIG_SND_SOC_MSM8916_WCD_DIGITAL) +=snd-soc-msm8916-digital.o
 obj-$(CONFIG_SND_SOC_MT6351)   += snd-soc-mt6351.o
diff --git a/sound/soc/codecs/motmdm-state.c b/sound/soc/codecs/motmdm-state.c
new file mode 100644
index ..a3b2870c5167
--- /dev/null
+++ b/sound/soc/codecs/motmdm-state.c
@@ -0,0 +1,177 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Motorola Mapphone MDM6600 voice call audio support
+ * Copyright 2018 - 2020 Tony Lindgren 
+ * Copyright 2020 - 2021 Pavel Machek 
+ *
+ * Designed to provide notifications about voice call state to the
+ * motmdm.c driver. This one listens on "gsmtty1".
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+
+#define MOTMDM_HEADER_LEN  5   /* U1234 */
+#define MOTMDM_AUDIO_MAX_LEN   128
+#define MOTMDM_VOICE_RESP_LEN  7   /* U1234~+CIEV= */
+
+struct motmdm_driver_data {
+   struct serdev_device *serdev;
+   unsigned char *buf;
+   size_t len;
+   spinlock_t lock;/* enable/disabled lock */
+};
+
+static BLOCKING_NOTIFIER_HEAD(modem_state_chain_head);
+
+int register_modem_state_notifier(struct notifier_block *nb)
+{
+   return blocking_notifier_chain_register(_state_chain_head, nb);
+}
+EXPORT_SYMBOL_GPL(register_modem_state_notifier);
+
+int unregister_modem_state_notifier(struct notifier_block *nb)
+{
+   return blocking_notifier_chain_unregister(_state_chain_head, nb);
+}
+EXPORT_SYMBOL_GPL(unregister_modem_state_notifier);
+
+static int modem_state_notifier_call_chain(unsigned long val)
+{
+   int ret;
+   ret = __blocking_notifier_call_chain(_state_chain_head, val, NULL,
+-1, NULL);
+   return notifier_to_errno(ret);
+}
+
+/* Parses the voice call state from unsolicited notifications on dlci1 */
+static int motmdm_voice_get_state(struct motmdm_driver_data *ddata,
+  const unsigned char *buf,
+  size_t len)
+{
+   struct device *dev = >serdev->dev;
+   bool enable;
+   const unsigned char *state;
+
+   if (len < MOTMDM_HEADER_LEN + MOTMDM_VOICE_RESP_LEN + 5)
+ 

Re: [PATCH -next] drivers: net: CONFIG_ATH9K select LEDS_CLASS

[Pavel Machek]

On Fri 2021-03-26 16:13:51, Zhang Jianhua wrote:
> If CONFIG_ATH9K=y, the following errors will be seen while compiling
> gpio.c
> 
> drivers/net/wireless/ath/ath9k/gpio.o: In function `ath_deinit_leds':
> gpio.c:(.text+0x604): undefined reference to `led_classdev_unregister'
> gpio.c:(.text+0x604): relocation truncated to fit: R_AARCH64_CALL26
> against undefined symbol `led_classdev_unregister'
> drivers/net/wireless/ath/ath9k/gpio.o: In function `ath_init_leds':
> gpio.c:(.text+0x708): undefined reference to `led_classdev_register_ext'
> gpio.c:(.text+0x708): relocation truncated to fit: R_AARCH64_CALL26
> against undefined symbol `led_classdev_register_ext'
> 
> Reported-by: Hulk Robot 
> Signed-off-by: Zhang Jianhua 

a) please cc led lists with led issue.

b) probably does not work as LED_CLASS depends on NEW_LEDS...?

Best regards,
Pavel

-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH 17/17] auxdisplay: ht16k33: Add segment display LED support

[Pavel Machek]

Hi!

> > > > > - err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS);
> > > > > + of_property_read_u32(node, "color", );
> > > > > + seg->led.name = devm_kasprintf(dev, GFP_KERNEL,
> > > > > + DRIVER_NAME ":%s:" LED_FUNCTION_BACKLIGHT,
> > > > > + color < LED_COLOR_ID_MAX ? led_colors[color] : 
> > > > > "");
> >
> > And would prefer not to see driver_name as part of LED name.
> 
> OK. So what should I use instead? Or just leave the part before the
> first colon empty?

I'd suggest auxdisplay:...:backlight.

And we should start documenting this somewhere.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH v2 2/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver

[Pavel Machek]

Hi!

> Introduce a new multiple GPIOs LED driver. This LED will made of
> multiple GPIOs (up to 8) and will map different brightness to different
> GPIOs states which defined in dts file.
> 
> Signed-off-by: Hermes Zhang 


> index ..7aef82701f86
> --- /dev/null
> +++ b/drivers/leds/simple/Kconfig
> @@ -0,0 +1,23 @@
> +menuconfig LEDS_SIMPLE
> + bool "Simple LED support"
> + depends on LEDS_CLASS
> + help
> +   This option enables simple leds support for the leds class.
> +   If unsure, say Y.

No need for new config symbol.

> + if (ret != 1 << gpios->ndescs) {

I'd do (ret != (1 << gpios->ndescs))

> + priv = devm_kzalloc(dev, sizeof(struct multi_gpio_led_priv)
> + + sizeof(u8) * nr_states , GFP_KERNEL);

Sizeof(u8) is always 1, no need for space before , . 

> + of_property_read_string(node, "default-state", );
> + if (!strcmp(state, "on"))
> + multi_gpio_led_set(>cdev, priv->cdev.max_brightness);
> + else
> + multi_gpio_led_set(>cdev, 0);

No need for default-state handling, unless you are using it.

Best regards,
Pavel

-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

[PATCH] remove Dan Murphy from TI from MAINTAINERS

[Pavel Machek]

Dan's address bounces, and has been bouncing for some time as he moved
to other projects.

I believe TI should be more careful with this, and should assign
alternate contacts for their drivers.

Anyway what we can do now is to remove the obsolete address.

Signed-off-by: Pavel Machek 

---

(I'm sending this directly; you can also have it as a pull request for
late -rc or with the LEDs in next -rc1... but I believe applying it
directly is easiest).

diff --git a/MAINTAINERS b/MAINTAINERS
index d92f85ca831d..11d83c30b737 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -10030,7 +10030,6 @@ F:  scripts/leaking_addresses.pl
 
 LED SUBSYSTEM
 M: Pavel Machek 
-R: Dan Murphy 
 L: linux-l...@vger.kernel.org
 S: Maintained
 T: git git://git.kernel.org/pub/scm/linux/kernel/git/pavel/linux-leds.git
@@ -10905,7 +10904,6 @@ T:  git git://linuxtv.org/media_tree.git
 F: drivers/media/radio/radio-maxiradio*
 
 MCAN MMIO DEVICE DRIVER
-M: Dan Murphy 
 M: Pankaj Sharma 
 L: linux-...@vger.kernel.org
 S: Maintained
@@ -17846,7 +17844,6 @@ S:  Maintained
 F: drivers/thermal/ti-soc-thermal/
 
 TI BQ27XXX POWER SUPPLY DRIVER
-R: Dan Murphy 
 F: drivers/power/supply/bq27xxx_battery.c
 F: drivers/power/supply/bq27xxx_battery_i2c.c
 F: include/linux/power/bq27xxx_battery.h
@@ -17981,7 +17978,6 @@ S:  Odd Fixes
 F: sound/soc/codecs/tas571x*
 
 TI TCAN4X5X DEVICE DRIVER
-M: Dan Murphy 
 L: linux-...@vger.kernel.org
 S: Maintained
 F: Documentation/devicetree/bindings/net/can/tcan4x5x.txt

-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH v3 2/2] leds: rt4505: Add support for Richtek RT4505 flash LED controller

[Pavel Machek]

Hi!

> Add support for RT4505 flash LED controller. It can support up to 1.5A
> flash current with hardware timeout and low input voltage protection.
> 
> Signed-off-by: ChiYuan Huang 
> Acked-by: Jacek Anaszewski 

Thank you, applied and pushed out.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: linux-next: Signed-off-by missing for commit in the leds tree

[Pavel Machek]

Hi!

> Commit
> 
>   feac7685def1 ("leds: Kconfig: LEDS_CLASS is usually selected.")
> 
> is missing a Signed-off-by from its author and committer.

Thank you, fixed.
Pavel


-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH 2/2] dt-binding: leds: Document leds-multi-gpio bindings

[Pavel Machek]

Hi!

> See below.

Please trim.

> > +  led-gpios:
> > +description: Array of one or more GPIOs pins used to control the LED.
> > +minItems: 1
> > +maxItems: 8  # Should be enough
> 
> We also have a case with multi color LEDs (which is probably a more common
> than multi intensity LED. So I am wondering how these both could co-exist.
> 
> From: 
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/devicetree/bindings/leds/leds-gpio.yaml?h=v5.12-rc4#n58
> 
> led-0 {
> gpios = <_pio 0 GPIO_ACTIVE_LOW>;
> linux,default-trigger = "disk-activity";
> function = LED_FUNCTION_DISK;
> };
> 
> Now 'gpios' (and in LED context) and 'led-gpios' is very close to each other
> and could easily be confused.
> 
> Perhaps this could be something like:
> 
> intensity-gpios = ...
> 
> or even simplified then just to gpios = <...>

...
> How would this sound?

Well, not too bad on a quick look.

Are you willing to implement such multi-color-multi-bit-multi-gpio
driver?

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH v2 1/1] leds: lgm: Improve Kconfig help

[Pavel Machek]

Hi!

> Remove unnecessary Kconfig symbol LEDS_BLINK
> Improve Kconfig help text to make it more useful.
> 
> Signed-off-by: Rahul Tanwar 

Thanks for the patch, I applied it by hand.

> +++ b/drivers/leds/blink/leds-lgm-sso.c
> @@ -1,8 +1,8 @@
>  // SPDX-License-Identifier: GPL-2.0
>  /*
> - * Intel Lightning Mountain SoC LED Serial Shift Output Controller driver
> + * Lightning Mountain SoC LED Serial Shift Output Controller driver
>   *
> - * Copyright (c) 2020 Intel Corporation.
> + * Copyright (c) 2021 Intel Corporation.
>   */
> 

Not taking this. Talk to your lawyers why.

> This electronic mail (including any attachments) may contain
  information that is privileged, confidential, and/or otherwise
  protected from disclosure to anyone other than its intended
  recipient(s). Any dissemination or use of this electronic mail or
  its contents (including any attachments) by persons other than the
  intended recipient(s) is strictly prohibited. If you have received
  this message in error, please notify us immediately by reply e-mail
  so that we may correct our internal records. Please then delete the
  original message (including any attachments) in its entirety.

And talk to them about this, too.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH v2 1/2] leds: rt4505: Add support for Richtek RT4505 flash LED controller

[Pavel Machek]

On Thu 2021-03-25 18:19:03, ChiYuan Huang wrote:
> Pavel Machek  於 2021年3月25日 週四 下午6:01寫道：
> >
> > Hi!
> >
> > > > >   create mode 100644 drivers/leds/flash/Kconfig
> > > > >   create mode 100644 drivers/leds/flash/Makefile
> > > > >   create mode 100644 drivers/leds/flash/leds-rt4505.c
> > > >
> > > > Acked-by: Jacek Anaszewski 
> > > >
> > > Any problem with this patch? Do I need to submit it again?
> >
> > It won't apply on current next.
> >
> > So please: Merge ACKs, reorder it so that docs goes first, port it to
> >
> > To gitolite.kernel.org:pub/scm/linux/kernel/git/pavel/linux-leds.git
> >34731ed13e8a..85674b0e40d9  for-next -> for-next
> >
> > and resubmit.
> 
> Thx. It's clear.
> So the next I need to do is
> 1. Merge ACKs
> 2. Reorder this patch from the docs first
> 
> After done, do I need to change the patch revision from v2 to v3
> before submitng it?

Most important step is

0. port it to the for-next tree.

And yes, you can increase patch revision.

Best regards,

Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH v2 1/2] leds: rt4505: Add support for Richtek RT4505 flash LED controller

[Pavel Machek]

Hi!

> > >   create mode 100644 drivers/leds/flash/Kconfig
> > >   create mode 100644 drivers/leds/flash/Makefile
> > >   create mode 100644 drivers/leds/flash/leds-rt4505.c
> >
> > Acked-by: Jacek Anaszewski 
> >
> Any problem with this patch? Do I need to submit it again?

It won't apply on current next.

So please: Merge ACKs, reorder it so that docs goes first, port it to

To gitolite.kernel.org:pub/scm/linux/kernel/git/pavel/linux-leds.git
   34731ed13e8a..85674b0e40d9  for-next -> for-next

and resubmit.

Thanks you,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH 5.10 000/150] 5.10.26-rc3 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 5.10.26 release.
> There are 150 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-5.10.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: PGP signature

Re: [PATCH 1/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver

[Pavel Machek]

Hi!

> > +   of_property_read_string(node, "default-state", );
> > +   if (!strcmp(state, "on"))
> > +   multi_gpio_led_set(>cdev, LED_FULL);
> > +   else
> > +   multi_gpio_led_set(>cdev, LED_OFF);
> 
> Again LED_FULL and LED_OFF...
> What about default-state = "keep" ?

Let's not support default-state unless you need it.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH 1/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver

[Pavel Machek]

On Wed 2021-03-24 15:56:30, Hermes Zhang wrote:
> From: Hermes Zhang 
> 
> Introduce a new multiple GPIOs LED driver. This LED will made of
> multiple GPIOs (up to 8) and will map different brightness to different
> GPIOs states which defined in dts file.
> 
> Signed-off-by: Hermes Zhang 
> ---
>  drivers/leds/Kconfig   |  12 +++
>  drivers/leds/Makefile  |   1 +
>  drivers/leds/leds-multi-gpio.c | 140 +
>  3 files changed, 153 insertions(+)
>  create mode 100644 drivers/leds/leds-multi-gpio.c

Let's put it into drivers/leds/simple. You may need to create it.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH 1/2] leds: leds-multi-gpio: Add multiple GPIOs LED driver

[Pavel Machek]

Hi!

> > From: Hermes Zhang 
> > 
> > Introduce a new multiple GPIOs LED driver. This LED will made of
> > multiple GPIOs (up to 8) and will map different brightness to different
> > GPIOs states which defined in dts file.
> 
> I wonder how many boards have such LEDs.
> 
> Also if it wouldn't be better to expand the original leds-gpio driver.
> Probably depends on how much larger would such expansion make the
> leds-gpio driver.

Let's start with separate.

> Use flexible array members. Allocate with
>   devm_kzalloc(dev, struct_size(priv, states, priv->nr_states),
>GFP_KERNEL)

Better yet, assume the brightness is 0..2^(num leds) and avoid this
complexity.

> Again LED_FULL and LED_OFF...
> What about default-state = "keep" ?
> 
> Hermes, do you actually have a device that controls LEDs this way? How
> many brightness options do they have?

He has two bits.

> Also I think this functionality could be easily incorporated into the
> existing leds-gpio driver, instead of creating new driver.

> Moreover your driver can control only one LED, so it needs to be
> probed multiple times for multiple LEDs. Meanwhile the leds-gpio driver
> can register multiple LEDs in one probe...

The current version is mostly fine. Let's not overcomplicate it.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH v3] dt-binding: leds: Document leds-multi-gpio bindings

[Pavel Machek]

On Wed 2021-03-24 06:58:21, Alexander Dahl wrote:
> Hello Hermes,
> 
> > Hermes Zhang  hat am 24.03.2021 03:48 geschrieben:
> > 
> >  
> > From: Hermes Zhang 
> > 
> > Document the device tree bindings of the multiple GPIOs LED driver
> > Documentation/devicetree/bindings/leds/leds-multi-gpio.yaml.
> > 
> > Signed-off-by: Hermes Zhang 
> > ---
> > 
> > Notes:
> > Add maxItems
> 
> What about the other part of the series? I think you should send both patches 
> together with an introduction message on both. If you only change one patch 
> for a new version spin of the series, just send the other one unchanged.
>

Please wrap emails after 72 columns or so.

> (It makes no sense to merge the binding as long as the driver is not merged, 
> otherwise you would end up with a binding without driver. So keeping them 
> together should help reviewers and maintainers.)
>

This is actually okay to do if you expect discussion and not a patch
merge. Series works, too.

Best regads,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH 3/6] dt-bindings: serdev: ngsm: Add binding for GNSS child node

[Pavel Machek]

On Fri 2021-03-05 11:52:38, Johan Hovold wrote:
> On Fri, Mar 05, 2021 at 11:46:35AM +0100, Pavel Machek wrote:
> > Hi!
> > 
> > > > > For motorola modem case, we may have a GNSS device on channel 4.
> > > > > Let's add that to the binding and example.
> > > > > 
> > > > > Signed-off-by: Tony Lindgren 
> > > > > ---
> > > > >  .../devicetree/bindings/serdev/serdev-ngsm.yaml  | 9 
> > > > > +
> > > > >  1 file changed, 9 insertions(+)
> > 
> > > 
> > > And since we're describing a mux, I think you need nodes for the virtual
> > > ports rather than a reg property in what should be a serial client. That
> > > is something like
> > > 
> > >   serial@nnn {
> > >   modem {
> > >   compatible = "etsi,ts27001-mux";
> > > 
> > >   serial@4 {
> > >   compatible = "etsi,ts27001-serial";
> > >   reg = <4>;
> > > 
> > >   gnss {
> > >   compatible = "motorola,motmdm-gnss";
> > >   };
> > >   };
> > >   };
> > >   };
> > > 
> > > This way you can actually use serdev for the client drivers (e.g. for
> > > gnss), and those drivers also be used for non-muxed ports if needed
> > > (e.g. over USB).
> > 
> > I have done changes you requested, and then hit "serdev is busy
> > because it can have at most one child" limit in the code. You have
> > pretty clean driver in your inbox, and no reply. No help with serdev
> > core limitations, either. Can you start to communicate?
> 
> It's on my list, but time is limited.

Everyone's time is limited. Do you have any time estimates?
Pavel

-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH 17/17] auxdisplay: ht16k33: Add segment display LED support

[Pavel Machek]

Hi!

> CC linux-leds (which I intended, but forgot to add)
> 
> cover letter at
> https://lore.kernel.org/linux-devicetree/20210322144848.1065067-1-ge...@linux-m68k.org/

Still does not tell me... riscv on fpga with 4 character display. What
is this? :-).


> On Tue, Mar 23, 2021 at 11:08 AM Robin van der Gracht  
> wrote:
> >
> > On 2021-03-22 15:48, Geert Uytterhoeven wrote:
> > > Instantiate a single LED for a segment display.  This allows the user
> > > to
> > > control display brightness and blinking through the LED class API and
> > > triggers, and exposes the display color.
> > >
> > > Signed-off-by: Geert Uytterhoeven 
> > > ---
> > > For setting display brightness, this could use the existing backlight
> > > support for frame buffer devices instantiated for dot-matrix displays.
> > > However, using the leds subsystem instead has the advantage that the
> > > driver can make use of the HT16K33's hardware blinking support, and can
> > > expose the display color.

We have multicolor support now...

> > > - err = ht16k33_brightness_set(priv, MAX_BRIGHTNESS);
> > > + of_property_read_u32(node, "color", );
> > > + seg->led.name = devm_kasprintf(dev, GFP_KERNEL,
> > > + DRIVER_NAME ":%s:" LED_FUNCTION_BACKLIGHT,
> > > + color < LED_COLOR_ID_MAX ? led_colors[color] : "");

And would prefer not to see driver_name as part of LED name.

> > > + err = ht16k33_brightness_set(priv, seg->led.brightness);
> > >   if (err)
> > >   return err;
> >
> > The LED class can pretty much do what the backlight class can and more.
> >
> > Maybe we can stop registering a backlight device in the fbdev case and
> > register a led device for both. This makes the code cleaner and drops
> > a dependency but will break backwards compatibility.
> >
> > I'd prefer a single solution that covers both use cases, but I'm not
> > sure about the 'breaking backwards compatibility' consequence...

For new drivers, breaking compatibility should not be a problem.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH 10/11] pragma once: delete few backslashes

[Pavel Machek]

Hi!
> > index e201b4b1655a..46704c341b17 100644
> > --- a/arch/arc/include/asm/cacheflush.h
> > +++ b/arch/arc/include/asm/cacheflush.h
> > @@ -112,6 +112,6 @@ do {
> > \
> >  } while (0)
> >  
> >  #define copy_from_user_page(vma, page, vaddr, dst, src, len)   
> > \
> > -   memcpy(dst, src, len);  \
> > +   memcpy(dst, src, len)
> >  This changebar also removes a semicolon.
> It looks plausibly correct, but the commit message ought to mention it.

Probably should use do{}while(0) trick.

Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH 00/11] pragma once: treewide conversion

[Pavel Machek]

Hi!

> > >  (a) the traditional include guard optimization HAS NO HIDDEN SEMANTIC
> > > MEANING. It's a pure optimization that doesn't actually change
> > > anything else. If you don't do the optimization, absolutely nothing
> > > changes.
> >
> > And if the parser is well written the optimisation is probably
> > irrelevant compared to the compile time.
> 
> That's actually surprisingly not even remotely true.
> 
> People always think that the optimization phases of a compiler are the
> expensive ones. And yes, there are certain optimizations that can be
> *really* expensive, and people just don't even do them because they
> are _so_ expensive and are exponential in time.

Well, linux kernel can be compiled in two _seconds_ if you take
compiler optimized for fast parsing... and quick code generation.

See "SUSE Labs Conference 2018 - Compiling the Linux kernel in a
second (give or take)" on youtube.

So yes, gcc's frontend may be slow, but that does not mean job can not
be done quickly by suitable compiler.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH 4.4 00/14] 4.4.263-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 4.4.263 release.
> There are 14 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.4.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel


-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 4.19 00/43] 4.19.183-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 4.19.183 release.
> There are 43 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.19.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel


-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 5.10 000/156] 5.10.26-rc2 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 5.10.26 release.
> There are 156 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-5.10.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: Getting Rust to work

[Pavel Machek]

Hi!

> > First try was on x86-32. It took me a whlie to figure out the problem,
> > hence the diff below.
> 
> Thanks a lot for trying it out and for the feedback! :-)

Thanks for your work :-).

> The supported architectures are in
> Documentation/rust/arch-support.rst, so we should link to that.
> 
> x86 32-bit and other architectures should eventually work -- we need
> to put some time into them and setting up a few things. For the moment
> we focused on x86_64 and arm64.

Actually x86-32 would be useful for me. That's my test machine.

> > Then I got failure because libclang (?) was not installed. I installed
> > it. But I guess that should be mentioned in docs as dependency.
> 
> Yeah, this is mentioned on the bindgen section in quick-start.rst, I
> will try to improve make it more clear, perhaps I'll a section for
> libclang itself.

Aha, sorry, it is possible I was not careful enough reading the docs.

> > With CONFIG_RUST unset, kernel builds for me. With CONFIG_RUST=y +
> > RUST_EXAMPLE=y... I get errors:
> 
> Typically it is due to a kernel option that enables some GCC flag that
> libclang does not know about -- I have a hack in place to make it
> work, but requires manual maintenance. Can you please attach your
> kernel config?

Done, in a private message.

> Meanwhile, please try building with Clang or LLVM instead, the GCC
> builds are very, very experimental. I'll make it even more explicit in
> the quick-start.

I tried, but newest in my distribution is clang-7, and kernel needs
clang-10.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Getting Rust to work

[Pavel Machek]

Hi!

First try was on x86-32. It took me a whlie to figure out the problem,
hence the diff below.

Next I was not sure what to do with rustup. I'm not sure if advice
below is good.

Then I got failure because libclang (?) was not installed. I installed
it. But I guess that should be mentioned in docs as dependency.

With CONFIG_RUST unset, kernel builds for me. With CONFIG_RUST=y +
RUST_EXAMPLE=y... I get errors:

  RUSTC L rust/core.o
  BINDGEN rust/bindings_generated.rs
error: unknown argument: '-fmacro-prefix-map=./='
error: unknown warning option '-Wno-frame-address'; did you mean 
'-Wno-address'? [-Wunknown-warning-option]
./arch/x86/include/asm/bitops.h:138:9: error: 'asm goto' constructs are not 
supported yet
./arch/x86/include/asm/bitops.h:162:9: error: 'asm goto' constructs are not 
supported yet
./arch/x86/include/asm/bitops.h:201:9: error: 'asm goto' constructs are not 
supported yet
./include/linux/list.h:282:9: warning: duplicate 'const' declaration specifier 
[-Wduplicate-decl-specifier]
./include/linux/list.h:318:27: warning: duplicate 'const' declaration specifier 
[-Wduplicate-decl-specifier]
./include/linux/list.h:821:10: warning: duplicate 'const' declaration specifier 
[-Wduplicate-decl-specifier]
./include/linux/list.h:830:10: warning: duplicate 'const' declaration specifier 
[-Wduplicate-decl-specifier]
./arch/x86/include/asm/atomic.h:83:9: error: 'asm goto' constructs are not 
supported yet
./arch/x86/include/asm/atomic.h:123:9: error: 'asm goto' constructs are not 
supported yet
./arch/x86/include/asm/atomic.h:137:9: error: 'asm goto' constructs are not 
supported yet
./arch/x86/include/asm/atomic.h:152:9: error: 'asm goto' constructs are not 
supported yet

$ gcc --version
gcc (Debian 8.3.0-6) 8.3.0
$ rustc --version
rustc 1.52.0-nightly (61edfd591 2021-03-20)

Ideas?

Best regards,
Pavel



diff --git a/Documentation/rust/quick-start.rst 
b/Documentation/rust/quick-start.rst
index 33b5cd33ecc5..d034369b3d43 100644
--- a/Documentation/rust/quick-start.rst
+++ b/Documentation/rust/quick-start.rst
@@ -16,6 +16,8 @@ Some of these requirements might be available from your Linux 
distribution
 under names like ``rustc``, ``rust-src``, ``rust-bindgen``, etc. However,
 at the time of writing, they are likely to not be recent enough.
 
+You need to have x86-64 or arm64 system.
+
 
 rustc
 *
@@ -32,6 +34,8 @@ Otherwise, fetch a standalone installer or install ``rustup`` 
from:
 
 https://www.rust-lang.org
 
+(if you have multiple toolchains, do rustup default 
nightly-i686-unknown-linux-gnu )
+
 
 Rust standard library source
 

-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

vDPA: explain in Kconfig what vDPA is, capitalise it consistenly

[Pavel Machek]

Not everyone knows what vDPA stands for, explain it in Kconfig.

Signed-off-by: Pavel Machek (CIP) 

diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
index ffd1e098bfd2..8cb37b3dd279 100644
--- a/drivers/vdpa/Kconfig
+++ b/drivers/vdpa/Kconfig
@@ -3,9 +3,9 @@ menuconfig VDPA
tristate "vDPA drivers"
depends on NET
help
- Enable this module to support vDPA device that uses a
- datapath which complies with virtio specifications with
- vendor specific control path.
+ Enable this module to support Virtual Data Path Acceleration
+ (vDPA) device that uses a datapath which complies with
+ virtio specifications with vendor specific control path.
 
 if VDPA
 
@@ -38,8 +38,8 @@ config MLX5_VDPA
bool
select VHOST_IOTLB
help
- Support library for Mellanox VDPA drivers. Provides code that is
- common for all types of VDPA drivers. The following drivers are 
planned:
+ Support library for Mellanox vDPA drivers. Provides code that is
+ common for all types of vDPA drivers. The following drivers are 
planned:
  net, block.
 
 config MLX5_VDPA_NET
@@ -47,7 +47,7 @@ config MLX5_VDPA_NET
select MLX5_VDPA
depends on MLX5_CORE
help
- VDPA network driver for ConnectX6 and newer. Provides offloading
+ vDPA network driver for ConnectX6 and newer. Provides offloading
  of virtio net datapath such that descriptors put on the ring will
  be executed by the hardware. It also supports a variety of stateless
  offloads depending on the actual device used and firmware version.

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany



signature.asc
Description: Digital signature

net/dev: fix information leak to userspace

[Pavel Machek]

dev_get_mac_address() does not always initialize whole
structure. Unfortunately, other code copies such structure to
userspace, leaking information. Fix it.

Signed-off-by: Pavel Machek (CIP) 
Cc: sta...@kernel.org

diff --git a/net/core/dev.c b/net/core/dev.c
index 6c5967e80132..28283a9eb63a 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -8949,11 +8949,9 @@ int dev_get_mac_address(struct sockaddr *sa, struct net 
*net, char *dev_name)
ret = -ENODEV;
goto unlock;
}
-   if (!dev->addr_len)
-   memset(sa->sa_data, 0, size);
-   else
-   memcpy(sa->sa_data, dev->dev_addr,
-  min_t(size_t, size, dev->addr_len));
+   memset(sa->sa_data, 0, size);
+   memcpy(sa->sa_data, dev->dev_addr,
+  min_t(size_t, size, dev->addr_len));
sa->sa_family = dev->type;
 
 unlock:


-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

vDPA: explain in Kconfig what vDPA is, capitalise it consistenly

[Pavel Machek]

Not everyone knows what vDPA stands for, explain it in Kconfig.

Signed-off-by: Pavel Machek (CIP) 

diff --git a/drivers/vdpa/Kconfig b/drivers/vdpa/Kconfig
index ffd1e098bfd2..8cb37b3dd279 100644
--- a/drivers/vdpa/Kconfig
+++ b/drivers/vdpa/Kconfig
@@ -3,9 +3,9 @@ menuconfig VDPA
tristate "vDPA drivers"
depends on NET
help
- Enable this module to support vDPA device that uses a
- datapath which complies with virtio specifications with
- vendor specific control path.
+ Enable this module to support Virtual Data Path Acceleration
+ (vDPA) device that uses a datapath which complies with
+ virtio specifications with vendor specific control path.
 
 if VDPA
 
@@ -38,8 +38,8 @@ config MLX5_VDPA
bool
select VHOST_IOTLB
help
- Support library for Mellanox VDPA drivers. Provides code that is
- common for all types of VDPA drivers. The following drivers are 
planned:
+ Support library for Mellanox vDPA drivers. Provides code that is
+ common for all types of vDPA drivers. The following drivers are 
planned:
  net, block.
 
 config MLX5_VDPA_NET
@@ -47,7 +47,7 @@ config MLX5_VDPA_NET
select MLX5_VDPA
depends on MLX5_CORE
help
- VDPA network driver for ConnectX6 and newer. Provides offloading
+ vDPA network driver for ConnectX6 and newer. Provides offloading
  of virtio net datapath such that descriptors put on the ring will
  be executed by the hardware. It also supports a variety of stateless
  offloads depending on the actual device used and firmware version.

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

enetc: fix bitfields, we are clearing wrong bits

[Pavel Machek]

Bitfield manipulation in enetc_mac_config() looks wrong. Fix
it. Untested.

Signed-off-by: Pavel Machek (CIP) 

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c 
b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 224fc37a6757..b85079493933 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -505,7 +505,7 @@ static void enetc_mac_config(struct enetc_hw *hw, 
phy_interface_t phy_mode)
if (phy_interface_mode_is_rgmii(phy_mode)) {
val = enetc_port_rd(hw, ENETC_PM0_IF_MODE);
val &= ~ENETC_PM0_IFM_EN_AUTO;
-   val &= ENETC_PM0_IFM_IFMODE_MASK;
+   val &= ~ENETC_PM0_IFM_IFMODE_MASK;
val |= ENETC_PM0_IFM_IFMODE_GMII | ENETC_PM0_IFM_RG;
enetc_port_wr(hw, ENETC_PM0_IF_MODE, val);
}

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Trojan horses on various lists was Re: EDAC list as Trojan Horse distribution ??

[Pavel Machek]

Hi!

> > I assume this is just to install a trojan horse when opening the
> > attached zip (also I assume most of you will work on linux and it
> > might not be a Problem for you anyhow   ;-) .
> >
> >  Virus total reports a Trojan horse, but only for with 2 out of 61
> > virus scan engines (and I have to admit, I did not knew K7AntiVirus
> > and Qihoo-360 before, all other engines reported the file as
> > clean!!! ).
> >
> > So be careful when you get some feedback to old requests from this
> > list

Happened to me, too, on different list on two different email
addresses. Did not reoccur.

Best regards,
Pavel

-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH 5.10 00/13] 5.10.25-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 5.10.25 release.
> There are 13 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-5.10.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 4.19 0/8] 4.19.182-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 4.19.182 release.
> There are 8 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.19.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH] ACPI: video: Add missing callback back for Sony VPCEH3U1E

[Pavel Machek]

On Fri 2021-03-19 17:44:32, Rafael J. Wysocki wrote:
> On Sat, Mar 13, 2021 at 11:27 AM Pavel Machek  wrote:
> >
> > On Fri 2021-03-12 11:24:30, chris.c...@canonical.com wrote:
> > > From: Chris Chiu 
> > >
> > > The .callback of the quirk for Sony VPCEH3U1E was unintetionally
> > > removed by the commit 25417185e9b5 ("ACPI: video: Add DMI quirk
> > > for GIGABYTE GB-BXBT-2807"). Add it back to make sure the quirk
> > > for Sony VPCEH3U1E works as expected.
> > >
> > > Signed-off-by: Chris Chiu 
> > > Reported-by: Pavel Machek 
> >
> > Thanks for doing this.
> >
> > Reviewed-by: Pavel Machek (CIP) 
> 
> Applied as 5.12-rc5 material, thanks!

Commit 25417185e9b5 made it into stable, so this one should be Cc:
stable as well. Sorry for noticing that late.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH 1/1] leds: lgm: Improve Kconfig help

[Pavel Machek]

Hi!


> >  > help
> >  > - Parallel to serial conversion, which is also called SSO
> >  > controller,
> >  > - can drive external shift register for LED outputs.
> >  > - This enables LED support for Serial Shift Output controller(SSO).
> >  > + This option enables support for LEDs connected to GPIO lines on
> >  > + Lightning Mountain(LGM) SoC. These LEDs are driven by a Serial
> >  > + Shift Output(SSO) controller. The driver supports hardware
> > 
> > What is Lightning Mountain? The codename is not widely known. Where
> > can we find that hardware? Notebooks? Phones? Only some development
> > boards?
> > 
> 
> Lightning Mountain is generically a network processor with a primary 
> targeted application as Gateway SoC. It has already been added as a 
> valid Intel Atom processor variant in 
> arch/x86/include/asm/intel-family.h as below:

Yep, but Kconfig users are not going to read header files.

If the SoC is not shipping in any products, state so.

> >  > *
> >  > - * Copyright (c) 2020 Intel Corporation.
> >  > + * Copyright (c) 2021 MaxLinear, Inc.
> >  > */
> >  >
> > 
> > I don't think you can do that, and I don't think you should be doing
> > it in the same patch.
> 
> Well noted. Will revert it back now and update later in a separate 
> patch. Thanks.

Don't. You can't update copyright like that.

Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH 5.10 041/290] net: enetc: force the RGMII speed and duplex instead of operating in inband mode

[Pavel Machek]

Hi!

> It has been reported that RGMII is broken in fixed-link, and that is not
> surprising considering the fact that no PHY is attached to the MAC in
> that case, but a switch.

Okay, but there's something wrong in the code.

> This brings us to the topic of the patch: the enetc driver should have
> not enabled the optional in-band status signaling for RGMII unconditionally,
> but should have forced the speed and duplex to what was resolved by
> phylink.

> +++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
> @@ -494,13 +494,20 @@ static void enetc_configure_port_mac(str
>  
>  static void enetc_mac_config(struct enetc_hw *hw, phy_interface_t phy_mode)
>  {
...
> + u32 val;
> +
> + if (phy_interface_mode_is_rgmii(phy_mode)) {
> + val = enetc_port_rd(hw, ENETC_PM0_IF_MODE);
> + val &= ~ENETC_PM0_IFM_EN_AUTO;
> + val &= ENETC_PM0_IFM_IFMODE_MASK;
> + val |= ENETC_PM0_IFM_IFMODE_GMII | ENETC_PM0_IFM_RG;
> + enetc_port_wr(hw, ENETC_PM0_IF_MODE, val);
> + }

val clears ENETC_PM0_IFM_EN_AUTO bit, then the bit is cleared again,
preserving just IFMODE bits, then ors the IFMODE bits with new values.

I believe this is needed:

Signed-off-by: Pavel Machek (CIP) 
Pavel

diff --git a/drivers/net/ethernet/freescale/enetc/enetc_pf.c 
b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
index 83187cd59fdd..446ad4c43fab 100644
--- a/drivers/net/ethernet/freescale/enetc/enetc_pf.c
+++ b/drivers/net/ethernet/freescale/enetc/enetc_pf.c
@@ -499,7 +499,7 @@ static void enetc_mac_config(struct enetc_hw *hw, 
phy_interface_t phy_mode)
if (phy_interface_mode_is_rgmii(phy_mode)) {
val = enetc_port_rd(hw, ENETC_PM0_IF_MODE);
val &= ~ENETC_PM0_IFM_EN_AUTO;
-   val &= ENETC_PM0_IFM_IFMODE_MASK;
+   val &= ~ENETC_PM0_IFM_IFMODE_MASK;
val |= ENETC_PM0_IFM_IFMODE_GMII | ENETC_PM0_IFM_RG;
enetc_port_wr(hw, ENETC_PM0_IF_MODE, val);
}


-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: PGP signature

Re: [PATCH 1/1] leds: lgm: Improve Kconfig help

[Pavel Machek]

Hi!

> Remove unnecessary Kconfig symbol LEDS_BLINK
> Improve Kconfig help text to make it more useful.
> 
> Signed-off-by: Rahul Tanwar 

> +++ b/drivers/leds/blink/Kconfig
> @@ -1,21 +1,19 @@
> -menuconfig LEDS_BLINK
> -   bool "LED Blink support"
> -   depends on LEDS_CLASS
> -   help
> - This option enables blink support for the leds class.
> - If unsure, say Y.
> -
> -if LEDS_BLINK
> -
> -config LEDS_BLINK_LGM
> -   tristate "LED support for Intel LGM SoC series"
> +config LEDS_LGM
> +   tristate "LED support for LGM SoC series"
>  depends on GPIOLIB
>  depends on LEDS_CLASS
>  depends on MFD_SYSCON
>  depends on OF
>  help
> - Parallel to serial conversion, which is also called SSO 
> controller,
> - can drive external shift register for LED outputs.
> - This enables LED support for Serial Shift Output controller(SSO).
> + This option enables support for LEDs connected to GPIO lines on
> + Lightning Mountain(LGM) SoC. These LEDs are driven by a Serial
> + Shift Output(SSO) controller. The driver supports hardware

What is Lightning Mountain? The codename is not widely known. Where
can we find that hardware? Notebooks? Phones? Only some development
boards?

If user is not likely to need the driver, say so.

> + blinking with a configurable LED update/blink frequency in two
> + modes, 2/4/8/10 Hz in low speed mode and 50/100/200/250
> KHz in

kHz? But I guess we don't need that here.

>*
> - * Copyright (c) 2020 Intel Corporation.
> + * Copyright (c) 2021 MaxLinear, Inc.
>*/
> 

I don't think you can do that, and I don't think you should be doing
it in the same patch.

Best regards,
Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: Digital signature

Re: [PATCH v3 2/2] rockchip: rk3399: Add support for FriendlyARM NanoPi R4S

[Pavel Machek]

On Tue 2021-03-16 16:34:50, Geert Uytterhoeven wrote:
> Hi Tianling,
> 
> CC Jacek, Pavel
> 
> On Tue, Mar 16, 2021 at 4:00 PM Tianling Shen  wrote:
> > On 2021-03-16 02:23 Geert Uytterhoeven  wrote:
> > > Personally, I'm not so fond of the -%u node names, and prefer
> > > -.  With the former, it's way too easy to have a silent
> > > override in your .dts(i) stack.
> > > Cfr. commit 45f5d5a9e34d3fe4 ("arm64: dts: renesas: r8a77995: draak:
> > > Fix backlight regulator name")
> >
> > How about using `lan-led`, `sys-led` and `wan-led` here?
> 
> Documentation/devicetree/bindings/leds/leds-gpio.yaml says "led-%u"
> is the preferred form, but that anything containing "led" as a substring
> is accepted.  So I'd go for "led-lan" etc.
> 
> BTW, you can validate your DTB against the leds-gpio DT bindings
> by running:
> 
> make dtbs_check
> DT_SCHEMA_FILES=Documentation/devicetree/bindings/leds/leds-gpio.yaml
> 
> Background info for CCed parties:
>
https://lore.kernel.org/linux-arm-kernel/20210316150033.15987-1-cns...@gmail.com/

I don't care much either way, lan-0 is okay as is lan-led.

but...

+   label = "nanopi-r4s:green:lan";
+   label = "nanopi-r4s:red:sys";
+   label = "nanopi-r4s:green:wan";


It would be good to have common labels, that means LED_FUNCTION_LAN,
LED_FUNCTION_WAN, and figuring out something better than "sys",
possibly LED_FUNCTION_FAULT?

Thanks,
Pavel

-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH 5.10 154/290] PCI/LINK: Remove bandwidth notification

[Pavel Machek]

Hi!

> From: Greg Kroah-Hartman 
> 
> From: Bjorn Helgaas 

Dup.

> Remove the bandwidth change notifications for now.  Hopefully we can add
> this back when we have a better understanding of why this happens and how
> we can make the messages useful instead of overwhelming.

This is stable, and even for mainline, I'd expect "depends on BROKEN"
in Kconfig, or something like that, so people can still work on fixing
it and so that we don't have huge changes floating around.

Best regards,
Pavel

> diff --git a/drivers/pci/pcie/Kconfig b/drivers/pci/pcie/Kconfig
> index 3946555a6042..45a2ef702b45 100644
> --- a/drivers/pci/pcie/Kconfig
> +++ b/drivers/pci/pcie/Kconfig
> @@ -133,14 +133,6 @@ config PCIE_PTM
> This is only useful if you have devices that support PTM, but it
> is safe to enable even if you don't.
>  
> -config PCIE_BW
> - bool "PCI Express Bandwidth Change Notification"
> - depends on PCIEPORTBUS
> - help
> -   This enables PCI Express Bandwidth Change Notification.  If
> -   you know link width or rate changes occur only to correct
> -   unreliable links, you may answer Y.

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 4.19 012/120] tcp: annotate tp->write_seq lockless reads

[Pavel Machek]

Hi!

> > From: Greg Kroah-Hartman 
> > 
> > From: Eric Dumazet 
> 
> Dup.

Aha, sorry, crossed mails. Still I wonder if hiding assignment into
macro is good:

> > --- a/net/ipv4/tcp_minisocks.c
> > +++ b/net/ipv4/tcp_minisocks.c
> > @@ -510,7 +510,7 @@ struct sock *tcp_create_openreq_child(co
> > newtp->app_limited = ~0U;
> >  
> > tcp_init_xmit_timers(newsk);
> > -   newtp->write_seq = newtp->pushed_seq = treq->snt_isn + 1;
> > +   WRITE_ONCE(newtp->write_seq, newtp->pushed_seq = treq->snt_isn + 1);
> 
> Would it be better to do assignment to pushed_seq outside of
> WRITE_ONCE macro? This is ... "interesting".

Best regards,
Pavel



-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 4.19 012/120] tcp: annotate tp->write_seq lockless reads

[Pavel Machek]

Hi!

> From: Greg Kroah-Hartman 
> 
> From: Eric Dumazet 

Dup.


> We need to add READ_ONCE() annotations, and also make
> sure write sides use corresponding WRITE_ONCE() to avoid
> store-tearing.

> @@ -1037,7 +1037,7 @@ new_segment:
>   sk->sk_wmem_queued += copy;
>   sk_mem_charge(sk, copy);
>   skb->ip_summed = CHECKSUM_PARTIAL;
> - tp->write_seq += copy;
> + WRITE_ONCE(tp->write_seq, tp->write_seq + copy);
>   TCP_SKB_CB(skb)->end_seq += copy;
>   tcp_skb_pcount_set(skb, 0);
>

I wonder if this needs to do READ_ONCE, too?

> @@ -1391,7 +1391,7 @@ new_segment:
>   if (!copied)
>   TCP_SKB_CB(skb)->tcp_flags &= ~TCPHDR_PSH;
>  
> - tp->write_seq += copy;
> + WRITE_ONCE(tp->write_seq, tp->write_seq + copy);
>   TCP_SKB_CB(skb)->end_seq += copy;
>   tcp_skb_pcount_set(skb, 0);
>

And here.

> @@ -2593,9 +2594,12 @@ int tcp_disconnect(struct sock *sk, int
>   sock_reset_flag(sk, SOCK_DONE);
>   tp->srtt_us = 0;
>   tp->rcv_rtt_last_tsecr = 0;
> - tp->write_seq += tp->max_window + 2;
> - if (tp->write_seq == 0)
> - tp->write_seq = 1;
> +
> + seq = tp->write_seq + tp->max_window + 2;
> + if (!seq)
> + seq = 1;
> + WRITE_ONCE(tp->write_seq, seq);

And here.

> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -510,7 +510,7 @@ struct sock *tcp_create_openreq_child(co
>   newtp->app_limited = ~0U;
>  
>   tcp_init_xmit_timers(newsk);
> - newtp->write_seq = newtp->pushed_seq = treq->snt_isn + 1;
> + WRITE_ONCE(newtp->write_seq, newtp->pushed_seq = treq->snt_isn + 1);

Would it be better to do assignment to pushed_seq outside of
WRITE_ONCE macro? This is ... "interesting".

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 4.19 011/120] tcp: annotate tp->copied_seq lockless reads

[Pavel Machek]

Hi!

> From: Greg Kroah-Hartman 
> 
> From: Eric Dumazet 

Two From: fields here.

> [ Upstream commit 7db48e983930285b765743ebd665aecf9850582b ]
> 
> There are few places where we fetch tp->copied_seq while
> this field can change from IRQ or other cpu.

And there are few such places even after the patch is applied; I
quoted them below.

Doing addition to variable without locking... is kind of
interesting. Are you sure it is okay?

> @@ -2112,7 +2112,7 @@ int tcp_recvmsg(struct sock *sk, struct
>   if (urg_offset < used) {
>   if (!urg_offset) {
>   if (!sock_flag(sk, SOCK_URGINLINE)) {
> - ++*seq;
> + WRITE_ONCE(*seq, *seq + 1);
>   urg_hole++;
>   offset++;
>   used--;
> @@ -2134,7 +2134,7 @@ int tcp_recvmsg(struct sock *sk, struct
>   }
>   }
>  
> - *seq += used;
> + WRITE_ONCE(*seq, *seq + used);
>   copied += used;
>   len -= used;
>  
> @@ -2163,7 +2163,7 @@ skip_copy:
>  
>   found_fin_ok:
>   /* Process the FIN. */
> - ++*seq;
> + WRITE_ONCE(*seq, *seq + 1);
>   if (!(flags & MSG_PEEK))
>   sk_eat_skb(sk, skb);
>   break;

Best regards,
Pavel
-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 5.10 000/290] 5.10.24-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 5.10.24 release.
> There are 290 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any kernel problems here: (Renesas boards
are still unavailable)

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-5.10.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel


-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 4.19 000/120] 4.19.181-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 4.19.181 release.
> There are 120 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any kernel problems here: (Renesas boards
are still unavailable)

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.4.y
https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.19.y
https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-5.10.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel


-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 4.4 00/75] 4.4.262-rc1 review

[Pavel Machek]

Hi!

> This is the start of the stable review cycle for the 4.4.262 release.
> There are 75 patches in this series, all will be posted as a response
> to this one.  If anyone has any issues with these being applied, please
> let me know.

CIP testing did not find any problems here:

https://gitlab.com/cip-project/cip-testing/linux-stable-rc-ci/-/tree/linux-4.4.y

Tested-by: Pavel Machek (CIP) 

Best regards,
Pavel

-- 
DENX Software Engineering GmbH,  Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany


signature.asc
Description: Digital signature

Re: [PATCH 2/4] leds: simatic-ipc-leds: add new driver for Siemens Industial PCs

[Pavel Machek]

Hi!

> > I would also be happy to include a fix to that script. My suggestion
> > would be to allow bus=platform, in which case a "devicename" will be
> > required and is allowed to have any value.
> 
> Furthermore it might be good to catch that in the led core instead of
> that script. Maybe warn() on dev registration when function/color/name
> seem off. Could later become "return -EINVAL"

I'm not sure if we want to change _existing_ funny names.

Would document such as below be helpful?

Could you describe the LEDs you have in similar format?

Best regards,
Pavel

-*- org -*-

It is somehow important to provide consistent interface to the
userland. LED devices have one problem there, and that is naming of
directories in /sys/class/leds. It would be nice if userland would
just know right "name" for given LED function, but situation got more
complex.

Anyway, if backwards compatibility is not an issue, new code should
use one of the "good" names from this list, and you should extend the
list where applicable.

Bad names are listed, too; in case you are writing application that
wants to use particular feature, you should probe for good name, first,
but then try the bad ones, too.

* Keyboards
  
Good: "input*:*:capslock"
Good: "input*:*:scrolllock"
Good: "input*:*:numlock"
Bad: "shift-key-light" (Motorola Droid 4, capslock)

Set of common keyboard LEDs, going back to PC AT or so.

Good: "platform::kbd_backlight"
Bad: "tpacpi::thinklight" (IBM/Lenovo Thinkpads)
Bad: "lp5523:kb{1,2,3,4,5,6}" (Nokia N900)

Frontlight/backlight of main keyboard.

Bad: "button-backlight" (Motorola Droid 4)

Some phones have touch buttons below screen; it is different from main
keyboard. And this is their backlight.

* Sound subsystem

Good: "platform:*:mute"
Good: "platform:*:micmute"

LEDs on notebook body, indicating that sound input / output is muted.

* System notification

Good: "status-led:{red,green,blue}" (Motorola Droid 4)
Bad: "lp5523:{r,g,b}" (Nokia N900)

Phones usually have multi-color status LED.

* Power management

Good: "platform:*:charging" (allwinner sun50i)

* Screen

Good: ":backlight" (Motorola Droid 4)


-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH 2/4] leds: simatic-ipc-leds: add new driver for Siemens Industial PCs

[Pavel Machek]

Hi!

> Maybe you explain the idea behind choosing only from that namespace? My
> guess would be high-level software being able to toggle leds totally
> indep of the device it runs on. Such software would have to do some
> really nasty directory listing, name parsing, dealing with multiple
> hits. Does such generic software already exist, maybe that would help
> me understand my "mapping problems" ?

It does not, but we want to have one... or at least not have current situation.

> The current class encodes, color, function and name into "name".
> 
> Maybe i am all wrong and should go for
> 
> {1 << 14, "simatic-ipc-error:red:" LED_FUNCTION_STATUS }
> {1 << 15, "simatic-ipc-run-stop:green:" LED_FUNCTION_STATUS}
> {..., "simatic-ipc-hdd0:red:" LED_FUNCTION_DISK }
> {..., "simatic-ipc-hdd1:red:" LED_FUNCTION_DISK }

Can you explain in plain english what the leds should do?

We don't want to have simatic-ipc- prefix there. tpacpi was really bad
example.

Best regards,
Pavel

-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH v2 2/4] leds: simatic-ipc-leds: add new driver for Siemens Industial PCs

[Pavel Machek]

> > +   struct led_classdev cdev;
> > +};
> > +
> > +static struct simatic_ipc_led simatic_ipc_leds_io[] = {
> > +   {1 << 15, "simatic-ipc:green:" LED_FUNCTION_STATUS "-1" },
> > +   {1 << 7,  "simatic-ipc:yellow:" LED_FUNCTION_STATUS "-1" },
> > +   {1 << 14, "simatic-ipc:red:" LED_FUNCTION_STATUS "-2" },
> > +   {1 << 6,  "simatic-ipc:yellow:" LED_FUNCTION_STATUS "-2" },
> > +   {1 << 13, "simatic-ipc:red:" LED_FUNCTION_STATUS "-3" },
> > +   {1 << 5,  "simatic-ipc:yellow:" LED_FUNCTION_STATUS "-3" },
> 
> Can you use BIT() macro here? And can it be sorted by the bit order?

There's nothing wrong with << and this order is fine.

But I still don't like the naming. simantic-ipc: prefix is
useless. Having 6 status leds is not good, either.

> > +   struct simatic_ipc_led *led =
> > +   container_of(led_cd, struct simatic_ipc_led, cdev);
> 
> One line?

80 columns. It is fine as it is.

Best regards,

Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature

Re: [PATCH] leds: lgm: fix gpiolib dependency

[Pavel Machek]

Hi!

> On 15/3/2021 5:44 pm, Rahul Tanwar wrote:
> 
> From: Arnd Bergmann 
> 
> From: Arnd Bergmann 
> 
...

> 
> diff
>  --git a/drivers/leds/blink/Kconfig b/drivers/leds/blink/Kconfig
> index 265b53476a80..6dedc58c47b3 100644

...
> 
> Acked-by: Rahul Tanwar 

You may want to do something with your mail configuration. This is
quite corrupted.

Pavel
-- 
http://www.livejournal.com/~pavelmachek


signature.asc
Description: PGP signature