* [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document
From: Pavel Machek @ 2024-04-08 10:51 UTC (permalink / raw)
To: phone-devel, kernel list, fiona.klute, martijn, samuel,
heikki.krogerus, gregkh, linux-usb, robh+dt,
krzysztof.kozlowski+dt, devicetree, megi
[-- Attachment #1: Type: text/plain, Size: 4013 bytes --]
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 <pavel@ucw.cz>
---
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 000000000000..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 <pavel@ucw.cz>
+
+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 <dt-bindings/interrupt-controller/irq.h>
+ #include <dt-bindings/gpio/gpio.h>
+
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ typec@2c {
+ compatible = "analogix,anx7688";
+ reg = <0x2c>;
+ interrupts = <8 IRQ_TYPE_EDGE_FALLING>;
+ interrupt-parent = <&gpio0>;
+
+ enable-gpios = <&pio 3 10 GPIO_ACTIVE_LOW>; /* PD10 */
+ reset-gpios = <&pio 3 6 GPIO_ACTIVE_HIGH>; /* PD6 */
+ cabledet-gpios = <&r_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 = <&usb_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 = <&usbotg_hs_ep>;
+ };
+ };
+ };
+ };
+ };
+ };
+...
--
People of Russia, stop Putin before his war on Ukraine escalates.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply related
* [PATCH] dt-bindings: pinctrl: mediatek: mt7622: add "gpio-ranges" property
From: Rafał Miłecki @ 2024-04-08 10:51 UTC (permalink / raw)
To: Sean Wang, Linus Walleij, Rob Herring, Krzysztof Kozlowski,
Conor Dooley
Cc: Matthias Brugger, AngeloGioacchino Del Regno, Hsin-Yi Wang,
linux-mediatek, linux-gpio, devicetree, linux-arm-kernel,
Rafał Miłecki
From: Rafał Miłecki <rafal@milecki.pl>
Allow specifying pin to GPIO mapping. It can be find in in-Linux DTS
file for MT7622.
Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
---
.../devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml b/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
index eaa65512161a..d211f2e403f2 100644
--- a/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/mediatek,mt7622-pinctrl.yaml
@@ -36,6 +36,9 @@ properties:
the amount of cells must be specified as 2. See the below mentioned gpio
binding representation for description of particular cells.
+ gpio-ranges:
+ maxItems: 1
+
interrupt-controller: true
interrupts:
--
2.35.3
^ permalink raw reply related
* Re: [PATCH 0/5] Add parsing for Zimop ISA extension
From: Andrew Jones @ 2024-04-08 11:03 UTC (permalink / raw)
To: Clément Léger
Cc: Deepak Gupta, Jonathan Corbet, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
Anup Patel, Shuah Khan, Atish Patra, linux-doc, linux-riscv,
linux-kernel, devicetree, kvm, kvm-riscv, linux-kselftest
In-Reply-To: <ddc5555a-3ae8-42e5-a08a-ca5ceaf0bf28@rivosinc.com>
On Mon, Apr 08, 2024 at 10:01:12AM +0200, Clément Léger wrote:
>
>
> On 05/04/2024 19:33, Deepak Gupta wrote:
> > On Fri, Apr 5, 2024 at 8:26 AM Andrew Jones <ajones@ventanamicro.com> wrote:
> >>
> >> On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote:
> >>> The Zimop ISA extension was ratified recently. This series adds support
> >>> for parsing it from riscv,isa, hwprobe export and kvm support for
> >>> Guest/VM.
> >>
> >> I'm not sure we need this. Zimop by itself isn't useful, so I don't know
> >> if we need to advertise it at all. When an extension comes along that
> >> redefines some MOPs, then we'll advertise that extension, but the fact
> >> Zimop is used for that extension is really just an implementation detail.
> >
> > Only situation I see this can be useful is this:--
> >
> > An implementer, implemented Zimops in CPU solely for the purpose that they can
> > run mainline distro & packages on their hardware and don't want to leverage any
> > feature which are built on top of Zimop.
>
> Yes, the rationale was that some binaries using extensions that overload
> MOPs could still be run. With Zimop exposed, the loader could determine
> if the binary can be executed without potentially crashing. We could
> also let the program run anyway but the execution could potentially
> crash unexpectedly, which IMHO is not really good for the user
> experience nor for debugging. I already think that the segfaults which
> happens when executing binaries that need some missing extension are not
> so easy to debug, so better add more guards.
OK. It's only one more extension out of dozens, so I won't complain more,
but I was thinking that binaries that use particular extensions would
check for those particular extensions (step 2), rather than Zimop.
Thanks,
drew
>
> >
> > As an example zicfilp and zicfiss are dependent on zimops. glibc can
> > do following
> >
> > 1) check elf header if binary was compiled with zicfiss and zicfilp,
> > if yes goto step 2, else goto step 6.
> > 2) check if zicfiss/zicfilp is available in hw via hwprobe, if yes
> > goto step 5. else goto step 3
> > 3) check if zimop is available via hwprobe, if yes goto step 6, else goto step 4
>
> I think you meant step 5 rather than step 6.
>
> Clément
>
> > 4) This binary won't be able to run successfully on this platform,
> > issue exit syscall. <-- termination
> > 5) issue prctl to enable shadow stack and landing pad for current task
> > <-- enable feature
> > 6) let the binary run <-- let the binary run because no harm can be done
^ permalink raw reply
* Re: [PATCH 0/5] Add parsing for Zimop ISA extension
From: Andrew Jones @ 2024-04-08 11:04 UTC (permalink / raw)
To: Clément Léger
Cc: Jonathan Corbet, Paul Walmsley, Palmer Dabbelt, Albert Ou,
Conor Dooley, Rob Herring, Krzysztof Kozlowski, Anup Patel,
Shuah Khan, Atish Patra, linux-doc, linux-riscv, linux-kernel,
devicetree, kvm, kvm-riscv, linux-kselftest
In-Reply-To: <20240404103254.1752834-1-cleger@rivosinc.com>
On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote:
> The Zimop ISA extension was ratified recently. This series adds support
> for parsing it from riscv,isa, hwprobe export and kvm support for
> Guest/VM.
>
> Clément Léger (5):
> dt-bindings: riscv: add Zimop ISA extension description
> riscv: add ISA extension parsing for Zimop
> riscv: hwprobe: export Zimop ISA extension
> RISC-V: KVM: Allow Zimop extension for Guest/VM
> KVM: riscv: selftests: Add Zimop extension to get-reg-list test
>
> Documentation/arch/riscv/hwprobe.rst | 4 ++++
> Documentation/devicetree/bindings/riscv/extensions.yaml | 5 +++++
> arch/riscv/include/asm/hwcap.h | 1 +
> arch/riscv/include/uapi/asm/hwprobe.h | 1 +
> arch/riscv/include/uapi/asm/kvm.h | 1 +
> arch/riscv/kernel/cpufeature.c | 1 +
> arch/riscv/kernel/sys_hwprobe.c | 1 +
> arch/riscv/kvm/vcpu_onereg.c | 2 ++
> tools/testing/selftests/kvm/riscv/get-reg-list.c | 4 ++++
> 9 files changed, 20 insertions(+)
>
> --
> 2.43.0
For the series,
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
^ permalink raw reply
* Re: [PATCH v3 1/2] dt-bindings: mailbox: arm,mhuv3: Add bindings
From: Cristian Marussi @ 2024-04-08 11:09 UTC (permalink / raw)
To: Jassi Brar
Cc: linux-kernel, linux-arm-kernel, devicetree, sudeep.holla, robh+dt,
krzysztof.kozlowski+dt, conor+dt
In-Reply-To: <CABb+yY07OQQ8gAOZM2iV7nh_P5sQPWwNPODfwnJNZoPk8MNg2w@mail.gmail.com>
On Sun, Apr 07, 2024 at 06:38:52PM -0500, Jassi Brar wrote:
> On Thu, Apr 4, 2024 at 1:25 AM Cristian Marussi
> <cristian.marussi@arm.com> wrote:
> >
> > Add bindings for the ARM MHUv3 Mailbox controller.
> >
Hi,
> > Signed-off-by: Cristian Marussi <cristian.marussi@arm.com>
> > ---
> > v2 -> v3
> > - fixed spurious tabs in dt_binding_check
> > v1 -> v2
> > - clarified extension descriptions around configurability and discoverability
> > - removed unused labels from the example
> > - using pattern properties to define interrupt-names
> > - bumped interrupt maxItems to 74 (allowing uo to 8 channels per extension)
> > ---
> > .../bindings/mailbox/arm,mhuv3.yaml | 217 ++++++++++++++++++
> > 1 file changed, 217 insertions(+)
> > create mode 100644 Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml b/Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml
> > new file mode 100644
> > index 000000000000..32a8bb711464
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/mailbox/arm,mhuv3.yaml
> > @@ -0,0 +1,217 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/mailbox/arm,mhuv3.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: ARM MHUv3 Mailbox Controller
> > +
> > +maintainers:
> > + - Sudeep Holla <sudeep.holla@arm.com>
> > + - Cristian Marussi <cristian.marussi@arm.com>
> > +
> > +description: |
> > + The Arm Message Handling Unit (MHU) Version 3 is a mailbox controller that
> > + enables unidirectional communications with remote processors through various
> > + possible transport protocols.
> > + The controller can optionally support a varying number of extensions that, in
> > + turn, enable different kinds of transport to be used for communication.
> > + Number, type and characteristics of each supported extension can be discovered
> > + dynamically at runtime.
> > +
> > + Given the unidirectional nature of the controller, an MHUv3 mailbox controller
> > + is composed of a MHU Sender (MHUS) containing a PostBox (PBX) block and a MHU
> > + Receiver (MHUR) containing a MailBox (MBX) block, where
> > +
> > + PBX is used to
> > + - Configure the MHU
> > + - Send Transfers to the Receiver
> > + - Optionally receive acknowledgment of a Transfer from the Receiver
> > +
> > + MBX is used to
> > + - Configure the MHU
> > + - Receive Transfers from the Sender
> > + - Optionally acknowledge Transfers sent by the Sender
> > +
> > + Both PBX and MBX need to be present and defined in the DT description if you
> > + need to establish a bidirectional communication, since you will have to
> > + acquire two distinct unidirectional channels, one for each block.
> > +
> > + As a consequence both blocks needs to be represented separately and specified
> > + as distinct DT nodes in order to properly describe their resources.
> > +
> > + Note that, though, thanks to the runtime discoverability, there is no need to
> > + identify the type of blocks with distinct compatibles.
> > +
> > + Following are the MHUv3 possible extensions.
> > +
> > + - Doorbell Extension (DBE): DBE defines a type of channel called a Doorbell
> > + Channel (DBCH). DBCH enables a single bit Transfer to be sent from the
> > + Sender to Receiver. The Transfer indicates that an event has occurred.
> > + When DBE is implemented, the number of DBCHs that an implementation of the
> > + MHU can support is between 1 and 128, numbered starting from 0 in ascending
> > + order and discoverable at run-time.
> > + Each DBCH contains 32 individual fields, referred to as flags, each of which
> > + can be used independently. It is possible for the Sender to send multiple
> > + Transfers at once using a single DBCH, so long as each Transfer uses
> > + a different flag in the DBCH.
> > + Optionally, data may be transmitted through an out-of-band shared memory
> > + region, wherein the MHU Doorbell is used strictly as an interrupt generation
> > + mechanism, but this is out of the scope of these bindings.
> > +
> > + - FastChannel Extension (FCE): FCE defines a type of channel called a Fast
> > + Channel (FCH). FCH is intended for lower overhead communication between
> > + Sender and Receiver at the expense of determinism. An FCH allows the Sender
> > + to update the channel value at any time, regardless of whether the previous
> > + value has been seen by the Receiver. When the Receiver reads the channel's
> > + content it gets the last value written to the channel.
> > + FCH is considered lossy in nature, and means that the Sender has no way of
> > + knowing if, or when, the Receiver will act on the Transfer.
> > + FCHs are expected to behave as RAM which generates interrupts when writes
> > + occur to the locations within the RAM.
> > + When FCE is implemented, the number of FCHs that an implementation of the
> > + MHU can support is between 1-1024, if the FastChannel word-size is 32-bits,
> > + or between 1-512, when the FastChannel word-size is 64-bits.
> > + FCHs are numbered from 0 in ascending order.
> > + Note that the number of FCHs and the word-size are implementation defined,
> > + not configurable but discoverable at run-time.
> > + Optionally, data may be transmitted through an out-of-band shared memory
> > + region, wherein the MHU FastChannel is used as an interrupt generation
> > + mechanism which carries also a pointer to such out-of-band data, but this
> > + is out of the scope of these bindings.
> > +
> > + - FIFO Extension (FE): FE defines a Channel type called a FIFO Channel (FFCH).
> > + FFCH allows a Sender to send
> > + - Multiple Transfers to the Receiver without having to wait for the
> > + previous Transfer to be acknowledged by the Receiver, as long as the
> > + FIFO has room for the Transfer.
> > + - Transfers which require the Receiver to provide acknowledgment.
> > + - Transfers which have in-band payload.
> > + In all cases, the data is guaranteed to be observed by the Receiver in the
> > + same order which the Sender sent it.
> > + When FE is implemented, the number of FFCHs that an implementation of the
> > + MHU can support is between 1 and 64, numbered starting from 0 in ascending
> > + order. The number of FFCHs, their depth (same for all implemented FFCHs) and
> > + the access-granularity are implementation defined, not configurable but
> > + discoverable at run-time.
> > + Optionally, additional data may be transmitted through an out-of-band shared
> > + memory region, wherein the MHU FIFO is used to transmit, in order, a small
> > + part of the payload (like a header) and a reference to the shared memory
> > + area holding the remaining, bigger, chunk of the payload, but this is out of
> > + the scope of these bindings.
> > +
> > +properties:
> > + compatible:
> > + const: arm,mhuv3
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + minItems: 1
> > + maxItems: 74
> > +
> > + interrupt-names:
> > + description: |
> > + The MHUv3 controller generates a number of events some of which are used
> > + to generate interrupts; as a consequence it can expose a varying number of
> > + optional PBX/MBX interrupts, representing the events generated during the
> > + operation of the various transport protocols associated with different
> > + extensions. All interrupts of the MHU are level-sensitive.
> > + Some of these optional interrupts are defined per-channel, where the
> > + number of channels effectively available is implementation defined and
> > + run-time discoverable.
> > + In the following names are enumerated using patterns, with per-channel
> > + interrupts implicitly capped at the maximum channels allowed by the
> > + specification for each extension type.
> > + For the sake of simplicity maxItems is anyway capped to a most plausible
> > + number, assuming way less channels would be implemented than actually
> > + possible.
> > +
> > + The only mandatory interrupts on the MHU are:
> > + - combined
> > + - mbx-fch-xfer-<N> but only if mbx-fcgrp-xfer-<N> is not implemented.
> > +
> > + minItems: 1
> > + maxItems: 74
> > + items:
> > + oneOf:
> > + - const: combined
> > + description: PBX/MBX Combined interrupt
> > + - const: combined-ffch
> > + description: PBX/MBX FIFO Combined interrupt
> > + - pattern: '^ffch-low-tide-[0-9]+$'
> > + description: PBX/MBX FIFO Channel <N> Low Tide interrupt
> > + - pattern: '^ffch-high-tide-[0-9]+$'
> > + description: PBX/MBX FIFO Channel <N> High Tide interrupt
> > + - pattern: '^ffch-flush-[0-9]+$'
> > + description: PBX/MBX FIFO Channel <N> Flush interrupt
> > + - pattern: '^mbx-dbch-xfer-[0-9]+$'
> > + description: MBX Doorbell Channel <N> Transfer interrupt
> > + - pattern: '^mbx-fch-xfer-[0-9]+$'
> > + description: MBX FastChannel <N> Transfer interrupt
> > + - pattern: '^mbx-fchgrp-xfer-[0-9]+$'
> > + description: MBX FastChannel <N> Group Transfer interrupt
> > + - pattern: '^mbx-ffch-xfer-[0-9]+$'
> > + description: MBX FIFO Channel <N> Transfer interrupt
> > + - pattern: '^pbx-dbch-xfer-ack-[0-9]+$'
> > + description: PBX Doorbell Channel <N> Transfer Ack interrupt
> > + - pattern: '^pbx-ffch-xfer-ack-[0-9]+$'
> > + description: PBX FIFO Channel <N> Transfer Ack interrupt
> > +
> Can we have optional subnodes (with different properties as required)
> for each extension type ?
>
Not sure if I have understood properly you request, but the type of extensions
present in a PBX/MBX block can be discovered at runtime together with their
characteristics (like the number of channels) so the specific DT properties
can be searched (or NOT) based on the features discovered at run-time:
are you asking for subnodes as a means of adding clarity to what can be
defined in a block depending on what it is (PBX vs MBX) and what
extensions it has ?
if that is the reason ... the convoluted names like pbx/mbx-dbch- etc...
was my attempt at thhat :D ... to give a hint at what you can define in a PBX
vs MBX block and what is related to each extensions...
Note that, though, since the PBX/MBX blocks are discoverable at runtime as such,
they are not identified as such in the DT (same compatible) so I would not have
anywayy the capability to check in the DT which is which (PBX/MBX) and if the
properties are appropriate or not, nor I could know which extensions are really
implemented, so such subnodes would ony be a way of grouping props in the DT
without adding any compile time check capability nor adding any improvement to
the runtime DT parsing proces...
...BUT, of course, I could be wrong and missing a something here, so I
am happy to corrected on the subnodes utility...
>
> > + '#mbox-cells':
> > + description: |
> > + The first argument in the consumers 'mboxes' property represents the
> > + extension type, the second is for the channel number while the third
> > + depends on extension type.
> > +
> > + Extension type for DBE is 0 and the third parameter represents the
> > + doorbell flag number to use.
> > + Extension type for FCE is 1, third parameter unused.
> > + Extension type for FE is 2, third parameter unused.
> > +
> > + mboxes = <&mhu 0 0 5>; // DBE, Doorbell Channel Window 0, doorbell flag 5.
> > + mboxes = <&mhu 0 1 7>; // DBE, Doorbell Channel Window 1, doorbell flag 7.
> > + mboxes = <&mhu 1 0 0>; // FCE, FastChannel Window 0.
> > + mboxes = <&mhu 1 3 0>; // FCE, FastChannel Window 3.
> > + mboxes = <&mhu 2 1 0>; // FE, FIFO Channel Window 1.
> > + mboxes = <&mhu 2 7 0>; // FE, FIFO Channel Window 7.
> >
> Please define the extension types, instead of 0, 1 and 2.
>
I'll do.
Thanks for the review.
Cristian
^ permalink raw reply
* Re: [PATCH v5 1/2] media: dt-bindings: nxp,imx8-jpeg:: Add clocks entries
From: Fabio Estevam @ 2024-04-08 11:14 UTC (permalink / raw)
To: Mirela Rabulea
Cc: shawnguo, robh+dt, krzysztof.kozlowski+dt, festevam,
alexander.stein, Frank.li, ming.qian, conor+dt, devicetree,
linux-arm-kernel, s.hauer, kernel, mchehab, hverkuil, linux-media,
imx, linux-kernel
In-Reply-To: <20240408075806.1194297-1-mirela.rabulea@nxp.com>
Hi Mirela,
On Mon, Apr 8, 2024 at 4:58 AM Mirela Rabulea <mirela.rabulea@nxp.com> wrote:
>
> From: Fabio Estevam <festevam@gmail.com>
>
> The JPEG decoder/encoder present in iMX8QXP and iMX8QM SoCs need
> the PER and IPG clocks to be functional, so add the clock entries.
>
> This also fixes the following schema warning:
>
> imx8qm-apalis-eval.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
> from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
>
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
Thanks for submitting this series.
In the Subject, you added an extra : "..nxp,imx8-jpeg:: Add". Please
remove the extra :.
My From and Signed-off-by tags e-mails do not match.
Please use the email from the Signed-off-by in the From field as well.
The same applies to patch 2/2.
Thanks
^ permalink raw reply
* Re: [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document
From: Krzysztof Kozlowski @ 2024-04-08 11:17 UTC (permalink / raw)
To: Pavel Machek, phone-devel, kernel list, fiona.klute, martijn,
samuel, heikki.krogerus, gregkh, linux-usb, robh+dt,
krzysztof.kozlowski+dt, devicetree, megi
In-Reply-To: <ZhPMHdt6r/4D99Zg@duo.ucw.cz>
On 08/04/2024 12:51, Pavel Machek wrote:
> 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 <pavel@ucw.cz>
...
> + 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.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH 0/5] Add parsing for Zimop ISA extension
From: Clément Léger @ 2024-04-08 11:19 UTC (permalink / raw)
To: Andrew Jones
Cc: Deepak Gupta, Jonathan Corbet, Paul Walmsley, Palmer Dabbelt,
Albert Ou, Conor Dooley, Rob Herring, Krzysztof Kozlowski,
Anup Patel, Shuah Khan, Atish Patra, linux-doc, linux-riscv,
linux-kernel, devicetree, kvm, kvm-riscv, linux-kselftest
In-Reply-To: <20240408-6c93f3f50b55234f3825ca33@orel>
On 08/04/2024 13:03, Andrew Jones wrote:
> On Mon, Apr 08, 2024 at 10:01:12AM +0200, Clément Léger wrote:
>>
>>
>> On 05/04/2024 19:33, Deepak Gupta wrote:
>>> On Fri, Apr 5, 2024 at 8:26 AM Andrew Jones <ajones@ventanamicro.com> wrote:
>>>>
>>>> On Thu, Apr 04, 2024 at 12:32:46PM +0200, Clément Léger wrote:
>>>>> The Zimop ISA extension was ratified recently. This series adds support
>>>>> for parsing it from riscv,isa, hwprobe export and kvm support for
>>>>> Guest/VM.
>>>>
>>>> I'm not sure we need this. Zimop by itself isn't useful, so I don't know
>>>> if we need to advertise it at all. When an extension comes along that
>>>> redefines some MOPs, then we'll advertise that extension, but the fact
>>>> Zimop is used for that extension is really just an implementation detail.
>>>
>>> Only situation I see this can be useful is this:--
>>>
>>> An implementer, implemented Zimops in CPU solely for the purpose that they can
>>> run mainline distro & packages on their hardware and don't want to leverage any
>>> feature which are built on top of Zimop.
>>
>> Yes, the rationale was that some binaries using extensions that overload
>> MOPs could still be run. With Zimop exposed, the loader could determine
>> if the binary can be executed without potentially crashing. We could
>> also let the program run anyway but the execution could potentially
>> crash unexpectedly, which IMHO is not really good for the user
>> experience nor for debugging. I already think that the segfaults which
>> happens when executing binaries that need some missing extension are not
>> so easy to debug, so better add more guards.
>
> OK. It's only one more extension out of dozens, so I won't complain more,
No worries, your point *is* valid since I'm not sure yet that the loader
will actually do that one day.
BTW, are you aware of any effort to make the elf dynamic loader
"smarter" and actually check for needed extensions to be present rather
than blindly running the elf and potentially catching SIGILL ?
Thanks,
Clément
> but I was thinking that binaries that use particular extensions would
> check for those particular extensions (step 2), rather than Zimop.
>
> Thanks,
> drew
>
>>
>>>
>>> As an example zicfilp and zicfiss are dependent on zimops. glibc can
>>> do following
>>>
>>> 1) check elf header if binary was compiled with zicfiss and zicfilp,
>>> if yes goto step 2, else goto step 6.
>>> 2) check if zicfiss/zicfilp is available in hw via hwprobe, if yes
>>> goto step 5. else goto step 3
>>> 3) check if zimop is available via hwprobe, if yes goto step 6, else goto step 4
>>
>> I think you meant step 5 rather than step 6.
>>
>> Clément
>>
>>> 4) This binary won't be able to run successfully on this platform,
>>> issue exit syscall. <-- termination
>>> 5) issue prctl to enable shadow stack and landing pad for current task
>>> <-- enable feature
>>> 6) let the binary run <-- let the binary run because no harm can be done
^ permalink raw reply
* Re: [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document
From: Pavel Machek @ 2024-04-08 11:21 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: phone-devel, kernel list, fiona.klute, martijn, samuel,
heikki.krogerus, gregkh, linux-usb, robh+dt,
krzysztof.kozlowski+dt, devicetree, megi
In-Reply-To: <ab9affc8-de68-4ec9-bdfc-02131191bc3a@linaro.org>
[-- Attachment #1: Type: text/plain, Size: 1199 bytes --]
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 <pavel@ucw.cz>
>
> ...
>
> > + 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.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]
^ permalink raw reply
* Re: [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document
From: Krzysztof Kozlowski @ 2024-04-08 11:24 UTC (permalink / raw)
To: Pavel Machek
Cc: phone-devel, kernel list, fiona.klute, martijn, samuel,
heikki.krogerus, gregkh, linux-usb, robh+dt,
krzysztof.kozlowski+dt, devicetree, megi
In-Reply-To: <ZhPTTxI4oTF3pgrk@duo.ucw.cz>
On 08/04/2024 13:21, Pavel Machek wrote:
> 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 <pavel@ucw.cz>
>>
>> ...
>>
>>> + 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?
The schematics you linked in the document at the beginning. Page 13. Do
you see these pins there? I saw only VCONN1_EN, but that's not a supply.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] dt-bindings: pinctrl: mediatek: mt7622: add "gpio-ranges" property
From: Krzysztof Kozlowski @ 2024-04-08 11:25 UTC (permalink / raw)
To: Rafał Miłecki, Sean Wang, Linus Walleij, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Matthias Brugger, AngeloGioacchino Del Regno, Hsin-Yi Wang,
linux-mediatek, linux-gpio, devicetree, linux-arm-kernel,
Rafał Miłecki
In-Reply-To: <20240408105128.30586-1-zajec5@gmail.com>
On 08/04/2024 12:51, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@milecki.pl>
>
> Allow specifying pin to GPIO mapping. It can be find in in-Linux DTS
> file for MT7622.
>
> Signed-off-by: Rafał Miłecki <rafal@milecki.pl>
Acked-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] arm64: dts: imx8mp-msc-sm2s: Add i2c{1,6} sda-/scl-gpios
From: Fabio Estevam @ 2024-04-08 11:31 UTC (permalink / raw)
To: Ian Ray
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, devicetree, imx,
linux-arm-kernel, linux-kernel
In-Reply-To: <20240408092449.6-1-ian.ray@gehealthcare.com>
Hi Ian,
> + pinctrl_i2c1_gpio: i2c1gpiogrp {
> + fsl,pins =
> + <MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14 0x400001c3>,
The Sion bit is unnecessary in the GPIO mode so you could pass 0x1c3 instead.
^ permalink raw reply
* Re: [PATCH] arm64: dts: imx8mp-msc-sm2s: Add i2c{1,6} sda-/scl-gpios
From: Ian Ray @ 2024-04-08 11:44 UTC (permalink / raw)
To: Fabio Estevam
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, devicetree, imx,
linux-arm-kernel, linux-kernel
In-Reply-To: <CAOMZO5B-vTRSfi=tNc_iZxnxYstL8JJOd_1rMf4ps9WHyfx0GQ@mail.gmail.com>
On Mon, Apr 08, 2024 at 08:31:53AM -0300, Fabio Estevam wrote:
>
> Hi Ian,
>
> > + pinctrl_i2c1_gpio: i2c1gpiogrp {
> > + fsl,pins =
> > + <MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14 0x400001c3>,
>
> The Sion bit is unnecessary in the GPIO mode so you could pass 0x1c3 instead.
Thank you -- I will submit a V2.
Would 0x1c2 be more correct? From the IMX8MPRM.pdf, it seems that the
lowest bit is reserved. Example: 8.2.4.158 SW_PAD_CTL_PAD_GPIO1_IO05
SW PAD Control Register (IOMUXC_SW_PAD_CTL_PAD_GPIO1_IO05).
^ permalink raw reply
* Re: [PATCH] arm64: dts: imx8mp-msc-sm2s: Add i2c{1,6} sda-/scl-gpios
From: Fabio Estevam @ 2024-04-08 11:48 UTC (permalink / raw)
To: Ian Ray
Cc: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, devicetree, imx,
linux-arm-kernel, linux-kernel
In-Reply-To: <ZhPYkGLM_b5IEKs2@de2cfed78370>
On Mon, Apr 8, 2024 at 8:44 AM Ian Ray <ian.ray@gehealthcare.com> wrote:
> Thank you -- I will submit a V2.
>
> Would 0x1c2 be more correct? From the IMX8MPRM.pdf, it seems that the
> lowest bit is reserved. Example: 8.2.4.158 SW_PAD_CTL_PAD_GPIO1_IO05
> SW PAD Control Register (IOMUXC_SW_PAD_CTL_PAD_GPIO1_IO05).
Yes, good catch. It is better not to write to the reserved bit 0, so
0x1c2 is preferred.
^ permalink raw reply
* Re: [PATCH v3 2/2] media: i2c: Add GC05A2 image sensor driver
From: Zhi Mao (毛智) @ 2024-04-08 11:50 UTC (permalink / raw)
To: mchehab@kernel.org, sakari.ailus@linux.intel.com,
robh+dt@kernel.org, kieran.bingham@ideasonboard.com,
krzysztof.kozlowski+dt@linaro.org
Cc: heiko@sntech.de, gerald.loacker@wolfvision.net,
linux-kernel@vger.kernel.org, yunkec@chromium.org,
linux-mediatek@lists.infradead.org, dan.scally@ideasonboard.com,
linux-media@vger.kernel.org,
Shengnan Wang (王圣男), hdegoede@redhat.com,
linus.walleij@linaro.org, andy.shevchenko@gmail.com,
Yaya Chang (張雅清), bingbu.cao@intel.com,
jacopo.mondi@ideasonboard.com, jernej.skrabec@gmail.com,
devicetree@vger.kernel.org, conor+dt@kernel.org,
Project_Global_Chrome_Upstream_Group, 10572168@qq.com,
hverkuil-cisco@xs4all.nl, tomi.valkeinen@ideasonboard.com,
linux-arm-kernel@lists.infradead.org, matthias.bgg@gmail.com,
laurent.pinchart@ideasonboard.com,
angelogioacchino.delregno@collabora.com, macromorgan@hotmail.com
In-Reply-To: <171248091995.2374960.12981271990757968652@ping.linuxembedded.co.uk>
Hi Kieran,
Thanks for your review this patch.
It seems that there are some difficult for us(Mediatek) to explain
these register setting comments.
As these settings are released by GC sensor vendor, and we have not
detailed datasheet described them.
And even if send the letter to ask sensor vendor, I am afraid there may
be not a clear response.
Can we just focus on the driver code function and control flow part?
On Sun, 2024-04-07 at 10:08 +0100, Kieran Bingham wrote:
>
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
> Hello,
>
> Thanks for helping extending the kernels sensor driver support.
>
> My comments below can likely be taken with a pinch of salt, as they
> are
> mostly around the tabled register values ... but we have many drivers
> which are binary blobs of sensor register values and I think it would
> be
> far more beneficial to clean these up where possible...
>
> So the first question is ... Can we ?
>
>
>
> Quoting Zhi Mao (2024-04-03 04:38:25)
> > Add a V4L2 sub-device driver for Galaxycore GC05A2 image sensor.
> >
> > Signed-off-by: Zhi Mao <zhi.mao@mediatek.com>
> > ---
> > drivers/media/i2c/Kconfig | 10 +
> > drivers/media/i2c/Makefile | 1 +
> > drivers/media/i2c/gc05a2.c | 1383
> ++++++++++++++++++++++++++++++++++++
> > 3 files changed, 1394 insertions(+)
> > create mode 100644 drivers/media/i2c/gc05a2.c
> >
> > diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig
> > index 56f276b920ab..97993bf160f9 100644
> > --- a/drivers/media/i2c/Kconfig
> > +++ b/drivers/media/i2c/Kconfig
> > @@ -70,6 +70,16 @@ config VIDEO_GC0308
> > To compile this driver as a module, choose M here: the
> > module will be called gc0308.
> >
> > +config VIDEO_GC05A2
> > + tristate "GalaxyCore gc05a2 sensor support"
> > + select V4L2_CCI_I2C
> > + help
> > + This is a Video4Linux2 sensor driver for the GalaxyCore
> gc05a2
> > + camera.
> > +
> > + To compile this driver as a module, choose M here: the
> > + module will be called gc05a2.
> > +
> > config VIDEO_GC2145
> > select V4L2_CCI_I2C
> > tristate "GalaxyCore GC2145 sensor support"
> > diff --git a/drivers/media/i2c/Makefile
> b/drivers/media/i2c/Makefile
> > index dfbe6448b549..8ed6faf0f854 100644
> > --- a/drivers/media/i2c/Makefile
> > +++ b/drivers/media/i2c/Makefile
> > @@ -38,6 +38,7 @@ obj-$(CONFIG_VIDEO_DW9768) += dw9768.o
> > obj-$(CONFIG_VIDEO_DW9807_VCM) += dw9807-vcm.o
> > obj-$(CONFIG_VIDEO_ET8EK8) += et8ek8/
> > obj-$(CONFIG_VIDEO_GC0308) += gc0308.o
> > +obj-$(CONFIG_VIDEO_GC05A2) += gc05a2.o
> > obj-$(CONFIG_VIDEO_GC2145) += gc2145.o
> > obj-$(CONFIG_VIDEO_HI556) += hi556.o
> > obj-$(CONFIG_VIDEO_HI846) += hi846.o
> > diff --git a/drivers/media/i2c/gc05a2.c
> b/drivers/media/i2c/gc05a2.c
> > new file mode 100644
> > index 000000000000..461d33055a3b
> > --- /dev/null
> > +++ b/drivers/media/i2c/gc05a2.c
> > @@ -0,0 +1,1383 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +/*
> > + * Driver for GalaxyCore gc05a2 image sensor
> > + *
> > + * Copyright 2024 MediaTek
> > + *
> > + * Zhi Mao <zhi.mao@mediatek.com>
> > + */
> > +#include <linux/array_size.h>
> > +#include <linux/bits.h>
> > +#include <linux/clk.h>
> > +#include <linux/container_of.h>
> > +#include <linux/delay.h>
> > +#include <linux/device.h>
> > +#include <linux/err.h>
> > +#include <linux/gpio/consumer.h>
> > +#include <linux/math64.h>
> > +#include <linux/mod_devicetable.h>
> > +#include <linux/pm_runtime.h>
> > +#include <linux/property.h>
> > +#include <linux/regulator/consumer.h>
> > +#include <linux/types.h>
> > +#include <linux/units.h>
> > +
> > +#include <media/v4l2-cci.h>
> > +#include <media/v4l2-ctrls.h>
> > +#include <media/v4l2-event.h>
> > +#include <media/v4l2-fwnode.h>
> > +#include <media/v4l2-subdev.h>
> > +
> > +#define GC05A2_REG_TEST_PATTERN_EN CCI_REG8(0x008c)
> > +#define GC05A2_REG_TEST_PATTERN_IDX CCI_REG8(0x008d)
> > +#define GC05A2_TEST_PATTERN_EN 0x01
> > +
> > +#define GC05A2_STREAMING_REG CCI_REG8(0x0100)
> > +
> > +#define GC05A2_FLIP_REG CCI_REG8(0x0101)
> > +#define GC05A2_FLIP_H_MASK BIT(0)
> > +#define GC05A2_FLIP_V_MASK BIT(1)
> > +
> > +#define GC05A2_EXP_REG CCI_REG16(0x0202)
> > +#define GC05A2_EXP_MARGIN 16
> > +#define GC05A2_EXP_MIN 4
> > +#define GC05A2_EXP_STEP 1
> > +
> > +#define GC05A2_AGAIN_REG CCI_REG16(0x0204)
> > +#define GC05A2_AGAIN_MIN 1024
> > +#define GC05A2_AGAIN_MAX (1024 * 16)
> > +#define GC05A2_AGAIN_STEP 1
> > +
> > +#define GC05A2_FRAME_LENGTH_REG CCI_REG16(0x0340)
> > +#define GC05A2_VTS_MAX 0xffff
> > +
> > +#define GC05A2_REG_CHIP_ID CCI_REG16(0x03f0)
> > +#define GC05A2_CHIP_ID 0x05a2
> > +
> > +#define GC05A2_NATIVE_WIDTH 2592
> > +#define GC05A2_NATIVE_HEIGHT 1944
> > +
> > +#define GC05A2_DEFAULT_CLK_FREQ (24 * HZ_PER_MHZ)
> > +#define GC05A2_MBUS_CODE MEDIA_BUS_FMT_SGRBG10_1X10
> > +#define GC05A2_DATA_LANES 2
> > +#define GC05A2_RGB_DEPTH 10
> > +#define GC05A2_SLEEP_US (2 * USEC_PER_MSEC)
> > +
> > +static const char *const gc05a2_test_pattern_menu[] = {
> > + "No Pattern", "Fade_to_gray_Color Bar", "Color Bar",
> > + "PN9", "Horizental_gradient", "Checkboard
> Pattern",
> > + "Slant", "Resolution", "Solid Black",
> > + "Solid White",
> > +};
> > +
> > +static const s64 gc05a2_link_freq_menu_items[] = {
> > + (448 * HZ_PER_MHZ),
> > + (224 * HZ_PER_MHZ),
> > +};
> > +
> > +static const char *const gc05a2_supply_name[] = {
> > + "avdd",
> > + "dvdd",
> > + "dovdd",
> > +};
> > +
> > +struct gc05a2 {
> > + struct device *dev;
> > + struct v4l2_subdev sd;
> > + struct media_pad pad;
> > +
> > + struct clk *xclk;
> > + struct regulator_bulk_data
> supplies[ARRAY_SIZE(gc05a2_supply_name)];
> > + struct gpio_desc *reset_gpio;
> > +
> > + struct v4l2_ctrl_handler ctrls;
> > + struct v4l2_ctrl *pixel_rate;
> > + struct v4l2_ctrl *link_freq;
> > + struct v4l2_ctrl *exposure;
> > + struct v4l2_ctrl *vblank;
> > + struct v4l2_ctrl *hblank;
> > + struct v4l2_ctrl *hflip;
> > + struct v4l2_ctrl *vflip;
> > +
> > + struct regmap *regmap;
> > + unsigned long link_freq_bitmap;
> > +
> > + /* True if the device has been identified */
> > + bool identified;
> > + const struct gc05a2_mode *cur_mode;
> > +};
> > +
> > +struct gc05a2_reg_list {
> > + u32 num_of_regs;
> > + const struct cci_reg_sequence *regs;
> > +};
> > +
> > +static const struct cci_reg_sequence mode_2592x1944[] = {
> > + /* system */
> > + { CCI_REG8(0x0135), 0x01 },
> > +
> > + /* pre_setting */
> > + { CCI_REG8(0x0084), 0x21 },
> > + { CCI_REG8(0x0d05), 0xcc },
> > + { CCI_REG8(0x0218), 0x00 },
> > + { CCI_REG8(0x005e), 0x48 },
> > + { CCI_REG8(0x0d06), 0x01 },
> > + { CCI_REG8(0x0007), 0x16 },
> > + { CCI_REG8(0x0101), 0x00 },
> > +
> > + /* analog */
> > + { CCI_REG8(0x0342), 0x07 },
> > + { CCI_REG8(0x0343), 0x28 },
> > + { CCI_REG8(0x0220), 0x07 },
> > + { CCI_REG8(0x0221), 0xd0 },
> > + { CCI_REG8(0x0202), 0x07 },
> > + { CCI_REG8(0x0203), 0x32 },
> > + { CCI_REG8(0x0340), 0x07 },
> > + { CCI_REG8(0x0341), 0xf0 },
> > + { CCI_REG8(0x0219), 0x00 },
> > + { CCI_REG8(0x0346), 0x00 },
> > + { CCI_REG8(0x0347), 0x04 },
> > + { CCI_REG8(0x0d14), 0x00 },
> > + { CCI_REG8(0x0d13), 0x05 },
> > + { CCI_REG8(0x0d16), 0x05 },
> > + { CCI_REG8(0x0d15), 0x1d },
> > + { CCI_REG8(0x00c0), 0x0a },
> > + { CCI_REG8(0x00c1), 0x30 },
> > + { CCI_REG8(0x034a), 0x07 },
> > + { CCI_REG8(0x034b), 0xa8 },
> > + { CCI_REG8(0x0e0a), 0x00 },
> > + { CCI_REG8(0x0e0b), 0x00 },
> > + { CCI_REG8(0x0e0e), 0x03 },
> > + { CCI_REG8(0x0e0f), 0x00 },
> > + { CCI_REG8(0x0e06), 0x0a },
> > + { CCI_REG8(0x0e23), 0x15 },
> > + { CCI_REG8(0x0e24), 0x15 },
> > + { CCI_REG8(0x0e2a), 0x10 },
> > + { CCI_REG8(0x0e2b), 0x10 },
> > + { CCI_REG8(0x0e17), 0x49 },
> > + { CCI_REG8(0x0e1b), 0x1c },
> > + { CCI_REG8(0x0e3a), 0x36 },
> > + { CCI_REG8(0x0d11), 0x84 },
> > + { CCI_REG8(0x0e52), 0x14 },
> > + { CCI_REG8(0x000b), 0x10 },
> > + { CCI_REG8(0x0008), 0x08 },
> > + { CCI_REG8(0x0223), 0x17 },
> > + { CCI_REG8(0x0d27), 0x39 },
> > + { CCI_REG8(0x0d22), 0x00 },
> > + { CCI_REG8(0x03f6), 0x0d },
> > + { CCI_REG8(0x0d04), 0x07 },
> > + { CCI_REG8(0x03f3), 0x72 },
> > + { CCI_REG8(0x03f4), 0xb8 },
> > + { CCI_REG8(0x03f5), 0xbc },
> > + { CCI_REG8(0x0d02), 0x73 },
> > +
> > + /* auto load start */
> > + { CCI_REG8(0x00cb), 0x00 },
> > +
> > + /* OUT 2592*1944 */
> > + { CCI_REG8(0x0350), 0x01 },
> > + { CCI_REG8(0x0353), 0x00 },
> > + { CCI_REG8(0x0354), 0x08 },
>
> > + { CCI_REG8(0x034c), 0x0a },
> > + { CCI_REG8(0x034d), 0x20 },
>
> Should/Could this be
> { CCI_REG16(0x034c), 2592 }, /* Width */
>
>
> > + { CCI_REG8(0x021f), 0x14 },
> > +
> > + /* MIPI */
> > + { CCI_REG8(0x0107), 0x05 },
> > + { CCI_REG8(0x0117), 0x01 },
> > + { CCI_REG8(0x0d81), 0x00 },
> > + { CCI_REG8(0x0d84), 0x0c },
> > + { CCI_REG8(0x0d85), 0xa8 },
> > + { CCI_REG8(0x0d86), 0x06 },
> > + { CCI_REG8(0x0d87), 0x55 },
> > + { CCI_REG8(0x0db3), 0x06 },
> > + { CCI_REG8(0x0db4), 0x08 },
> > + { CCI_REG8(0x0db5), 0x1e },
> > + { CCI_REG8(0x0db6), 0x02 },
> > + { CCI_REG8(0x0db8), 0x12 },
> > + { CCI_REG8(0x0db9), 0x0a },
> > + { CCI_REG8(0x0d93), 0x06 },
> > + { CCI_REG8(0x0d94), 0x09 },
> > + { CCI_REG8(0x0d95), 0x0d },
> > + { CCI_REG8(0x0d99), 0x0b },
> > + { CCI_REG8(0x0084), 0x01 },
> > +
> > + /* OUT */
> > + { CCI_REG8(0x0110), 0x01 },
> > +};
> > +
> > +static const struct cci_reg_sequence mode_1280x720[] = {
> > + /* system */
> > + { CCI_REG8(0x0135), 0x05 },
>
> In 2592x1944 this is 0x01. Do you have a datasheet? Can you explain
> why
> they are different? Can you add register definitions that have names
> to
> make this more maintainable or extendable in the future?
>
> There's discussion in the recent series improving the IMX258 which
> makes
> me wonder if we should try harder to have sensor drivers with clearer
> definitions.
>
>
> > +
> > + /*pre_setting*/
>
> /* pre_setting */ ?
>
> > + { CCI_REG8(0x0084), 0x21 },
> > + { CCI_REG8(0x0d05), 0xcc },
> > + { CCI_REG8(0x0218), 0x80 },
> > + { CCI_REG8(0x005e), 0x49 },
> > + { CCI_REG8(0x0d06), 0x81 },
> > + { CCI_REG8(0x0007), 0x16 },
> > + { CCI_REG8(0x0101), 0x00 },
>
> In 2592x1944, only register 0x0218 differs. Why? What is that? Can it
> be
> broken out to a function that applies the correct configuration at
> startuup based on a parameter instead of duplicating this table set?
>
> > +
> > + /* analog */
> > + { CCI_REG8(0x0342), 0x07 },
> > + { CCI_REG8(0x0343), 0x10 },
> > + { CCI_REG8(0x0220), 0x07 },
> > + { CCI_REG8(0x0221), 0xd0 },
> > + { CCI_REG8(0x0202), 0x03 },
> > + { CCI_REG8(0x0203), 0x32 },
> > + { CCI_REG8(0x0340), 0x04 },
> > + { CCI_REG8(0x0341), 0x08 },
> > + { CCI_REG8(0x0219), 0x00 },
> > + { CCI_REG8(0x0346), 0x01 },
> > + { CCI_REG8(0x0347), 0x00 },
> > + { CCI_REG8(0x0d14), 0x00 },
> > + { CCI_REG8(0x0d13), 0x05 },
> > + { CCI_REG8(0x0d16), 0x05 },
> > + { CCI_REG8(0x0d15), 0x1d },
> > + { CCI_REG8(0x00c0), 0x0a },
> > + { CCI_REG8(0x00c1), 0x30 },
> > + { CCI_REG8(0x034a), 0x05 },
> > + { CCI_REG8(0x034b), 0xb0 },
> > + { CCI_REG8(0x0e0a), 0x00 },
> > + { CCI_REG8(0x0e0b), 0x00 },
> > + { CCI_REG8(0x0e0e), 0x03 },
> > + { CCI_REG8(0x0e0f), 0x00 },
> > + { CCI_REG8(0x0e06), 0x0a },
> > + { CCI_REG8(0x0e23), 0x15 },
> > + { CCI_REG8(0x0e24), 0x15 },
> > + { CCI_REG8(0x0e2a), 0x10 },
> > + { CCI_REG8(0x0e2b), 0x10 },
> > + { CCI_REG8(0x0e17), 0x49 },
> > + { CCI_REG8(0x0e1b), 0x1c },
> > + { CCI_REG8(0x0e3a), 0x36 },
> > + { CCI_REG8(0x0d11), 0x84 },
> > + { CCI_REG8(0x0e52), 0x14 },
> > + { CCI_REG8(0x000b), 0x0e },
> > + { CCI_REG8(0x0008), 0x03 },
> > + { CCI_REG8(0x0223), 0x16 },
> > + { CCI_REG8(0x0d27), 0x39 },
> > + { CCI_REG8(0x0d22), 0x00 },
> > + { CCI_REG8(0x03f6), 0x0d },
> > + { CCI_REG8(0x0d04), 0x07 },
> > + { CCI_REG8(0x03f3), 0x72 },
> > + { CCI_REG8(0x03f4), 0xb8 },
> > + { CCI_REG8(0x03f5), 0xbc },
> > + { CCI_REG8(0x0d02), 0x73 },
> > +
>
> Are any of those able to be broken out to named register to be more
> clear in their intent?
>
> > + /* auto load start */
> > + { CCI_REG8(0x00cb), 0xfc },
> > +
>
> Why is this auto load start so different to the other modes 'auto
> load
> start'? What do the bits refer to ?
>
> > + /* OUT 1280x720 */
> > + { CCI_REG8(0x0350), 0x01 },
> > + { CCI_REG8(0x0353), 0x00 },
> > + { CCI_REG8(0x0354), 0x0c },
>
> > + { CCI_REG8(0x034c), 0x05 },
> > + { CCI_REG8(0x034d), 0x00 },
>
> Should/Could this be
> { CCI_REG16(0x034c), 1280 },
>
> Are there any other register settings that would make more sense to
> be
> in decimal units that match their actual context?
>
>
> > + { CCI_REG8(0x021f), 0x14 },
>
> I don't see a setting for 720/0x2d0. Do these registers only set the
> width?
>
> > +
> > + /* MIPI */
> > + { CCI_REG8(0x0107), 0x05 },
> > + { CCI_REG8(0x0117), 0x01 },
> > + { CCI_REG8(0x0d81), 0x00 },
> > + { CCI_REG8(0x0d84), 0x06 },
> > + { CCI_REG8(0x0d85), 0x40 },
> > + { CCI_REG8(0x0d86), 0x03 },
> > + { CCI_REG8(0x0d87), 0x21 },
> > + { CCI_REG8(0x0db3), 0x03 },
> > + { CCI_REG8(0x0db4), 0x04 },
> > + { CCI_REG8(0x0db5), 0x0d },
> > + { CCI_REG8(0x0db6), 0x01 },
> > + { CCI_REG8(0x0db8), 0x04 },
> > + { CCI_REG8(0x0db9), 0x06 },
> > + { CCI_REG8(0x0d93), 0x03 },
> > + { CCI_REG8(0x0d94), 0x04 },
> > + { CCI_REG8(0x0d95), 0x05 },
> > + { CCI_REG8(0x0d99), 0x06 },
> > + { CCI_REG8(0x0084), 0x01 },
> > +
> > + /* OUT */
>
> Out where? What is out?
>
> > + { CCI_REG8(0x0110), 0x01 },
> > +};
> > +
> > +static const struct cci_reg_sequence mode_table_common[] = {
> > + { GC05A2_STREAMING_REG, 0x00 },
> > + /* system */
> > + { CCI_REG8(0x0315), 0xd4 },
> > + { CCI_REG8(0x0d06), 0x01 },
> > + { CCI_REG8(0x0a70), 0x80 },
> > + { CCI_REG8(0x031a), 0x00 },
> > + { CCI_REG8(0x0314), 0x00 },
> > + { CCI_REG8(0x0130), 0x08 },
> > + { CCI_REG8(0x0132), 0x01 },
> > + { CCI_REG8(0x0136), 0x38 },
> > + { CCI_REG8(0x0137), 0x03 },
> > + { CCI_REG8(0x0134), 0x5b },
> > + { CCI_REG8(0x031c), 0xe0 },
> > + { CCI_REG8(0x0d82), 0x14 },
> > + { CCI_REG8(0x0dd1), 0x56 },
> > +
> > + /* gate_mode */
> > + { CCI_REG8(0x0af4), 0x01 },
> > + { CCI_REG8(0x0002), 0x10 },
> > + { CCI_REG8(0x00c3), 0x34 },
> > +
> > + /* auto load start */
>
> The previous 'auto load start' referenced 0x00cb ?
>
> > + { CCI_REG8(0x00c4), 0x00 },
> > + { CCI_REG8(0x00c5), 0x01 },
> > + { CCI_REG8(0x0af6), 0x00 },
> > + { CCI_REG8(0x0ba0), 0x17 },
> > + { CCI_REG8(0x0ba1), 0x00 },
> > + { CCI_REG8(0x0ba2), 0x00 },
> > + { CCI_REG8(0x0ba3), 0x00 },
> > + { CCI_REG8(0x0ba4), 0x03 },
> > + { CCI_REG8(0x0ba5), 0x00 },
> > + { CCI_REG8(0x0ba6), 0x00 },
> > + { CCI_REG8(0x0ba7), 0x00 },
> > + { CCI_REG8(0x0ba8), 0x40 },
> > + { CCI_REG8(0x0ba9), 0x00 },
> > + { CCI_REG8(0x0baa), 0x00 },
> > + { CCI_REG8(0x0bab), 0x00 },
> > + { CCI_REG8(0x0bac), 0x40 },
> > + { CCI_REG8(0x0bad), 0x00 },
> > + { CCI_REG8(0x0bae), 0x00 },
> > + { CCI_REG8(0x0baf), 0x00 },
> > + { CCI_REG8(0x0bb0), 0x02 },
> > + { CCI_REG8(0x0bb1), 0x00 },
> > + { CCI_REG8(0x0bb2), 0x00 },
> > + { CCI_REG8(0x0bb3), 0x00 },
> > + { CCI_REG8(0x0bb8), 0x02 },
> > + { CCI_REG8(0x0bb9), 0x00 },
> > + { CCI_REG8(0x0bba), 0x00 },
> > + { CCI_REG8(0x0bbb), 0x00 },
> > + { CCI_REG8(0x0a70), 0x80 },
> > + { CCI_REG8(0x0a71), 0x00 },
> > + { CCI_REG8(0x0a72), 0x00 },
> > + { CCI_REG8(0x0a66), 0x00 },
> > + { CCI_REG8(0x0a67), 0x80 },
> > + { CCI_REG8(0x0a4d), 0x4e },
> > + { CCI_REG8(0x0a50), 0x00 },
> > + { CCI_REG8(0x0a4f), 0x0c },
> > + { CCI_REG8(0x0a66), 0x00 },
> > + { CCI_REG8(0x00ca), 0x00 },
> > + { CCI_REG8(0x00cc), 0x00 },
> > + { CCI_REG8(0x00cd), 0x00 },
> > + { CCI_REG8(0x0aa1), 0x00 },
> > + { CCI_REG8(0x0aa2), 0xe0 },
> > + { CCI_REG8(0x0aa3), 0x00 },
> > + { CCI_REG8(0x0aa4), 0x40 },
> > + { CCI_REG8(0x0a90), 0x03 },
> > + { CCI_REG8(0x0a91), 0x0e },
> > + { CCI_REG8(0x0a94), 0x80 },
> > +
> > + /* standby */
> > + { CCI_REG8(0x0af6), 0x20 },
> > + { CCI_REG8(0x0b00), 0x91 },
> > + { CCI_REG8(0x0b01), 0x17 },
> > + { CCI_REG8(0x0b02), 0x01 },
> > + { CCI_REG8(0x0b03), 0x00 },
> > + { CCI_REG8(0x0b04), 0x01 },
> > + { CCI_REG8(0x0b05), 0x17 },
> > + { CCI_REG8(0x0b06), 0x01 },
> > + { CCI_REG8(0x0b07), 0x00 },
> > + { CCI_REG8(0x0ae9), 0x01 },
> > + { CCI_REG8(0x0aea), 0x02 },
> > + { CCI_REG8(0x0ae8), 0x53 },
> > + { CCI_REG8(0x0ae8), 0x43 },
> > +
> > + /* gain_partition */
> > + { CCI_REG8(0x0af6), 0x30 },
> > + { CCI_REG8(0x0b00), 0x08 },
> > + { CCI_REG8(0x0b01), 0x0f },
> > + { CCI_REG8(0x0b02), 0x00 },
> > + { CCI_REG8(0x0b04), 0x1c },
> > + { CCI_REG8(0x0b05), 0x24 },
> > + { CCI_REG8(0x0b06), 0x00 },
> > + { CCI_REG8(0x0b08), 0x30 },
> > + { CCI_REG8(0x0b09), 0x40 },
> > + { CCI_REG8(0x0b0a), 0x00 },
> > + { CCI_REG8(0x0b0c), 0x0e },
> > + { CCI_REG8(0x0b0d), 0x2a },
> > + { CCI_REG8(0x0b0e), 0x00 },
> > + { CCI_REG8(0x0b10), 0x0e },
> > + { CCI_REG8(0x0b11), 0x2b },
> > + { CCI_REG8(0x0b12), 0x00 },
> > + { CCI_REG8(0x0b14), 0x0e },
> > + { CCI_REG8(0x0b15), 0x23 },
> > + { CCI_REG8(0x0b16), 0x00 },
> > + { CCI_REG8(0x0b18), 0x0e },
> > + { CCI_REG8(0x0b19), 0x24 },
> > + { CCI_REG8(0x0b1a), 0x00 },
> > + { CCI_REG8(0x0b1c), 0x0c },
> > + { CCI_REG8(0x0b1d), 0x0c },
> > + { CCI_REG8(0x0b1e), 0x00 },
> > + { CCI_REG8(0x0b20), 0x03 },
> > + { CCI_REG8(0x0b21), 0x03 },
> > + { CCI_REG8(0x0b22), 0x00 },
> > + { CCI_REG8(0x0b24), 0x0e },
> > + { CCI_REG8(0x0b25), 0x0e },
> > + { CCI_REG8(0x0b26), 0x00 },
> > + { CCI_REG8(0x0b28), 0x03 },
> > + { CCI_REG8(0x0b29), 0x03 },
> > + { CCI_REG8(0x0b2a), 0x00 },
> > + { CCI_REG8(0x0b2c), 0x12 },
> > + { CCI_REG8(0x0b2d), 0x12 },
> > + { CCI_REG8(0x0b2e), 0x00 },
> > + { CCI_REG8(0x0b30), 0x08 },
> > + { CCI_REG8(0x0b31), 0x08 },
> > + { CCI_REG8(0x0b32), 0x00 },
> > + { CCI_REG8(0x0b34), 0x14 },
> > + { CCI_REG8(0x0b35), 0x14 },
> > + { CCI_REG8(0x0b36), 0x00 },
> > + { CCI_REG8(0x0b38), 0x10 },
> > + { CCI_REG8(0x0b39), 0x10 },
> > + { CCI_REG8(0x0b3a), 0x00 },
> > + { CCI_REG8(0x0b3c), 0x16 },
> > + { CCI_REG8(0x0b3d), 0x16 },
> > + { CCI_REG8(0x0b3e), 0x00 },
> > + { CCI_REG8(0x0b40), 0x10 },
> > + { CCI_REG8(0x0b41), 0x10 },
> > + { CCI_REG8(0x0b42), 0x00 },
> > + { CCI_REG8(0x0b44), 0x19 },
> > + { CCI_REG8(0x0b45), 0x19 },
> > + { CCI_REG8(0x0b46), 0x00 },
> > + { CCI_REG8(0x0b48), 0x16 },
> > + { CCI_REG8(0x0b49), 0x16 },
> > + { CCI_REG8(0x0b4a), 0x00 },
> > + { CCI_REG8(0x0b4c), 0x19 },
> > + { CCI_REG8(0x0b4d), 0x19 },
> > + { CCI_REG8(0x0b4e), 0x00 },
> > + { CCI_REG8(0x0b50), 0x16 },
> > + { CCI_REG8(0x0b51), 0x16 },
> > + { CCI_REG8(0x0b52), 0x00 },
> > + { CCI_REG8(0x0b80), 0x01 },
> > + { CCI_REG8(0x0b81), 0x00 },
> > + { CCI_REG8(0x0b82), 0x00 },
> > + { CCI_REG8(0x0b84), 0x00 },
> > + { CCI_REG8(0x0b85), 0x00 },
> > + { CCI_REG8(0x0b86), 0x00 },
> > + { CCI_REG8(0x0b88), 0x01 },
> > + { CCI_REG8(0x0b89), 0x6a },
> > + { CCI_REG8(0x0b8a), 0x00 },
> > + { CCI_REG8(0x0b8c), 0x00 },
> > + { CCI_REG8(0x0b8d), 0x01 },
> > + { CCI_REG8(0x0b8e), 0x00 },
> > + { CCI_REG8(0x0b90), 0x01 },
> > + { CCI_REG8(0x0b91), 0xf6 },
> > + { CCI_REG8(0x0b92), 0x00 },
> > + { CCI_REG8(0x0b94), 0x00 },
> > + { CCI_REG8(0x0b95), 0x02 },
> > + { CCI_REG8(0x0b96), 0x00 },
> > + { CCI_REG8(0x0b98), 0x02 },
> > + { CCI_REG8(0x0b99), 0xc4 },
> > + { CCI_REG8(0x0b9a), 0x00 },
> > + { CCI_REG8(0x0b9c), 0x00 },
> > + { CCI_REG8(0x0b9d), 0x03 },
> > + { CCI_REG8(0x0b9e), 0x00 },
> > + { CCI_REG8(0x0ba0), 0x03 },
> > + { CCI_REG8(0x0ba1), 0xd8 },
> > + { CCI_REG8(0x0ba2), 0x00 },
> > + { CCI_REG8(0x0ba4), 0x00 },
> > + { CCI_REG8(0x0ba5), 0x04 },
> > + { CCI_REG8(0x0ba6), 0x00 },
> > + { CCI_REG8(0x0ba8), 0x05 },
> > + { CCI_REG8(0x0ba9), 0x4d },
> > + { CCI_REG8(0x0baa), 0x00 },
> > + { CCI_REG8(0x0bac), 0x00 },
> > + { CCI_REG8(0x0bad), 0x05 },
> > + { CCI_REG8(0x0bae), 0x00 },
> > + { CCI_REG8(0x0bb0), 0x07 },
> > + { CCI_REG8(0x0bb1), 0x3e },
> > + { CCI_REG8(0x0bb2), 0x00 },
> > + { CCI_REG8(0x0bb4), 0x00 },
> > + { CCI_REG8(0x0bb5), 0x06 },
> > + { CCI_REG8(0x0bb6), 0x00 },
> > + { CCI_REG8(0x0bb8), 0x0a },
> > + { CCI_REG8(0x0bb9), 0x1a },
> > + { CCI_REG8(0x0bba), 0x00 },
> > + { CCI_REG8(0x0bbc), 0x09 },
> > + { CCI_REG8(0x0bbd), 0x36 },
> > + { CCI_REG8(0x0bbe), 0x00 },
> > + { CCI_REG8(0x0bc0), 0x0e },
> > + { CCI_REG8(0x0bc1), 0x66 },
> > + { CCI_REG8(0x0bc2), 0x00 },
> > + { CCI_REG8(0x0bc4), 0x10 },
> > + { CCI_REG8(0x0bc5), 0x06 },
> > + { CCI_REG8(0x0bc6), 0x00 },
> > + { CCI_REG8(0x02c1), 0xe0 },
> > + { CCI_REG8(0x0207), 0x04 },
> > + { CCI_REG8(0x02c2), 0x10 },
> > + { CCI_REG8(0x02c3), 0x74 },
> > + { CCI_REG8(0x02c5), 0x09 },
> > + { CCI_REG8(0x02c1), 0xe0 },
> > + { CCI_REG8(0x0207), 0x04 },
> > + { CCI_REG8(0x02c2), 0x10 },
> > + { CCI_REG8(0x02c5), 0x09 },
> > + { CCI_REG8(0x02c1), 0xe0 },
> > + { CCI_REG8(0x0207), 0x04 },
> > + { CCI_REG8(0x02c2), 0x10 },
> > + { CCI_REG8(0x02c5), 0x09 },
> > +
> > + /* auto load CH_GAIN */
> > + { CCI_REG8(0x0aa1), 0x15 },
> > + { CCI_REG8(0x0aa2), 0x50 },
> > + { CCI_REG8(0x0aa3), 0x00 },
> > + { CCI_REG8(0x0aa4), 0x09 },
> > + { CCI_REG8(0x0a90), 0x25 },
> > + { CCI_REG8(0x0a91), 0x0e },
> > + { CCI_REG8(0x0a94), 0x80 },
> > +
> > + /* ISP */
> > + { CCI_REG8(0x0050), 0x00 },
> > + { CCI_REG8(0x0089), 0x83 },
> > + { CCI_REG8(0x005a), 0x40 },
> > + { CCI_REG8(0x00c3), 0x35 },
> > + { CCI_REG8(0x00c4), 0x80 },
> > + { CCI_REG8(0x0080), 0x10 },
> > + { CCI_REG8(0x0040), 0x12 },
> > + { CCI_REG8(0x0053), 0x0a },
> > + { CCI_REG8(0x0054), 0x44 },
> > + { CCI_REG8(0x0055), 0x32 },
> > + { CCI_REG8(0x0058), 0x89 },
> > + { CCI_REG8(0x004a), 0x03 },
> > + { CCI_REG8(0x0048), 0xf0 },
> > + { CCI_REG8(0x0049), 0x0f },
> > + { CCI_REG8(0x0041), 0x20 },
> > + { CCI_REG8(0x0043), 0x0a },
> > + { CCI_REG8(0x009d), 0x08 },
> > + { CCI_REG8(0x0236), 0x40 },
> > +
> > + /* gain */
>
> Is the gain configurable? Is this analogue gain? digital gain? or
> colour
> balanace gains ?
>
>
> > + { CCI_REG8(0x0204), 0x04 },
> > + { CCI_REG8(0x0205), 0x00 },
> > + { CCI_REG8(0x02b3), 0x00 },
> > + { CCI_REG8(0x02b4), 0x00 },
> > + { CCI_REG8(0x009e), 0x01 },
> > + { CCI_REG8(0x009f), 0x94 },
> > +
> > + /* auto load REG */
> > + { CCI_REG8(0x0aa1), 0x10 },
> > + { CCI_REG8(0x0aa2), 0xf8 },
> > + { CCI_REG8(0x0aa3), 0x00 },
> > + { CCI_REG8(0x0aa4), 0x1f },
> > + { CCI_REG8(0x0a90), 0x11 },
> > + { CCI_REG8(0x0a91), 0x0e },
> > + { CCI_REG8(0x0a94), 0x80 },
> > + { CCI_REG8(0x03fe), 0x00 },
> > + { CCI_REG8(0x0a90), 0x00 },
> > + { CCI_REG8(0x0a70), 0x00 },
> > + { CCI_REG8(0x0a67), 0x00 },
> > + { CCI_REG8(0x0af4), 0x29 },
> > +
> > + /* DPHY */
> > + { CCI_REG8(0x0d80), 0x07 },
> > + { CCI_REG8(0x0dd3), 0x18 },
> > +
> > + /* CISCTL_Reset */
> > + { CCI_REG8(0x031c), 0x80 },
> > + { CCI_REG8(0x03fe), 0x30 },
> > + { CCI_REG8(0x0d17), 0x06 },
> > + { CCI_REG8(0x03fe), 0x00 },
> > + { CCI_REG8(0x0d17), 0x00 },
> > + { CCI_REG8(0x031c), 0x93 },
> > + { CCI_REG8(0x03fe), 0x00 },
> > + { CCI_REG8(0x031c), 0x80 },
> > + { CCI_REG8(0x03fe), 0x30 },
> > + { CCI_REG8(0x0d17), 0x06 },
> > + { CCI_REG8(0x03fe), 0x00 },
> > + { CCI_REG8(0x0d17), 0x00 },
> > + { CCI_REG8(0x031c), 0x93 },
> > +};
> > +
> > +struct gc05a2_mode {
> > + u32 width;
> > + u32 height;
> > + const struct gc05a2_reg_list reg_list;
> > +
> > + u32 hts; /* Horizontal timining size */
> > + u32 vts_def; /* Default vertical timining size */
> > + u32 vts_min; /* Min vertical timining size */
> > +};
> > +
> > +/* Declare modes in order, from biggest to smallest height. */
> > +static const struct gc05a2_mode gc05a2_modes[] = {
> > + {
> > + /* 2592*1944@30fps */
> > + .width = GC05A2_NATIVE_WIDTH,
> > + .height = GC05A2_NATIVE_HEIGHT,
> > + .reg_list = {
> > + .num_of_regs = ARRAY_SIZE(mode_2592x1944),
> > + .regs = mode_2592x1944,
> > + },
> > + .hts = 3664,
> > + .vts_def = 2032,
> > + .vts_min = 2032,
> > + },
> > + {
> > + /* 1280*720@60fps */
> > + .width = 1280,
> > + .height = 720,
> > + .reg_list = {
> > + .num_of_regs = ARRAY_SIZE(mode_1280x720),
> > + .regs = mode_1280x720,
> > + },
> > + .hts = 3616,
> > + .vts_def = 1032,
> > + .vts_min = 1032,
> > + },
> > +};
> > +
> > +static inline struct gc05a2 *to_gc05a2(struct v4l2_subdev *sd)
> > +{
> > + return container_of(sd, struct gc05a2, sd);
> > +}
> > +
> > +static int gc05a2_power_on(struct device *dev)
> > +{
> > + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > + struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > + int ret;
> > +
> > + ret = regulator_bulk_enable(ARRAY_SIZE(gc05a2_supply_name),
> > + gc05a2->supplies);
> > + if (ret < 0) {
> > + dev_err(gc05a2->dev, "failed to enable regulators:
> %d\n", ret);
> > + return ret;
> > + }
> > +
> > + ret = clk_prepare_enable(gc05a2->xclk);
> > + if (ret < 0) {
> >
> + regulator_bulk_disable(ARRAY_SIZE(gc05a2_supply_name)
> ,
> > + gc05a2->supplies);
> > + dev_err(gc05a2->dev, "clk prepare enable
> failed\n");
> > + return ret;
> > + }
> > +
> > + fsleep(GC05A2_SLEEP_US);
> > +
> > + gpiod_set_value_cansleep(gc05a2->reset_gpio, 0);
> > + fsleep(GC05A2_SLEEP_US);
> > +
> > + return 0;
> > +}
> > +
> > +static int gc05a2_power_off(struct device *dev)
> > +{
> > + struct v4l2_subdev *sd = dev_get_drvdata(dev);
> > + struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > +
> > + clk_disable_unprepare(gc05a2->xclk);
> > + gpiod_set_value_cansleep(gc05a2->reset_gpio, 1);
> > + regulator_bulk_disable(ARRAY_SIZE(gc05a2_supply_name),
> > + gc05a2->supplies);
> > +
> > + return 0;
> > +}
> > +
> > +static int gc05a2_enum_mbus_code(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state
> *sd_state,
> > + struct v4l2_subdev_mbus_code_enum
> *code)
> > +{
> > + if (code->index > 0)
> > + return -EINVAL;
> > +
> > + code->code = GC05A2_MBUS_CODE;
> > +
> > + return 0;
> > +}
> > +
> > +static int gc05a2_enum_frame_size(struct v4l2_subdev *subdev,
> > + struct v4l2_subdev_state
> *sd_state,
> > + struct
> v4l2_subdev_frame_size_enum *fse)
> > +{
> > + if (fse->code != GC05A2_MBUS_CODE)
> > + return -EINVAL;
> > +
> > + if (fse->index >= ARRAY_SIZE(gc05a2_modes))
> > + return -EINVAL;
> > +
> > + fse->min_width = gc05a2_modes[fse->index].width;
> > + fse->max_width = gc05a2_modes[fse->index].width;
> > + fse->min_height = gc05a2_modes[fse->index].height;
> > + fse->max_height = gc05a2_modes[fse->index].height;
> > +
> > + return 0;
> > +}
> > +
> > +static int gc05a2_update_cur_mode_controls(struct gc05a2 *gc05a2,
> > + const struct gc05a2_mode
> *mode)
> > +{
> > + s64 exposure_max, h_blank;
> > + int ret;
> > +
> > + ret = __v4l2_ctrl_modify_range(gc05a2->vblank,
> > + mode->vts_min - mode-
> >height,
> > + GC05A2_VTS_MAX - mode-
> >height, 1,
> > + mode->vts_def - mode-
> >height);
> > + if (ret) {
> > + dev_err(gc05a2->dev, "VB ctrl range update
> failed\n");
> > + return ret;
> > + }
> > +
> > + h_blank = mode->hts - mode->width;
> > + ret = __v4l2_ctrl_modify_range(gc05a2->hblank, h_blank,
> h_blank, 1,
> > + h_blank);
> > + if (ret) {
> > + dev_err(gc05a2->dev, "HB ctrl range update
> failed\n");
> > + return ret;
> > + }
> > +
> > + exposure_max = mode->vts_def - GC05A2_EXP_MARGIN;
> > + ret = __v4l2_ctrl_modify_range(gc05a2->exposure,
> GC05A2_EXP_MIN,
> > + exposure_max,
> GC05A2_EXP_STEP,
> > + exposure_max);
> > + if (ret) {
> > + dev_err(gc05a2->dev, "exposure ctrl range update
> failed\n");
> > + return ret;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static void gc05a2_update_pad_format(struct gc05a2 *gc08a3,
> > + const struct gc05a2_mode
> *mode,
> > + struct v4l2_mbus_framefmt
> *fmt)
> > +{
> > + fmt->width = mode->width;
> > + fmt->height = mode->height;
> > + fmt->code = GC05A2_MBUS_CODE;
> > + fmt->field = V4L2_FIELD_NONE;
> > + fmt->colorspace = V4L2_COLORSPACE_RAW;
> > + fmt->ycbcr_enc = V4L2_MAP_YCBCR_ENC_DEFAULT(fmt-
> >colorspace);
> > + fmt->quantization = V4L2_QUANTIZATION_FULL_RANGE;
> > + fmt->xfer_func = V4L2_XFER_FUNC_NONE;
> > +}
> > +
> > +static int gc05a2_set_format(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + struct v4l2_subdev_format *fmt)
> > +{
> > + struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > + struct v4l2_mbus_framefmt *mbus_fmt;
> > + struct v4l2_rect *crop;
> > + const struct gc05a2_mode *mode;
> > +
> > + mode = v4l2_find_nearest_size(gc05a2_modes,
> ARRAY_SIZE(gc05a2_modes),
> > + width, height, fmt-
> >format.width,
> > + fmt->format.height);
> > +
> > + /* update crop info to subdev state */
> > + crop = v4l2_subdev_state_get_crop(state, 0);
> > + crop->width = mode->width;
> > + crop->height = mode->height;
> > +
> > + /* update fmt info to subdev state */
> > + gc05a2_update_pad_format(gc05a2, mode, &fmt->format);
> > + mbus_fmt = v4l2_subdev_state_get_format(state, 0);
> > + *mbus_fmt = fmt->format;
> > +
> > + if (fmt->which == V4L2_SUBDEV_FORMAT_TRY)
> > + return 0;
> > + gc05a2->cur_mode = mode;
> > + gc05a2_update_cur_mode_controls(gc05a2, mode);
> > +
> > + return 0;
> > +}
> > +
> > +static int gc05a2_get_selection(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state,
> > + struct v4l2_subdev_selection *sel)
> > +{
> > + switch (sel->target) {
> > + case V4L2_SEL_TGT_CROP_DEFAULT:
> > + case V4L2_SEL_TGT_CROP:
> > + sel->r = *v4l2_subdev_state_get_crop(state, 0);
> > + break;
> > + case V4L2_SEL_TGT_CROP_BOUNDS:
> > + sel->r.top = 0;
> > + sel->r.left = 0;
> > + sel->r.width = GC05A2_NATIVE_WIDTH;
> > + sel->r.height = GC05A2_NATIVE_HEIGHT;
> > + break;
> > + default:
> > + return -EINVAL;
> > + }
> > +
> > + return 0;
> > +}
> > +
> > +static int gc05a2_init_state(struct v4l2_subdev *sd,
> > + struct v4l2_subdev_state *state)
> > +{
> > + struct v4l2_subdev_format fmt = {
> > + .which = V4L2_SUBDEV_FORMAT_TRY,
> > + .pad = 0,
> > + .format = {
> > + .code = GC05A2_MBUS_CODE,
> > + .width = gc05a2_modes[0].width,
> > + .height = gc05a2_modes[0].height,
> > + },
> > + };
> > +
> > + gc05a2_set_format(sd, state, &fmt);
> > +
> > + return 0;
> > +}
> > +
> > +static int gc05a2_set_ctrl_hflip(struct gc05a2 *gc05a2, u32
> ctrl_val)
> > +{
> > + int ret;
> > + u64 val;
> > +
> > + ret = cci_read(gc05a2->regmap, GC05A2_FLIP_REG, &val,
> NULL);
> > + if (ret) {
> > + dev_err(gc05a2->dev, "read hflip register failed:
> %d\n", ret);
> > + return ret;
> > + }
> > +
> > + return cci_update_bits(gc05a2->regmap, GC05A2_FLIP_REG,
> > + GC05A2_FLIP_H_MASK,
> > + ctrl_val ? GC05A2_FLIP_H_MASK : 0,
> NULL);
> > +}
> > +
> > +static int gc05a2_set_ctrl_vflip(struct gc05a2 *gc05a2, u32
> ctrl_val)
> > +{
> > + int ret;
> > + u64 val;
> > +
> > + ret = cci_read(gc05a2->regmap, GC05A2_FLIP_REG, &val,
> NULL);
> > + if (ret) {
> > + dev_err(gc05a2->dev, "read vflip register failed:
> %d\n", ret);
> > + return ret;
> > + }
> > +
> > + return cci_update_bits(gc05a2->regmap, GC05A2_FLIP_REG,
> > + GC05A2_FLIP_V_MASK,
> > + ctrl_val ? GC05A2_FLIP_V_MASK : 0,
> NULL);
> > +}
> > +
> > +static int gc05a2_test_pattern(struct gc05a2 *gc05a2, u32
> pattern_menu)
> > +{
> > + u32 pattern;
> > + int ret;
> > +
> > + if (pattern_menu) {
> > + switch (pattern_menu) {
> > + case 1:
> > + case 2:
> > + case 3:
> > + case 4:
> > + case 5:
> > + case 6:
> > + case 7:
> > + pattern = pattern_menu << 4;
> > + break;
> > +
> > + case 8:
> > + pattern = 0;
> > + break;
> > +
> > + case 9:
> > + pattern = 4;
> > + break;
> > +
> > + default:
> > + pattern = 0x00;
> > + break;
> > + }
>
> This is fairly terse. Can we add comments, or definitions for the
> types
> or such so that the above is easier to interpret?
>
> > +
> > + ret = cci_write(gc05a2->regmap,
> GC05A2_REG_TEST_PATTERN_IDX,
> > + pattern, NULL);
> > + if (ret)
> > + return ret;
> > +
> > + return cci_write(gc05a2->regmap,
> GC05A2_REG_TEST_PATTERN_EN,
> > + GC05A2_TEST_PATTERN_EN, NULL);
> > + } else {
> > + return cci_write(gc05a2->regmap,
> GC05A2_REG_TEST_PATTERN_EN,
> > + 0x00, NULL);
> > + }
> > +}
> > +
> > +static int gc05a2_set_ctrl(struct v4l2_ctrl *ctrl)
> > +{
> > + struct gc05a2 *gc05a2 =
> > + container_of(ctrl->handler, struct gc05a2, ctrls);
> > + int ret = 0;
> > + s64 exposure_max;
> > + struct v4l2_subdev_state *state;
> > + const struct v4l2_mbus_framefmt *format;
> > +
> > + state = v4l2_subdev_get_locked_active_state(&gc05a2->sd);
> > + format = v4l2_subdev_state_get_format(state, 0);
> > +
> > + if (ctrl->id == V4L2_CID_VBLANK) {
> > + /* Update max exposure while meeting expected
> vblanking */
> > + exposure_max = format->height + ctrl->val -
> GC05A2_EXP_MARGIN;
> > + __v4l2_ctrl_modify_range(gc05a2->exposure,
> > + gc05a2->exposure->minimum,
> > + exposure_max, gc05a2-
> >exposure->step,
> > + exposure_max);
> > + }
> > +
> > + /*
> > + * Applying V4L2 control value only happens
> > + * when power is on for streaming.
> > + */
> > + if (!pm_runtime_get_if_active(gc05a2->dev))
> > + return 0;
> > +
> > + switch (ctrl->id) {
> > + case V4L2_CID_EXPOSURE:
> > + ret = cci_write(gc05a2->regmap, GC05A2_EXP_REG,
> > + ctrl->val, NULL);
> > + break;
> > +
> > + case V4L2_CID_ANALOGUE_GAIN:
> > + ret = cci_write(gc05a2->regmap, GC05A2_AGAIN_REG,
> > + ctrl->val, NULL);
> > + break;
> > +
> > + case V4L2_CID_VBLANK:
> > + ret = cci_write(gc05a2->regmap,
> GC05A2_FRAME_LENGTH_REG,
> > + gc05a2->cur_mode->height + ctrl-
> >val, NULL);
> > + break;
> > +
> > + case V4L2_CID_HFLIP:
> > + ret = gc05a2_set_ctrl_hflip(gc05a2, ctrl->val);
> > + break;
> > +
> > + case V4L2_CID_VFLIP:
> > + ret = gc05a2_set_ctrl_vflip(gc05a2, ctrl->val);
> > + break;
> > +
> > + case V4L2_CID_TEST_PATTERN:
> > + ret = gc05a2_test_pattern(gc05a2, ctrl->val);
> > + break;
> > +
> > + default:
> > + break;
> > + }
> > +
> > + pm_runtime_put(gc05a2->dev);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct v4l2_ctrl_ops gc05a2_ctrl_ops = {
> > + .s_ctrl = gc05a2_set_ctrl,
> > +};
> > +
> > +static int gc05a2_identify_module(struct gc05a2 *gc05a2)
> > +{
> > + u64 val;
> > + int ret;
> > +
> > + if (gc05a2->identified)
> > + return 0;
> > +
> > + ret = cci_read(gc05a2->regmap, GC05A2_REG_CHIP_ID, &val,
> NULL);
> > + if (ret)
> > + return ret;
> > +
> > + if (val != GC05A2_CHIP_ID) {
> > + dev_err(gc05a2->dev, "chip id mismatch:
> 0x%x!=0x%llx",
> > + GC05A2_CHIP_ID, val);
> > + return -ENXIO;
> > + }
> > +
> > + gc05a2->identified = true;
> > +
> > + return 0;
> > +}
> > +
> > +static int gc05a2_start_streaming(struct gc05a2 *gc05a2)
> > +{
> > + const struct gc05a2_mode *mode;
> > + const struct gc05a2_reg_list *reg_list;
> > + int ret;
> > +
> > + ret = pm_runtime_resume_and_get(gc05a2->dev);
> > + if (ret < 0)
> > + return ret;
> > +
> > + ret = gc05a2_identify_module(gc05a2);
> > + if (ret)
> > + goto err_rpm_put;
> > +
> > + ret = cci_multi_reg_write(gc05a2->regmap,
> > + mode_table_common,
> > + ARRAY_SIZE(mode_table_common),
> NULL);
> > + if (ret)
> > + goto err_rpm_put;
> > +
> > + mode = gc05a2->cur_mode;
> > + reg_list = &mode->reg_list;
> > +
> > + ret = cci_multi_reg_write(gc05a2->regmap,
> > + reg_list->regs, reg_list-
> >num_of_regs, NULL);
> > + if (ret < 0)
> > + goto err_rpm_put;
> > +
> > + ret = __v4l2_ctrl_handler_setup(&gc05a2->ctrls);
> > + if (ret < 0) {
> > + dev_err(gc05a2->dev, "could not sync v4l2
> controls\n");
> > + goto err_rpm_put;
> > + }
> > +
> > + ret = cci_write(gc05a2->regmap, GC05A2_STREAMING_REG, 1,
> NULL);
> > + if (ret < 0) {
> > + dev_err(gc05a2->dev, "write STREAMING_REG failed:
> %d\n", ret);
> > + goto err_rpm_put;
> > + }
> > +
> > + return 0;
> > +
> > +err_rpm_put:
> > + pm_runtime_put(gc05a2->dev);
> > + return ret;
> > +}
> > +
> > +static int gc05a2_stop_streaming(struct gc05a2 *gc05a2)
> > +{
> > + int ret;
> > +
> > + ret = cci_write(gc05a2->regmap, GC05A2_STREAMING_REG, 0,
> NULL);
> > + if (ret < 0)
> > + dev_err(gc05a2->dev, "could not sent stop streaming
> %d\n", ret);
> > +
> > + pm_runtime_put(gc05a2->dev);
> > + return ret;
> > +}
> > +
> > +static int gc05a2_s_stream(struct v4l2_subdev *subdev, int enable)
> > +{
> > + struct gc05a2 *gc05a2 = to_gc05a2(subdev);
> > + struct v4l2_subdev_state *state;
> > + int ret;
> > +
> > + state = v4l2_subdev_lock_and_get_active_state(subdev);
> > +
> > + if (enable)
> > + ret = gc05a2_start_streaming(gc05a2);
> > + else
> > + ret = gc05a2_stop_streaming(gc05a2);
> > +
> > + v4l2_subdev_unlock_state(state);
> > +
> > + return ret;
> > +}
> > +
> > +static const struct v4l2_subdev_video_ops gc05a2_video_ops = {
> > + .s_stream = gc05a2_s_stream,
> > +};
> > +
> > +static const struct v4l2_subdev_pad_ops gc05a2_subdev_pad_ops = {
> > + .enum_mbus_code = gc05a2_enum_mbus_code,
> > + .enum_frame_size = gc05a2_enum_frame_size,
> > + .get_fmt = v4l2_subdev_get_fmt,
> > + .set_fmt = gc05a2_set_format,
> > + .get_selection = gc05a2_get_selection,
> > +};
> > +
> > +static const struct v4l2_subdev_core_ops gc05a2_core_ops = {
> > + .subscribe_event = v4l2_ctrl_subdev_subscribe_event,
> > + .unsubscribe_event = v4l2_event_subdev_unsubscribe,
> > +};
> > +
> > +static const struct v4l2_subdev_ops gc05a2_subdev_ops = {
> > + .core = &gc05a2_core_ops,
> > + .video = &gc05a2_video_ops,
> > + .pad = &gc05a2_subdev_pad_ops,
> > +};
> > +
> > +static const struct v4l2_subdev_internal_ops gc05a2_internal_ops =
> {
> > + .init_state = gc05a2_init_state,
> > +};
> > +
> > +static int gc05a2_get_regulators(struct device *dev, struct gc05a2
> *gc05a2)
> > +{
> > + unsigned int i;
> > +
> > + for (i = 0; i < ARRAY_SIZE(gc05a2_supply_name); i++)
> > + gc05a2->supplies[i].supply = gc05a2_supply_name[i];
> > +
> > + return devm_regulator_bulk_get(dev,
> ARRAY_SIZE(gc05a2_supply_name),
> > + gc05a2->supplies);
> > +}
> > +
> > +static int gc05a2_parse_fwnode(struct gc05a2 *gc05a2)
> > +{
> > + struct fwnode_handle *endpoint;
> > + struct v4l2_fwnode_endpoint bus_cfg = {
> > + .bus_type = V4L2_MBUS_CSI2_DPHY,
> > + };
> > + int ret;
> > + struct device *dev = gc05a2->dev;
> > +
> > + endpoint =
> > + fwnode_graph_get_endpoint_by_id(dev_fwnode(dev), 0,
> 0,
> >
> + FWNODE_GRAPH_ENDPOINT
> _NEXT);
> > + if (!endpoint) {
> > + dev_err(dev, "endpoint node not found\n");
> > + return -EINVAL;
> > + }
> > +
> > + ret = v4l2_fwnode_endpoint_alloc_parse(endpoint, &bus_cfg);
> > + if (ret) {
> > + dev_err(dev, "parsing endpoint node failed\n");
> > + goto done;
> > + }
> > +
> > + ret = v4l2_link_freq_to_bitmap(dev,
> bus_cfg.link_frequencies,
> >
> + bus_cfg.nr_of_link_frequencies
> ,
> > + gc05a2_link_freq_menu_items,
> >
> + ARRAY_SIZE(gc05a2_link_freq_me
> nu_items),
> > + &gc05a2->link_freq_bitmap);
> > + if (ret)
> > + goto done;
> > +
> > +done:
> > + v4l2_fwnode_endpoint_free(&bus_cfg);
> > + fwnode_handle_put(endpoint);
> > + return ret;
> > +}
> > +
> > +static u64 gc05a2_to_pixel_rate(u32 f_index)
> > +{
> > + u64 pixel_rate =
> > + gc05a2_link_freq_menu_items[f_index] * 2 *
> GC05A2_DATA_LANES;
> > +
> > + return div_u64(pixel_rate, GC05A2_RGB_DEPTH);
> > +}
> > +
> > +static int gc05a2_init_controls(struct gc05a2 *gc05a2)
> > +{
> > + struct i2c_client *client = v4l2_get_subdevdata(&gc05a2-
> >sd);
> > + const struct gc05a2_mode *mode = &gc05a2_modes[0];
> > + const struct v4l2_ctrl_ops *ops = &gc05a2_ctrl_ops;
> > + struct v4l2_fwnode_device_properties props;
> > + struct v4l2_ctrl_handler *ctrl_hdlr;
> > + s64 exposure_max, h_blank;
> > + int ret;
> > +
> > + ctrl_hdlr = &gc05a2->ctrls;
> > + ret = v4l2_ctrl_handler_init(ctrl_hdlr, 9);
> > + if (ret)
> > + return ret;
> > +
> > + gc05a2->hflip = v4l2_ctrl_new_std(ctrl_hdlr,
> &gc05a2_ctrl_ops,
> > + V4L2_CID_HFLIP, 0, 1, 1,
> 0);
> > + gc05a2->vflip = v4l2_ctrl_new_std(ctrl_hdlr,
> &gc05a2_ctrl_ops,
> > + V4L2_CID_VFLIP, 0, 1, 1,
> 0);
> > + v4l2_ctrl_cluster(2, &gc05a2->hflip);
> > +
> > + gc05a2->link_freq =
> > + v4l2_ctrl_new_int_menu(ctrl_hdlr,
> > + &gc05a2_ctrl_ops,
> > + V4L2_CID_LINK_FREQ,
> >
> + ARRAY_SIZE(gc05a2_link_freq_menu_items
> ) - 1,
> > + 0,
> > + gc05a2_link_freq_menu_items);
> > + if (gc05a2->link_freq)
> > + gc05a2->link_freq->flags |=
> V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > + gc05a2->pixel_rate =
> > + v4l2_ctrl_new_std(ctrl_hdlr,
> > + &gc05a2_ctrl_ops,
> > + V4L2_CID_PIXEL_RATE, 0,
> > + gc05a2_to_pixel_rate(0),
> > + 1,
> > + gc05a2_to_pixel_rate(0));
> > +
> > + gc05a2->vblank =
> > + v4l2_ctrl_new_std(ctrl_hdlr,
> > + &gc05a2_ctrl_ops,
> V4L2_CID_VBLANK,
> > + mode->vts_min - mode->height,
> > + GC05A2_VTS_MAX - mode->height, 1,
> > + mode->vts_def - mode->height);
> > +
> > + h_blank = mode->hts - mode->width;
> > + gc05a2->hblank = v4l2_ctrl_new_std(ctrl_hdlr,
> &gc05a2_ctrl_ops,
> > + V4L2_CID_HBLANK,
> h_blank, h_blank, 1,
> > + h_blank);
> > + if (gc05a2->hblank)
> > + gc05a2->hblank->flags |= V4L2_CTRL_FLAG_READ_ONLY;
> > +
> > + v4l2_ctrl_new_std(ctrl_hdlr, &gc05a2_ctrl_ops,
> > + V4L2_CID_ANALOGUE_GAIN, GC05A2_AGAIN_MIN,
> > + GC05A2_AGAIN_MAX, GC05A2_AGAIN_STEP,
> > + GC05A2_AGAIN_MIN);
> > +
> > + exposure_max = mode->vts_def - GC05A2_EXP_MARGIN;
> > + gc05a2->exposure = v4l2_ctrl_new_std(ctrl_hdlr,
> &gc05a2_ctrl_ops,
> > + V4L2_CID_EXPOSURE,
> GC05A2_EXP_MIN,
> > + exposure_max,
> GC05A2_EXP_STEP,
> > + exposure_max);
> > +
> > + v4l2_ctrl_new_std_menu_items(ctrl_hdlr, &gc05a2_ctrl_ops,
> > + V4L2_CID_TEST_PATTERN,
> >
> + ARRAY_SIZE(gc05a2_test_pattern_m
> enu) - 1,
> > + 0, 0,
> gc05a2_test_pattern_menu);
> > +
> > + /* register properties to fwnode (e.g. rotation,
> orientation) */
> > + ret = v4l2_fwnode_device_parse(&client->dev, &props);
> > + if (ret)
> > + goto error_ctrls;
> > +
> > + ret = v4l2_ctrl_new_fwnode_properties(ctrl_hdlr, ops,
> &props);
> > + if (ret)
> > + goto error_ctrls;
> > +
> > + if (ctrl_hdlr->error) {
> > + ret = ctrl_hdlr->error;
> > + goto error_ctrls;
> > + }
> > +
> > + gc05a2->sd.ctrl_handler = ctrl_hdlr;
> > +
> > + return 0;
> > +
> > +error_ctrls:
> > + v4l2_ctrl_handler_free(ctrl_hdlr);
> > +
> > + return ret;
> > +}
> > +
> > +static int gc05a2_probe(struct i2c_client *client)
> > +{
> > + struct device *dev = &client->dev;
> > + struct gc05a2 *gc05a2;
> > + int ret;
> > +
> > + gc05a2 = devm_kzalloc(dev, sizeof(*gc05a2), GFP_KERNEL);
> > + if (!gc05a2)
> > + return -ENOMEM;
> > +
> > + gc05a2->dev = dev;
> > +
> > + ret = gc05a2_parse_fwnode(gc05a2);
> > + if (ret)
> > + return ret;
> > +
> > + gc05a2->regmap = devm_cci_regmap_init_i2c(client, 16);
> > + if (IS_ERR(gc05a2->regmap))
> > + return dev_err_probe(dev, PTR_ERR(gc05a2->regmap),
> > + "failed to init CCI\n");
> > +
> > + gc05a2->xclk = devm_clk_get(dev, NULL);
> > + if (IS_ERR(gc05a2->xclk))
> > + return dev_err_probe(dev, PTR_ERR(gc05a2->xclk),
> > + "failed to get xclk\n");
> > +
> > + ret = clk_set_rate(gc05a2->xclk, GC05A2_DEFAULT_CLK_FREQ);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "failed to set xclk
> frequency\n");
> > +
> > + ret = gc05a2_get_regulators(dev, gc05a2);
> > + if (ret < 0)
> > + return dev_err_probe(dev, ret,
> > + "failed to get regulators\n");
> > +
> > + gc05a2->reset_gpio = devm_gpiod_get(dev, "reset",
> GPIOD_OUT_LOW);
> > + if (IS_ERR(gc05a2->reset_gpio))
> > + return dev_err_probe(dev, PTR_ERR(gc05a2-
> >reset_gpio),
> > + "failed to get gpio\n");
> > +
> > + v4l2_i2c_subdev_init(&gc05a2->sd, client,
> &gc05a2_subdev_ops);
> > + gc05a2->sd.internal_ops = &gc05a2_internal_ops;
> > + gc05a2->cur_mode = &gc05a2_modes[0];
> > +
> > + ret = gc05a2_init_controls(gc05a2);
> > + if (ret)
> > + return dev_err_probe(dev, ret,
> > + "failed to init controls\n");
> > +
> > + gc05a2->sd.flags |= V4L2_SUBDEV_FL_HAS_DEVNODE |
> > + V4L2_SUBDEV_FL_HAS_EVENTS;
> > + gc05a2->pad.flags = MEDIA_PAD_FL_SOURCE;
> > + gc05a2->sd.dev = &client->dev;
> > + gc05a2->sd.entity.function = MEDIA_ENT_F_CAM_SENSOR;
> > +
> > + ret = media_entity_pads_init(&gc05a2->sd.entity, 1,
> &gc05a2->pad);
> > + if (ret < 0) {
> > + dev_err(dev, "could not register media entity\n");
> > + goto err_v4l2_ctrl_handler_free;
> > + }
> > +
> > + gc05a2->sd.state_lock = gc05a2->ctrls.lock;
> > + ret = v4l2_subdev_init_finalize(&gc05a2->sd);
> > + if (ret < 0) {
> > + dev_err(dev, "v4l2 subdev init error: %d\n", ret);
> > + goto err_media_entity_cleanup;
> > + }
> > +
> > + pm_runtime_set_active(gc05a2->dev);
> > + pm_runtime_enable(gc05a2->dev);
> > + pm_runtime_set_autosuspend_delay(gc05a2->dev, 1000);
> > + pm_runtime_use_autosuspend(gc05a2->dev);
> > + pm_runtime_idle(gc05a2->dev);
> > +
> > + ret = v4l2_async_register_subdev_sensor(&gc05a2->sd);
> > + if (ret < 0) {
> > + dev_err(dev, "could not register v4l2 device\n");
> > + goto err_rpm;
> > + }
> > +
> > + return 0;
> > +
> > +err_rpm:
> > + pm_runtime_disable(gc05a2->dev);
> > + v4l2_subdev_cleanup(&gc05a2->sd);
> > +
> > +err_media_entity_cleanup:
> > + media_entity_cleanup(&gc05a2->sd.entity);
> > +
> > +err_v4l2_ctrl_handler_free:
> > + v4l2_ctrl_handler_free(&gc05a2->ctrls);
> > +
> > + return ret;
> > +}
> > +
> > +static void gc05a2_remove(struct i2c_client *client)
> > +{
> > + struct v4l2_subdev *sd = i2c_get_clientdata(client);
> > + struct gc05a2 *gc05a2 = to_gc05a2(sd);
> > +
> > + v4l2_async_unregister_subdev(&gc05a2->sd);
> > + v4l2_subdev_cleanup(sd);
> > + media_entity_cleanup(&gc05a2->sd.entity);
> > + v4l2_ctrl_handler_free(&gc05a2->ctrls);
> > +
> > + pm_runtime_disable(&client->dev);
> > + if (!pm_runtime_status_suspended(&client->dev))
> > + gc05a2_power_off(gc05a2->dev);
> > + pm_runtime_set_suspended(&client->dev);
> > +}
> > +
> > +static const struct of_device_id gc05a2_of_match[] = {
> > + { .compatible = "galaxycore,gc05a2" },
> > + {}
> > +};
> > +MODULE_DEVICE_TABLE(of, gc05a2_of_match);
> > +
> > +static DEFINE_RUNTIME_DEV_PM_OPS(gc05a2_pm_ops,
> > + gc05a2_power_off,
> > + gc05a2_power_on,
> > + NULL);
> > +
> > +static struct i2c_driver gc05a2_i2c_driver = {
> > + .driver = {
> > + .of_match_table = gc05a2_of_match,
> > + .pm = pm_ptr(&gc05a2_pm_ops),
> > + .name = "gc05a2",
> > + },
> > + .probe = gc05a2_probe,
> > + .remove = gc05a2_remove,
> > +};
> > +module_i2c_driver(gc05a2_i2c_driver);
> > +
> > +MODULE_DESCRIPTION("GalaxyCore gc05a2 Camera driver");
> > +MODULE_AUTHOR("Zhi Mao <zhi.mao@mediatek.com>");
> > +MODULE_LICENSE("GPL");
> > --
> > 2.25.1
> >
^ permalink raw reply
* Re: [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document
From: Ondřej Jirman @ 2024-04-08 11:51 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Pavel Machek, phone-devel, kernel list, fiona.klute, martijn,
samuel, heikki.krogerus, gregkh, linux-usb, robh+dt,
krzysztof.kozlowski+dt, devicetree
In-Reply-To: <ab9affc8-de68-4ec9-bdfc-02131191bc3a@linaro.org>
Hi Krzysztof,
On Mon, Apr 08, 2024 at 01:17:32PM GMT, Krzysztof Kozlowski wrote:
> On 08/04/2024 12:51, Pavel Machek wrote:
> > 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 <pavel@ucw.cz>
>
> ...
>
> > + 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.
Which schematics?
ANX7688 has VCONN1/2_EN GPIOs that control a switching of VCONN power supply
to resective CCx pins. That's just a switch signal. Power for VCONN needs
to come from somewhere and the driver needs to enable the regulator at
the appropriate time only.
On Pinephone it can't be an always on power supply and needs to be enabled
only when used due to HW design of the circuit. (default without ANX driver
initialized would be to shove 5V to both CC pins, which breaks Type-C spec)
I2C supply is needed for I2C bus to work, apparently. There's nothing
that says that I2C pull-ups supply has to come from supplies powering the
chip. I2C I/O is open drain and the device needs to enable a bus supply
in order to communicate.
You can say that bus master should be managing the bus supply, but you'd still
have a problem that each device may be behind a voltage translator, and
logically, bus master driver should care only about its side of the bus then.
Both side of level shifter need the pull-up power enabled.
You can also make an argument that bus supply can be always on, but that
causes several other issues on Pinephone due to shared nature of most
resources like these. There are other devices on shared power rails, that
need to be turned off during sleep, etc.
Kind regards,
o.
> I think this represents some other part of component which was added
> here only for convenience.
>
>
>
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document
From: Ondřej Jirman @ 2024-04-08 11:52 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: Pavel Machek, phone-devel, kernel list, fiona.klute, martijn,
samuel, heikki.krogerus, gregkh, linux-usb, robh+dt,
krzysztof.kozlowski+dt, devicetree
In-Reply-To: <e7841ad2-fa3d-442d-804d-51f12e05c234@linaro.org>
On Mon, Apr 08, 2024 at 01:24:03PM GMT, Krzysztof Kozlowski wrote:
> On 08/04/2024 13:21, Pavel Machek wrote:
> > 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 <pavel@ucw.cz>
> >>
> >> ...
> >>
> >>> + 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?
>
> The schematics you linked in the document at the beginning. Page 13. Do
> you see these pins there? I saw only VCONN1_EN, but that's not a supply.
The supply is U1308.
regards,
o.
> Best regards,
> Krzysztof
>
^ permalink raw reply
* Re: [PATCH] dt-bindings: extcon: ptn5150: Document the 'port' node
From: Fabio Estevam @ 2024-04-08 11:58 UTC (permalink / raw)
To: Krzysztof Kozlowski
Cc: cw00.choi, myungjoo.ham, robh, conor+dt, devicetree, marex,
Fabio Estevam
In-Reply-To: <c6edf937-dd58-44f7-b620-09dd452f6921@kernel.org>
On Sat, Apr 6, 2024 at 8:26 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > Would it be OK if I send a v2 without the USB C connector description
> > and address your other comments?
>
> No, because I think this should be the connector. Look at datasheet of
> ptn5150 and ptn5110. Aren't both describing similar hardware?
>
> Instead adding some sort of hacked-hardware-representation, please
> investigate why your previous commit broke things.
Yes, you are right.
I don't have access to any board with a PTN5150, so I can't debug it myself.
^ permalink raw reply
* Re: [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document
From: Krzysztof Kozlowski @ 2024-04-08 11:59 UTC (permalink / raw)
To: Ondřej Jirman, Pavel Machek, phone-devel, kernel list,
fiona.klute, martijn, samuel, heikki.krogerus, gregkh, linux-usb,
robh+dt, krzysztof.kozlowski+dt, devicetree
In-Reply-To: <e6vvuttix5k5fioy7q44ick5wj6u5gleh7mht36s4zjjcym7vy@bziejyohtc4b>
On 08/04/2024 13:52, Ondřej Jirman wrote:
> On Mon, Apr 08, 2024 at 01:24:03PM GMT, Krzysztof Kozlowski wrote:
>> On 08/04/2024 13:21, Pavel Machek wrote:
>>> 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 <pavel@ucw.cz>
>>>>
>>>> ...
>>>>
>>>>> + 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?
>>
>> The schematics you linked in the document at the beginning. Page 13. Do
>> you see these pins there? I saw only VCONN1_EN, but that's not a supply.
>
> The supply is U1308.
That's not a supply to anx7688.
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCHv3 1/2] dt-bindings: usb: typec: anx7688: start a binding document
From: Krzysztof Kozlowski @ 2024-04-08 12:01 UTC (permalink / raw)
To: Ondřej Jirman, Pavel Machek, phone-devel, kernel list,
fiona.klute, martijn, samuel, heikki.krogerus, gregkh, linux-usb,
robh+dt, krzysztof.kozlowski+dt, devicetree
In-Reply-To: <35tqaktf533qtpaquvzb7p5juupjyakktstlqgr2hqretnt7lv@chubnabkyqjz>
On 08/04/2024 13:51, Ondřej Jirman wrote:
> Hi Krzysztof,
>
> On Mon, Apr 08, 2024 at 01:17:32PM GMT, Krzysztof Kozlowski wrote:
>> On 08/04/2024 12:51, Pavel Machek wrote:
>>> 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 <pavel@ucw.cz>
>>
>> ...
>>
>>> + 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.
>
> Which schematics?
>
> ANX7688 has VCONN1/2_EN GPIOs that control a switching of VCONN power supply
> to resective CCx pins. That's just a switch signal. Power for VCONN needs
> to come from somewhere and the driver needs to enable the regulator at
> the appropriate time only.
>
> On Pinephone it can't be an always on power supply and needs to be enabled
> only when used due to HW design of the circuit. (default without ANX driver
> initialized would be to shove 5V to both CC pins, which breaks Type-C spec)
>
> I2C supply is needed for I2C bus to work, apparently. There's nothing
> that says that I2C pull-ups supply has to come from supplies powering the
> chip. I2C I/O is open drain and the device needs to enable a bus supply
> in order to communicate.
No, that's misunderstanding of DT. These are not supplies to anx7688.
Bus supply is not related to anx7688.
>
> You can say that bus master should be managing the bus supply, but you'd still
> have a problem that each device may be behind a voltage translator, and
> logically, bus master driver should care only about its side of the bus then.
> Both side of level shifter need the pull-up power enabled.
Again, that's nothing related to anx7688. If you want to add it here,
why not to every device everywhere?
>
> You can also make an argument that bus supply can be always on, but that
> causes several other issues on Pinephone due to shared nature of most
> resources like these. There are other devices on shared power rails, that
> need to be turned off during sleep, etc.
>
No, do not add non-existing properties on this device as workaround for
other issues.
Please drop these two supplies *and all other which do not exist* on
anx7688.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v6 1/2] media: dt-bindings: nxp,imx8-jpeg: Add clocks entries
From: Mirela Rabulea @ 2024-04-08 12:06 UTC (permalink / raw)
To: shawnguo, robh+dt, krzysztof.kozlowski+dt, festevam, festevam,
alexander.stein, Frank.li, ming.qian
Cc: conor+dt, devicetree, linux-arm-kernel, s.hauer, kernel, mchehab,
hverkuil, linux-media, imx, linux-kernel
From: Fabio Estevam <festevam@denx.de>
The JPEG decoder/encoder present in iMX8QXP and iMX8QM SoCs need
the PER and IPG clocks to be functional, so add the clock entries.
This also fixes the following schema warning:
imx8qm-apalis-eval.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
Signed-off-by: Fabio Estevam <festevam@denx.de>
Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
---
Changes since v5:
- Remove one extra ":" from subject (thanks Fabio!)
- Use <festevam@denx.de> address for both Author and Signed-of (Fabio's feedback)
Changes since v4:
- Remove redundant description and update subject prefix (per Krzysztof's feddback)
Changes since v3:
- Add items for clocks (per Krzysztof's feddback)
- Add description for clocks (per Conor's feddback to the other similar patch from Alexander)
- Add "media:" to the subject
- Add Mirela's signed-off
- For the similar patches that were sent for this issue, should Co-developed-by/Signed-off-by be added? Alexander Stein? Frank Li?
Changes since v2:
- Remove clock-names. (Mirela)
.../devicetree/bindings/media/nxp,imx8-jpeg.yaml | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
index 3d9d1db37040..2be30c5fdc83 100644
--- a/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
+++ b/Documentation/devicetree/bindings/media/nxp,imx8-jpeg.yaml
@@ -31,6 +31,11 @@ properties:
reg:
maxItems: 1
+ clocks:
+ items:
+ - description: AXI DMA engine clock for fetching JPEG bitstream from memory (per)
+ - description: IP bus clock for register access (ipg)
+
interrupts:
description: |
There are 4 slots available in the IP, which the driver may use
@@ -49,6 +54,7 @@ properties:
required:
- compatible
- reg
+ - clocks
- interrupts
- power-domains
@@ -56,12 +62,15 @@ additionalProperties: false
examples:
- |
+ #include <dt-bindings/clock/imx8-lpcg.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
#include <dt-bindings/firmware/imx/rsrc.h>
jpegdec: jpegdec@58400000 {
compatible = "nxp,imx8qxp-jpgdec";
reg = <0x58400000 0x00050000 >;
+ clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
+ <&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 310 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 311 IRQ_TYPE_LEVEL_HIGH>,
@@ -76,6 +85,8 @@ examples:
jpegenc: jpegenc@58450000 {
compatible = "nxp,imx8qm-jpgenc", "nxp,imx8qxp-jpgenc";
reg = <0x58450000 0x00050000 >;
+ clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
+ <&img_jpeg__lpcg IMX_LPCG_CLK_4>;
interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 306 IRQ_TYPE_LEVEL_HIGH>,
<GIC_SPI 307 IRQ_TYPE_LEVEL_HIGH>,
--
2.25.1
^ permalink raw reply related
* [PATCH v6 2/2] arm64: dts: imx8-ss-img: Remove JPEG clock-names
From: Mirela Rabulea @ 2024-04-08 12:06 UTC (permalink / raw)
To: shawnguo, robh+dt, krzysztof.kozlowski+dt, festevam, festevam,
alexander.stein, Frank.li, ming.qian
Cc: conor+dt, devicetree, linux-arm-kernel, s.hauer, kernel, mchehab,
hverkuil, linux-media, imx, linux-kernel
In-Reply-To: <20240408120654.1196880-1-mirela.rabulea@nxp.com>
From: Fabio Estevam <festevam@denx.de>
Per nxp,imx8-jpeg.yaml, the clock-names entry is not valid.
Remove them.
Signed-off-by: Fabio Estevam <festevam@denx.de>
---
Changes since v5:
- Use <festevam@denx.de> address for both Author and Signed-of (Fabio's feedback)
Changes since v4:
- None
Changes since v3:
- Just added "imx8-ss-img:" in the subject
Changes since v2:
- Newly introduced.
arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi | 2 --
1 file changed, 2 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
index e7783cc2d830..77d2928997b4 100644
--- a/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8-ss-img.dtsi
@@ -21,7 +21,6 @@ jpegdec: jpegdec@58400000 {
interrupts = <GIC_SPI 309 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
<&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
- clock-names = "per", "ipg";
assigned-clocks = <&img_jpeg_dec_lpcg IMX_LPCG_CLK_0>,
<&img_jpeg_dec_lpcg IMX_LPCG_CLK_4>;
assigned-clock-rates = <200000000>, <200000000>;
@@ -35,7 +34,6 @@ jpegenc: jpegenc@58450000 {
interrupts = <GIC_SPI 305 IRQ_TYPE_LEVEL_HIGH>;
clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
<&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
- clock-names = "per", "ipg";
assigned-clocks = <&img_jpeg_enc_lpcg IMX_LPCG_CLK_0>,
<&img_jpeg_enc_lpcg IMX_LPCG_CLK_4>;
assigned-clock-rates = <200000000>, <200000000>;
--
2.25.1
^ permalink raw reply related
* [PATCH V2 1/2] arm64: dts: imx8mp-msc-sm2s: do not write i2c pinctrl reserved bit
From: Ian Ray @ 2024-04-08 12:23 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Ian Ray, devicetree, imx, linux-arm-kernel, linux-kernel
Better not to write to the reserved bit.
Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
---
.../boot/dts/freescale/imx8mp-msc-sm2s.dtsi | 24 +++++++++----------
1 file changed, 12 insertions(+), 12 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
index 61c2a63efc6d..940bdbe115a3 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
@@ -602,38 +602,38 @@ pinctrl_flexspi0: flexspi0grp {
pinctrl_i2c1: i2c1grp {
fsl,pins =
- <MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL 0x400001c3>,
- <MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA 0x400001c3>;
+ <MX8MP_IOMUXC_I2C1_SCL__I2C1_SCL 0x400001c2>,
+ <MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA 0x400001c2>;
};
pinctrl_i2c2: i2c2grp {
fsl,pins =
- <MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL 0x400001c3>,
- <MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA 0x400001c3>;
+ <MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL 0x400001c2>,
+ <MX8MP_IOMUXC_I2C2_SDA__I2C2_SDA 0x400001c2>;
};
pinctrl_i2c3: i2c3grp {
fsl,pins =
- <MX8MP_IOMUXC_I2C3_SCL__I2C3_SCL 0x400001c3>,
- <MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA 0x400001c3>;
+ <MX8MP_IOMUXC_I2C3_SCL__I2C3_SCL 0x400001c2>,
+ <MX8MP_IOMUXC_I2C3_SDA__I2C3_SDA 0x400001c2>;
};
pinctrl_i2c4: i2c4grp {
fsl,pins =
- <MX8MP_IOMUXC_I2C4_SCL__I2C4_SCL 0x400001c3>,
- <MX8MP_IOMUXC_I2C4_SDA__I2C4_SDA 0x400001c3>;
+ <MX8MP_IOMUXC_I2C4_SCL__I2C4_SCL 0x400001c2>,
+ <MX8MP_IOMUXC_I2C4_SDA__I2C4_SDA 0x400001c2>;
};
pinctrl_i2c5: i2c5grp {
fsl,pins =
- <MX8MP_IOMUXC_SPDIF_TX__I2C5_SCL 0x400001c3>,
- <MX8MP_IOMUXC_SPDIF_RX__I2C5_SDA 0x400001c3>;
+ <MX8MP_IOMUXC_SPDIF_TX__I2C5_SCL 0x400001c2>,
+ <MX8MP_IOMUXC_SPDIF_RX__I2C5_SDA 0x400001c2>;
};
pinctrl_i2c6: i2c6grp {
fsl,pins =
- <MX8MP_IOMUXC_SAI5_RXFS__I2C6_SCL 0x400001c3>,
- <MX8MP_IOMUXC_SAI5_RXC__I2C6_SDA 0x400001c3>;
+ <MX8MP_IOMUXC_SAI5_RXFS__I2C6_SCL 0x400001c2>,
+ <MX8MP_IOMUXC_SAI5_RXC__I2C6_SDA 0x400001c2>;
};
pinctrl_lcd0_backlight: lcd0-backlightgrp {
--
2.39.2
^ permalink raw reply related
* [PATCH V2 2/2] arm64: dts: imx8mp-msc-sm2s: Add i2c{1,6} sda-/scl-gpios
From: Ian Ray @ 2024-04-08 12:23 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Shawn Guo,
Sascha Hauer, Pengutronix Kernel Team, Fabio Estevam
Cc: Ian Ray, devicetree, imx, linux-arm-kernel, linux-kernel
In-Reply-To: <20240408122321.464-1-ian.ray@gehealthcare.com>
Add i2c{1,6} sda-/scl-gpios with the corresponding pinmux entries.
Signed-off-by: Ian Ray <ian.ray@gehealthcare.com>
---
.../boot/dts/freescale/imx8mp-msc-sm2s.dtsi | 22 +++++++++++++++++--
1 file changed, 20 insertions(+), 2 deletions(-)
diff --git a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
index 940bdbe115a3..aeb557fe9dd6 100644
--- a/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8mp-msc-sm2s.dtsi
@@ -200,8 +200,11 @@ ethphy1: ethernet-phy@1 {
};
&i2c1 {
- pinctrl-names = "default";
+ pinctrl-names = "default", "gpio";
pinctrl-0 = <&pinctrl_i2c1>;
+ pinctrl-1 = <&pinctrl_i2c1_gpio>;
+ scl-gpios = <&gpio5 14 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+ sda-gpios = <&gpio5 15 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
clock-frequency = <400000>;
status = "okay";
@@ -241,8 +244,11 @@ &i2c5 {
};
&i2c6 {
- pinctrl-names = "default";
+ pinctrl-names = "default", "gpio";
pinctrl-0 = <&pinctrl_i2c6>;
+ pinctrl-1 = <&pinctrl_i2c6_gpio>;
+ scl-gpios = <&gpio3 19 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
+ sda-gpios = <&gpio3 20 (GPIO_ACTIVE_HIGH | GPIO_OPEN_DRAIN)>;
clock-frequency = <400000>;
status = "okay";
@@ -606,6 +612,12 @@ pinctrl_i2c1: i2c1grp {
<MX8MP_IOMUXC_I2C1_SDA__I2C1_SDA 0x400001c2>;
};
+ pinctrl_i2c1_gpio: i2c1gpiogrp {
+ fsl,pins =
+ <MX8MP_IOMUXC_I2C1_SCL__GPIO5_IO14 0x1c2>,
+ <MX8MP_IOMUXC_I2C1_SDA__GPIO5_IO15 0x1c2>;
+ };
+
pinctrl_i2c2: i2c2grp {
fsl,pins =
<MX8MP_IOMUXC_I2C2_SCL__I2C2_SCL 0x400001c2>,
@@ -636,6 +648,12 @@ pinctrl_i2c6: i2c6grp {
<MX8MP_IOMUXC_SAI5_RXC__I2C6_SDA 0x400001c2>;
};
+ pinctrl_i2c6_gpio: i2c6gpiogrp {
+ fsl,pins =
+ <MX8MP_IOMUXC_SAI5_RXFS__GPIO3_IO19 0x1c2>,
+ <MX8MP_IOMUXC_SAI5_RXC__GPIO3_IO20 0x1c2>;
+ };
+
pinctrl_lcd0_backlight: lcd0-backlightgrp {
fsl,pins =
<MX8MP_IOMUXC_GPIO1_IO05__GPIO1_IO05 0x41>;
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v6 1/2] media: dt-bindings: nxp,imx8-jpeg: Add clocks entries
From: Krzysztof Kozlowski @ 2024-04-08 12:31 UTC (permalink / raw)
To: Mirela Rabulea, shawnguo, robh+dt, krzysztof.kozlowski+dt,
festevam, festevam, alexander.stein, Frank.li, ming.qian
Cc: conor+dt, devicetree, linux-arm-kernel, s.hauer, kernel, mchehab,
hverkuil, linux-media, imx, linux-kernel
In-Reply-To: <20240408120654.1196880-1-mirela.rabulea@nxp.com>
On 08/04/2024 14:06, Mirela Rabulea wrote:
> From: Fabio Estevam <festevam@denx.de>
>
> The JPEG decoder/encoder present in iMX8QXP and iMX8QM SoCs need
> the PER and IPG clocks to be functional, so add the clock entries.
>
> This also fixes the following schema warning:
>
> imx8qm-apalis-eval.dtb: jpegdec@58400000: 'assigned-clock-rates', 'assigned-clocks', 'clock-names', 'clocks' do not match any of the regexes: 'pinctrl-[0-9]+'
> from schema $id: http://devicetree.org/schemas/media/nxp,imx8-jpeg.yaml#
>
> Signed-off-by: Fabio Estevam <festevam@denx.de>
> Signed-off-by: Mirela Rabulea <mirela.rabulea@nxp.com>
> ---
This is like third version today? Give people chance to review your code
and wait 24h before postings.
Best regards,
Krzysztof
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox