* [PATCH v3 25/25] media:i2c: imx258: Use v4l2_link_freq_to_bitmap helper
From: git @ 2024-04-03 15:03 UTC (permalink / raw)
To: linux-media
Cc: dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, pavel, phone-devel, Luis Garcia
In-Reply-To: <20240403150355.189229-1-git@luigi311.com>
From: Luis Garcia <git@luigi311.com>
Use the v4l2_link_freq_to_bitmap() helper to figure out which
driver-supported link freq can be used on a given system.
Signed-off-by: Luis Garcia <git@luigi311.com>
---
drivers/media/i2c/imx258.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
index 4c117c4829f1..038f40a1f800 100644
--- a/drivers/media/i2c/imx258.c
+++ b/drivers/media/i2c/imx258.c
@@ -674,6 +674,7 @@ struct imx258 {
/* Current mode */
const struct imx258_mode *cur_mode;
+ unsigned long link_freq_bitmap;
const struct imx258_link_freq_config *link_freq_configs;
const s64 *link_freq_menu_items;
unsigned int lane_mode_idx;
@@ -1533,6 +1534,17 @@ static int imx258_probe(struct i2c_client *client)
return ret;
}
+ ret = v4l2_link_freq_to_bitmap(&client->dev,
+ ep.link_frequencies,
+ ep.nr_of_link_frequencies,
+ imx258->link_freq_menu_items,
+ ARRAY_SIZE(link_freq_menu_items_19_2),
+ &imx258->link_freq_bitmap);
+ if (ret) {
+ dev_err(&client->dev, "Link frequency not supported\n");
+ goto error_endpoint_free;
+ }
+
/* Get number of data lanes */
switch (ep.bus.mipi_csi2.num_data_lanes) {
case 2:
--
2.42.0
^ permalink raw reply related
* Re: Fixing the devicetree of Rock 5 Model B (and possibly others)
From: Pratham Patel @ 2024-04-03 15:26 UTC (permalink / raw)
To: Sebastian Reichel
Cc: Saravana Kannan, Dragan Simic, devicetree, linux-arm-kernel,
linux-rockchip, linux-kernel, regressions, stable
On Wed Apr 3, 2024 at 7:22 PM IST, Sebastian Reichel wrote:
> Hi,
>
> On Wed, Apr 03, 2024 at 01:03:07AM +0000, Pratham Patel wrote:
> > > > > Also, can you give the output of <debugfs>/devices_deferred for the
> > > > > good vs bad case?
> > > >
> > > > I can't provide you with requested output from the bad case, since the
> > > > kernel never moves past this to an initramfs rescue shell, but following
> > > > is the output from v6.8.1 (**with aforementioned patch reverted**).
> > > >
> > > > # cat /sys/kernel/debug/devices_deferred
> > > > fc400000.usb platform: wait for supplier /phy@fed90000/usb3-port
> > > > 1-0022 typec_fusb302: cannot register tcpm port
> > > > fc000000.usb platform: wait for supplier /phy@fed80000/usb3-port
> > > >
> > > > It seems that v6.8.2 works _without needing to revert the patch_. I will
> > > > have to look into this sometime this week but it seems like
> > > > a8037ceb8964 (arm64: dts: rockchip: drop rockchip,trcm-sync-tx-only from rk3588 i2s)
> > > > seems to be the one that fixed the root issue. I will have to test it
> > > > sometime later this week.
> > >
> > > Ok, once you find the patch that fixes things, let me know too.
> >
> > Will do!
>
> FWIW the v6.8.1 kernel referenced above is definitely patched, since
> upstream's Rock 5B DT does neither describe fusb302, nor the USB
> port it is connected to.
>
> We have a few Rock 5B in Kernel CI and upstream boots perfectly
> fine:
>
> https://lava.collabora.dev/scheduler/device_type/rk3588-rock-5b
Hmm, weird then. I can confirm that v6.8.1 doesn't _always_ boot. It
boots some times but still fails a majority of times. There is a
2 out of 10 chance that v6.8.1 will not boot. If you keep rebooting
enough times, you might get it to boot but the next boot is
likely to be borked. :(
That said, v6.8.2 might still have the same issue, but the probably of a
failed boot might be _lesser_ than v6.8.1 (from what I saw). I will
verify that behaviour sometime tomorrow or day after tomorrow.
>
> So it could be one of your downstream patches, which is introducing
> this problem.
I thought so too. So I built a vanilla kernel from the release tarball
of v6.8.1, using GCC + arm64 defconfig. I also tried using LLVM just in
case but noticed the same result.
-- Pratham Patel
^ permalink raw reply
* Re: [PATCH net-next v6 11/17] dt-bindings: net: pse-pd: Add another way of describing several PSE PIs
From: Oleksij Rempel @ 2024-04-03 15:27 UTC (permalink / raw)
To: Rob Herring
Cc: Kory Maincent, David S. Miller, Eric Dumazet, Jakub Kicinski,
Paolo Abeni, Jonathan Corbet, Luis Chamberlain, Russ Weight,
Greg Kroah-Hartman, Rafael J. Wysocki, Krzysztof Kozlowski,
Conor Dooley, Mark Brown, Frank Rowand, Andrew Lunn,
Heiner Kallweit, Russell King, Thomas Petazzoni, netdev,
linux-kernel, linux-doc, devicetree, Dent Project
In-Reply-To: <20240403144448.GB3508225-robh@kernel.org>
On Wed, Apr 03, 2024 at 09:44:48AM -0500, Rob Herring wrote:
> On Tue, Apr 02, 2024 at 05:47:58PM +0200, Oleksij Rempel wrote:
> > On Tue, Apr 02, 2024 at 08:26:37AM -0500, Rob Herring wrote:
> > > > + pairsets:
> > > > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > > > + description:
> > > > + List of phandles, each pointing to the power supply for the
> > > > + corresponding pairset named in 'pairset-names'. This property
> > > > + aligns with IEEE 802.3-2022, Section 33.2.3 and 145.2.4.
> > > > + PSE Pinout Alternatives (as per IEEE 802.3-2022 Table 145\u20133)
> > > > + |-----------|---------------|---------------|---------------|---------------|
> > > > + | Conductor | Alternative A | Alternative A | Alternative B | Alternative B |
> > > > + | | (MDI-X) | (MDI) | (X) | (S) |
> > > > + |-----------|---------------|---------------|---------------|---------------|
> > > > + | 1 | Negative VPSE | Positive VPSE | \u2014 | \u2014 |
> > > > + | 2 | Negative VPSE | Positive VPSE | \u2014 | \u2014 |
> > > > + | 3 | Positive VPSE | Negative VPSE | \u2014 | \u2014 |
> > > > + | 4 | \u2014 | \u2014 | Negative VPSE | Positive VPSE |
> > > > + | 5 | \u2014 | \u2014 | Negative VPSE | Positive VPSE |
> > > > + | 6 | Positive VPSE | Negative VPSE | \u2014 | \u2014 |
> > > > + | 7 | \u2014 | \u2014 | Positive VPSE | Negative VPSE |
> > > > + | 8 | \u2014 | \u2014 | Positive VPSE | Negative VPSE |
> > > > + minItems: 1
> > > > + maxItems: 2
> > >
> > > "pairsets" does not follow the normal design pattern of foos, foo-names,
> > > and #foo-cells. You could add #foo-cells I suppose, but what would cells
> > > convey? I don't think it's a good fit for what you need.
> > >
> > > The other oddity is the number of entries and the names are fixed. That
> > > is usually defined per consumer.
> > >
> > > As each entry is just a power rail, why can't the regulator binding be
> > > used here?
> >
> > I'm not against describing it consequent with regulator till the wire
> > end, but right now I have no idea how it should be described by using
> > regulator bindings. There are maximum 2 rails going in to PSE PI on one
> > side and 4 rails with at least 5 combinations supported by standard on
> > other side. Instead of inventing anything new, I suggested to describe
> > supported output combinations by using IEEE 802.3 standard.
>
> There's 4 combinations above, what's the 5th combination? SPE?
The 5th combination is PoE4 where two rails are supplying power at same
time.
First 4 variants for PoE: one or two positive rails are attached (but
only one is used at same time) to pairs 1-2 or 3-4, or 5-6, or 7-8. Or
support all of combinations if some advanced PSE PI is present. PSE PI
is kind of MUX for regulators.
One more variant in case of PoE4: two positive rail are attached at same
time, one to 1-2, second to 5-6. May be one more variant with opposite
polarity, this will be the 6th combination.
> Seems to me you just describe the 2 rails going to the connector and
> then describe all the variations the connector supports. The PSE
> (h/w) has little to do with which variations are supported, right?
No. In case of mutli-channel PSE, it needs to know if channels are
attached to one port or to different ports. PSE is not only responsible
to enable the power, it runs classification of devices attached to the
port, so it will decide, which rail should be enabled.
> For example, MDI-X vs. MDI support is determined by the PHY, right?
Yes and No. Until PSE do not start supplying power, PHY will not be able to
start communication with the remote PHY, so it will not be able to
detect MDI/X configuration.
Polarity configuration is important for user space or user to get
information about supported pin configuration and if possible,
change the configuration.
> Or it has to be supported by both the PHY and PSE?
In most cases PSE and PHY work independently from each other, they just
share same port. Potential exception are:
- in case data line should not be shared with power lines, we need to
know what pins are used for power, this information would help to
provide PHY configuration.
- in case PHY autoneg signals disturb PoE classification, we need to
coordinate PHY and PSE states.
Regards,
Oleksij
--
Pengutronix e.K. | |
Steuerwalder Str. 21 | http://www.pengutronix.de/ |
31137 Hildesheim, Germany | Phone: +49-5121-206917-0 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-5555 |
^ permalink raw reply
* Re: [PATCH v4 0/5] Pinephone video out fixes (flipping between two frames)
From: Frank Oltmanns @ 2024-04-03 15:31 UTC (permalink / raw)
To: Michael Turquette, Stephen Boyd, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Maxime Ripard
Cc: Guido Günther, Purism Kernel Team, Ondrej Jirman,
Neil Armstrong, Jessica Zhang, Sam Ravnborg, Maarten Lankhorst,
Thomas Zimmermann, David Airlie, Daniel Vetter, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, linux-clk, linux-arm-kernel,
linux-sunxi, linux-kernel, dri-devel, devicetree, stable,
Diego Roversi, Erico Nunes
In-Reply-To: <20240310-pinephone-pll-fixes-v4-0-46fc80c83637@oltmanns.dev>
Dear clk and sunxi-ng maintainers,
Patches 1-4 have been reviewed and there are no pending issues. If there
is something else you need me to do to get this applied, please let me
know.
Thanks,
Frank
On 2024-03-10 at 14:21:10 +0100, Frank Oltmanns <frank@oltmanns.dev> wrote:
> On some pinephones the video output sometimes freezes (flips between two
> frames) [1]. It seems to be that the reason for this behaviour is that
> PLL-MIPI is outside its limits, and the GPU is not running at a fixed
> rate.
>
> In this patch series I propose the following changes:
> 1. sunxi-ng: Adhere to the following constraints given in the
> Allwinner A64 Manual regarding PLL-MIPI:
> * M/N <= 3
> * (PLL_VIDEO0)/M >= 24MHz
> * 500MHz <= clockrate <= 1400MHz
>
> 2. Remove two operating points from the A64 DTS OPPs, so that the GPU
> runs at a fixed rate of 432 MHz.
>
> Note, that when pinning the GPU to 432 MHz the issue [1] completely
> disappears for me. I've searched the BSP and could not find any
> indication that supports the idea of having the three OPPs. The only
> frequency I found in the BPSs for A64 is 432 MHz, which has also proven
> stable for me.
>
> I very much appreciate your feedback!
>
> [1] https://gitlab.com/postmarketOS/pmaports/-/issues/805
>
> Signed-off-by: Frank Oltmanns <frank@oltmanns.dev>
> ---
> Changes in v4:
> - sunxi-ng: common: Address review comments.
> - Link to v3: https://lore.kernel.org/r/20240304-pinephone-pll-fixes-v3-0-94ab828f269a@oltmanns.dev
>
> Changes in v3:
> - dts: Pin GPU to 432 MHz.
> - nkm and a64: Move minimum and maximum rate handling to the common part
> of the sunxi-ng driver.
> - Removed st7703 patch from series.
> - Link to v2: https://lore.kernel.org/r/20240205-pinephone-pll-fixes-v2-0-96a46a2d8c9b@oltmanns.dev
>
> Changes in v2:
> - dts: Increase minimum GPU frequency to 192 MHz.
> - nkm and a64: Add minimum and maximum rate for PLL-MIPI.
> - nkm: Use the same approach for skipping invalid rates in
> ccu_nkm_find_best() as in ccu_nkm_find_best_with_parent_adj().
> - nkm: Improve names for ratio struct members and hence get rid of
> describing comments.
> - nkm and a64: Correct description in the commit messages: M/N <= 3
> - Remove patches for nm as they were not needed.
> - st7703: Rework the commit message to cover more background for the
> change.
> - Link to v1: https://lore.kernel.org/r/20231218-pinephone-pll-fixes-v1-0-e238b6ed6dc1@oltmanns.dev
>
> ---
> Frank Oltmanns (5):
> clk: sunxi-ng: common: Support minimum and maximum rate
> clk: sunxi-ng: a64: Set minimum and maximum rate for PLL-MIPI
> clk: sunxi-ng: nkm: Support constraints on m/n ratio and parent rate
> clk: sunxi-ng: a64: Add constraints on PLL-MIPI's n/m ratio and parent rate
> arm64: dts: allwinner: a64: Run GPU at 432 MHz
>
> arch/arm64/boot/dts/allwinner/sun50i-a64.dtsi | 8 --------
> drivers/clk/sunxi-ng/ccu-sun50i-a64.c | 14 +++++++++-----
> drivers/clk/sunxi-ng/ccu_common.c | 19 +++++++++++++++++++
> drivers/clk/sunxi-ng/ccu_common.h | 3 +++
> drivers/clk/sunxi-ng/ccu_nkm.c | 21 +++++++++++++++++++++
> drivers/clk/sunxi-ng/ccu_nkm.h | 2 ++
> 6 files changed, 54 insertions(+), 13 deletions(-)
> ---
> base-commit: dcb6c8ee6acc6c347caec1e73fb900c0f4ff9806
> change-id: 20231218-pinephone-pll-fixes-0ccdfde273e4
>
> Best regards,
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: adc: ad7173: add support for ad411x
From: David Lechner @ 2024-04-03 15:40 UTC (permalink / raw)
To: Ceclan, Dumitru
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel
In-Reply-To: <25cb3514-1281-49a8-9e9b-40ead9b050dc@gmail.com>
On Wed, Apr 3, 2024 at 2:43 AM Ceclan, Dumitru <mitrutzceclan@gmail.com> wrote:
>
> On 01/04/2024 22:37, David Lechner wrote:
> > On Mon, Apr 1, 2024 at 10:10 AM Dumitru Ceclan via B4 Relay
> > <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> >>
> >> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> ...
>
> >> properties:
> >> reg:
> >> + description:
> >> + Reg values 16-19 are only permitted for ad4111/ad4112 current channels.
> >> minimum: 0
> >> - maximum: 15
> >> + maximum: 19
> >
> > This looks wrong. Isn't reg describing the number of logical channels
> > (# of channel config registers)?
> >
> > After reviewing the driver, I see that > 16 is used as a way of
> > flagging current inputs, but still seems like the wrong way to do it.
> > See suggestion below.
> >
>
> This was a suggestion from Jonathan, maybe I implemented it wrong.
> Other alternative that came to my mind: attribute "adi,current-channel".
Having a boolean flag like this would make more sense to me if we
don't agree that the suggestion below is simpler.
> >>
> >> diff-channels:
> >> + description:
> >> + For using current channels specify only the positive channel.
> >> + (IIN2+, IIN2−) -> diff-channels = <2 0>
> >
> > I find this a bit confusing since 2 is already VIN2 and 0 is already
> > VIN0. I think it would make more sense to assign unique channel
> > numbers individually to the negative and positive current inputs.
> > Also, I think it makes sense to use the same numbers that the
> > registers in the datasheet use (8 - 11 for negative and 12 to 15 for
> > positive).
> >
> > So: (IIN2+, IIN2−) -> diff-channels = <13 10>
> >
> >
> It would mean for the user to look in the datasheet at the possible
> channel INPUT configurations values, decode the bit field into two
> integer values and place it here (0110101010) -> 13 10. This is
> cumbersome for just choosing current input 2.
It could be documented in the devicetree bindings, just as it is done
in adi,ad4130.yaml so that users of the bindings don't have to
decipher the datasheet.
>
> >> +
> >> + Family AD411x supports a dedicated VCOM voltage input.
> >> + To select it set the second channel to 16.
> >> + (VIN2, VCOM) -> diff-channels = <2 16>
> >
> > The 411x datasheets call this pin VINCOM so calling it VCOM here is a
> > bit confusing.
> >
>
> Sure, I'll rename to VINCOM.
>
> > Also, do we need to add a vincom-supply to get this voltage? Or is it
> > safe to assume it is always connected to AVSS? The datasheet seems to
> > indicate that the latter is the case. But then it also has this
> > special case (at least for AD4116, didn't check all datasheets)
> > "VIN10, VINCOM (single-ended or differential pair)". If it can be used
> > as part of a fully differential input, we probably need some extra
> > flag to indicate that case.
> >
>
> I cannot see any configuration options for these use cases. All inputs
> are routed to the same mux and routed to the differential positive and
> negative ADC inputs.
>
> "VIN10, VINCOM (single-ended or differential pair)" the only difference
> between these two use cases is if you connected VINCOM to AVSS (with
> unipolar coding) or not with bipolar encoding. The channel is still
> measuring the difference between the two selected inputs and comparing
> to the selected reference.
>
> > Similarly, do we need special handling for ADCIN15 on AD4116? It has a
> > "(pseudo differential or differential pair)" notation that other
> > inputs don't. In other words, it is more like VINCOM than it is to the
> > other ADCINxx pins. So we probably need an adcin15-supply for this pin
> > to properly get the right channel configuration. I.e. the logic in the
> > IIO driver would be if adcin15-supply is present, any channels that
> > use this input are pseudo-differential, otherwise any channels that
> > use it are fully differential.
> >
>
> I cannot seem to understand what would a adcin15-supply be needed for.
> This input, the same as all others, enters the mux and is routed to
> either positive or negative input of the ADC.
>
> The voltage on the ADCIN15 pin is not important to the user, just the
> difference in voltage between that pin and the other one selected.
>
These suggestions come from some recent discussion about
pseudo-differential vs. fully differential inputs (e.g. search the IIO
mailing list for AD7380).
So what I suggested here might be more technically correct according
to what I got out of that discussion. But for this specific case, I
agree it is good enough to just treat all inputs as always
fully-differential to keep things from getting too unwieldy.
> >> items:
> >> minimum: 0
> >> maximum: 31
> >> @@ -166,7 +191,6 @@ allOf:
> >> - $ref: /schemas/spi/spi-peripheral-props.yaml#
> >>
> >> # Only ad7172-4, ad7173-8 and ad7175-8 support vref2
> >> - # Other models have [0-3] channel registers
> >
> > Did you forget to remove
> >
> > reg:
> > maximum: 3
> >
> > from this if statement that this comment is referring to?
> >
> >
>
>
> Other way around, forgot in a previous patch to remove the comment.
> I'll move this change to a precursor patch.
>
> >> - if:
> >> properties:
> >> compatible:
> >> @@ -187,6 +211,37 @@ allOf:
> >> - vref
> >> - refout-avss
> >> - avdd
> >> +
> >> + - if:
> >> + properties:
> >> + compatible:
> >> + contains:
> >> + enum:
> >> + - adi,ad4114
> >> + - adi,ad4115
> >> + - adi,ad4116
> >> + - adi,ad7173-8
> >> + - adi,ad7175-8
> >> + then:
> >> + patternProperties:
> >> + "^channel@[0-9a-f]$":
> >> + properties:
> >> + reg:
> >> + maximum: 15
> >
> > As with the previous reg comment, this if statement should not be
> > needed since maximum should not be changed to 19.
> >
>
> We'll see what is the best approach regarding the current channels,
> perhaps the one you mentioned in the later reply with always configuring
> like the temp channel.
>
> >> +
> >> + - if:
> >> + properties:
> >> + compatible:
> >> + contains:
> >> + enum:
> >> + - adi,ad7172-2
> >> + - adi,ad7175-2
> >> + - adi,ad7176-2
> >> + - adi,ad7177-2
> >> + then:
> >> + patternProperties:
> >> + "^channel@[0-9a-f]$":
> >> + properties:
> >> reg:
> >> maximum: 3
> >
> > It looks to me like AD7172-4 actually has 8 possible channels rather
> > than 16. So it would need a special condition as well. But that is a
> > bug in the previous bindings and should therefore be fixed in a
> > separate patch.
>
> It is addressed already in the binding:
> "
> - if:
> properties:
> compatible:
> contains:
> const: adi,ad7172-4
> [...]
> maximum: 7
> "
Ah, I missed it hiding with adi,reference-select overrides.
^ permalink raw reply
* Re: [PATCH v5 02/10] dt-bindings: mailbox: Add mboxes property for CMDQ secure driver
From: Conor Dooley @ 2024-04-03 15:46 UTC (permalink / raw)
To: Shawn Sung
Cc: CK Hu, Jassi Brar, AngeloGioacchino Del Regno, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Matthias Brugger,
Jason-JH . Lin, Houlong Wei, linux-kernel, devicetree,
linux-arm-kernel, linux-mediatek
In-Reply-To: <20240403102602.32155-3-shawn.sung@mediatek.com>
[-- Attachment #1: Type: text/plain, Size: 1430 bytes --]
On Wed, Apr 03, 2024 at 06:25:54PM +0800, Shawn Sung wrote:
> From: "Jason-JH.Lin" <jason-jh.lin@mediatek.com>
>
> Add mboxes to define a GCE loopping thread as a secure irq handler.
> This property is only required if CMDQ secure driver is supported.
>
> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> Signed-off-by: Hsiao Chien Sung <shawn.sung@mediatek.com>
> ---
> .../bindings/mailbox/mediatek,gce-mailbox.yaml | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> index cef9d76013985..c0d80cc770899 100644
> --- a/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> +++ b/Documentation/devicetree/bindings/mailbox/mediatek,gce-mailbox.yaml
> @@ -49,6 +49,16 @@ properties:
> items:
> - const: gce
>
> + mediatek,gce-events:
> + description:
> + The event id which is mapping to the specific hardware event signal
> + to gce. The event id is defined in the gce header
> + include/dt-bindings/gce/<chip>-gce.h of each chips.
Missing any info here about when this should be used, hint - you have it
in the commit message.
> + $ref: /schemas/types.yaml#/definitions/uint32-arrayi
Why is the ID used by the CMDQ service not fixed for each SoC?
Cheers,
Conor
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v6 2/6] interconnect: icc-clk: Remove tristate from INTERCONNECT_CLK
From: kernel test robot @ 2024-04-03 15:42 UTC (permalink / raw)
To: Varadarajan Narayanan, andersson, konrad.dybcio, mturquette,
sboyd, robh, krzysztof.kozlowski+dt, conor+dt, djakov,
dmitry.baryshkov, quic_anusha, linux-arm-msm, linux-clk,
devicetree, linux-kernel, linux-pm
Cc: oe-kbuild-all, kernel test robot
In-Reply-To: <20240402103406.3638821-3-quic_varada@quicinc.com>
Hi Varadarajan,
kernel test robot noticed the following build errors:
[auto build test ERROR on clk/clk-next]
[also build test ERROR on robh/for-next linus/master v6.9-rc2 next-20240403]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Varadarajan-Narayanan/dt-bindings-interconnect-Add-Qualcomm-IPQ9574-support/20240402-223729
base: https://git.kernel.org/pub/scm/linux/kernel/git/clk/linux.git clk-next
patch link: https://lore.kernel.org/r/20240402103406.3638821-3-quic_varada%40quicinc.com
patch subject: [PATCH v6 2/6] interconnect: icc-clk: Remove tristate from INTERCONNECT_CLK
config: arm64-defconfig (https://download.01.org/0day-ci/archive/20240403/202404032328.7zrla6d9-lkp@intel.com/config)
compiler: aarch64-linux-gcc (GCC) 13.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240403/202404032328.7zrla6d9-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202404032328.7zrla6d9-lkp@intel.com/
All errors (new ones prefixed by >>):
aarch64-linux-ld: Unexpected GOT/PLT entries detected!
aarch64-linux-ld: Unexpected run-time procedure linkages detected!
aarch64-linux-ld: drivers/clk/qcom/clk-cbf-8996.o: in function `qcom_msm8996_cbf_icc_remove':
>> drivers/clk/qcom/clk-cbf-8996.c:257:(.text+0x10): undefined reference to `icc_clk_unregister'
aarch64-linux-ld: drivers/clk/qcom/clk-cbf-8996.o: in function `qcom_msm8996_cbf_icc_register':
>> drivers/clk/qcom/clk-cbf-8996.c:244:(.text+0x360): undefined reference to `icc_clk_register'
vim +257 drivers/clk/qcom/clk-cbf-8996.c
12dc71953e664f Dmitry Baryshkov 2023-05-12 234
12dc71953e664f Dmitry Baryshkov 2023-05-12 235 static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev, struct clk_hw *cbf_hw)
12dc71953e664f Dmitry Baryshkov 2023-05-12 236 {
12dc71953e664f Dmitry Baryshkov 2023-05-12 237 struct device *dev = &pdev->dev;
12dc71953e664f Dmitry Baryshkov 2023-05-12 238 struct clk *clk = devm_clk_hw_get_clk(dev, cbf_hw, "cbf");
12dc71953e664f Dmitry Baryshkov 2023-05-12 239 const struct icc_clk_data data[] = {
12dc71953e664f Dmitry Baryshkov 2023-05-12 240 { .clk = clk, .name = "cbf", },
12dc71953e664f Dmitry Baryshkov 2023-05-12 241 };
12dc71953e664f Dmitry Baryshkov 2023-05-12 242 struct icc_provider *provider;
12dc71953e664f Dmitry Baryshkov 2023-05-12 243
12dc71953e664f Dmitry Baryshkov 2023-05-12 @244 provider = icc_clk_register(dev, CBF_MASTER_NODE, ARRAY_SIZE(data), data);
12dc71953e664f Dmitry Baryshkov 2023-05-12 245 if (IS_ERR(provider))
12dc71953e664f Dmitry Baryshkov 2023-05-12 246 return PTR_ERR(provider);
12dc71953e664f Dmitry Baryshkov 2023-05-12 247
12dc71953e664f Dmitry Baryshkov 2023-05-12 248 platform_set_drvdata(pdev, provider);
12dc71953e664f Dmitry Baryshkov 2023-05-12 249
12dc71953e664f Dmitry Baryshkov 2023-05-12 250 return 0;
12dc71953e664f Dmitry Baryshkov 2023-05-12 251 }
12dc71953e664f Dmitry Baryshkov 2023-05-12 252
abaf59c470a7c9 Uwe Kleine-König 2023-09-11 253 static void qcom_msm8996_cbf_icc_remove(struct platform_device *pdev)
12dc71953e664f Dmitry Baryshkov 2023-05-12 254 {
12dc71953e664f Dmitry Baryshkov 2023-05-12 255 struct icc_provider *provider = platform_get_drvdata(pdev);
12dc71953e664f Dmitry Baryshkov 2023-05-12 256
12dc71953e664f Dmitry Baryshkov 2023-05-12 @257 icc_clk_unregister(provider);
12dc71953e664f Dmitry Baryshkov 2023-05-12 258 }
12dc71953e664f Dmitry Baryshkov 2023-05-12 259 #define qcom_msm8996_cbf_icc_sync_state icc_sync_state
12dc71953e664f Dmitry Baryshkov 2023-05-12 260 #else
12dc71953e664f Dmitry Baryshkov 2023-05-12 261 static int qcom_msm8996_cbf_icc_register(struct platform_device *pdev, struct clk_hw *cbf_hw)
12dc71953e664f Dmitry Baryshkov 2023-05-12 262 {
12dc71953e664f Dmitry Baryshkov 2023-05-12 263 dev_warn(&pdev->dev, "CONFIG_INTERCONNECT is disabled, CBF clock is fixed\n");
12dc71953e664f Dmitry Baryshkov 2023-05-12 264
12dc71953e664f Dmitry Baryshkov 2023-05-12 265 return 0;
12dc71953e664f Dmitry Baryshkov 2023-05-12 266 }
abaf59c470a7c9 Uwe Kleine-König 2023-09-11 267 #define qcom_msm8996_cbf_icc_remove(pdev) { }
12dc71953e664f Dmitry Baryshkov 2023-05-12 268 #define qcom_msm8996_cbf_icc_sync_state NULL
12dc71953e664f Dmitry Baryshkov 2023-05-12 269 #endif
12dc71953e664f Dmitry Baryshkov 2023-05-12 270
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply
* Re: [PATCH v6 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support
From: Conor Dooley @ 2024-04-03 15:49 UTC (permalink / raw)
To: Tan Chun Hau
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Rob Herring, Emil Renner Berthing, Rob Herring,
Krzysztof Kozlowski, Conor Dooley, Maxime Coquelin,
Alexandre Torgue, Simon Horman, Bartosz Golaszewski,
Andrew Halaney, Jisheng Zhang, Uwe Kleine-König,
Russell King, Ley Foon Tan, Jee Heng Sia, netdev, devicetree,
linux-kernel, linux-stm32, linux-arm-kernel, linux-riscv
In-Reply-To: <20240403100549.78719-2-chunhau.tan@starfivetech.com>
[-- Attachment #1: Type: text/plain, Size: 802 bytes --]
On Wed, Apr 03, 2024 at 03:05:49AM -0700, Tan Chun Hau wrote:
> Add StarFive JH8100 dwmac support.
> The JH8100 dwmac shares the same driver code as the JH7110 dwmac
> and has only one reset signal.
>
> Please refer to below:
>
> JH8100: reset-names = "stmmaceth";
> JH7110: reset-names = "stmmaceth", "ahb";
> JH7100: reset-names = "ahb";
>
> Example usage of JH8100 in the device tree:
>
> gmac0: ethernet@16030000 {
> compatible = "starfive,jh8100-dwmac",
> "starfive,jh7110-dwmac",
> "snps,dwmac-5.20";
> ...
> };
>
> Signed-off-by: Tan Chun Hau <chunhau.tan@starfivetech.com>
How come you didn't pick up Rob's r-b?
https://lore.kernel.org/all/20240328204202.GA308290-robh@kernel.org/
Thanks,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v14 01/18] irqchip/sifive-plic: Convert PLIC driver into a platform driver
From: Lad, Prabhakar @ 2024-04-03 15:49 UTC (permalink / raw)
To: Anup Patel
Cc: Geert Uytterhoeven, Palmer Dabbelt, Paul Walmsley,
Thomas Gleixner, Rob Herring, Krzysztof Kozlowski, Frank Rowand,
Conor Dooley, Marc Zyngier, Björn Töpel, Atish Patra,
Andrew Jones, Sunil V L, Saravana Kannan, Anup Patel, linux-riscv,
linux-arm-kernel, linux-kernel, devicetree
In-Reply-To: <CAK9=C2VgiRcQjBEPmZjdcMf221omKS8ntdcenSE7G__4xYcCUA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1368 bytes --]
On Wed, Apr 3, 2024 at 3:17 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> On Wed, Apr 3, 2024 at 2:01 PM Lad, Prabhakar
> <prabhakar.csengg@gmail.com> wrote:
> >
> > Hi Anup,
> >
> > On Thu, Feb 22, 2024 at 9:41 AM Anup Patel <apatel@ventanamicro.com> wrote:
> > >
> > > The PLIC driver does not require very early initialization so convert
> > > it into a platform driver.
> > >
> > > After conversion, the PLIC driver is probed after CPUs are brought-up
> > > so setup cpuhp state after context handler of all online CPUs are
> > > initialized otherwise PLIC driver crashes for platforms with multiple
> > > PLIC instances.
> > >
> > > Signed-off-by: Anup Patel <apatel@ventanamicro.com>
> > > ---
> > > drivers/irqchip/irq-sifive-plic.c | 101 ++++++++++++++++++------------
> > > 1 file changed, 61 insertions(+), 40 deletions(-)
> > >
> > This patch seems to have broken things on RZ/Five SoC, after reverting
> > this patch I get to boot it back again on v6.9-rc2. Looks like there
> > is some probe order issue after switching to platform driver?
>
> Yes, this is most likely related to probe ordering based on your DT.
>
> Can you share the failing boot log and DT ?
non working case, https://paste.debian.net/1312947/
after reverting, https://paste.debian.net/1312948/
(attached is the DTB)
Cheers,
Prabhakar
[-- Attachment #2: r9a07g043f01-smarc.dtb --]
[-- Type: application/octet-stream, Size: 23379 bytes --]
^ permalink raw reply
* Re: [PATCH 2/3] arm64: dts: qcom: qdu1000-idp: enable USB nodes
From: Dmitry Baryshkov @ 2024-04-03 15:52 UTC (permalink / raw)
To: Krishna Kurapati PSSNV
Cc: Bjorn Andersson, Krzysztof Kozlowski, Komal Bajaj,
Bjorn Andersson, Konrad Dybcio, Rob Herring, Conor Dooley,
linux-arm-msm, devicetree, linux-kernel, Amrit Anand
In-Reply-To: <5354493b-63de-43bb-9871-73918f123661@quicinc.com>
On Wed, 3 Apr 2024 at 10:50, Krishna Kurapati PSSNV
<quic_kriskura@quicinc.com> wrote:
>
>
>
> On 4/2/2024 4:25 AM, Bjorn Andersson wrote:
> > On Tue, Mar 19, 2024 at 11:52:15AM +0200, Dmitry Baryshkov wrote:
> >> On Tue, 19 Mar 2024 at 11:11, Komal Bajaj <quic_kbajaj@quicinc.com> wrote:
> >>>
> >>> Enable both USB controllers and associated hsphy and qmp phy
> >>> nodes on QDU1000 IDP. Add the usb type B port linked with the
> >>> DWC3 USB controller switched to OTG mode and tagged with
> >>> usb-role-switch.
> >>>
> >>> Co-developed-by: Amrit Anand <quic_amrianan@quicinc.com>
> >>> Signed-off-by: Amrit Anand <quic_amrianan@quicinc.com>
> >>> Signed-off-by: Komal Bajaj <quic_kbajaj@quicinc.com>
> >>> ---
> >>> arch/arm64/boot/dts/qcom/qdu1000-idp.dts | 65 ++++++++++++++++++++++++
> >>> 1 file changed, 65 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/qdu1000-idp.dts b/arch/arm64/boot/dts/qcom/qdu1000-idp.dts
> >>> index 89b84fb0f70a..26442e707b5e 100644
> >>> --- a/arch/arm64/boot/dts/qcom/qdu1000-idp.dts
> >>> +++ b/arch/arm64/boot/dts/qcom/qdu1000-idp.dts
> >>> @@ -46,6 +46,33 @@ ppvar_sys: ppvar-sys-regulator {
> >>> regulator-boot-on;
> >>> };
> >>>
> >>> + usb_conn_gpio: usb-conn-gpio {
> >>> + compatible = "gpio-usb-b-connector";
> >>
> >> If this board has only a USB-B connector, can it really handle USB 3.0?
> >>
> >
> > Here's a USB 3.0 Type-B cable, so no problem there:
> > https://en.wikipedia.org/wiki/USB_hardware#/media/File:USB_3.0_plug,_type_B_-_1709.jpg
> >
> >
> > @Komal, please confirm that this is the connector you have on the IDP?
> >
>
> Hi Bjorn,
>
> Sorry for the confusion. The QDU1000 IDP has a Type-C connector. The
> type-c switch present between SoC and the connector is HD3SS3220 (from TI).
>
> I think Dmitry's comment was that if it is 3.0, is it Type-C ? and if
> it is Type-C, then the compatible written in the being
> "gpio-usb-b-connector" would mean that there is a Type-B connector for
> someone who looks at the DT. (Dmitry, Please correct me if I understood
> the comment wrong).
>
> I tried to push a series for adding a compatible to gpio conn driver
> [1] to resolve this and explained the connection specifics to Dmitry [2]
> and he suggested me to add a compatible for just the switch present on
> qdu1000 idp.
>
> Dmitry, Krzysztof,
>
> I was looking into the code again and it turns out there is a driver
> specific to HD3SS3220 switch [3] in linux already. I tried to check if
> it can be reused here but that driver relies on I2C communication
> between the SoC and the HD3SS3220 chip to get information on role
> switch. But in QDU1000 IDP board, there is no I2C communication present
> between SoC and the switch. Those lines have been cut off. The SoC only
> knows about VBUS/ID pins (other than DM/DP/SS Lanes) and no other I2C
> connections between the switch and the SoC. We still need to make use of
> vbus/id pins to decide which role we need to shift into. Can we still go
> ahead with using usb-conn-gpio driver by adding the compatible
> (qcom,qdu1000-hd3ss3220) and using it in DT ?
Is Qualcomm a manufacturer of the device? It is not.
Is qdu1000 a part of the device? It is not.
So the compatible string should be "ti,hd3ss3220". Which is fine to be
used in the platform driver. Just describe the differences in the
schema.
>
> Let me know your thoughts on this.
>
> [1]:
> https://lore.kernel.org/all/6f2df222-36d4-468e-99a7-9c48fae85aa9@quicinc.com/
>
> [2]:
> https://lore.kernel.org/all/6f2df222-36d4-468e-99a7-9c48fae85aa9@quicinc.com/
>
> [3]:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/usb/typec/hd3ss3220.c?h=v6.9-rc2
>
> Regards,
> Krishna,
>
>
>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH V2 RESEND 6/6] arm64: dts: qcom: sm8650: Add video and camera clock controllers
From: Dmitry Baryshkov @ 2024-04-03 15:54 UTC (permalink / raw)
To: Jagadeesh Kona
Cc: Bjorn Andersson, Konrad Dybcio, Michael Turquette, Stephen Boyd,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Vladimir Zapolskiy, linux-arm-msm, linux-clk, devicetree,
linux-kernel, Taniya Das, Satya Priya Kakitapalli, Ajit Pandey,
Imran Shaik
In-Reply-To: <b3464321-0c52-4c41-9198-e9e7b16aa419@quicinc.com>
On Wed, 3 Apr 2024 at 10:16, Jagadeesh Kona <quic_jkona@quicinc.com> wrote:
>
>
>
> On 3/25/2024 11:38 AM, Jagadeesh Kona wrote:
> >
> >
> > On 3/21/2024 6:43 PM, Dmitry Baryshkov wrote:
> >> On Thu, 21 Mar 2024 at 11:27, Jagadeesh Kona <quic_jkona@quicinc.com>
> >> wrote:
> >>>
> >>> Add device nodes for video and camera clock controllers on Qualcomm
> >>> SM8650 platform.
> >>>
> >>> Signed-off-by: Jagadeesh Kona <quic_jkona@quicinc.com>
> >>> ---
> >>> arch/arm64/boot/dts/qcom/sm8650.dtsi | 28 ++++++++++++++++++++++++++++
> >>> 1 file changed, 28 insertions(+)
> >>>
> >>> diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>> b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>> index 32c0a7b9aded..d862aa6be824 100644
> >>> --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>> +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi
> >>> @@ -4,6 +4,8 @@
> >>> */
> >>>
> >>> #include <dt-bindings/clock/qcom,rpmh.h>
> >>> +#include <dt-bindings/clock/qcom,sm8450-videocc.h>
> >>> +#include <dt-bindings/clock/qcom,sm8650-camcc.h>
> >>> #include <dt-bindings/clock/qcom,sm8650-dispcc.h>
> >>> #include <dt-bindings/clock/qcom,sm8650-gcc.h>
> >>> #include <dt-bindings/clock/qcom,sm8650-gpucc.h>
> >>> @@ -3110,6 +3112,32 @@ opp-202000000 {
> >>> };
> >>> };
> >>>
> >>> + videocc: clock-controller@aaf0000 {
> >>> + compatible = "qcom,sm8650-videocc";
> >>> + reg = <0 0x0aaf0000 0 0x10000>;
> >>> + clocks = <&bi_tcxo_div2>,
> >>> + <&gcc GCC_VIDEO_AHB_CLK>;
> >>> + power-domains = <&rpmhpd RPMHPD_MMCX>;
> >>> + required-opps = <&rpmhpd_opp_low_svs>;
> >>
> >> The required-opps should no longer be necessary.
> >>
> >
> > Sure, will check and remove this if not required.
>
>
> I checked further on this and without required-opps, if there is no vote
> on the power-domain & its peer from any other consumers, when runtime
> get is called on device, it enables the power domain just at the minimum
> non-zero level. But in some cases, the minimum non-zero level of
> power-domain could be just retention and is not sufficient for clock
> controller to operate, hence required-opps property is needed to specify
> the minimum level required on power-domain for this clock controller.
In which cases? If it ends up with the retention vote, it is a bug
which must be fixed.
>
> Thanks,
> Jagadeesh
>
> >
> >>> + #clock-cells = <1>;
> >>> + #reset-cells = <1>;
> >>> + #power-domain-cells = <1>;
> >>> + };
> >>> +
> >>> + camcc: clock-controller@ade0000 {
> >>> + compatible = "qcom,sm8650-camcc";
> >>> + reg = <0 0x0ade0000 0 0x20000>;
> >>> + clocks = <&gcc GCC_CAMERA_AHB_CLK>,
> >>> + <&bi_tcxo_div2>,
> >>> + <&bi_tcxo_ao_div2>,
> >>> + <&sleep_clk>;
> >>> + power-domains = <&rpmhpd RPMHPD_MMCX>;
> >>> + required-opps = <&rpmhpd_opp_low_svs>;
> >>> + #clock-cells = <1>;
> >>> + #reset-cells = <1>;
> >>> + #power-domain-cells = <1>;
> >>> + };
> >>> +
> >>> mdss: display-subsystem@ae00000 {
> >>> compatible = "qcom,sm8650-mdss";
> >>> reg = <0 0x0ae00000 0 0x1000>;
> >>> --
> >>> 2.43.0
> >>>
> >>>
> >>
> >>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH 3/6] iio: adc: ad7173: refactor channel configuration parsing
From: David Lechner @ 2024-04-03 15:55 UTC (permalink / raw)
To: Ceclan, Dumitru
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel
In-Reply-To: <1d777161-7d86-4d45-91bc-c7653504b890@gmail.com>
On Wed, Apr 3, 2024 at 5:01 AM Ceclan, Dumitru <mitrutzceclan@gmail.com> wrote:
>
> On 01/04/2024 22:39, David Lechner wrote:
> > On Mon, Apr 1, 2024 at 10:10 AM Dumitru Ceclan via B4 Relay
> > <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> >>
> >> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> >>
> >> Move configurations regarding number of channels from
> >> *_fw_parse_device_config to *_fw_parse_channel_config.
> >>
> >> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
> >> ---
> >
> > Commit messages need to explain _why_ the change is being made [1]. It
> > is not obvious to me why this needs to be moved.
> >
> > [1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#the-canonical-patch-format
>
>
> Jonathan Cameron:
>
> "
> > + if (num_channels == 0)
> > + return dev_err_probe(dev, -ENODATA, "No channels specified\n");
> > + indio_dev->num_channels = num_channels;
> > + st->num_channels = num_channels;
>
> I'm not seeing benefit of duplication here really and logically it feels like
> a lot of this last chunk would sit better in ad7173_fw_parse_channel_config()
>
> Perhaps that's a job for a future tidying up patch.
> "
> https://lore.kernel.org/all/20240303162148.3ad91aa2@jic23-huawei/
>
Thanks.
A Link: and Suggested-by: in the commit message with this info would
be a reasonable way to communicate this.
I looks like this is also adding an additional check " if
(num_channels > st->info->num_channels)" in addition to moving
existing code. It would be helpful to have the reason for this in the
commit message as well.
With the suggested additions to the commit message...
Reviewed-by: David Lechner <dlechner@baylibre.com>
^ permalink raw reply
* Re: [PATCH 2/2] dt-bindings: display: bridge: lt8912b: document 'lontium,pn-swap' property
From: Conor Dooley @ 2024-04-03 16:02 UTC (permalink / raw)
To: Alexandru Ardelean
Cc: linux-kernel, dri-devel, devicetree, adrien.grassein,
andrzej.hajda, neil.armstrong, rfoss, Laurent.pinchart, jonas,
jernej.skrabec, airlied, daniel, maarten.lankhorst, mripard,
tzimmermann, robh, krzysztof.kozlowski+dt, conor+dt,
stefan.eichenberger, francesco.dolcini, marius.muresan,
irina.muresan
In-Reply-To: <CAH3L5QooAXDYAxOdMkPrW1mx04ZgTv_kMU5VSAby9J3Hb_RFOg@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 278 bytes --]
On Wed, Apr 03, 2024 at 09:16:31AM +0300, Alexandru Ardelean wrote:
> >
> > > + type: boolean
> >
> > The type here should be flag.
>
> ack; i'll change the type
I prob shoulda said, its "$ref: /schemas/types.yaml#/definitions/flag"
instead of "type: boolean".
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH 4/6] iio: adc: ad7173: refactor ain and vref selection
From: David Lechner @ 2024-04-03 16:02 UTC (permalink / raw)
To: Ceclan, Dumitru
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Jonathan Cameron, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel
In-Reply-To: <78cab1a4-e085-4df5-bb8c-277fd5ec3d70@gmail.com>
On Wed, Apr 3, 2024 at 5:03 AM Ceclan, Dumitru <mitrutzceclan@gmail.com> wrote:
>
> On 01/04/2024 22:40, David Lechner wrote:
> > On Mon, Apr 1, 2024 at 10:10 AM Dumitru Ceclan via B4 Relay
> > <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> >>
> >> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
> >>
> >> Move validation of analog inputs and reference voltage selection to
> >> separate functions.
> >>
> >> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
> >> ---
> >
> > Same as my comment on PATCH 3/6. We would like to know why this change
> > is being made.
>
> Move validation of analog inputs and reference voltage selection to
> separate functions to reduce the size of the channel config parsing function.
>
> Good?
Better. But it still only says what is being done and doesn't answer
the question "why?".
"to reduce the size of the function to make it easier to read"
explains why reducing the size of the function makes it an
improvement.
^ permalink raw reply
* Re: [PATCH v3 18/25] dt-bindings: media: imx258: Add alternate compatible strings
From: Conor Dooley @ 2024-04-03 16:14 UTC (permalink / raw)
To: git
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, sakari.ailus, devicetree, imx, linux-arm-kernel,
linux-kernel, pavel, phone-devel
In-Reply-To: <20240403150355.189229-19-git@luigi311.com>
[-- Attachment #1: Type: text/plain, Size: 1743 bytes --]
On Wed, Apr 03, 2024 at 09:03:47AM -0600, git@luigi311.com wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>
> There are a number of variants of the imx258 modules that can not
> be differentiated at runtime, so add compatible strings for the
> PDAF variant.
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Luis Garcia <git@luigi311.com>
> ---
> .../devicetree/bindings/media/i2c/sony,imx258.yaml | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> index bee61a443b23..c978abc0cdb3 100644
> --- a/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> +++ b/Documentation/devicetree/bindings/media/i2c/sony,imx258.yaml
> @@ -13,11 +13,16 @@ description: |-
> IMX258 is a diagonal 5.867mm (Type 1/3.06) 13 Mega-pixel CMOS active pixel
> type stacked image sensor with a square pixel array of size 4208 x 3120. It
> is programmable through I2C interface. Image data is sent through MIPI
> - CSI-2.
> + CSI-2. The sensor exists in two different models, a standard variant
> + (IMX258) and a variant with phase detection autofocus (IMX258-PDAF).
> + The camera module does not expose the model through registers, so the
> + exact model needs to be specified.
>
> properties:
> compatible:
> - const: sony,imx258
> + enum:
> + - sony,imx258
> + - sony,imx258-pdaf
Does the pdaf variant support all of the features/is it register
compatible with the regular variant? If it is, the regular variant
should be a fallback compatible.
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* Re: [PATCH v3 19/25] media: i2c: imx258: Change register settings for variants of the sensor
From: Sakari Ailus @ 2024-04-03 16:18 UTC (permalink / raw)
To: git
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, devicetree, imx, linux-arm-kernel, linux-kernel, pavel,
phone-devel
In-Reply-To: <20240403150355.189229-20-git@luigi311.com>
Hi Luis, Dave,
On Wed, Apr 03, 2024 at 09:03:48AM -0600, git@luigi311.com wrote:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
>
> Sony have advised that there are variants of the IMX258 sensor which
> require slightly different register configuration to the mainline
> imx258 driver defaults.
>
> There is no available run-time detection for the variant, so add
> configuration via the DT compatible string.
>
> The Vision Components imx258 module supports PDAF, so add the
> register differences for that variant
>
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Luis Garcia <git@luigi311.com>
> ---
> drivers/media/i2c/imx258.c | 48 ++++++++++++++++++++++++++++++++++----
> 1 file changed, 44 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index 775d957c9b87..fa48da212037 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -6,6 +6,7 @@
> #include <linux/delay.h>
> #include <linux/i2c.h>
> #include <linux/module.h>
> +#include <linux/of_device.h>
> #include <linux/pm_runtime.h>
> #include <linux/regulator/consumer.h>
> #include <media/v4l2-ctrls.h>
> @@ -321,8 +322,6 @@ static const struct imx258_reg mipi_642mbps_24mhz_4l[] = {
>
> static const struct imx258_reg mode_common_regs[] = {
> { 0x3051, 0x00 },
> - { 0x3052, 0x00 },
> - { 0x4E21, 0x14 },
> { 0x6B11, 0xCF },
> { 0x7FF0, 0x08 },
> { 0x7FF1, 0x0F },
> @@ -345,7 +344,6 @@ static const struct imx258_reg mode_common_regs[] = {
> { 0x7FA8, 0x03 },
> { 0x7FA9, 0xFE },
> { 0x7B24, 0x81 },
> - { 0x7B25, 0x00 },
> { 0x6564, 0x07 },
> { 0x6B0D, 0x41 },
> { 0x653D, 0x04 },
> @@ -460,6 +458,33 @@ static const struct imx258_reg mode_1048_780_regs[] = {
> { 0x034F, 0x0C },
> };
>
> +struct imx258_variant_cfg {
> + const struct imx258_reg *regs;
> + unsigned int num_regs;
> +};
> +
> +static const struct imx258_reg imx258_cfg_regs[] = {
> + { 0x3052, 0x00 },
> + { 0x4E21, 0x14 },
> + { 0x7B25, 0x00 },
> +};
> +
> +static const struct imx258_variant_cfg imx258_cfg = {
> + .regs = imx258_cfg_regs,
> + .num_regs = ARRAY_SIZE(imx258_cfg_regs),
> +};
> +
> +static const struct imx258_reg imx258_pdaf_cfg_regs[] = {
> + { 0x3052, 0x01 },
> + { 0x4E21, 0x10 },
> + { 0x7B25, 0x01 },
> +};
> +
> +static const struct imx258_variant_cfg imx258_pdaf_cfg = {
> + .regs = imx258_pdaf_cfg_regs,
> + .num_regs = ARRAY_SIZE(imx258_pdaf_cfg_regs),
> +};
> +
> static const char * const imx258_test_pattern_menu[] = {
> "Disabled",
> "Solid Colour",
> @@ -637,6 +662,8 @@ struct imx258 {
> struct v4l2_subdev sd;
> struct media_pad pad;
>
> + const struct imx258_variant_cfg *variant_cfg;
> +
> struct v4l2_ctrl_handler ctrl_handler;
> /* V4L2 Controls */
> struct v4l2_ctrl *link_freq;
> @@ -1104,6 +1131,14 @@ static int imx258_start_streaming(struct imx258 *imx258)
> return ret;
> }
>
> + ret = imx258_write_regs(imx258, imx258->variant_cfg->regs,
> + imx258->variant_cfg->num_regs);
> + if (ret) {
> + dev_err(&client->dev, "%s failed to set variant config\n",
> + __func__);
> + return ret;
> + }
> +
> ret = imx258_write_reg(imx258, IMX258_CLK_BLANK_STOP,
> IMX258_REG_VALUE_08BIT,
> imx258->csi2_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK ?
> @@ -1492,6 +1527,10 @@ static int imx258_probe(struct i2c_client *client)
>
> imx258->csi2_flags = ep.bus.mipi_csi2.flags;
>
> + imx258->variant_cfg = of_device_get_match_data(&client->dev);
You'll also need to keep this working for ACPI based systems. I.e. in
practice remove "of_" prefix here and add the non-PDAF variant data to the
relevant ACPI ID list.
> + if (!imx258->variant_cfg)
> + imx258->variant_cfg = &imx258_cfg;
> +
> /* Initialize subdev */
> v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
>
> @@ -1579,7 +1618,8 @@ MODULE_DEVICE_TABLE(acpi, imx258_acpi_ids);
> #endif
>
> static const struct of_device_id imx258_dt_ids[] = {
> - { .compatible = "sony,imx258" },
> + { .compatible = "sony,imx258", .data = &imx258_cfg },
> + { .compatible = "sony,imx258-pdaf", .data = &imx258_pdaf_cfg },
> { /* sentinel */ }
> };
> MODULE_DEVICE_TABLE(of, imx258_dt_ids);
--
Kind regards,
Sakari Ailus
^ permalink raw reply
* [PATCH 0/4] Introduce clock support for Airoha EN7581 SoC
From: Lorenzo Bianconi @ 2024-04-03 16:20 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
This series is based on the following series:
https://patchwork.kernel.org/project/linux-arm-kernel/cover/cover.1709975956.git.lorenzo@kernel.org/
Lorenzo Bianconi (4):
dt-bindings: clock: airoha: add EN7581 binding
arm64: dts: airoha: Add EN7581 clock node
clk: en7523: make pcie clk_ops accessible through of_device_id struct
clk: en7523: add EN7581 support
.../bindings/clock/airoha,en7523-scu.yaml | 26 ++-
arch/arm64/boot/dts/airoha/en7581.dtsi | 9 +
drivers/clk/clk-en7523.c | 155 ++++++++++++++++--
3 files changed, 171 insertions(+), 19 deletions(-)
--
2.44.0
^ permalink raw reply
* [PATCH 1/4] dt-bindings: clock: airoha: add EN7581 binding
From: Lorenzo Bianconi @ 2024-04-03 16:20 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <cover.1712160869.git.lorenzo@kernel.org>
Introduce Airoha EN7581 entry in Airoha EN7523 clock binding
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
.../bindings/clock/airoha,en7523-scu.yaml | 26 +++++++++++++++++--
1 file changed, 24 insertions(+), 2 deletions(-)
diff --git a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
index 79b0752faa91..cf893d4c74cd 100644
--- a/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
+++ b/Documentation/devicetree/bindings/clock/airoha,en7523-scu.yaml
@@ -29,10 +29,13 @@ description: |
properties:
compatible:
items:
- - const: airoha,en7523-scu
+ - enum:
+ - airoha,en7523-scu
+ - airoha,en7581-scu
reg:
- maxItems: 2
+ minItems: 2
+ maxItems: 3
"#clock-cells":
description:
@@ -45,6 +48,25 @@ required:
- reg
- '#clock-cells'
+allOf:
+ - if:
+ properties:
+ compatible:
+ const: airoha,en7523-scu
+ then:
+ properties:
+ reg:
+ maxItems: 2
+
+ - if:
+ properties:
+ compatible:
+ const: airoha,en7581-scu
+ then:
+ properties:
+ reg:
+ maxItems: 3
+
additionalProperties: false
examples:
--
2.44.0
^ permalink raw reply related
* [PATCH 2/4] arm64: dts: airoha: Add EN7581 clock node
From: Lorenzo Bianconi @ 2024-04-03 16:20 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <cover.1712160869.git.lorenzo@kernel.org>
Introduce the Airoha EN7581 clock node in Airoha EN7581 dtsi
Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
arch/arm64/boot/dts/airoha/en7581.dtsi | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/arch/arm64/boot/dts/airoha/en7581.dtsi b/arch/arm64/boot/dts/airoha/en7581.dtsi
index 55eb1762fb11..a1daaaef0de0 100644
--- a/arch/arm64/boot/dts/airoha/en7581.dtsi
+++ b/arch/arm64/boot/dts/airoha/en7581.dtsi
@@ -2,6 +2,7 @@
#include <dt-bindings/interrupt-controller/irq.h>
#include <dt-bindings/interrupt-controller/arm-gic.h>
+#include <dt-bindings/clock/en7523-clk.h>
/ {
interrupt-parent = <&gic>;
@@ -150,5 +151,13 @@ uart1: serial@1fbf0000 {
interrupts = <GIC_SPI 18 IRQ_TYPE_LEVEL_HIGH>;
clock-frequency = <1843200>;
};
+
+ scu: system-controller@1fa20000 {
+ compatible = "airoha,en7581-scu";
+ reg = <0x0 0x1fa20000 0x0 0x400>,
+ <0x0 0x1fb00000 0x0 0x1000>,
+ <0x0 0x1fbe3400 0x0 0xfc>;
+ #clock-cells = <1>;
+ };
};
};
--
2.44.0
^ permalink raw reply related
* [PATCH 3/4] clk: en7523: make pcie clk_ops accessible through of_device_id struct
From: Lorenzo Bianconi @ 2024-04-03 16:20 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <cover.1712160869.git.lorenzo@kernel.org>
Make pcie clk_ops structure accessible through of_device_id data
pointer in order to define multiple clk_ops for each supported SoC.
This is a preliminary patch to introduce EN7581 clock support.
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/clk/clk-en7523.c | 25 +++++++++++++------------
1 file changed, 13 insertions(+), 12 deletions(-)
diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index 7cde328495e2..c7def87b74c6 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -145,11 +145,6 @@ static const struct en_clk_desc en7523_base_clks[] = {
}
};
-static const struct of_device_id of_match_clk_en7523[] = {
- { .compatible = "airoha,en7523-scu", },
- { /* sentinel */ }
-};
-
static unsigned int en7523_get_base_rate(void __iomem *base, unsigned int i)
{
const struct en_clk_desc *desc = &en7523_base_clks[i];
@@ -247,14 +242,9 @@ static void en7523_pci_unprepare(struct clk_hw *hw)
static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
void __iomem *np_base)
{
- static const struct clk_ops pcie_gate_ops = {
- .is_enabled = en7523_pci_is_enabled,
- .prepare = en7523_pci_prepare,
- .unprepare = en7523_pci_unprepare,
- };
struct clk_init_data init = {
.name = "pcie",
- .ops = &pcie_gate_ops,
+ .ops = of_device_get_match_data(dev),
};
struct en_clk_gate *cg;
@@ -264,7 +254,7 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
cg->base = np_base;
cg->hw.init = &init;
- en7523_pci_unprepare(&cg->hw);
+ init.ops->unprepare(&cg->hw);
if (clk_hw_register(dev, &cg->hw))
return NULL;
@@ -333,6 +323,17 @@ static int en7523_clk_probe(struct platform_device *pdev)
return r;
}
+static const struct clk_ops en7523_pcie_ops = {
+ .is_enabled = en7523_pci_is_enabled,
+ .prepare = en7523_pci_prepare,
+ .unprepare = en7523_pci_unprepare,
+};
+
+static const struct of_device_id of_match_clk_en7523[] = {
+ { .compatible = "airoha,en7523-scu", .data = &en7523_pcie_ops },
+ { /* sentinel */ }
+};
+
static struct platform_driver clk_en7523_drv = {
.probe = en7523_clk_probe,
.driver = {
--
2.44.0
^ permalink raw reply related
* [PATCH 4/4] clk: en7523: add EN7581 support
From: Lorenzo Bianconi @ 2024-04-03 16:20 UTC (permalink / raw)
To: linux-clk
Cc: mturquette, sboyd, linux-arm-kernel, robh+dt,
krzysztof.kozlowski+dt, conor+dt, nbd, john, devicetree, dd,
catalin.marinas, will, upstream, lorenzo.bianconi83,
angelogioacchino.delregno
In-Reply-To: <cover.1712160869.git.lorenzo@kernel.org>
Introduce EN7581 clock support to clk-en7523 driver.
Tested-by: Zhengping Zhang <zhengping.zhang@airoha.com>
Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
drivers/clk/clk-en7523.c | 130 +++++++++++++++++++++++++++++++++++++--
1 file changed, 125 insertions(+), 5 deletions(-)
diff --git a/drivers/clk/clk-en7523.c b/drivers/clk/clk-en7523.c
index c7def87b74c6..51a6c0cc7f58 100644
--- a/drivers/clk/clk-en7523.c
+++ b/drivers/clk/clk-en7523.c
@@ -4,13 +4,16 @@
#include <linux/clk-provider.h>
#include <linux/io.h>
#include <linux/of.h>
+#include <linux/of_device.h>
#include <linux/platform_device.h>
#include <dt-bindings/clock/en7523-clk.h>
#define REG_PCI_CONTROL 0x88
#define REG_PCI_CONTROL_PERSTOUT BIT(29)
#define REG_PCI_CONTROL_PERSTOUT1 BIT(26)
+#define REG_PCI_CONTROL_REFCLK_EN0 BIT(23)
#define REG_PCI_CONTROL_REFCLK_EN1 BIT(22)
+#define REG_PCI_CONTROL_PERSTOUT2 BIT(16)
#define REG_GSW_CLK_DIV_SEL 0x1b4
#define REG_EMI_CLK_DIV_SEL 0x1b8
#define REG_BUS_CLK_DIV_SEL 0x1bc
@@ -18,10 +21,25 @@
#define REG_SPI_CLK_FREQ_SEL 0x1c8
#define REG_NPU_CLK_DIV_SEL 0x1fc
#define REG_CRYPTO_CLKSRC 0x200
-#define REG_RESET_CONTROL 0x834
+#define REG_RESET_CONTROL2 0x830
+#define REG_RESET2_CONTROL_PCIE2 BIT(27)
+#define REG_RESET_CONTROL1 0x834
#define REG_RESET_CONTROL_PCIEHB BIT(29)
#define REG_RESET_CONTROL_PCIE1 BIT(27)
#define REG_RESET_CONTROL_PCIE2 BIT(26)
+/* EN7581 */
+#define REG_PCIE0_MEM 0x00
+#define REG_PCIE0_MEM_MASK 0x04
+#define REG_PCIE1_MEM 0x08
+#define REG_PCIE1_MEM_MASK 0x0c
+#define REG_PCIE2_MEM 0x10
+#define REG_PCIE2_MEM_MASK 0x14
+#define REG_PCIE_RESET_OPEN_DRAIN 0x018c
+#define REG_PCIE_RESET_OPEN_DRAIN_MASK GENMASK(2, 0)
+#define REG_NP_SCU_PCIC 0x88
+#define REG_NP_SCU_SSTR 0x9c
+#define REG_PCIE_XSI0_SEL_MASK GENMASK(14, 13)
+#define REG_PCIE_XSI1_SEL_MASK GENMASK(12, 11)
struct en_clk_desc {
int id;
@@ -207,14 +225,14 @@ static int en7523_pci_prepare(struct clk_hw *hw)
usleep_range(1000, 2000);
/* Reset to default */
- val = readl(np_base + REG_RESET_CONTROL);
+ val = readl(np_base + REG_RESET_CONTROL1);
mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
REG_RESET_CONTROL_PCIEHB;
- writel(val & ~mask, np_base + REG_RESET_CONTROL);
+ writel(val & ~mask, np_base + REG_RESET_CONTROL1);
usleep_range(1000, 2000);
- writel(val | mask, np_base + REG_RESET_CONTROL);
+ writel(val | mask, np_base + REG_RESET_CONTROL1);
msleep(100);
- writel(val & ~mask, np_base + REG_RESET_CONTROL);
+ writel(val & ~mask, np_base + REG_RESET_CONTROL1);
usleep_range(5000, 10000);
/* Release device */
@@ -262,6 +280,64 @@ static struct clk_hw *en7523_register_pcie_clk(struct device *dev,
return &cg->hw;
}
+static int en7581_pci_is_enabled(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ u32 val, mask;
+
+ mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1;
+ val = readl(cg->base + REG_PCI_CONTROL);
+ return (val & mask) == mask;
+}
+
+static int en7581_pci_prepare(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ void __iomem *np_base = cg->base;
+ u32 val, mask;
+
+ mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
+ REG_RESET_CONTROL_PCIEHB;
+ val = readl(np_base + REG_RESET_CONTROL1);
+ writel(val & ~mask, np_base + REG_RESET_CONTROL1);
+ val = readl(np_base + REG_RESET_CONTROL2);
+ writel(val & ~REG_RESET2_CONTROL_PCIE2, np_base + REG_RESET_CONTROL2);
+ usleep_range(5000, 10000);
+
+ mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1 |
+ REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT2 |
+ REG_PCI_CONTROL_PERSTOUT;
+ val = readl(np_base + REG_PCI_CONTROL);
+ writel(val | mask, np_base + REG_PCI_CONTROL);
+ msleep(250);
+
+ return 0;
+}
+
+static void en7581_pci_unprepare(struct clk_hw *hw)
+{
+ struct en_clk_gate *cg = container_of(hw, struct en_clk_gate, hw);
+ void __iomem *np_base = cg->base;
+ u32 val, mask;
+
+ mask = REG_PCI_CONTROL_REFCLK_EN0 | REG_PCI_CONTROL_REFCLK_EN1 |
+ REG_PCI_CONTROL_PERSTOUT1 | REG_PCI_CONTROL_PERSTOUT2 |
+ REG_PCI_CONTROL_PERSTOUT;
+ val = readl(np_base + REG_PCI_CONTROL);
+ writel(val & ~mask, np_base + REG_PCI_CONTROL);
+ usleep_range(1000, 2000);
+
+ mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2 |
+ REG_RESET_CONTROL_PCIEHB;
+ val = readl(np_base + REG_RESET_CONTROL1);
+ writel(val | mask, np_base + REG_RESET_CONTROL1);
+ mask = REG_RESET_CONTROL_PCIE1 | REG_RESET_CONTROL_PCIE2;
+ writel(val | mask, np_base + REG_RESET_CONTROL1);
+ val = readl(np_base + REG_RESET_CONTROL2);
+ writel(val | REG_RESET_CONTROL_PCIE2, np_base + REG_RESET_CONTROL2);
+ msleep(100);
+}
+
static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_data *clk_data,
void __iomem *base, void __iomem *np_base)
{
@@ -291,6 +367,37 @@ static void en7523_register_clocks(struct device *dev, struct clk_hw_onecell_dat
clk_data->num = EN7523_NUM_CLOCKS;
}
+static int en7581_clk_hw_init(struct platform_device *pdev,
+ void __iomem *base,
+ void __iomem *np_base)
+{
+ void __iomem *pb_base;
+ u32 val;
+
+ pb_base = devm_platform_ioremap_resource(pdev, 2);
+ if (IS_ERR(pb_base))
+ return PTR_ERR(pb_base);
+
+ val = readl(np_base + REG_NP_SCU_SSTR);
+ val &= ~(REG_PCIE_XSI0_SEL_MASK | REG_PCIE_XSI1_SEL_MASK);
+ writel(val, np_base + REG_NP_SCU_SSTR);
+ val = readl(np_base + REG_NP_SCU_PCIC);
+ writel(val | 3, np_base + REG_NP_SCU_PCIC);
+
+ writel(0x20000000, pb_base + REG_PCIE0_MEM);
+ writel(0xfc000000, pb_base + REG_PCIE0_MEM_MASK);
+ writel(0x24000000, pb_base + REG_PCIE1_MEM);
+ writel(0xfc000000, pb_base + REG_PCIE1_MEM_MASK);
+ writel(0x28000000, pb_base + REG_PCIE2_MEM);
+ writel(0xfc000000, pb_base + REG_PCIE2_MEM_MASK);
+
+ val = readl(base + REG_PCIE_RESET_OPEN_DRAIN);
+ writel(val | REG_PCIE_RESET_OPEN_DRAIN_MASK,
+ base + REG_PCIE_RESET_OPEN_DRAIN);
+
+ return 0;
+}
+
static int en7523_clk_probe(struct platform_device *pdev)
{
struct device_node *node = pdev->dev.of_node;
@@ -306,6 +413,12 @@ static int en7523_clk_probe(struct platform_device *pdev)
if (IS_ERR(np_base))
return PTR_ERR(np_base);
+ if (of_device_is_compatible(node, "airoha,en7581-scu")) {
+ r = en7581_clk_hw_init(pdev, base, np_base);
+ if (r)
+ return r;
+ }
+
clk_data = devm_kzalloc(&pdev->dev,
struct_size(clk_data, hws, EN7523_NUM_CLOCKS),
GFP_KERNEL);
@@ -329,8 +442,15 @@ static const struct clk_ops en7523_pcie_ops = {
.unprepare = en7523_pci_unprepare,
};
+static const struct clk_ops en7581_pcie_ops = {
+ .is_enabled = en7581_pci_is_enabled,
+ .prepare = en7581_pci_prepare,
+ .unprepare = en7581_pci_unprepare,
+};
+
static const struct of_device_id of_match_clk_en7523[] = {
{ .compatible = "airoha,en7523-scu", .data = &en7523_pcie_ops },
+ { .compatible = "airoha,en7581-scu", .data = &en7581_pcie_ops },
{ /* sentinel */ }
};
--
2.44.0
^ permalink raw reply related
* Re: [PATCH v3 21/25] drivers: media: i2c: imx258: Use macros
From: Sakari Ailus @ 2024-04-03 16:23 UTC (permalink / raw)
To: git
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, devicetree, imx, linux-arm-kernel, linux-kernel, pavel,
phone-devel, Ondrej Jirman
In-Reply-To: <20240403150355.189229-22-git@luigi311.com>
Hi Luis,
On Wed, Apr 03, 2024 at 09:03:50AM -0600, git@luigi311.com wrote:
> From: Luis Garcia <git@luigi311.com>
>
> Use understandable macros instead of raw values.
>
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> Signed-off-by: Luis Garcia <git@luigi311.com>
> ---
> drivers/media/i2c/imx258.c | 434 ++++++++++++++++++-------------------
> 1 file changed, 207 insertions(+), 227 deletions(-)
>
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index e2ecf6109516..30352c33f63c 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -33,8 +33,6 @@
> #define IMX258_VTS_30FPS_VGA 0x034c
> #define IMX258_VTS_MAX 65525
>
> -#define IMX258_REG_VTS 0x0340
> -
> /* HBLANK control - read only */
> #define IMX258_PPL_DEFAULT 5352
>
> @@ -90,6 +88,53 @@
> #define IMX258_PIXEL_ARRAY_WIDTH 4208U
> #define IMX258_PIXEL_ARRAY_HEIGHT 3120U
>
> +/* regs */
> +#define IMX258_REG_PLL_MULT_DRIV 0x0310
> +#define IMX258_REG_IVTPXCK_DIV 0x0301
> +#define IMX258_REG_IVTSYCK_DIV 0x0303
> +#define IMX258_REG_PREPLLCK_VT_DIV 0x0305
> +#define IMX258_REG_IOPPXCK_DIV 0x0309
> +#define IMX258_REG_IOPSYCK_DIV 0x030b
> +#define IMX258_REG_PREPLLCK_OP_DIV 0x030d
> +#define IMX258_REG_PHASE_PIX_OUTEN 0x3030
> +#define IMX258_REG_PDPIX_DATA_RATE 0x3032
> +#define IMX258_REG_SCALE_MODE 0x0401
> +#define IMX258_REG_SCALE_MODE_EXT 0x3038
> +#define IMX258_REG_AF_WINDOW_MODE 0x7bcd
> +#define IMX258_REG_FRM_LENGTH_CTL 0x0350
> +#define IMX258_REG_CSI_LANE_MODE 0x0114
> +#define IMX258_REG_X_EVN_INC 0x0381
> +#define IMX258_REG_X_ODD_INC 0x0383
> +#define IMX258_REG_Y_EVN_INC 0x0385
> +#define IMX258_REG_Y_ODD_INC 0x0387
> +#define IMX258_REG_BINNING_MODE 0x0900
> +#define IMX258_REG_BINNING_TYPE_V 0x0901
> +#define IMX258_REG_FORCE_FD_SUM 0x300d
> +#define IMX258_REG_DIG_CROP_X_OFFSET 0x0408
> +#define IMX258_REG_DIG_CROP_Y_OFFSET 0x040a
> +#define IMX258_REG_DIG_CROP_IMAGE_WIDTH 0x040c
> +#define IMX258_REG_DIG_CROP_IMAGE_HEIGHT 0x040e
> +#define IMX258_REG_SCALE_M 0x0404
> +#define IMX258_REG_X_OUT_SIZE 0x034c
> +#define IMX258_REG_Y_OUT_SIZE 0x034e
> +#define IMX258_REG_X_ADD_STA 0x0344
> +#define IMX258_REG_Y_ADD_STA 0x0346
> +#define IMX258_REG_X_ADD_END 0x0348
> +#define IMX258_REG_Y_ADD_END 0x034a
> +#define IMX258_REG_EXCK_FREQ 0x0136
> +#define IMX258_REG_CSI_DT_FMT 0x0112
> +#define IMX258_REG_LINE_LENGTH_PCK 0x0342
> +#define IMX258_REG_SCALE_M_EXT 0x303a
> +#define IMX258_REG_FRM_LENGTH_LINES 0x0340
> +#define IMX258_REG_FINE_INTEG_TIME 0x0200
> +#define IMX258_REG_PLL_IVT_MPY 0x0306
> +#define IMX258_REG_PLL_IOP_MPY 0x030e
> +#define IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H 0x0820
> +#define IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L 0x0822
> +
> +#define REG8(a, v) { a, v }
> +#define REG16(a, v) { a, ((v) >> 8) & 0xff }, { (a) + 1, (v) & 0xff }
The patch is nice but these macros are better replaced by the V4L2 CCI
helper that also offers register access functions. Could you add a patch to
convert the driver to use it (maybe after this one)?
> +
> struct imx258_reg {
> u16 address;
> u8 val;
> @@ -145,179 +190,139 @@ struct imx258_mode {
> * lane data rate when using 2 lanes, thus allowing a maximum of 15fps.
> */
> static const struct imx258_reg mipi_1267mbps_19_2mhz_2l[] = {
> - { 0x0136, 0x13 },
> - { 0x0137, 0x33 },
> - { 0x0301, 0x0A },
> - { 0x0303, 0x02 },
> - { 0x0305, 0x03 },
> - { 0x0306, 0x00 },
> - { 0x0307, 0xC6 },
> - { 0x0309, 0x0A },
> - { 0x030B, 0x01 },
> - { 0x030D, 0x02 },
> - { 0x030E, 0x00 },
> - { 0x030F, 0xD8 },
> - { 0x0310, 0x00 },
> -
> - { 0x0114, 0x01 },
> - { 0x0820, 0x09 },
> - { 0x0821, 0xa6 },
> - { 0x0822, 0x66 },
> - { 0x0823, 0x66 },
> + REG16(IMX258_REG_EXCK_FREQ, 0x1333),
> + REG8(IMX258_REG_IVTPXCK_DIV, 10),
> + REG8(IMX258_REG_IVTSYCK_DIV, 2),
> + REG8(IMX258_REG_PREPLLCK_VT_DIV, 3),
> + REG16(IMX258_REG_PLL_IVT_MPY, 0x00C6),
> + REG8(IMX258_REG_IOPPXCK_DIV, 10),
> + REG8(IMX258_REG_IOPSYCK_DIV, 1),
> + REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> + REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> + REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> + REG8(IMX258_REG_CSI_LANE_MODE, 1),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x09A6),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x6666),
> };
>
> static const struct imx258_reg mipi_1267mbps_19_2mhz_4l[] = {
> - { 0x0136, 0x13 },
> - { 0x0137, 0x33 },
> - { 0x0301, 0x05 },
> - { 0x0303, 0x02 },
> - { 0x0305, 0x03 },
> - { 0x0306, 0x00 },
> - { 0x0307, 0xC6 },
> - { 0x0309, 0x0A },
> - { 0x030B, 0x01 },
> - { 0x030D, 0x02 },
> - { 0x030E, 0x00 },
> - { 0x030F, 0xD8 },
> - { 0x0310, 0x00 },
> -
> - { 0x0114, 0x03 },
> - { 0x0820, 0x13 },
> - { 0x0821, 0x4C },
> - { 0x0822, 0xCC },
> - { 0x0823, 0xCC },
> + REG16(IMX258_REG_EXCK_FREQ, 0x1333),
> + REG8(IMX258_REG_IVTPXCK_DIV, 5),
> + REG8(IMX258_REG_IVTSYCK_DIV, 2),
> + REG8(IMX258_REG_PREPLLCK_VT_DIV, 3),
> + REG16(IMX258_REG_PLL_IVT_MPY, 0x00C6),
> + REG8(IMX258_REG_IOPPXCK_DIV, 10),
> + REG8(IMX258_REG_IOPSYCK_DIV, 1),
> + REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> + REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> + REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> + REG8(IMX258_REG_CSI_LANE_MODE, 3),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x134C),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0xCCCC),
> };
>
> static const struct imx258_reg mipi_1272mbps_24mhz_2l[] = {
> - { 0x0136, 0x18 },
> - { 0x0137, 0x00 },
> - { 0x0301, 0x0a },
> - { 0x0303, 0x02 },
> - { 0x0305, 0x04 },
> - { 0x0306, 0x00 },
> - { 0x0307, 0xD4 },
> - { 0x0309, 0x0A },
> - { 0x030B, 0x01 },
> - { 0x030D, 0x02 },
> - { 0x030E, 0x00 },
> - { 0x030F, 0xD8 },
> - { 0x0310, 0x00 },
> -
> - { 0x0114, 0x01 },
> - { 0x0820, 0x13 },
> - { 0x0821, 0x4C },
> - { 0x0822, 0xCC },
> - { 0x0823, 0xCC },
> + REG16(IMX258_REG_EXCK_FREQ, 0x1800),
> + REG8(IMX258_REG_IVTPXCK_DIV, 10),
> + REG8(IMX258_REG_IVTSYCK_DIV, 2),
> + REG8(IMX258_REG_PREPLLCK_VT_DIV, 4),
> + REG16(IMX258_REG_PLL_IVT_MPY, 0x00D4),
> + REG8(IMX258_REG_IOPPXCK_DIV, 10),
> + REG8(IMX258_REG_IOPSYCK_DIV, 1),
> + REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> + REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> + REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> + REG8(IMX258_REG_CSI_LANE_MODE, 1),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x134C),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0xCCCC),
> };
>
> static const struct imx258_reg mipi_1272mbps_24mhz_4l[] = {
> - { 0x0136, 0x18 },
> - { 0x0137, 0x00 },
> - { 0x0301, 0x05 },
> - { 0x0303, 0x02 },
> - { 0x0305, 0x04 },
> - { 0x0306, 0x00 },
> - { 0x0307, 0xD4 },
> - { 0x0309, 0x0A },
> - { 0x030B, 0x01 },
> - { 0x030D, 0x02 },
> - { 0x030E, 0x00 },
> - { 0x030F, 0xD8 },
> - { 0x0310, 0x00 },
> -
> - { 0x0114, 0x03 },
> - { 0x0820, 0x13 },
> - { 0x0821, 0xE0 },
> - { 0x0822, 0x00 },
> - { 0x0823, 0x00 },
> + REG16(IMX258_REG_EXCK_FREQ, 0x1800),
> + REG8(IMX258_REG_IVTPXCK_DIV, 5),
> + REG8(IMX258_REG_IVTSYCK_DIV, 2),
> + REG8(IMX258_REG_PREPLLCK_VT_DIV, 4),
> + REG16(IMX258_REG_PLL_IVT_MPY, 0x00D4),
> + REG8(IMX258_REG_IOPPXCK_DIV, 10),
> + REG8(IMX258_REG_IOPSYCK_DIV, 1),
> + REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> + REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> + REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> + REG8(IMX258_REG_CSI_LANE_MODE, 3),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x13E0),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
> };
>
> static const struct imx258_reg mipi_640mbps_19_2mhz_2l[] = {
> - { 0x0136, 0x13 },
> - { 0x0137, 0x33 },
> - { 0x0301, 0x05 },
> - { 0x0303, 0x02 },
> - { 0x0305, 0x03 },
> - { 0x0306, 0x00 },
> - { 0x0307, 0x64 },
> - { 0x0309, 0x0A },
> - { 0x030B, 0x01 },
> - { 0x030D, 0x02 },
> - { 0x030E, 0x00 },
> - { 0x030F, 0xD8 },
> - { 0x0310, 0x00 },
> -
> - { 0x0114, 0x01 },
> - { 0x0820, 0x05 },
> - { 0x0821, 0x00 },
> - { 0x0822, 0x00 },
> - { 0x0823, 0x00 },
> + REG16(IMX258_REG_EXCK_FREQ, 0x1333),
> + REG8(IMX258_REG_IVTPXCK_DIV, 5),
> + REG8(IMX258_REG_IVTSYCK_DIV, 2),
> + REG8(IMX258_REG_PREPLLCK_VT_DIV, 3),
> + REG16(IMX258_REG_PLL_IVT_MPY, 0x0064),
> + REG8(IMX258_REG_IOPPXCK_DIV, 10),
> + REG8(IMX258_REG_IOPSYCK_DIV, 1),
> + REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> + REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> + REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> + REG8(IMX258_REG_CSI_LANE_MODE, 1),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x0500),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
> };
>
> static const struct imx258_reg mipi_640mbps_19_2mhz_4l[] = {
> - { 0x0136, 0x13 },
> - { 0x0137, 0x33 },
> - { 0x0301, 0x05 },
> - { 0x0303, 0x02 },
> - { 0x0305, 0x03 },
> - { 0x0306, 0x00 },
> - { 0x0307, 0x64 },
> - { 0x0309, 0x0A },
> - { 0x030B, 0x01 },
> - { 0x030D, 0x02 },
> - { 0x030E, 0x00 },
> - { 0x030F, 0xD8 },
> - { 0x0310, 0x00 },
> -
> - { 0x0114, 0x03 },
> - { 0x0820, 0x0A },
> - { 0x0821, 0x00 },
> - { 0x0822, 0x00 },
> - { 0x0823, 0x00 },
> + REG16(IMX258_REG_EXCK_FREQ, 0x1333),
> + REG8(IMX258_REG_IVTPXCK_DIV, 5),
> + REG8(IMX258_REG_IVTSYCK_DIV, 2),
> + REG8(IMX258_REG_PREPLLCK_VT_DIV, 3),
> + REG16(IMX258_REG_PLL_IVT_MPY, 0x0064),
> + REG8(IMX258_REG_IOPPXCK_DIV, 10),
> + REG8(IMX258_REG_IOPSYCK_DIV, 1),
> + REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> + REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> + REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> + REG8(IMX258_REG_CSI_LANE_MODE, 3),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x0A00),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
> };
>
> static const struct imx258_reg mipi_642mbps_24mhz_2l[] = {
> - { 0x0136, 0x18 },
> - { 0x0137, 0x00 },
> - { 0x0301, 0x05 },
> - { 0x0303, 0x02 },
> - { 0x0305, 0x04 },
> - { 0x0306, 0x00 },
> - { 0x0307, 0x6B },
> - { 0x0309, 0x0A },
> - { 0x030B, 0x01 },
> - { 0x030D, 0x02 },
> - { 0x030E, 0x00 },
> - { 0x030F, 0xD8 },
> - { 0x0310, 0x00 },
> -
> - { 0x0114, 0x01 },
> - { 0x0820, 0x0A },
> - { 0x0821, 0x00 },
> - { 0x0822, 0x00 },
> - { 0x0823, 0x00 },
> + REG16(IMX258_REG_EXCK_FREQ, 0x1800),
> + REG8(IMX258_REG_IVTPXCK_DIV, 5),
> + REG8(IMX258_REG_IVTSYCK_DIV, 2),
> + REG8(IMX258_REG_PREPLLCK_VT_DIV, 4),
> + REG16(IMX258_REG_PLL_IVT_MPY, 0x006B),
> + REG8(IMX258_REG_IOPPXCK_DIV, 10),
> + REG8(IMX258_REG_IOPSYCK_DIV, 1),
> + REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> + REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> + REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> + REG8(IMX258_REG_CSI_LANE_MODE, 1),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x0A00),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
> };
>
> static const struct imx258_reg mipi_642mbps_24mhz_4l[] = {
> - { 0x0136, 0x18 },
> - { 0x0137, 0x00 },
> - { 0x0301, 0x05 },
> - { 0x0303, 0x02 },
> - { 0x0305, 0x04 },
> - { 0x0306, 0x00 },
> - { 0x0307, 0x6B },
> - { 0x0309, 0x0A },
> - { 0x030B, 0x01 },
> - { 0x030D, 0x02 },
> - { 0x030E, 0x00 },
> - { 0x030F, 0xD8 },
> - { 0x0310, 0x00 },
> -
> - { 0x0114, 0x03 },
> - { 0x0820, 0x0A },
> - { 0x0821, 0x00 },
> - { 0x0822, 0x00 },
> - { 0x0823, 0x00 },
> + REG16(IMX258_REG_EXCK_FREQ, 0x1800),
> + REG8(IMX258_REG_IVTPXCK_DIV, 5),
> + REG8(IMX258_REG_IVTSYCK_DIV, 2),
> + REG8(IMX258_REG_PREPLLCK_VT_DIV, 4),
> + REG16(IMX258_REG_PLL_IVT_MPY, 0x006B),
> + REG8(IMX258_REG_IOPPXCK_DIV, 10),
> + REG8(IMX258_REG_IOPSYCK_DIV, 1),
> + REG8(IMX258_REG_PREPLLCK_OP_DIV, 2),
> + REG16(IMX258_REG_PLL_IOP_MPY, 0x00D8),
> + REG8(IMX258_REG_PLL_MULT_DRIV, 0),
> +
> + REG8(IMX258_REG_CSI_LANE_MODE, 3),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_H, 0x0A00),
> + REG16(IMX258_REG_REQ_LINK_BIT_RATE_MBPS_L, 0x0000),
> };
>
> static const struct imx258_reg mode_common_regs[] = {
> @@ -363,45 +368,29 @@ static const struct imx258_reg mode_common_regs[] = {
> { 0x7423, 0xD7 },
> { 0x5F04, 0x00 },
> { 0x5F05, 0xED },
> - { 0x0112, 0x0A },
> - { 0x0113, 0x0A },
> - { 0x0342, 0x14 },
> - { 0x0343, 0xE8 },
> - { 0x0344, 0x00 },
> - { 0x0345, 0x00 },
> - { 0x0346, 0x00 },
> - { 0x0347, 0x00 },
> - { 0x0348, 0x10 },
> - { 0x0349, 0x6F },
> - { 0x034A, 0x0C },
> - { 0x034B, 0x2F },
> - { 0x0381, 0x01 },
> - { 0x0383, 0x01 },
> - { 0x0385, 0x01 },
> - { 0x0387, 0x01 },
> - { 0x0404, 0x00 },
> - { 0x0408, 0x00 },
> - { 0x0409, 0x00 },
> - { 0x040A, 0x00 },
> - { 0x040B, 0x00 },
> - { 0x040C, 0x10 },
> - { 0x040D, 0x70 },
> - { 0x3038, 0x00 },
> - { 0x303A, 0x00 },
> - { 0x303B, 0x10 },
> - { 0x300D, 0x00 },
> - { 0x0350, 0x00 },
> - { 0x0204, 0x00 },
> - { 0x0205, 0x00 },
> - { 0x020E, 0x01 },
> - { 0x020F, 0x00 },
> - { 0x0210, 0x01 },
> - { 0x0211, 0x00 },
> - { 0x0212, 0x01 },
> - { 0x0213, 0x00 },
> - { 0x0214, 0x01 },
> - { 0x0215, 0x00 },
> - { 0x7BCD, 0x00 },
> + REG16(IMX258_REG_CSI_DT_FMT, 0x0a0a),
> + REG16(IMX258_REG_LINE_LENGTH_PCK, 5352),
> + REG16(IMX258_REG_X_ADD_STA, 0),
> + REG16(IMX258_REG_Y_ADD_STA, 0),
> + REG16(IMX258_REG_X_ADD_END, 4207),
> + REG16(IMX258_REG_Y_ADD_END, 3119),
> + REG8(IMX258_REG_X_EVN_INC, 1),
> + REG8(IMX258_REG_X_ODD_INC, 1),
> + REG8(IMX258_REG_Y_EVN_INC, 1),
> + REG8(IMX258_REG_Y_ODD_INC, 1),
> + REG16(IMX258_REG_DIG_CROP_X_OFFSET, 0),
> + REG16(IMX258_REG_DIG_CROP_Y_OFFSET, 0),
> + REG16(IMX258_REG_DIG_CROP_IMAGE_WIDTH, 4208),
> + REG8(IMX258_REG_SCALE_MODE_EXT, 0),
> + REG16(IMX258_REG_SCALE_M_EXT, 16),
> + REG8(IMX258_REG_FORCE_FD_SUM, 0),
> + REG8(IMX258_REG_FRM_LENGTH_CTL, 0),
> + REG16(IMX258_REG_ANALOG_GAIN, 0),
> + REG16(IMX258_REG_GR_DIGITAL_GAIN, 256),
> + REG16(IMX258_REG_R_DIGITAL_GAIN, 256),
> + REG16(IMX258_REG_B_DIGITAL_GAIN, 256),
> + REG16(IMX258_REG_GB_DIGITAL_GAIN, 256),
> + REG8(IMX258_REG_AF_WINDOW_MODE, 0),
> { 0x94DC, 0x20 },
> { 0x94DD, 0x20 },
> { 0x94DE, 0x20 },
> @@ -414,48 +403,39 @@ static const struct imx258_reg mode_common_regs[] = {
> { 0x941B, 0x50 },
> { 0x9519, 0x50 },
> { 0x951B, 0x50 },
> - { 0x3030, 0x00 },
> - { 0x3032, 0x00 },
> - { 0x0220, 0x00 },
> + REG8(IMX258_REG_PHASE_PIX_OUTEN, 0),
> + REG8(IMX258_REG_PDPIX_DATA_RATE, 0),
> + REG8(IMX258_REG_HDR, 0),
> };
>
> static const struct imx258_reg mode_4208x3120_regs[] = {
> - { 0x0900, 0x00 },
> - { 0x0901, 0x11 },
> - { 0x0401, 0x00 },
> - { 0x0405, 0x10 },
> - { 0x040E, 0x0C },
> - { 0x040F, 0x30 },
> - { 0x034C, 0x10 },
> - { 0x034D, 0x70 },
> - { 0x034E, 0x0C },
> - { 0x034F, 0x30 },
> + REG8(IMX258_REG_BINNING_MODE, 0),
> + REG8(IMX258_REG_BINNING_TYPE_V, 0x11),
> + REG8(IMX258_REG_SCALE_MODE, 0),
> + REG16(IMX258_REG_SCALE_M, 16),
> + REG16(IMX258_REG_DIG_CROP_IMAGE_HEIGHT, 3120),
> + REG16(IMX258_REG_X_OUT_SIZE, 4208),
> + REG16(IMX258_REG_Y_OUT_SIZE, 3120),
> };
>
> static const struct imx258_reg mode_2104_1560_regs[] = {
> - { 0x0900, 0x01 },
> - { 0x0901, 0x12 },
> - { 0x0401, 0x01 },
> - { 0x0405, 0x20 },
> - { 0x040E, 0x06 },
> - { 0x040F, 0x18 },
> - { 0x034C, 0x08 },
> - { 0x034D, 0x38 },
> - { 0x034E, 0x06 },
> - { 0x034F, 0x18 },
> + REG8(IMX258_REG_BINNING_MODE, 1),
> + REG8(IMX258_REG_BINNING_TYPE_V, 0x12),
> + REG8(IMX258_REG_SCALE_MODE, 1),
> + REG16(IMX258_REG_SCALE_M, 32),
> + REG16(IMX258_REG_DIG_CROP_IMAGE_HEIGHT, 1560),
> + REG16(IMX258_REG_X_OUT_SIZE, 2104),
> + REG16(IMX258_REG_Y_OUT_SIZE, 1560),
> };
>
> static const struct imx258_reg mode_1048_780_regs[] = {
> - { 0x0900, 0x01 },
> - { 0x0901, 0x14 },
> - { 0x0401, 0x01 },
> - { 0x0405, 0x40 },
> - { 0x040E, 0x03 },
> - { 0x040F, 0x0C },
> - { 0x034C, 0x04 },
> - { 0x034D, 0x18 },
> - { 0x034E, 0x03 },
> - { 0x034F, 0x0C },
> + REG8(IMX258_REG_BINNING_MODE, 1),
> + REG8(IMX258_REG_BINNING_TYPE_V, 0x14),
> + REG8(IMX258_REG_SCALE_MODE, 1),
> + REG16(IMX258_REG_SCALE_M, 64),
> + REG16(IMX258_REG_DIG_CROP_IMAGE_HEIGHT, 780),
> + REG16(IMX258_REG_X_OUT_SIZE, 1048),
> + REG16(IMX258_REG_Y_OUT_SIZE, 780),
> };
>
> struct imx258_variant_cfg {
> @@ -923,7 +903,7 @@ static int imx258_set_ctrl(struct v4l2_ctrl *ctrl)
> }
> break;
> case V4L2_CID_VBLANK:
> - ret = imx258_write_reg(imx258, IMX258_REG_VTS,
> + ret = imx258_write_reg(imx258, IMX258_REG_FRM_LENGTH_LINES,
> IMX258_REG_VALUE_16BIT,
> imx258->cur_mode->height + ctrl->val);
> break;
--
Kind regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH v3 23/25] drivers: media: i2c: imx258: Add support for powerdown gpio
From: Sakari Ailus @ 2024-04-03 16:25 UTC (permalink / raw)
To: git
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, devicetree, imx, linux-arm-kernel, linux-kernel, pavel,
phone-devel, Ondrej Jirman
In-Reply-To: <20240403150355.189229-24-git@luigi311.com>
Hi Luis, Ondrej,
On Wed, Apr 03, 2024 at 09:03:52AM -0600, git@luigi311.com wrote:
> From: Luis Garcia <git@luigi311.com>
>
> On some boards powerdown signal needs to be deasserted for this
> sensor to be enabled.
>
> Signed-off-by: Ondrej Jirman <megi@xff.cz>
> Signed-off-by: Luis Garcia <git@luigi311.com>
> ---
> drivers/media/i2c/imx258.c | 13 +++++++++++++
> 1 file changed, 13 insertions(+)
>
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index 30352c33f63c..163f04f6f954 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -679,6 +679,8 @@ struct imx258 {
> unsigned int lane_mode_idx;
> unsigned int csi2_flags;
>
> + struct gpio_desc *powerdown_gpio;
> +
> /*
> * Mutex for serialized access:
> * Protect sensor module set pad format and start/stop streaming safely.
> @@ -1213,6 +1215,8 @@ static int imx258_power_on(struct device *dev)
> struct imx258 *imx258 = to_imx258(sd);
> int ret;
>
> + gpiod_set_value_cansleep(imx258->powerdown_gpio, 0);
What does the spec say? Should this really happen before switching on the
supplies below?
> +
> ret = regulator_bulk_enable(IMX258_NUM_SUPPLIES,
> imx258->supplies);
> if (ret) {
> @@ -1224,6 +1228,7 @@ static int imx258_power_on(struct device *dev)
> ret = clk_prepare_enable(imx258->clk);
> if (ret) {
> dev_err(dev, "failed to enable clock\n");
> + gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
> regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
> }
>
> @@ -1238,6 +1243,8 @@ static int imx258_power_off(struct device *dev)
> clk_disable_unprepare(imx258->clk);
> regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
>
> + gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
> +
> return 0;
> }
>
> @@ -1541,6 +1548,12 @@ static int imx258_probe(struct i2c_client *client)
> if (!imx258->variant_cfg)
> imx258->variant_cfg = &imx258_cfg;
>
> + /* request optional power down pin */
> + imx258->powerdown_gpio = devm_gpiod_get_optional(&client->dev, "powerdown",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(imx258->powerdown_gpio))
> + return PTR_ERR(imx258->powerdown_gpio);
> +
> /* Initialize subdev */
> v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
>
--
Regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH v14 01/18] irqchip/sifive-plic: Convert PLIC driver into a platform driver
From: Samuel Holland @ 2024-04-03 16:28 UTC (permalink / raw)
To: Lad, Prabhakar, Anup Patel
Cc: devicetree, Conor Dooley, Geert Uytterhoeven, Marc Zyngier,
Anup Patel, Atish Patra, linux-kernel, Saravana Kannan,
Björn Töpel, Rob Herring, Palmer Dabbelt,
Krzysztof Kozlowski, Paul Walmsley, Thomas Gleixner, Frank Rowand,
linux-riscv, linux-arm-kernel, Andrew Jones
In-Reply-To: <CA+V-a8ser=hDmst6+XSeOWaEoOd+iY3Ys6bYBWDa5UYPfT+Pug@mail.gmail.com>
Hi Prabhakar,
On 2024-04-03 10:49 AM, Lad, Prabhakar wrote:
> On Wed, Apr 3, 2024 at 3:17 PM Anup Patel <apatel@ventanamicro.com> wrote:
>>
>> On Wed, Apr 3, 2024 at 2:01 PM Lad, Prabhakar
>> <prabhakar.csengg@gmail.com> wrote:
>>>
>>> Hi Anup,
>>>
>>> On Thu, Feb 22, 2024 at 9:41 AM Anup Patel <apatel@ventanamicro.com> wrote:
>>>>
>>>> The PLIC driver does not require very early initialization so convert
>>>> it into a platform driver.
>>>>
>>>> After conversion, the PLIC driver is probed after CPUs are brought-up
>>>> so setup cpuhp state after context handler of all online CPUs are
>>>> initialized otherwise PLIC driver crashes for platforms with multiple
>>>> PLIC instances.
>>>>
>>>> Signed-off-by: Anup Patel <apatel@ventanamicro.com>
>>>> ---
>>>> drivers/irqchip/irq-sifive-plic.c | 101 ++++++++++++++++++------------
>>>> 1 file changed, 61 insertions(+), 40 deletions(-)
>>>>
>>> This patch seems to have broken things on RZ/Five SoC, after reverting
>>> this patch I get to boot it back again on v6.9-rc2. Looks like there
>>> is some probe order issue after switching to platform driver?
>>
>> Yes, this is most likely related to probe ordering based on your DT.
>>
>> Can you share the failing boot log and DT ?
>
> non working case, https://paste.debian.net/1312947/
Looks like you need to add "keep_bootcon" to your kernel command line to get a
full log here.
> after reverting, https://paste.debian.net/1312948/
> (attached is the DTB)
I don't see anything suspicious between the "riscv-intc" lines and the "Fixed
dependency cycle(s)" lines that looks like it would depend on the PLIC IRQ
domain. Maybe there is some driver that does not handle -EPROBE_DEFER? It's hard
to tell without the full log from the failure case.
Regards,
Samuel
^ permalink raw reply
* Re: [PATCH v3 24/25] drivers: media: i2c: imx258: Add support for reset gpio
From: Sakari Ailus @ 2024-04-03 16:28 UTC (permalink / raw)
To: git
Cc: linux-media, dave.stevenson, jacopo.mondi, mchehab, robh,
krzysztof.kozlowski+dt, conor+dt, shawnguo, s.hauer, kernel,
festevam, devicetree, imx, linux-arm-kernel, linux-kernel, pavel,
phone-devel, Ondrej Jirman
In-Reply-To: <20240403150355.189229-25-git@luigi311.com>
Hi Luis,
Could you unify the subject prefix for the driver patches, please? E.g.
"media: imx258: " would be fine.
On Wed, Apr 03, 2024 at 09:03:53AM -0600, git@luigi311.com wrote:
> From: Luis Garcia <git@luigi311.com>
>
> It was documented in DT, but not implemented.
>
> Signed-off-by: Ondrej Jirman <megous@megous.com>
> Signed-off-by: Luis Garcia <git@luigi311.com>
> ---
> drivers/media/i2c/imx258.c | 14 +++++++++++++-
> 1 file changed, 13 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/media/i2c/imx258.c b/drivers/media/i2c/imx258.c
> index 163f04f6f954..4c117c4829f1 100644
> --- a/drivers/media/i2c/imx258.c
> +++ b/drivers/media/i2c/imx258.c
> @@ -680,6 +680,7 @@ struct imx258 {
> unsigned int csi2_flags;
>
> struct gpio_desc *powerdown_gpio;
> + struct gpio_desc *reset_gpio;
>
> /*
> * Mutex for serialized access:
> @@ -1232,7 +1233,11 @@ static int imx258_power_on(struct device *dev)
> regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
> }
>
> - return ret;
> + gpiod_set_value_cansleep(imx258->reset_gpio, 0);
> +
> + usleep_range(400, 500);
You could mention this at least in the commit message.
> +
> + return 0;
> }
>
> static int imx258_power_off(struct device *dev)
> @@ -1243,6 +1248,7 @@ static int imx258_power_off(struct device *dev)
> clk_disable_unprepare(imx258->clk);
> regulator_bulk_disable(IMX258_NUM_SUPPLIES, imx258->supplies);
>
> + gpiod_set_value_cansleep(imx258->reset_gpio, 1);
Same question than on the other GPIO: does this belong here?
> gpiod_set_value_cansleep(imx258->powerdown_gpio, 1);
>
> return 0;
> @@ -1554,6 +1560,12 @@ static int imx258_probe(struct i2c_client *client)
> if (IS_ERR(imx258->powerdown_gpio))
> return PTR_ERR(imx258->powerdown_gpio);
>
> + /* request optional reset pin */
> + imx258->reset_gpio = devm_gpiod_get_optional(&client->dev, "reset",
> + GPIOD_OUT_HIGH);
> + if (IS_ERR(imx258->reset_gpio))
> + return PTR_ERR(imx258->reset_gpio);
> +
> /* Initialize subdev */
> v4l2_i2c_subdev_init(&imx258->sd, client, &imx258_subdev_ops);
>
--
Regards,
Sakari Ailus
^ 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