Re: [PATCH AUTOSEL 6.1 08/25] KVM: selftests: Add test for uaccesses to non-existent vgic-v2 CPUIF
Hi! > Assert that accesses to a non-existent vgic-v2 CPU interface > consistently fail across the various KVM device attr ioctls. This also > serves as a regression test for a bug wherein KVM hits a NULL > dereference when the CPUID specified in the ioctl is invalid. > > Note that there is no need to print the observed errno, as TEST_ASSERT() > will take care of it. I don't think this fixes the bug... and thus we should not need it in stable. 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] alloc_tag: Tighten file permissions on /proc/allocinfo
On Fri 2024-04-26 04:25:40, Matthew Wilcox wrote: > On Thu, Apr 25, 2024 at 08:58:34PM -0400, Kent Overstreet wrote: > > On Thu, Apr 25, 2024 at 05:43:33PM -0700, Kees Cook wrote: > > > All this said, I'm still not excited about any of these files living > > > in /proc at all -- we were supposed to use /sys for this kind of thing, > > > but its interface wasn't great for this kind of more "free-form" data, > > > and debugfs isn't good for production interfaces. /proc really should > > > only have pid information -- we end up exposing these top-level files to > > > every mount namespace with a /proc mount. :( But that's a yet-to-be-solved > > > problem... > > > > It really wouldn't be that hard to relax the 4k file limit in sysfs. > > It's a lot harder to relax the GregKH opposition to multiple values per > file in sysfs. With all the "vulnerability" files including multiple-files with english text, you may be able to renegotiate that :-). Joking, really the vulnerability files should be fixed. Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH] alloc_tag: Tighten file permissions on /proc/allocinfo
Hi! > > > > > The /proc/allocinfo file exposes a tremendous about of information > > > > > about > > > > > kernel build details, memory allocations (obviously), and potentially > > > > > even image layout (due to ordering). As this is intended to be > > > > > consumed > > > > > by system owners (like /proc/slabinfo), use the same file permissions > > > > > as > > > > > there: 0400. > > > > > > > > Err... > > > > > > > > The side effect of locking down more and more reporting interfaces is > > > > that programs that consume those interfaces now have to run as root. > > > > > > sudo cat /proc/allocinfo | analyse-that-fie > > > > Even that is still an annoyance, but I'm thinking more about a future > > daemon to collect this every n seconds - that really shouldn't need to > > be root. > > Yeah, that would preclude some nice usecases. Could we maybe use > CAP_SYS_ADMIN checks instead? That way we can still use it from a > non-root process? CAP_SYS_ADMIN is really not suitable, as it can do changes to the system. On working system, allocinfo is really not dangerous, it just may make exploits harder. CAP_KERNEL_OBSERVER or something... Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [ANNOUNCE] 5.10.214-rt106
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
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
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
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
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
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
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: [PATCH v3 12/25] media: i2c: imx258: Allow configuration of clock lane behaviour
On Thu 2024-04-04 16:29:11, Luigi311 wrote: > On 4/3/24 12:48, Pavel Machek wrote: > > Hi! > > > >> The sensor supports the clock lane either remaining in HS mode > >> during frame blanking, or dropping to LP11. > >> > >> Add configuration of the mode via V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK. > > > >> + ret = imx258_write_reg(imx258, IMX258_CLK_BLANK_STOP, > >> + IMX258_REG_VALUE_08BIT, > >> + imx258->csi2_flags & > >> V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK ? > >> + 1 : 0); > > > > !! can be used to turn value into 1/0. I find it easier to read than ? > > 1 : 0 combination, but possibly that's fine, too. > > > > Best regards, > > Pavel > > > > I assume you mean by using > > !!(imx258->csi2_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK) > > I can go ahead and use that instead Yes, I'd do that. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH v3 00/25] imx258 improvement series
Hi! Thanks for doing this. I had some comments on 5, 9, 10, 11, 12, 21 and 22. You can add "Reviewed-by: Pavel Machek " to the rest. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH v3 22/25] dt-bindings: media: imx258: Add binding for powerdown-gpio
Hi! > From: Luis Garcia > > Add powerdown-gpio binding as it is required for some boards "." at end of sentence. > Signed-off-by: Ondrej Jirman > Signed-off-by: Luis Garcia If the patch is from Ondrej, he should be in From:, I believe. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH v3 12/25] media: i2c: imx258: Allow configuration of clock lane behaviour
Hi! > The sensor supports the clock lane either remaining in HS mode > during frame blanking, or dropping to LP11. > > Add configuration of the mode via V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK. > + ret = imx258_write_reg(imx258, IMX258_CLK_BLANK_STOP, > +IMX258_REG_VALUE_08BIT, > +imx258->csi2_flags & > V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK ? > +1 : 0); !! can be used to turn value into 1/0. I find it easier to read than ? 1 : 0 combination, but possibly that's fine, too. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH v3 11/25] media: i2c: imx258: Add get_selection for pixel array information
Hi!
> Libcamera requires the cropping information for each mode, so
> add this information to the driver.
> @@ -116,6 +124,9 @@ struct imx258_mode {
> u32 link_freq_index;
> /* Default register values */
> struct imx258_reg_list reg_list;
> +
> + /* Analog crop rectangle. */
No need for "." at the end, as it is not above.
> + struct v4l2_rect crop;
> };
If the crop is same in all modes, should we have it in common place?
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
signature.asc
Description: PGP signature
Re: [PATCH v3 10/25] media: i2c: imx258: Follow normal V4L2 behaviours for clipping exposure
On Wed 2024-04-03 09:03:39, g...@luigi311.com wrote: > From: Dave Stevenson > > V4L2 sensor drivers are expected are expected to clip the supported Remove one copy of "are expected". Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH v3 09/25] media: i2c: imx258: Add support for running on 2 CSI data lanes
Hi!
> +/*
> + * 4208x3120 @ 30 fps needs 1267Mbps/lane, 4 lanes.
> + * To avoid further computation of clock settings, adopt the same per
> + * lane data rate when using 2 lanes, thus allowing a maximum of 15fps.
> + */
> +static const struct imx258_reg mipi_1267mbps_19_2mhz_2l[] = {
> + { 0x0136, 0x13 },
> + { 0x0137, 0x33 },
> + { 0x0301, 0x0A },
> + { 0x0303, 0x02 },
> + { 0x0305, 0x03 },
> + { 0x0306, 0x00 },
> + { 0x0307, 0xC6 },
> + { 0x0309, 0x0A },
> + { 0x030B, 0x01 },
> + { 0x030D, 0x02 },
> + { 0x030E, 0x00 },
> + { 0x030F, 0xD8 },
> + { 0x0310, 0x00 },
> +
> + { 0x0114, 0x01 },
> + { 0x0820, 0x09 },
> + { 0x0821, 0xa6 },
> + { 0x0822, 0x66 },
> + { 0x0823, 0x66 },
> +};
> +
> +static const struct imx258_reg mipi_1267mbps_19_2mhz_4l[] = {
> { 0x0136, 0x13 },
> { 0x0137, 0x33 },
> { 0x0301, 0x05 },
I wish we did not have to copy all the magic values like this.
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
signature.asc
Description: PGP signature
Re: [PATCH v3 05/25] media: i2c: imx258: Add regulator control
Hi!
> The device tree bindings define the relevant regulators for the
> sensor, so update the driver to request the regulators and control
> them at the appropriate times.
> @@ -995,9 +1007,19 @@ static int imx258_power_on(struct device *dev)
> struct imx258 *imx258 = to_imx258(sd);
> int ret;
>
> + ret = regulator_bulk_enable(IMX258_NUM_SUPPLIES,
> + imx258->supplies);
> + if (ret) {
Will this make it fail for all current users?
Best regards,
Pavel
--
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
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: [PATCH AUTOSEL 6.1 3/7] x86/hyperv: Use slow_virt_to_phys() in page transition hypervisor callback
Hi! > In preparation for temporarily marking pages not present during a > transition between encrypted and decrypted, use slow_virt_to_phys() > in the hypervisor callback. As long as the PFN is correct, This seems to be preparation for something we don't plan to do in -stable. Please drop. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature ___ devel mailing list de...@linuxdriverproject.org http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
Re: [PATCHv2 2/2] usb: typec: anx7688: Add driver for ANX7688 USB-C HDMI bridge
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
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
Re: [PATCH AUTOSEL 6.1 10/12] enic: Avoid false positive under FORTIFY_SOURCE
Hi! > From: Kees Cook > > [ Upstream commit 40b9385dd8e6a0515e1c9cd06a277483556b7286 ] > > FORTIFY_SOURCE has been ignoring 0-sized destinations while the kernel > code base has been converted to flexible arrays. In order to enforce > the 0-sized destinations (e.g. with __counted_by), the remaining 0-sized > destinations need to be handled. Unfortunately, struct vic_provinfo > resists full conversion, as it contains a flexible array of flexible > arrays, which is only possible with the 0-sized fake flexible array. > > Use unsafe_memcpy() to avoid future false positives under > CONFIG_FORTIFY_SOURCE. This prepares for future chagnes, but I don't believe we'll port them to stable. Best regards, Pavel -- 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
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
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: Future handling of complex RGB devices on Linux v2
Hi! > For all these reasons the display analogy really is a bit fit for these > keyboards > we tried to come up with a universal coordinate system for these at the > beginning > of the thread and we failed ... I quite liked the coordinate system proposal. I can propose this: Vendor maps the keyboard lights to a grid. That would be something 16x8 for thinkpad X220. Then we provide functionality to query "is a working pixel there" and "what kind of key is at this pixel" -- I guess we can use input keycodes for that. Multiple pixels can map to one keycode. (And then we make best effort to map normal keyboards into similar grids). Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: Future handling of complex RGB devices on Linux v2
Hi! > > To be honest, I think the kernel shouldn't include too much high-level > > complexity. If there is a desire to implement a generic display device on > > top of the RGB device, this should be a configurable service running in > > user space. The kernel should provide an interface to expose this emulated > > display as a "real" display to applications - unless this can also be done > > entirely in user space in a generic way. > > We really need to stop seeing per key addressable RGB keyboards as displays: > > 1. Some "pixels" are non square > 2. Not all "pixels" have the same width-height ratio They are quite close to square usually. > 3. Not all rows have the same amount of pixels True for cellphone displays, too. Rounded corners. > 4. There are holes in the rows like between the enter key and then numpad True for cellphone displays, too. Hole for camera. > 5. Some "pixels" have multiple LEDs beneath them. These might be addressable >per LEDs are the sub-pixels ? What about a 2 key wide backspace key vs >the 1 key wide + another key (some non US layouts) in place of the > backspace? >This will be "2 pixels" in some layout and 1 pixel with maybe / maybe-not >2 subpixels where the sub-pixels may/may not be individually addressable ? Treat those "sub pixels" as pixels. They will be in same matrix as the rest. > For all these reasons the display analogy really is a bit fit for these > keyboards > we tried to come up with a universal coordinate system for these at the > beginning > of the thread and we failed ... I'd suggest trying harder this time :-). Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: Future handling of complex RGB devices on Linux v2
Hi!
> > > so after more feedback from the OpenRGB maintainers I came up with an even
> > > more generic proposal:
> > > https://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/3916#note_1753072869
> >
> > > >evaluate-set-command ioctl taking:
> > > >{
> > > > enum command /* one of supported_commands */
> > > > union data
> > > > {
> > > > char raw[3072],
> > > > {
> > > >
> > > > },
> >
> > Yeah, so ... this is not a interface. This is a backdoor to pass
> > arbitrary data. That's not going to fly.
> >
> > For keyboards, we don't need complete new interface; we reasonable
> > extensions over existing display APIs -- keyboards are clearly 2D.
>
> I suppose they could be seen as *a* display, but if you are referring
> to DRM KMS UAPI, then no, I don't see that fitting at all:
So -- we already have very similar displays in
drivers/auxdisplay. drivers/auxdisplay/cfag12864b.c is 128x64 display,
1-bit display for example.
> - the "pixel grid" is not orthogonal, it's not a rectangle, and it
> might not be a grid at all
It is quite close to orthogonal. I'd suggest simply pretending it is
orthogonal grid with some pixels missing :-). We already have
cellphone displays with rounded corners and holes in them, so I
suspect handling of missing pixels will be neccessary anyway.
> - Timings and video modes? DRM KMS has always been somewhat awkward for
> display devices that do not have an inherent scanout cycle and timings
> totally depend on the amount of pixels updated at a time
> (FB_DAMAGE_CLIPS), e.g. USB displays (not USB-C DP alt mode).
> They do work, but they are very different from the usual hardware
> involved with KMS, require special consideration in userspace, and
> they still are actual displays while what we're talking about here
> are not.
As you say, there are other displays with similar problems.
> - KMS has no concept of programmed autonomous animations, and likely
> never will. They are not useful with actual displays.
Yep. We need some kind of extension here, and this is likely be quite
vendor-specific, as animations will differ between vendors. I guess
"please play pattern xyzzy with parametrs 3 and 5" might be enough for this.
> - Userspace will try to light up KMS outputs automatically and extend
> the traditional desktop there. This was already a problem for
> head-mounted displays (HMD) where it made no sense. That was worked
> around with an in-kernel list of HMDs and some KMS property
> quirking.
This will need fixing for cfag12864b.c, no? Perhaps userspace should
simply ignore anything smaller than 240x160 or something...
> Modern KMS UAPI very much aims to be a generic UAPI that abstracts
> display devices. It already breaks down a little for things like USB
> displays and virtual machines (e.g. qemu, vmware, especially with
> remote viewers), which I find unfortunate. With HMDs the genericity
> breaks down in other ways, but I'd claim HMDs are a better fit still
> than full-featured VM virtual displays (cursor plane hijacking). With
> non-displays like keyboards the genericity would be completely lost, as
> they won't work at all the same way as displays. You cannot even show
> proper images there, only coarse light patterns *IF* you actually know
> the pixel layout. But the pixel layout is(?) hardware-specific which is
> the opposite of generic.
>
> While you could dress keyboard lights etc. up with DRM KMS UAPI, the
> userspace would have to be written from scratch for them, and you
> somehow need to make existing KMS userspace to never touch those
> devices. What's the point of using DRM KMS UAPI in the first place,
> then?
Well, at least we have good UAPI to work with. Other options were 100
files in /sys/class/leds, pretending it is a linear array of leds,
just passing raw data around, and pretending it is grid of RGB888
data.
Anyway, there are devices such as these: (8x8 LED display).
https://www.laskakit.cz/8x8-led-matice-s-max7219-3mm-cervena/
We should think about what interface we want for these, and then I
believe we should make RGB keyboards use something similar.
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
signature.asc
Description: PGP signature
Re: Future handling of complex RGB devices on Linux v2
Hi! > > Yeah, so ... this is not a interface. This is a backdoor to pass > > arbitrary data. That's not going to fly. > > Pavel, Note the data will be interpreted by a kernel driver and > not passed directly to the hw. Yes, still not flying :-). > With that said I tend to agree that this seems to be a bit too > generic. Exactly. > Given that these devices are all different in various ways > and that we only want this for devices which cannot be accessed > from userspace directly (so a limit set of devices) I really > think that just doing custom ioctls per vendor is best. I don't think that's good idea in the long term. Kernel should provide hardware abstraction, so that userspace does not need to know about hardware. Obviously there are exceptions, but this should not be one of those. BR, 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
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: Future handling of complex RGB devices on Linux v2
Hi!
> so after more feedback from the OpenRGB maintainers I came up with an even
> more generic proposal:
> https://gitlab.com/CalcProgrammer1/OpenRGB/-/issues/3916#note_1753072869
> >evaluate-set-command ioctl taking:
> >{
> > enum command /* one of supported_commands */
> > union data
> > {
> > char raw[3072],
> > {
> >
> > },
Yeah, so ... this is not a interface. This is a backdoor to pass
arbitrary data. That's not going to fly.
For keyboards, we don't need complete new interface; we reasonable
extensions over existing display APIs -- keyboards are clearly 2D.
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
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
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
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: [RFC] string: Allow 2-argument strscpy()
Hi! > > Using sizeof(dst) is the overwhelmingly common case for strscpy(). > > Instead of requiring this everywhere, allow a 2-argument version to be > > used that will use the sizeof() internally. > > Yeah, this is definitely the case. I have a ton of patches replacing > strncpy with strscpy [1] and many of them match the pattern of: > | strscpy(dest, src, sizeof(dest)) > > BTW, this hack for function overloading is insane. Never really looked into > it before. This hack is insane, but this is also highly confusing, please don't do this. BR, 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
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
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
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
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
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
Re: Implement per-key keyboard backlight as auxdisplay?
Hi! > > And while I would personally hate it, you can imagine a use case where > > you'd like a keypress to have a visual effect around the key you > > pressed. A kind of force feedback, if you will. I don't actually know, > > and correct me if I'm wrong, but feels like implementing that outside of > > the input subsystem would be non-trivial. > > Actually I think it does not belong to the input subsystem as it is, > where the goal is to deliver keystrokes and gestures to userspace. The > "force feedback" kind of fits, but not really practical, again because > of lack of geometry info. It is also not really essential to be fully > and automatically handled by the kernel. So I think the best way is > > to So that's actually big question. If the common usage is "run bad apple demo on keyboard" than pretty clearly it should be display. If the common usage is "computer is asking yes/no question, so highlight yes and no buttons", then there are good arguments why input should handle that (as it does capslock led, for example). Actually I could imagine "real" use when shift / control /alt backlight would indicate sticky-shift keys for handicapped. It seems they are making mice with backlit buttons. If the main use is highlight this key whereever it is, then it should be input. But I suspect may use is just fancy colors and it should be display. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: Implement per-key keyboard backlight as auxdisplay?
Hi! > > > And then storing RGB in separate bytes, so userspace will then > > > always send a buffer of 192 bytes per line (64x3) x 14 rows > > > = 3072 bytes. With the kernel driver ignoring parts of > > > the buffer where there are no actual keys. > > That's really really weird interface. If you are doing RGB888 64x14, > > lets make it a ... display? :-). > > > > ioctl always sending 3072 bytes is really a hack. > > > > Small displays exist and are quite common, surely we'd handle this as > > a display: > > https://pajenicko.cz/displeje/graficky-oled-displej-0-66-64x48-i2c-bily-wemos-d1-mini > > It is 64x48. > > > > And then there's this: > > https://pajenicko.cz/displeje/maticovy-8x8-led-displej-s-radicem-max7219 > > and this: > > https://pajenicko.cz/displeje/maticovy-8x32-led-displej-s-radicem-max7219 > > > > One of them is 8x8. > > > > Surely those should be displays, too? > > But what about a light bar with, lets say, 3 zones. Is that a 3x1 display? > > And what about a mouse having lit mousebuttons and a single led light bar at > the wrist: a 2x2 display, but one is thin but long and one is not used? So indeed LEDs can arranged into various shapes. Like a ring, or this: * * * * * * * https://pajenicko.cz/led-moduly?page=2 Dunno. Sounds like a display is still a best match for them. Some of modules are RGB, some are single-color only, I'm sure there will be various bit depths. I guess we can do 3x1 and 2x2 displays. Or we could try to solve keyboards and ignore those for now. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: Implement per-key keyboard backlight as auxdisplay?
Hi! > >> 2. Implement per-key keyboards as auxdisplay > >> > >> - Pro: > >> > >> - Already has a concept for led positions > >> > >> - Is conceptually closer to "multiple leds forming a singular > >> entity" > >> > >> - Con: > >> > >> - No preexisting UPower support > >> > >> - No concept for special hardware lightning modes > >> > >> - No support for arbitrary led outlines yet (e.g. ISO style > >> enter-key) > > > > Please do this one. > > Ok, so based on the discussion so far and Pavel's feedback lets try to > design a custom userspace API for this. I do not believe that auxdisplay > is a good fit because: Ok, so lets call this a "display". These days, framebuffers and drm handles displays. My proposal is to use similar API as other displays. > So my proposal would be an ioctl interface (ioctl only no r/w) > using /dev/rgbkbd0 /dev/rgbkdb1, etc. registered as a misc chardev. > > For per key controllable rgb LEDs we need to discuss a coordinate > system. I propose using a fixed size of 16 rows of 64 keys, > so 64x16 in standard WxH notation. > > And then storing RGB in separate bytes, so userspace will then > always send a buffer of 192 bytes per line (64x3) x 14 rows > = 3072 bytes. With the kernel driver ignoring parts of > the buffer where there are no actual keys. That's really really weird interface. If you are doing RGB888 64x14, lets make it a ... display? :-). ioctl always sending 3072 bytes is really a hack. Small displays exist and are quite common, surely we'd handle this as a display: https://pajenicko.cz/displeje/graficky-oled-displej-0-66-64x48-i2c-bily-wemos-d1-mini It is 64x48. And then there's this: https://pajenicko.cz/displeje/maticovy-8x8-led-displej-s-radicem-max7219 and this: https://pajenicko.cz/displeje/maticovy-8x32-led-displej-s-radicem-max7219 One of them is 8x8. Surely those should be displays, too? And yes, we'd probably want some extra ioctls on top, for example to map from input device to this and back, and maybe for various effects, too. And yes, I realize that display with holes in it and with some pixels bigger than others is weird, but it still looks like a display to me. (And phones have high-res displays with rounded corners and holes in them, so... we'll need to deal with weird displays anyway). Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: Implement per-key keyboard backlight as auxdisplay?
Hi! > We have an upcoming device that has a per-key keyboard backlight, but does > the control completely via a wmi/acpi interface. So no usable hidraw here > for a potential userspace driver implementation ... > > So a quick summary for the ideas floating in this thread so far: > > 1. Expand leds interface allowing arbitrary modes with semi arbitrary > optional attributes: > - Con: > > - Violates the simplicity paradigm of the leds interface (e.g. with > this one leds entry controls possible multiple leds) Let's not do this. > 2. Implement per-key keyboards as auxdisplay > > - Pro: > > - Already has a concept for led positions > > - Is conceptually closer to "multiple leds forming a singular entity" > > - Con: > > - No preexisting UPower support > > - No concept for special hardware lightning modes > > - No support for arbitrary led outlines yet (e.g. ISO style enter-key) Please do this one. > 3. Implement in input subsystem > > - Pro: > > - Preexisting concept for keys and key purpose > > - Con: > > - Not in scope for subsystem > > - No other preexisting light infrastructure Or negotiate with input people to do this. > 4. Implement a simple leds driver only supporting a small subset of the > capabilities and make it disable-able for a userspace driver to take over No. Kernel should abstract this away. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
End of 4.14 autosel? Re: [PATCH AUTOSEL 4.14 3/6] drm/crtc: Fix uninit-value bug in drm_mode_setcrtc
Hi!
> > > From: Ziqi Zhao
> > >
> > > [ Upstream commit 3823119b9c2b5f9e9b760336f75bc989b805cde6 ]
> > >
> > > The connector_set contains uninitialized values when allocated with
> > > kmalloc_array. However, in the "out" branch, the logic assumes that any
> > > element in connector_set would be equal to NULL if failed to
> > > initialize, which causes the bug reported by Syzbot. The fix is to use
> > > an extra variable to keep track of how many connectors are initialized
> > > indeed, and use that variable to decrease any refcounts in the "out"
> > > branch.
> > >
> > > Reported-by: syzbot+4fad2e57beb6397ab...@syzkaller.appspotmail.com
> > > Signed-off-by: Ziqi Zhao
> > > Reported-and-tested-by:
> > > syzbot+4fad2e57beb6397ab...@syzkaller.appspotmail.com
> > > Tested-by: Harshit Mogalapalli
> > > Link: https://lore.kernel.org/r/20230721161446.8602-1-astraj...@yahoo.com
> > > Signed-off-by: Maxime Ripard
> > > Signed-off-by: Sasha Levin
> >
> > This commit fixes an uninitialized value, but introduces a new
> > one. Please backport 6e455f5dcdd1 ("drm/crtc: fix uninitialized variable
> > use") from v6.7-rc6 to go with it.
>
> I'll take 6e455f5dcdd1 too, thanks!
So, what is the status of 4.14-stable? Last one was
marked. .. well. .. as last one, so perhaps AUTOSEL should start
ignoring it, too?
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
signature.asc
Description: PGP signature
Front camera on pinephone
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: [ibm-acpi-devel] [PATCH AUTOSEL 6.1 12/24] platform/x86: thinkpad_acpi: fix for incorrect fan reporting on some ThinkPad systems
Hi!
> [ Upstream commit 66e92e23a72761f5b53f970aeb1badc5fd92fc74 ]
>
> Some ThinkPad systems ECFW use non-standard addresses for fan control
> and reporting. This patch adds support for such ECFW so that it can report
> the correct fan values.
> Tested on Thinkpads L13 Yoga Gen 2 and X13 Yoga Gen 2.
This is just a new feature, and is > 200 lines. We should not have
this in stable.
BR,
Pavel
> Suggested-by: Mark Pearson
> Signed-off-by: Vishnu Sankar
> Reviewed-by: Hans de Goede
> Reviewed-by: Ilpo Järvinen
> Link: https://lore.kernel.org/r/20231214134702.166464-1-vishnu...@gmail.com
> Signed-off-by: Ilpo Järvinen
> Signed-off-by: Sasha Levin
> ---
> drivers/platform/x86/thinkpad_acpi.c | 98
> 1 file changed, 85 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/platform/x86/thinkpad_acpi.c
> b/drivers/platform/x86/thinkpad_acpi.c
> index 05a55bc31c796..6edd2e294750e 100644
> --- a/drivers/platform/x86/thinkpad_acpi.c
> +++ b/drivers/platform/x86/thinkpad_acpi.c
> @@ -8149,8 +8149,19 @@ static struct ibm_struct volume_driver_data = {
> * TPACPI_FAN_WR_TPEC is also available and should be used to
> * command the fan. The X31/X40/X41 seems to have 8 fan levels,
> * but the ACPI tables just mention level 7.
> + *
> + * TPACPI_FAN_RD_TPEC_NS:
> + * This mode is used for a few ThinkPads (L13 Yoga Gen2, X13 Yoga Gen2
> etc.)
> + * that are using non-standard EC locations for reporting fan speeds.
> + * Currently these platforms only provide fan rpm reporting.
> + *
> */
>
> +#define FAN_RPM_CAL_CONST 491520 /* FAN RPM calculation offset for some
> non-standard ECFW */
> +
> +#define FAN_NS_CTRL_STATUS BIT(2) /* Bit which determines control
> is enabled or not */
> +#define FAN_NS_CTRL BIT(4) /* Bit which determines control
> is by host or EC */
> +
> enum { /* Fan control constants */
> fan_status_offset = 0x2f, /* EC register 0x2f */
> fan_rpm_offset = 0x84, /* EC register 0x84: LSB, 0x85 MSB (RPM)
> @@ -8158,6 +8169,11 @@ enum { /* Fan control
> constants */
> fan_select_offset = 0x31, /* EC register 0x31 (Firmware 7M)
> bit 0 selects which fan is active */
>
> + fan_status_offset_ns = 0x93,/* Special status/control offset for
> non-standard EC Fan1 */
> + fan2_status_offset_ns = 0x96, /* Special status/control offset for
> non-standard EC Fan2 */
> + fan_rpm_status_ns = 0x95, /* Special offset for Fan1 RPM status
> for non-standard EC */
> + fan2_rpm_status_ns = 0x98, /* Special offset for Fan2 RPM status
> for non-standard EC */
> +
> TP_EC_FAN_FULLSPEED = 0x40, /* EC fan mode: full speed */
> TP_EC_FAN_AUTO = 0x80, /* EC fan mode: auto fan control */
>
> @@ -8168,6 +8184,7 @@ enum fan_status_access_mode {
> TPACPI_FAN_NONE = 0,/* No fan status or control */
> TPACPI_FAN_RD_ACPI_GFAN,/* Use ACPI GFAN */
> TPACPI_FAN_RD_TPEC, /* Use ACPI EC regs 0x2f, 0x84-0x85 */
> + TPACPI_FAN_RD_TPEC_NS, /* Use non-standard ACPI EC regs (eg:
> L13 Yoga gen2 etc.) */
> };
>
> enum fan_control_access_mode {
> @@ -8195,6 +8212,8 @@ static u8 fan_control_desired_level;
> static u8 fan_control_resume_level;
> static int fan_watchdog_maxinterval;
>
> +static bool fan_with_ns_addr;
> +
> static struct mutex fan_mutex;
>
> static void fan_watchdog_fire(struct work_struct *ignored);
> @@ -8325,6 +8344,15 @@ static int fan_get_status(u8 *status)
> }
>
> break;
> + case TPACPI_FAN_RD_TPEC_NS:
> + /* Default mode is AUTO which means controlled by EC */
> + if (!acpi_ec_read(fan_status_offset_ns, ))
> + return -EIO;
> +
> + if (status)
> + *status = s;
> +
> + break;
>
> default:
> return -ENXIO;
> @@ -8341,7 +8369,8 @@ static int fan_get_status_safe(u8 *status)
> if (mutex_lock_killable(_mutex))
> return -ERESTARTSYS;
> rc = fan_get_status();
> - if (!rc)
> + /* NS EC doesn't have register with level settings */
> + if (!rc && !fan_with_ns_addr)
> fan_update_desired_level(s);
> mutex_unlock(_mutex);
>
> @@ -8368,7 +8397,13 @@ static int fan_get_speed(unsigned int *speed)
>
> if (likely(speed))
> *speed = (hi << 8) | lo;
> + break;
> + case TPACPI_FAN_RD_TPEC_NS:
> + if (!acpi_ec_read(fan_rpm_status_ns, ))
> + return -EIO;
>
> + if (speed)
> + *speed = lo ? FAN_RPM_CAL_CONST / lo : 0;
> break;
>
> default:
> @@ -8380,7 +8415,7 @@
Re: Implement per-key keyboard backlight as auxdisplay?
On Mon 2023-10-23 13:44:46, Miguel Ojeda wrote: > On Mon, Oct 23, 2023 at 1:40 PM Jani Nikula > wrote: > > > > One could also reasonably make the argument that controlling the > > individual keyboard key backlights should be part of the input > > subsystem. It's not a display per se. (Unless you actually have small > > displays on the keycaps, and I think that's a thing too.) > > > > There's force feedback, there could be light feedback? There's also > > drivers/input/input-leds.c for the keycaps that have leds, like caps > > lock, num lock, etc. > > > > Anyway, just throwing ideas around, no strong opinions, really. > > Yeah, sounds quite reasonable too, in fact it may make more sense > there given the LEDs are associated per-key rather than being an > uniform matrix in a rectangle if I understand correctly. If the input > subsystem wants to take it, that would be great. Unfortunately we are getting no input from input subsystem. Question seems to be more of "is auxdisplay willing to take it if it is done properly"? Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: Implement per-key keyboard backlight as auxdisplay?
Hi!
> >> So... a bit of rationale. The keyboard does not really fit into the
> >> LED subsystem; LEDs are expected to be independent ("hdd led") and not
> >> a matrix of them.
> >
> > Makes sense.
> >
> >> We do see various strange displays these days -- they commonly have
> >> rounded corners and holes in them. I'm not sure how that's currently
> >> supported, but I believe it is reasonable to view keyboard as a
> >> display with slightly weird placing of pixels.
> >>
> >> Plus, I'd really like to play tetris on one of those :-).
> >>
> >> So, would presenting them as auxdisplay be acceptable? Or are there
> >> better options?
> >
> > It sounds like a fair use case -- auxdisplay are typically simple
> > character-based or small graphical displays, e.g. 128x64, that may not
> > be a "main" / usual screen as typically understood, but the concept is
> > a bit fuzzy and we are a bit of a catch-all.
> >
> > And "keyboard backlight display with a pixel/color per-key" does not
> > sound like a "main" screen, and having some cute effects displayed
> > there are the kind of thing that one could do in the usual small
> > graphical ones too. :)
> >
> > But if somebody prefers to create new categories (or subcategories
> > within auxdisplay) to hold these, that could be nice too (in the
> > latter case, I would perhaps suggest reorganizing all of the existing
> > ones while at it).
>
> One could also reasonably make the argument that controlling the
> individual keyboard key backlights should be part of the input
> subsystem. It's not a display per se. (Unless you actually have small
> displays on the keycaps, and I think that's a thing too.)
While it would not be completely crazy to do that... I believe the
backlight is more of a display and less of a keyboard. Plus input
subystem is very far away from supporting this, and we had no input
from input people here.
I don't think LED subsystem is right place for this, and I believe
auxdisplay makes slightly more sense than input.
Unless someone steps up, I'd suggest Werner tries to implement this as
an auxdisplay. [And yes, this will not be simple task. RGB on LED is
different from RGB on display. But there are other LED displays, so
auxdisplay should handle this. Plus pixels are really funnily
shaped. But displays with missing pixels -- aka holes for camera --
are common in phones, and I believe we'll get variable pixel densities
-- less dense over camera -- too. So displays will have to deal with
these in the end.]
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
signature.asc
Description: PGP signature
Re: [PATCH 2/6] led-uclass: honour ->label field populated by driver's own .bind
Hi!
> > > > > > diff --git a/drivers/led/led-uclass.c b/drivers/led/led-uclass.c
> > > > > > index 5a5d07b9a7..0232fa84de 100644
> > > > > > --- a/drivers/led/led-uclass.c
> > > > > > +++ b/drivers/led/led-uclass.c
> > > > > > @@ -71,7 +71,9 @@ static int led_post_bind(struct udevice *dev)
> > > > > > struct led_uc_plat *uc_plat = dev_get_uclass_plat(dev);
> > > > > > const char *default_state;
> > > > > > -uc_plat->label = dev_read_string(dev, "label");
> > > > > > +if (!uc_plat->label)
> > > > > > +uc_plat->label = dev_read_string(dev, "label");
> > > > > > +
> > > > >
> > > > > One thing I have to wonder about is, why does this controller have
> > > > > label
> > > > > property in the top-level node , what is that used for ?
> > > > >
> > > > > (see Linux Documentation/devicetree/bindings/leds/leds-lp55xx.yaml)
> > > > >
> > > > > Reviewed-by: Marek Vasut
> > > >
> > > > Reading the linux driver, it seems that the top-level label, if any, is
> > > > used as part of the naming for individual channels if they don't have
> > > > individual chan-name properties:
> > > >
> > > >
> > > > if (pdata->led_config[chan].name) {
> > > > led->cdev.name = pdata->led_config[chan].name;
> > > > } else {
> > > > snprintf(name, sizeof(name), "%s:channel%d",
> > > > pdata->label ? : chip->cl->name, chan);
> > > > led->cdev.name = name;
> > > > }
> > > >
> > > > but I think the rationale in d1188adb2dabc is a bit weak, since the only
> > > > example also does have individual chan-name properties.
> > > >
> > > > [Complete aside: At first I thought it was related to the multi-color
> > > > LED work that has been ongoing for many many years (I think there was an
> > > > LWN article at some point), where this could be exposed as a single
> > > > multi-color LED, as opposed to the "traditional" three/four individual
> > > > LEDs. In the former case, there would only be one sysfs entry, but with
> > > > attributes exposing the multicolor functionality. I must admit I don't
> > > > know the status of that work, when something reaches v31,
> > > > http://archive.lwn.net:8080/linux-kernel/20200722071055.GA8984@amd/t/ ,
> > > > it's hard to know if it ever lands, or if pieces of it has landed.]
> > >
> > > +CC Pavel
>
> I think you want to coordinate the effort with Rasmus here .
This is u-boot, not Linux, right?
Anyway cc-ing linux-leds may be better than cc-ing me directly, but
don't expect much.
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 AUTOSEL 6.6 09/11] drm/amd: Fix UBSAN array-index-out-of-bounds for Powerplay headers
Hi! > > > From: Alex Deucher > > > > > > [ Upstream commit 49afe91370b86566857a3c2c39612cf098110885 ] > > > > > > For pptable structs that use flexible array sizes, use flexible arrays. > > > > > > Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2039926 > > > Reviewed-by: Mario Limonciello > > > Acked-by: Christian König > > > Signed-off-by: Alex Deucher > > > Signed-off-by: Sasha Levin > > > > I don't think any of these UBSAN flexible array changes are stable material. > > I'll drop it, but in general we've been taking kasan/ubsan/kcsan/... > annotation fixes since it enables (easier) testing on the LTS trees, and > for example finding issues specific to those LTS trees. I believe they should not be in stable, either. In any case, Documentation/process/stable-kernel-rules.rst should be updated, because it contradicts current practice. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH AUTOSEL 6.6 09/11] drm/amd: Fix UBSAN array-index-out-of-bounds for Powerplay headers
Hi! > > > From: Alex Deucher > > > > > > [ Upstream commit 49afe91370b86566857a3c2c39612cf098110885 ] > > > > > > For pptable structs that use flexible array sizes, use flexible arrays. > > > > > > Link: https://bugs.launchpad.net/ubuntu/+source/linux/+bug/2039926 > > > Reviewed-by: Mario Limonciello > > > Acked-by: Christian König > > > Signed-off-by: Alex Deucher > > > Signed-off-by: Sasha Levin > > > > I don't think any of these UBSAN flexible array changes are stable material. > > I'll drop it, but in general we've been taking kasan/ubsan/kcsan/... > annotation fixes since it enables (easier) testing on the LTS trees, and > for example finding issues specific to those LTS trees. I believe they should not be in stable, either. In any case, Documentation/process/stable-kernel-rules.rst should be updated, because it contradicts current practice. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH v2 0/8] dma-buf: heaps: Add secure heap
Hi! > This patchset adds three secure heaps: > 1) secure_mtk_cm: secure chunk memory for MediaTek SVP (Secure Video Path). >The buffer is reserved for the secure world after bootup and it is used >for vcodec's ES/working buffer; > 2) secure_mtk_cma: secure CMA memory for MediaTek SVP. This buffer is >dynamically reserved for the secure world and will be got when we start >playing secure videos, Once the security video playing is complete, the >CMA will be released. This heap is used for the vcodec's frame buffer. > 3) secure_cma: Use the kerne CMA ops as the allocation ops. >currently it is a draft version for Vijay and Jaskaran. Is there high-level description of what the security goals here are, somewhere? BR, 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
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
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 3/7] Input: synaptics-rmi4 - f12: use hardcoded values for aftermarket touch ICs
Hi! > Some replacement displays include third-party touch ICs which are > devoid of register descriptors. Create a fake data register descriptor > for such ICs and provide hardcoded default values. > > It isn't possible to reliably determine if the touch IC is original or > not, so these fallback values are offered as an alternative to the error > path when register descriptors aren't available. > > Signed-off-by: methanal I guess we should have full/real name here. Best regards, Pavel --
Re: [PATCH 0/2] arm64: dts: qcom: longcheer l8910 and l9100: Enable RGB LED
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: [PATCH 1/2] arm64: dts: qcom: sdm845-oneplus: enable flash LED
Hi!
> + led-1 {
> + function = LED_FUNCTION_FLASH;
> + color = ;
That's warm white, not yellow. We should likely create a define for
that.
BR,
Pavel
--
Re: [PATCH RFC] dt-bindings: display: document display panel occlusions
Hi! > > An orthogonal issue is labeling all of those regions. I think we should > > start with fully obscured areas and maybe less readable ones like the > > waterfall edges. Still, different features should live on different > > masks – even if we don't attach meaningfull labels (like 'notch' or > > 'camera cutout') to them right away. > > > > > > What do you all think of that? I didn't see this approach considered in > > any of the earlier discussions, yet it seems so elegant to me. Am I > > missing something? > > I think the unfortunate truth is that approximating notches and rounded > corners exclusively with regular arcs at the cost of pixel accuracy is > just such a no-brainer. Pixel masks would be pixel accurate, but there > is no benefit compared to a slightly underfit curve. Pixel accuracy may be important for low-resolution displays. And... I have certain low-resolution displays in mind. There are keyboards with individual backlights below each key. Maybe they could/should be treated similarily to displays with occlusions? But we'll really need to be pixel-perfect for that. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH v6 2/2] arm64: dts: qcom: sm8250-xiaomi-pipa: Add initial device tree
Hi! > > Initial support for Xiaomi Pad 6 tablet, that have sm8250 soc. > > > > Signed-off-by: Luka Panio > > +++ b/arch/arm64/boot/dts/qcom/sm8250-xiaomi-pipa.dts > > @@ -0,0 +1,625 @@ > > +// SPDX-License-Identifier: BSD-3-Clause > > If there are no other copyrights here, why did you use BSD-3 license? We like BSD license for dts files. (But dual GPL or BSD is more common. Not that it matters). > > +/* > > + * Copyright (c) 2023 luka177 > > + */ This should have real name. Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH 00/10] Fix confusion around MAX_ORDER
On Thu 2023-09-28 18:57:18, Paolo Bonzini wrote: > On 9/28/23 09:50, Mikulas Patocka wrote: > > > > Fix the bugs and then change the definition of MAX_ORDER to be > > > > inclusive: the range of orders user can ask from buddy allocator is > > > > 0..MAX_ORDER now. > > I think that exclusive MAX_ORDER is more intuitive in the C language - > > i.e. if you write "for (i = 0; i < MAX_ORDER; i++)", you are supposed to > > loop over all allowed values. If you declare an array "void > > *array[MAX_ORDER];" you are supposed to hold a value for each allowed > > order. > > > > Pascal has for loops and array dimensions with inclusive ranges - and it > > is more prone to off-by-one errors. > > I agree it's somewhat confusing either way but the ship has sailed, the > patch has been included in Linux for several months. Just make sure people don't backport it to stable. Fixes: (the commit that causes the semantic change) should do the trick. BR, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: Implement per-key keyboard backlight as auxdisplay?
Hi!
> coming from the leds mailing list I'm writing with Pavel how to best handle
> per-key RGB keyboards.
>
> His suggestion was that it could be implemented as an aux display, but he
> also suggested that I ask first if this fits.
Thanks for doing this.
> The specific keyboard RGB controller I want to implement takes 6*21 rgb
> values. However not every one is actually mapped to a physical key. e.g. the
> bottom row needs less entries because of the space bar. Additionally the
> keys are ofc not in a straight line from top to bottom.
So... a bit of rationale. The keyboard does not really fit into the
LED subsystem; LEDs are expected to be independent ("hdd led") and not
a matrix of them.
We do see various strange displays these days -- they commonly have
rounded corners and holes in them. I'm not sure how that's currently
supported, but I believe it is reasonable to view keyboard as a
display with slightly weird placing of pixels.
Plus, I'd really like to play tetris on one of those :-).
So, would presenting them as auxdisplay be acceptable? Or are there
better options?
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
signature.asc
Description: PGP signature
Re: Implement per-key keyboard backlight as auxdisplay?
Hi!
> > The specific keyboard RGB controller I want to implement takes 6*21 rgb
> > values. However not every one is actually mapped to a physical key. e.g. the
> > bottom row needs less entries because of the space bar. Additionally the
> > keys are ofc not in a straight line from top to bottom.
>
> So... a bit of rationale. The keyboard does not really fit into the
> LED subsystem; LEDs are expected to be independent ("hdd led") and not
> a matrix of them.
>
> We do see various strange displays these days -- they commonly have
> rounded corners and holes in them. I'm not sure how that's currently
> supported, but I believe it is reasonable to view keyboard as a
> display with slightly weird placing of pixels.
>
> Plus, I'd really like to play tetris on one of those :-).
>
> So, would presenting them as auxdisplay be acceptable? Or are there
> better options?
Oh and... My existing keyboard membrane-based Chicony, and it is from
time when PS/2 was still in wide use. I am slowly looking for a new
keyboard. If you know of one with nice mechanical switches, RGB
backlight with known protocol, and hopefully easily available in Czech
republic, let me know.
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
signature.asc
Description: PGP signature
Re: droid4 -- weird behaviour when attempting to use usb host
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
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: [Bridge] [PATCH AUTOSEL 4.14 6/8] netfilter: ebtables: fix fortify warnings in size_entry_mwt()
Hi! > [ Upstream commit a7ed3465daa240bdf01a5420f64336fee879c09d ] > > When compiling with gcc 13 and CONFIG_FORTIFY_SOURCE=y, the following > warning appears: > > In function ‘fortify_memcpy_chk’, > inlined from ‘size_entry_mwt’ at net/bridge/netfilter/ebtables.c:2118:2: > ./include/linux/fortify-string.h:592:25: error: call to > ‘__read_overflow2_field’ > declared with attribute warning: detected read beyond size of field (2nd > parameter); > maybe use struct_group()? [-Werror=attribute-warning] > 592 | __read_overflow2_field(q_size_field, size); > | > ^~ This is not queued for 4.19. Mistake? 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 AUTOSEL 6.1 18/22] drm/mediatek: dp: Change logging to dev for mtk_dp_aux_transfer()
Hi!
> From: AngeloGioacchino Del Regno
>
> [ Upstream commit fd70e2019bfbcb0ed90c5e23839bf510ce6acf8f ]
>
> Change logging from drm_{err,info}() to dev_{err,info}() in functions
> mtk_dp_aux_transfer() and mtk_dp_aux_do_transfer(): this will be
> essential to avoid getting NULL pointer kernel panics if any kind
> of error happens during AUX transfers happening before the bridge
> is attached.
>
> This may potentially start happening in a later commit implementing
> aux-bus support, as AUX transfers will be triggered from the panel
> driver (for EDID) before the mtk-dp bridge gets attached, and it's
> done in preparation for the same.
This is preparation for patches we are not going to apply to
stable. Please drop.
BR,
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: [dm-devel] [PATCH] fix writing to the filesystem after unmount
On Thu 2023-09-07 11:44:57, Jan Kara wrote: > On Wed 06-09-23 18:52:39, Mikulas Patocka wrote: > > On Wed, 6 Sep 2023, Christian Brauner wrote: > > > On Wed, Sep 06, 2023 at 06:01:06PM +0200, Mikulas Patocka wrote: > > > > > > BTW. what do you think that unmount of a frozen filesystem should > > > > > > properly > > > > > > do? Fail with -EBUSY? Or, unfreeze the filesystem and unmount it? > > > > > > Or > > > > > > something else? > > > > > > > > > > In my opinion we should refuse to unmount frozen filesystems and log > > > > > an > > > > > error that the filesystem is frozen. Waiting forever isn't a good idea > > > > > in my opinion. > > > > > > > > But lvm may freeze filesystems anytime - so we'd get randomly returned > > > > errors then. > > > > > > So? Or you might hang at anytime. > > > > lvm doesn't keep logical volumes suspended for a prolonged amount of time. > > It will unfreeze them after it made updates to the dm table and to the > > metadata. So, it won't hang forever. > > > > I think it's better to sleep for a short time in umount than to return an > > error. > > I think we've got too deep down into "how to fix things" but I'm not 100% > sure what the "bug" actually is. In the initial posting Mikulas writes "the > kernel writes to the filesystem after unmount successfully returned" - is > that really such a big issue? Anybody else can open the device and write to > it as well. Or even mount the device again. So userspace that relies on > this is kind of flaky anyway (and always has been). Umm. No? I admin my own systems; I'm responsible for my userspace. Maybe I'm in single user mode. Noone writes to my block devices without my permissions. By mount, I give such permission to the kernel. By umount, I take such permission away. There's nothing flaky about that. Kernel is simply buggy. Fix it. [Remember that "you should umount before disconnecting USB devices to prevent data corruption"? How is that working with kernel writing to devices after umount?] Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
Re: [dm-devel] [PATCH] fix writing to the filesystem after unmount
Hi! > What I wanted to suggest is that we should provide means how to make sure > block device is not being modified and educate admins and tool authors > about them. Because just doing "umount /dev/sda1" and thinking this means > that /dev/sda1 is unused now simply is not enough in today's world for > multiple reasons and we cannot solve it just in the kernel. It better be enough. And I'm pretty sure it is true in single-user mode, or for usb sticks, or... Simply fix the kernel. No need to re-educate anyone. Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature -- dm-devel mailing list dm-devel@redhat.com https://listman.redhat.com/mailman/listinfo/dm-devel
Re: [PATCH AUTOSEL 4.14 1/9] drm/radeon: Fix integer overflow in radeon_cs_parser_init
Hi! > From: hackyzh002 > > [ Upstream commit f828b681d0cd566f86351c0b913e6cb6ed8c7b9c ] > > The type of size is unsigned, if size is 0x4000, there will be an > integer overflow, size will be zero after size *= sizeof(uint32_t), > will cause uninitialized memory to be referenced later I only got the first patch of the series via lkml, rest seems to have been lost somewhere :-(. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH AUTOSEL 4.14 1/9] drm/radeon: Fix integer overflow in radeon_cs_parser_init
Hi! > From: hackyzh002 > > [ Upstream commit f828b681d0cd566f86351c0b913e6cb6ed8c7b9c ] > > The type of size is unsigned, if size is 0x4000, there will be an > integer overflow, size will be zero after size *= sizeof(uint32_t), > will cause uninitialized memory to be referenced later I only got the first patch of the series via lkml, rest seems to have been lost somewhere :-(. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH AUTOSEL 4.14 5/6] fbdev: imsttfb: Release framebuffer and dealloc cmap on error path
Hi!
> From: Helge Deller
>
> [ Upstream commit 5cf9a090a39c97f4506b7b53739d469b1c05a7e9 ]
>
> Add missing cleanups in error path.
If we insist this is important enough for -stable, it will need
tweaking. The function returns void, so we can't return a value.
Best regards,
Pavel
> +++ b/drivers/video/fbdev/imsttfb.c
> @@ -1452,9 +1452,13 @@ static void init_imstt(struct fb_info *info)
> FBINFO_HWACCEL_FILLRECT |
> FBINFO_HWACCEL_YPAN;
>
> - fb_alloc_cmap(>cmap, 0, 0);
> + if (fb_alloc_cmap(>cmap, 0, 0)) {
> + framebuffer_release(info);
> + return -ENODEV;
> + }
>
> if (register_framebuffer(info) < 0) {
> + fb_dealloc_cmap(>cmap);
> framebuffer_release(info);
> return;
> }
--
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] video/aperture: fix typos
Hi! > > Am 04.04.23 um 06:01 schrieb Sui Jingfeng: > >> EFI FB, VESA FB or VGA FB etc are belong to firmware based framebuffer > >> driver. > > > ... > I fixed that before applying, also removed the "are" in the sentence > above, since it sounded off and repharsed subject line as "Fix typos > in comments". I seem to remember that 'all your bases are belong to us' is an old meme, but that was probably not intentional here. Best regards, Pavel --
Re: [PATCH] drm/display: Add missing OLED Vesa brightnesses definitions
On Wed 2023-03-22 10:05:13, Rodrigo Siqueira wrote: > Cc: Anthony Koo > Cc: Iswara Negulendran > Cc: Felipe Clark > Cc: Harry Wentland > Signed-off-by: Rodrigo Siqueira Some changelog would be useful. > +++ b/include/drm/display/drm_dp.h > @@ -977,6 +977,8 @@ > # define DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP (1 << 5) > # define DP_EDP_DYNAMIC_BACKLIGHT_CAP(1 << 6) > # define DP_EDP_VBLANK_BACKLIGHT_UPDATE_CAP (1 << 7) > +#define DP_EDP_OLED_VESA_BRIGHTNESS_ON 0x80 > +# define DP_EDP_OLED_VESA_CAP(1 << 4) > Are you fixing a compile problem? Otherwise this should go in with the user... BR, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH] drm/display: Add missing OLED Vesa brightnesses definitions
On Wed 2023-03-22 10:05:13, Rodrigo Siqueira wrote: > Cc: Anthony Koo > Cc: Iswara Negulendran > Cc: Felipe Clark > Cc: Harry Wentland > Signed-off-by: Rodrigo Siqueira Some changelog would be useful. > +++ b/include/drm/display/drm_dp.h > @@ -977,6 +977,8 @@ > # define DP_EDP_BACKLIGHT_FREQ_AUX_SET_CAP (1 << 5) > # define DP_EDP_DYNAMIC_BACKLIGHT_CAP(1 << 6) > # define DP_EDP_VBLANK_BACKLIGHT_UPDATE_CAP (1 << 7) > +#define DP_EDP_OLED_VESA_BRIGHTNESS_ON 0x80 > +# define DP_EDP_OLED_VESA_CAP(1 << 4) > Are you fixing a compile problem? Otherwise this should go in with the user... BR, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
AUXdisplay for LED arrays, keyboards with per-key LEDs -- was Re: [PATCH v2 2/2] leds: add aw20xx driver
Hi! > > +config LEDS_AW200XX > > + tristate "LED support for Awinic AW20036/AW20054/AW20072" > > + depends on LEDS_CLASS > > + depends on I2C > > + help > > + This option enables support for the AW20036/AW20054/AW20072 LED > > driver. > > + It is a 3x12/6x9/6x12 matrix LED driver programmed via > > + an I2C interface, up to 36/54/72 LEDs or 12/18/24 RGBs, > > + 3 pattern controllers for auto breathing or group dimming control. > > I'm afraid this should be handled as a display, not as an array of > individual LEDs. You probably want to see AUXILIARY DISPLAY DRIVERS M: Miguel Ojeda S: Maintained F: Documentation/devicetree/bindings/auxdisplay/ F: drivers/auxdisplay/ F: include/linux/cfag12864b.h And this brings another question... ...sooner or later we'll see LED displays with around 100 pixels in almost rectangular grid. Minority of the pixels will have funny shapes. How will we handle those? Pretend it is regular display with some pixels missing? How do we handle cellphone displays with rounded corners and holes for front camera? And yes, such crazy displays are being manufactured -- it is called keyboard with per-key backlight... https://www.reddit.com/r/MechanicalKeyboards/comments/8dtvgo/keyboard_with_individually_programmable_leds/ Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH v4 0/4] Add PinePhone Pro display support
Hi! > This series add support for the display present in the PinePhone Pro. > > Patch #1 adds a driver for panels using the Himax HX8394 panel controller, > such as the HSD060BHW4 720x1440 TFT LCD panel present in the PinePhone Pro. > > Patch #2 adds a devicetree binding schema for this driver and patch #3 adds > an entry for the driver in the MAINTAINERS file. > > Finally patch #4 adds the needed devicetree nodes in the PinePhone Pro DTS, > to enable both the display and the touchscreen. This makes the upstream DTS > much more usable and will allow for example to enable support for the phone > in the Fedora distribution. Thanks for the series. Please cc: phone-de...@vger.kernel.org with future patches. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [Bridge] [PATCH] treewide: Convert del_timer*() to timer_shutdown*()
On Tue 2022-12-20 13:45:19, Steven Rostedt wrote: > [ > Linus, > > I ran the script against your latest master branch: > commit b6bb9676f2165d518b35ba3bea5f1fcfc0d969bf > > As the timer_shutdown*() code is now in your tree, I figured > we can start doing the conversions. At least add the trivial ones > now as Thomas suggested that this gets applied at the end of the > merge window, to avoid conflicts with linux-next during the > development cycle. I can wait to Friday to run it again, and > resubmit. > > What is the best way to handle this? > ] > > From: "Steven Rostedt (Google)" > > Due to several bugs caused by timers being re-armed after they are > shutdown and just before they are freed, a new state of timers was added > called "shutdown". After a timer is set to this state, then it can no > longer be re-armed. > > The following script was run to find all the trivial locations where > del_timer() or del_timer_sync() is called in the same function that the > object holding the timer is freed. It also ignores any locations where the > timer->function is modified between the del_timer*() and the free(), as > that is not considered a "trivial" case. > > This was created by using a coccinelle script and the following commands: LED parts looks good to me. Getting it in just before -rc1 would be best solution for me. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [Intel-gfx] [PATCH] treewide: Convert del_timer*() to timer_shutdown*()
On Tue 2022-12-20 13:45:19, Steven Rostedt wrote: > [ > Linus, > > I ran the script against your latest master branch: > commit b6bb9676f2165d518b35ba3bea5f1fcfc0d969bf > > As the timer_shutdown*() code is now in your tree, I figured > we can start doing the conversions. At least add the trivial ones > now as Thomas suggested that this gets applied at the end of the > merge window, to avoid conflicts with linux-next during the > development cycle. I can wait to Friday to run it again, and > resubmit. > > What is the best way to handle this? > ] > > From: "Steven Rostedt (Google)" > > Due to several bugs caused by timers being re-armed after they are > shutdown and just before they are freed, a new state of timers was added > called "shutdown". After a timer is set to this state, then it can no > longer be re-armed. > > The following script was run to find all the trivial locations where > del_timer() or del_timer_sync() is called in the same function that the > object holding the timer is freed. It also ignores any locations where the > timer->function is modified between the del_timer*() and the free(), as > that is not considered a "trivial" case. > > This was created by using a coccinelle script and the following commands: LED parts looks good to me. Getting it in just before -rc1 would be best solution for me. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH] treewide: Convert del_timer*() to timer_shutdown*()
On Tue 2022-12-20 13:45:19, Steven Rostedt wrote: > [ > Linus, > > I ran the script against your latest master branch: > commit b6bb9676f2165d518b35ba3bea5f1fcfc0d969bf > > As the timer_shutdown*() code is now in your tree, I figured > we can start doing the conversions. At least add the trivial ones > now as Thomas suggested that this gets applied at the end of the > merge window, to avoid conflicts with linux-next during the > development cycle. I can wait to Friday to run it again, and > resubmit. > > What is the best way to handle this? > ] > > From: "Steven Rostedt (Google)" > > Due to several bugs caused by timers being re-armed after they are > shutdown and just before they are freed, a new state of timers was added > called "shutdown". After a timer is set to this state, then it can no > longer be re-armed. > > The following script was run to find all the trivial locations where > del_timer() or del_timer_sync() is called in the same function that the > object holding the timer is freed. It also ignores any locations where the > timer->function is modified between the del_timer*() and the free(), as > that is not considered a "trivial" case. > > This was created by using a coccinelle script and the following commands: LED parts looks good to me. Getting it in just before -rc1 would be best solution for me. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH v2 01/11] pwm: Make .get_state() callback return an error code
Hi!
> .get_state() might fail in some cases. To make it possible that a driver
> signals such a failure change the prototype of .get_state() to return an
> error code.
>
> This patch was created using coccinelle and the following semantic patch:
>
> @p1@
> identifier getstatefunc;
> identifier driver;
> @@
> struct pwm_ops driver = {
> ...,
> .get_state = getstatefunc
> ,...
> };
>
> @p2@
> identifier p1.getstatefunc;
> identifier chip, pwm, state;
> @@
> -void
> +int
> getstatefunc(struct pwm_chip *chip, struct pwm_device *pwm, struct pwm_state
> *state)
> {
>...
> - return;
> + return 0;
>...
> }
>
> plus the actual change of the prototype in include/linux/pwm.h (plus some
> manual fixing of indentions and empty lines).
>
> So for now all drivers return success unconditionally. They are adapted
> in the following patches to make the changes easier reviewable.
>
> Signed-off-by: Uwe Kleine-König
LED part:
Acked-by: Pavel Machek
Best regards,
Pavel
> static const struct pwm_ops ti_sn_pwm_ops = {
> diff --git a/drivers/leds/rgb/leds-qcom-lpg.c
> b/drivers/leds/rgb/leds-qcom-lpg.c
> index 02f51cc61837..741cc2fd817d 100644
> --- a/drivers/leds/rgb/leds-qcom-lpg.c
> +++ b/drivers/leds/rgb/leds-qcom-lpg.c
> @@ -968,8 +968,8 @@ static int lpg_pwm_apply(struct pwm_chip *chip, struct
> pwm_device *pwm,
> return ret;
> }
>
> -static void lpg_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> - struct pwm_state *state)
> +static int lpg_pwm_get_state(struct pwm_chip *chip, struct pwm_device *pwm,
> + struct pwm_state *state)
> {
> struct lpg *lpg = container_of(chip, struct lpg, pwm);
> struct lpg_channel *chan = >channels[pwm->hwpwm];
> @@ -982,20 +982,20 @@ static void lpg_pwm_get_state(struct pwm_chip *chip,
> struct pwm_device *pwm,
>
> ret = regmap_read(lpg->map, chan->base + LPG_SIZE_CLK_REG, );
> if (ret)
> - return;
> + return 0;
>
> refclk = lpg_clk_rates[val & PWM_CLK_SELECT_MASK];
> if (refclk) {
> ret = regmap_read(lpg->map, chan->base + LPG_PREDIV_CLK_REG,
> );
> if (ret)
> - return;
> + return 0;
>
> pre_div = lpg_pre_divs[FIELD_GET(PWM_FREQ_PRE_DIV_MASK, val)];
> m = FIELD_GET(PWM_FREQ_EXP_MASK, val);
>
> ret = regmap_bulk_read(lpg->map, chan->base + PWM_VALUE_REG,
> _value, sizeof(pwm_value));
> if (ret)
> - return;
> + return 0;
>
> state->period = DIV_ROUND_UP_ULL((u64)NSEC_PER_SEC *
> LPG_RESOLUTION * pre_div * (1 << m), refclk);
> state->duty_cycle = DIV_ROUND_UP_ULL((u64)NSEC_PER_SEC *
> pwm_value * pre_div * (1 << m), refclk);
> @@ -1006,13 +1006,15 @@ static void lpg_pwm_get_state(struct pwm_chip *chip,
> struct pwm_device *pwm,
>
> ret = regmap_read(lpg->map, chan->base + PWM_ENABLE_CONTROL_REG, );
> if (ret)
> - return;
> + return 0;
>
> state->enabled = FIELD_GET(LPG_ENABLE_CONTROL_OUTPUT, val);
> state->polarity = PWM_POLARITY_NORMAL;
>
> if (state->duty_cycle > state->period)
> state->duty_cycle = state->period;
> +
> + return 0;
> }
--
People of Russia, stop Putin before his war on Ukraine escalates.
signature.asc
Description: PGP signature
Re: 6.1-rc: names of video outputs changed?
Hi! > > I used to be able to do: > > > > pavel@duo:~$ xrandr --output HDMI1 --mode 1920x1080 --primary > > warning: output HDMI1 not found; ignoring > > pavel@duo:~$ xrandr --output VGA1 --mode 1280x1024 --below HDMI1 > > warning: output VGA1 not found; ignoring > > > > ...but now I have to do: > > > > pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1 > > xrandr: cannot find crtc for output VGA-1 > > pavel@duo:~$ xrandr --output LVDS-1 --off > > pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1 > > > > Notice the change from HDMI1 to HDMI-1. I believe that's new in 6.1 or > > so. Who did it and why? Hardware is thinkpad x220, and this breaks my > > scripts :-(. > > Are you sure you didn't just switch from intel ddx to modesetting ddx? How do I tell? I don't think I did such changes recently. It is Debian 10.13 system (rather old) so I don't think it did update for me. Thanks and best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
6.1-rc: names of video outputs changed?
Hi! I used to be able to do: pavel@duo:~$ xrandr --output HDMI1 --mode 1920x1080 --primary warning: output HDMI1 not found; ignoring pavel@duo:~$ xrandr --output VGA1 --mode 1280x1024 --below HDMI1 warning: output VGA1 not found; ignoring ...but now I have to do: pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1 xrandr: cannot find crtc for output VGA-1 pavel@duo:~$ xrandr --output LVDS-1 --off pavel@duo:~$ xrandr --output VGA-1 --mode 1280x1024 --below HDMI-1 Notice the change from HDMI1 to HDMI-1. I believe that's new in 6.1 or so. Who did it and why? Hardware is thinkpad x220, and this breaks my scripts :-(. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH 4.19 174/229] x86/entry: Work around Clang __bdos() bug
Hi!
> From: Kees Cook
>
> [ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]
>
> Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
> and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
> offset. Work around this by using a direct assignment of an empty
> instance. Avoids this warning:
>
> ../include/linux/fortify-string.h:309:4: warning: call to
> __write_overflow_field declared with 'warn
> ing' attribute: detected write beyond size of field (1st parameter); maybe
> use struct_group()? [-Wat
> tribute-warning]
> __write_overflow_field(p_size_field, size);
> ^
>
> which was isolated to the memset() call in xen_load_idt().
>
> Note that this looks very much like another bug that was worked around:
> https://github.com/ClangBuiltLinux/linux/issues/1592
We don't have CONFIG_UBSAN_BOUNDS in 4.19, so maybe we don't need this
one?
Best regards,
Pavel
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -752,6 +752,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
> {
> static DEFINE_SPINLOCK(lock);
> static struct trap_info traps[257];
> + static const struct trap_info zero = { };
> unsigned out;
>
> trace_xen_cpu_load_idt(desc);
> @@ -761,7 +762,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
> memcpy(this_cpu_ptr(_desc), desc, sizeof(idt_desc));
>
> out = xen_convert_trap_info(desc, traps, false);
> - memset([out], 0, sizeof(traps[0]));
> + traps[out] = zero;
>
> xen_mc_flush();
> if (HYPERVISOR_set_trap_table(traps))
> --
> 2.35.1
>
>
--
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 AUTOSEL 4.19 16/25] wifi: ath10k: reset pointer after memory free to avoid potential use-after-free
Hi!
> From: Wen Gong
>
> [ Upstream commit 1e1cb8e0b73e6f39a9d4a7a15d940b1265387eb5 ]
>
> When running suspend test, kernel crash happened in ath10k, and it is
> fixed by commit b72a4aff947b ("ath10k: skip ath10k_halt during suspend
> for driver state RESTARTING").
>
> Currently the crash is fixed, but as a common code style, it is better
> to set the pointer to NULL after memory is free.
>
> This is to address the code style and it will avoid potential bug of
> use-after-free.
We don't have this patch in 4.19:
b72a4aff947b ("ath10k: skip ath10k_halt during suspend for driver state
RESTARTING").
We probably should take that one, as this may depend on it. On the
other hand, we don't need this one as it is just a cleanup...
Best regards,
Pavel
> +++ b/drivers/net/wireless/ath/ath10k/htt_rx.c
> @@ -302,12 +302,16 @@ void ath10k_htt_rx_free(struct ath10k_htt *htt)
> ath10k_htt_get_vaddr_ring(htt),
> htt->rx_ring.base_paddr);
>
> + ath10k_htt_config_paddrs_ring(htt, NULL);
> +
> dma_free_coherent(htt->ar->dev,
> sizeof(*htt->rx_ring.alloc_idx.vaddr),
> htt->rx_ring.alloc_idx.vaddr,
> htt->rx_ring.alloc_idx.paddr);
> + htt->rx_ring.alloc_idx.vaddr = NULL;
>
> kfree(htt->rx_ring.netbufs_ring);
> + htt->rx_ring.netbufs_ring = NULL;
> }
>
> static inline struct sk_buff *ath10k_htt_rx_netbuf_pop(struct ath10k_htt
> *htt)
> @@ -641,8 +645,10 @@ int ath10k_htt_rx_alloc(struct ath10k_htt *htt)
> ath10k_htt_get_rx_ring_size(htt),
> vaddr_ring,
> htt->rx_ring.base_paddr);
> + ath10k_htt_config_paddrs_ring(htt, NULL);
> err_dma_ring:
> kfree(htt->rx_ring.netbufs_ring);
> + htt->rx_ring.netbufs_ring = NULL;
> err_netbuf:
> return -ENOMEM;
> }
> --
> 2.35.1
--
DENX Software Engineering GmbH, Managing Director: Wolfgang Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
signature.asc
Description: PGP signature
___
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
Re: [PATCH AUTOSEL 5.10 04/16] powerpc/hw_breakpoint: Avoid relying on caller synchronization
Hi! > From: Marco Elver > > [ Upstream commit f95e5a3d59011eec1257d0e76de1e1f8969d426f ] > > Internal data structures (cpu_bps, task_bps) of powerpc's hw_breakpoint > implementation have relied on nr_bp_mutex serializing access to them. > > Before overhauling synchronization of kernel/events/hw_breakpoint.c, > introduce 2 spinlocks to synchronize cpu_bps and task_bps respectively, > thus avoiding reliance on callers synchronizing powerpc's > hw_breakpoint. This is preparation, not a bugfix, not sure why it was picked for 5.10. 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 AUTOSEL 4.19 5/6] x86/entry: Work around Clang __bdos() bug
Hi!
> From: Kees Cook
>
> [ Upstream commit 3e1730842f142add55dc658929221521a9ea62b6 ]
>
> Clang produces a false positive when building with CONFIG_FORTIFY_SOURCE=y
> and CONFIG_UBSAN_BOUNDS=y when operating on an array with a dynamic
> offset. Work around this by using a direct assignment of an empty
> instance. Avoids this warning:
>
> ../include/linux/fortify-string.h:309:4: warning: call to
> __write_overflow_field declared with 'warn
> ing' attribute: detected write beyond size of field (1st parameter); maybe
> use struct_group()? [-Wat
> tribute-warning]
> __write_overflow_field(p_size_field, size);
> ^
>
> which was isolated to the memset() call in xen_load_idt().
>
> Note that this looks very much like another bug that was worked around:
> https://github.com/ClangBuiltLinux/linux/issues/1592
At least in 4.19, there's no UBSAN_BOUNDS. Sounds like we don't need
it in old kernels?
Best regards,
Pavel
> +++ b/arch/x86/xen/enlighten_pv.c
> @@ -752,6 +752,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
> {
> static DEFINE_SPINLOCK(lock);
> static struct trap_info traps[257];
> + static const struct trap_info zero = { };
> unsigned out;
>
> trace_xen_cpu_load_idt(desc);
> @@ -761,7 +762,7 @@ static void xen_load_idt(const struct desc_ptr *desc)
> memcpy(this_cpu_ptr(_desc), desc, sizeof(idt_desc));
>
> out = xen_convert_trap_info(desc, traps, false);
> - memset([out], 0, sizeof(traps[0]));
> + traps[out] = zero;
>
> xen_mc_flush();
> if (HYPERVISOR_set_trap_table(traps))
> --
> 2.35.1
--
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 AUTOSEL 5.10 05/46] drm/panfrost: Handle HW_ISSUE_TTRX_2968_TTRX_3162
Hi! > From: Alyssa Rosenzweig > > [ Upstream commit 382435709516c1a7dc3843872792abf95e786c83 ] > > Add handling for the HW_ISSUE_TTRX_2968_TTRX_3162 quirk. Logic ported > from kbase. kbase lists this workaround as used on Mali-G57. AFAICT this quirk is not used anywhere in 5.10, and its users are not queued. We don't need this in 5.10. 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 AUTOSEL 5.10 01/46] drm/r128: Fix undefined behavior due to shift overflowing the constant
Hi! In this series, I only received patches up-to 42/46. Is problem at sender or receiver? Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH v6 12/13] leds: flash: mt6370: Add MediaTek MT6370 flashlight support
Hi! > From: Alice Chen > > The MediaTek MT6370 is a highly-integrated smart power management IC, > which includes a single cell Li-Ion/Li-Polymer switching battery > charger, a USB Type-C & Power Delivery (PD) controller, dual Flash > LED current sources, a RGB LED driver, a backlight WLED driver, > a display bias driver and a general LDO for portable devices. > > The Flash LED in MT6370 has 2 channels and support torch/strobe mode. > Add the support of MT6370 FLASH LED. > > Signed-off-by: Alice Chen > +config LEDS_MT6370_FLASHLIGHT > + tristate "Flash LED Support for MediaTek MT6370 PMIC" > + depends on LEDS_CLASS I'd name it just LEDS_MT6370_FLASH. > +// SPDX-License-Identifier: GPL-2.0-only > +/* > + * Copyright (C) 2022 Richtek Technology Corp. > + * > + * Author: Alice Chen " at end of line. The series is quite big, would it be possible to submit LED changes in separate series? Thanks, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH v6 11/13] leds: rgb: mt6370: Add MediaTek MT6370 current sink type LED Indicator support
Hi!
> From: ChiYuan Huang
>
> The MediaTek MT6370 is a highly-integrated smart power management IC,
> which includes a single cell Li-Ion/Li-Polymer switching battery
> charger, a USB Type-C & Power Delivery (PD) controller, dual
> Flash LED current sources, a RGB LED driver, a backlight WLED driver,
> a display bias driver and a general LDO for portable devices.
>
> In MediaTek MT6370, there are four channel current-sink RGB LEDs that
> support hardware pattern for constant current, PWM, and breath mode.
> Isink4 channel can also be used as a CHG_VIN power good indicator.
>
> +config LEDS_MT6370_RGB
> + tristate "LED Support for MediaTek MT6370 PMIC"
> + depends on MFD_MT6370
> + select LINEAR_RANGE
> + help
> + Say Y here to enable support for MT6370_RGB LED device.
> + In MT6370, there are four channel current-sink LED drivers that
> + support hardware pattern for constant current, PWM, and breath mode.
> + Isink4 channel can also be used as a CHG_VIN power good indicator.
That does not really belong here.
> +struct mt6370_priv {
> + /* Per LED access lock */
> + struct mutex lock;
Do we really need per-led locking?
> +static int mt6370_gen_breath_pattern(struct mt6370_priv *priv,
> + struct led_pattern *pattern, u32 len,
> + u8 *pattern_val, u32 val_len)
> +{
> + enum mt6370_led_ranges sel_range;
> + struct led_pattern *curr;
> + unsigned int sel;
> + u8 val[P_MAX_PATTERNS / 2] = {};
> + int i;
> +
> + if (len < P_MAX_PATTERNS && val_len < P_MAX_PATTERNS / 2)
> + return -EINVAL;
> +
> + /*
> + * Pattern list
> + * tr1: byte 0, b'[7: 4]
> + * tr2: byte 0, b'[3: 0]
> + * tf1: byte 1, b'[7: 4]
> + * tf2: byte 1, b'[3: 0]
> + * ton: byte 2, b'[7: 4]
> + * toff: byte 2, b'[3: 0]
> + */
> + for (i = 0; i < P_MAX_PATTERNS; i++) {
> + curr = pattern + i;
> +
> + sel_range = i == P_LED_TOFF ? R_LED_TOFF : R_LED_TRFON;
> +
> + linear_range_get_selector_within(priv->ranges + sel_range,
> + curr->delta_t, );
> +
> + val[i / 2] |= sel << (4 * ((i + 1) % 2));
> + }
> +
> + memcpy(pattern_val, val, 3);
> +
> + return 0;
> +}
I wonder how this works... you are not creating private sysfs
interface, are you?
> +static int mt6370_init_led_properties(struct mt6370_led *led,
> + struct led_init_data *init_data)
> +{
> + struct mt6370_priv *priv = led->priv;
> + struct device *dev = priv->dev;
> + struct led_classdev *lcdev;
> + struct fwnode_handle *child;
> + enum mt6370_led_ranges sel_range;
> + u32 max_uA, max_level;
> + const char * const states[] = { "off", "keep", "on" };
We'd really preffer not to add "keep" / "on" support unless you need
it.
> + if (ret)
> + return dev_err_probe(dev, ret,
> + "led %d, no color
> specified\n",
> + led->index);
led->LED.
> + if (num_color < 2)
> + return dev_err_probe(dev, -EINVAL,
> + "Multicolor must include
> 2 or more led channel\n");
"LED channels".
> +static int mt6370_isnk_init_default_state(struct mt6370_led *led)
> +{
> + struct mt6370_priv *priv = led->priv;
> + unsigned int enable, level;
> + int ret;
> +
> + ret = mt6370_get_led_brightness(priv, led->index, );
> + if (ret)
> + return ret;
> +
> + ret = regmap_field_read(priv->fields[F_RGB_EN], );
> + if (ret)
> + return ret;
> +
> + if (!(enable & MT6370_CHEN_BIT(led->index)))
> + level = LED_OFF;
Just use 0 instead of LED_OFF.
Best regards,
Pavel
--
People of Russia, stop Putin before his war on Ukraine escalates.
signature.asc
Description: PGP signature
Re: [PATCH v6 04/13] dt-bindings: leds: Add MediaTek MT6370 flashlight
On Fri 2022-07-22 18:23:58, ChiaEn Wu wrote: > From: Alice Chen > > Add MediaTek MT6370 flashlight binding documentation. > > Signed-off-by: Alice Chen > Reviewed-by: Krzysztof Kozlowski You'll need to get sign-offs right... And review from dt people before this can be applied. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH 6/6] i2c: Make remove callback return void
Hi! > From: Uwe Kleine-König > > The value returned by an i2c driver's remove function is mostly ignored. > (Only an error message is printed if the value is non-zero that the > error is ignored.) > > So change the prototype of the remove function to return no value. This > way driver authors are not tempted to assume that passing an error to > the upper layer is a good idea. All drivers are adapted accordingly. > There is no intended change of behaviour, all callbacks were prepared to > return 0 before. > > Signed-off-by: Uwe Kleine-König 2-4: Acked-by: Pavel Machek Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH 6/6] i2c: Make remove callback return void
Hi! > From: Uwe Kleine-König > > The value returned by an i2c driver's remove function is mostly ignored. > (Only an error message is printed if the value is non-zero that the > error is ignored.) > > So change the prototype of the remove function to return no value. This > way driver authors are not tempted to assume that passing an error to > the upper layer is a good idea. All drivers are adapted accordingly. > There is no intended change of behaviour, all callbacks were prepared to > return 0 before. > > Signed-off-by: Uwe Kleine-König 2-4: Acked-by: Pavel Machek Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [Openipmi-developer] [PATCH 6/6] i2c: Make remove callback return void
Hi! > From: Uwe Kleine-König > > The value returned by an i2c driver's remove function is mostly ignored. > (Only an error message is printed if the value is non-zero that the > error is ignored.) > > So change the prototype of the remove function to return no value. This > way driver authors are not tempted to assume that passing an error to > the upper layer is a good idea. All drivers are adapted accordingly. > There is no intended change of behaviour, all callbacks were prepared to > return 0 before. > > Signed-off-by: Uwe Kleine-König 2-4: Acked-by: Pavel Machek Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature ___ Openipmi-developer mailing list Openipmi-developer@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openipmi-developer
Re: [PATCH 6/8] dt-bindings: backlight: Update Lee Jones' email address
Hi! > Going forward, I'll be using my kernel.org for upstream work. > Acked-by: Pavel Machek Let me know if you want to take it through the LED tree. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH v5 11/13] leds: mt6370: Add MediaTek MT6370 current sink type LED Indicator support
Hi! > The MediaTek MT6370 is a highly-integrated smart power management IC, > which includes a single cell Li-Ion/Li-Polymer switching battery > charger, a USB Type-C & Power Delivery (PD) controller, dual > Flash LED current sources, a RGB LED driver, a backlight WLED driver, > a display bias driver and a general LDO for portable devices. > > In MediaTek MT6370, there are four channel current-sink RGB LEDs that > support hardware pattern for constant current, PWM, and breath mode. > Isink4 channel can also be used as a CHG_VIN power good indicator. > > Signed-off-by: ChiYuan Huang > index a49979f..71bacb5 100644 > --- a/drivers/leds/Kconfig > +++ b/drivers/leds/Kconfig > @@ -244,6 +244,20 @@ config LEDS_MT6323 > This option enables support for on-chip LED drivers found on > Mediatek MT6323 PMIC. > > +config LEDS_MT6370_RGB > + tristate "LED Support for MediaTek MT6370 PMIC" > + depends on LEDS_CLASS > + depends on MFD_MT6370 > + select LINEAR_RANGE > + help > + Say Y here to enable support for MT6370_RGB LED device. > + In MT6370, there are four channel current-sink LED drivers that > + support hardware pattern for constant current, PWM, and breath mode. > + Isink4 channel can also be used as a CHG_VIN power good Should this go to leds/rgb directory, and should it depend on multicolor framework? Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH AUTOSEL 4.9 08/13] video: fbdev: simplefb: Check before clk_put() not needed
Hi! > [ Upstream commit 5491424d17bdeb7b7852a59367858251783f8398 ] > > clk_put() already checks the clk ptr using !clk and IS_ERR() > so there is no need to check it again before calling it. Nice cleanup, but not a bugfix; we don't need it in -stable. 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 AUTOSEL 4.9 05/13] video: fbdev: skeletonfb: Fix syntax errors in comments
Hi!
> From: Xiang wangx
>
> [ Upstream commit fc378794a2f7a19cf26010dc33b89ba608d4c70f ]
>
> Delete the redundant word 'its'.
Calling typo in comment "syntax error" is ... interesting. Anyway, we
don't need this in stable.
Best regards,
Pavel
> +++ b/drivers/video/fbdev/skeletonfb.c
> @@ -96,7 +96,7 @@ static struct fb_fix_screeninfo xxxfb_fix = {
>
> /*
> * Modern graphical hardware not only supports pipelines but some
> - * also support multiple monitors where each display can have its
> + * also support multiple monitors where each display can have
> * its own unique data. In this case each display could be
> * represented by a separate framebuffer device thus a separate
> * struct fb_info. Now the struct xxx_par represents the graphics
> --
> 2.35.1
--
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 AUTOSEL 4.19 04/22] drm/vc4: crtc: Use an union to store the page flip callback
Hi! > From: Maxime Ripard > > [ Upstream commit 2523e9dcc3be91bf9fdc0d1e542557ca00bbef42 ] > > We'll need to extend the vc4_async_flip_state structure to rely on > another callback implementation, so let's move the current one into a > union. This and [04/22] drm/vc4: crtc: Use an union to store the page flip callback is queued for 4.9 / 4.19, preparing for change that is not queued into 4.19. Please drop at least from 4.9 and 4.19. 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
