* Re: [PATCH 1/3] of: Add a helper to free property struct
From: Jonathan Cameron @ 2024-04-06 16:47 UTC (permalink / raw)
To: Rob Herring; +Cc: Saravana Kannan, devicetree, linux-kernel
In-Reply-To: <20240404-dt-cleanup-free-v1-1-c60e6cba8da9@kernel.org>
On Thu, 04 Apr 2024 09:15:10 -0500
Rob Herring <robh@kernel.org> wrote:
> Freeing a property struct is 3 kfree()'s which is duplicated in multiple
> spots. Add a helper, __of_prop_free(), and replace all the open coded
> cases in the DT code.
>
> Signed-off-by: Rob Herring <robh@kernel.org>
Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com>
^ permalink raw reply
* Re: [PATCH v2 11/11] iio: dac: support the ad9739a RF DAC
From: Jonathan Cameron @ 2024-04-06 16:41 UTC (permalink / raw)
To: Nuno Sa
Cc: linux-iio, devicetree, Dragos Bogdan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Olivier Moysan
In-Reply-To: <20240405-iio-backend-axi-dac-v2-11-293bab7d5552@analog.com>
On Fri, 5 Apr 2024 17:00:09 +0200
Nuno Sa <nuno.sa@analog.com> wrote:
> The AD9739A is a 14-bit, 2.5 GSPS high performance RF DACs that are capable
> of synthesizing wideband signals from DC up to 3 GHz.
>
> A dual-port, source synchronous, LVDS interface simplifies the digital
> interface with existing FGPA/ASIC technology. On-chip controllers are used
> to manage external and internal clock domain variations over temperature to
> ensure reliable data transfer from the host to the DAC core.
>
> Co-developed-by: Dragos Bogdan <dragos.bogdan@analog.com>
> Signed-off-by: Dragos Bogdan <dragos.bogdan@analog.com>
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
The only thing I really have remaining questions on is the choice
of chan_spec with altvoltage and voltage channels. Why does that
split make sense? It's odd enough that some comments in the code would
be a good thing to add.
Jonathan
> new file mode 100644
> index 000000000000..9b91d66f826c
> --- /dev/null
> +++ b/drivers/iio/dac/ad9739a.c
> @@ -0,0 +1,454 @@
> +
> +static struct iio_chan_spec ad9739a_channels[] = {
> + {
> + .type = IIO_ALTVOLTAGE,
So this looks a little unusual. Perhaps some comments on why it
is appropriate to have this channel.
In reality there is only one channel I think?
> + .indexed = 1,
> + .output = 1,
> + .scan_index = -1,
> + },
> + {
> + .type = IIO_VOLTAGE,
> + .indexed = 1,
> + .info_mask_shared_by_all = BIT(IIO_CHAN_INFO_SAMP_FREQ),
> + .output = 1,
> + .ext_info = ad9739a_ext_info,
> + .scan_type = {
> + .sign = 's',
> + .storagebits = 16,
> + .realbits = 16,
> + },
> + }
> +};
^ permalink raw reply
* Re: [PATCH v2 09/11] iio: backend: add new functionality
From: Jonathan Cameron @ 2024-04-06 16:32 UTC (permalink / raw)
To: Nuno Sa
Cc: linux-iio, devicetree, Dragos Bogdan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Olivier Moysan
In-Reply-To: <20240405-iio-backend-axi-dac-v2-9-293bab7d5552@analog.com>
On Fri, 5 Apr 2024 17:00:07 +0200
Nuno Sa <nuno.sa@analog.com> wrote:
> This adds the needed backend ops for supporting a backend inerfacing
> with an high speed dac. The new ops are:
>
> * data_source_set();
> * set_sampling_freq();
> * extend_chan_spec();
> * ext_info_set();
> * ext_info_get().
>
> Also to note the new helpers that are meant to be used by the backends
> when extending an IIO channel (adding extended info):
>
> * iio_backend_ext_info_set();
> * iio_backend_ext_info_get().
>
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
Whilst the code for the backend retrieval callback is simple
I wonder if we are better off just not having it for now.
Keep the infrastructure that checks for the default approach not working
but don't actually provide the alternative until we need it.
Advantage is pretty minor though so maybe just keep it.
Unless others have strong opinions, up to you to decide whether to keep it.
One trivial thing noticed inline.
> ---
> drivers/iio/industrialio-backend.c | 179 +++++++++++++++++++++++++++++++++++++
> include/linux/iio/backend.h | 49 ++++++++++
> 2 files changed, 228 insertions(+)
>
> diff --git a/drivers/iio/industrialio-backend.c b/drivers/iio/industrialio-backend.c
> index 2fea2bbbe47f..ac554798897f 100644
> --- a/drivers/iio/industrialio-backend.c
> +++ b/drivers/iio/industrialio-backend.c
> @@ -29,6 +29,7 @@
> *
> * Copyright (C) 2023-2024 Analog Devices Inc.
> */
> +#include "asm-generic/errno-base.h"
You'll need a strong reason if you want to do that include rather than
a normal one like linux/errno.h
> #define dev_fmt(fmt) "iio-backend: " fmt
>
> #include <linux/cleanup.h>
> @@ -43,10 +44,12 @@
> #include <linux/types.h>
>
> #include <linux/iio/backend.h>
> +#include <linux/iio/iio.h>
>
^ permalink raw reply
* Re: [PATCH v2 03/11] iio: buffer-dma: Enable buffer write support
From: Jonathan Cameron @ 2024-04-06 16:23 UTC (permalink / raw)
To: Nuno Sa
Cc: linux-iio, devicetree, Dragos Bogdan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Olivier Moysan, Paul Cercueil, Alexandru Ardelean
In-Reply-To: <20240405-iio-backend-axi-dac-v2-3-293bab7d5552@analog.com>
On Fri, 5 Apr 2024 17:00:01 +0200
Nuno Sa <nuno.sa@analog.com> wrote:
> From: Paul Cercueil <paul@crapouillou.net>
>
> Adding write support to the buffer-dma code is easy - the write()
> function basically needs to do the exact same thing as the read()
> function: dequeue a block, read or write the data, enqueue the block
> when entirely processed.
>
> Therefore, the iio_buffer_dma_read() and the new iio_buffer_dma_write()
> now both call a function iio_buffer_dma_io(), which will perform this
> task.
>
> Note that we preemptively reset block->bytes_used to the buffer's size
> in iio_dma_buffer_request_update(), as in the future the
> iio_dma_buffer_enqueue() function won't reset it.
>
> Signed-off-by: Paul Cercueil <paul@crapouillou.net>
> Reviewed-by: Alexandru Ardelean <ardeleanalex@gmail.com>
> Signed-off-by: Nuno Sa <nuno.sa@analog.com>
One trivial comment on alignment that I noticed whilst reminding
myself of this patch. Otherwise looks good.
> +
> +/**
> + * iio_dma_buffer_read() - DMA buffer read callback
> + * @buffer: Buffer to read form
> + * @n: Number of bytes to read
> + * @user_buffer: Userspace buffer to copy the data to
> + *
> + * Should be used as the read callback for iio_buffer_access_ops
> + * struct for DMA buffers.
> + */
> +int iio_dma_buffer_read(struct iio_buffer *buffer, size_t n,
> + char __user *user_buffer)
Prefer aligning char with after the (
> +{
> + return iio_dma_buffer_io(buffer, n, user_buffer, false);
> +}
> EXPORT_SYMBOL_GPL(iio_dma_buffer_read);
>
> +/**
> + * iio_dma_buffer_write() - DMA buffer write callback
> + * @buffer: Buffer to read form
> + * @n: Number of bytes to read
> + * @user_buffer: Userspace buffer to copy the data from
> + *
> + * Should be used as the write callback for iio_buffer_access_ops
> + * struct for DMA buffers.
> + */
> +int iio_dma_buffer_write(struct iio_buffer *buffer, size_t n,
> + const char __user *user_buffer)
> +{
> + return iio_dma_buffer_io(buffer, n,
> + (__force __user char *)user_buffer, true);
> +}
> +EXPORT_SYMBOL_GPL(iio_dma_buffer_write);
>
^ permalink raw reply
* Re: [PATCH v2 00/11] iio: dac: support IIO backends on the output direction
From: Jonathan Cameron @ 2024-04-06 16:19 UTC (permalink / raw)
To: Nuno Sa
Cc: linux-iio, devicetree, Dragos Bogdan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Olivier Moysan, Paul Cercueil, Alexandru Ardelean
In-Reply-To: <20240405-iio-backend-axi-dac-v2-0-293bab7d5552@analog.com>
On Fri, 5 Apr 2024 16:59:58 +0200
Nuno Sa <nuno.sa@analog.com> wrote:
> Hi Jonathan,
>
> Here it goes version 2 of the output backend series. The main points in
> here:
> - The refactoring the DMA BUF api for setup. I pretty much like how it
> turned out. Note that Paul's patch ("iio: buffer-dmaengine: Support
> specifying buffer direction") had to be updated accordingly.
> - Introduction of the struct iio_info callback for getting the backend.
> I'm not sure about this one as we have no user for it and we may not
> have one for sometime. I like how the "default" implementation for
> getting the backend turned out and it should cover 99% of the cases. It
> will only fail if the iio parent device is not the same device where we
> bound the backend.
I've not looked at it yet, but this description makes me think perhaps
that should be ripped out for now? We can bring it back if we ever need it.
> - As mentioned above, we now get the backend from the iio device
> matching the IIO parent device with the device used when getting the
> backend. This should cover almost all the cases I think. Should be very
> unlikely to use a different device in devm_iio_backend_get() and
> devm_iio_device_alloc().
>
> For the bindings, I still did not addressed Rob's point about dma-names.
> I did reply [1] but still no feedback.
>
> Anyways, full log:
>
> v1:
> * https://lore.kernel.org/all/20240328-iio-backend-axi-dac-v1-0-afc808b3fde3@analog.com/
>
> v2:
> * Patch 1:
> - New patch.
>
> * Patch 4:
> - Make things consistent with the triggered buffer case.
>
> * Patch 6:
> - Fixed description as it's an output device;
> - Avoid duplicating the "bindings" word in the commit title.
>
> * Patch 7:
> - Renamed vdd_3_3-supply -> vdd-3p3-supply;
> - Added IRQ and vref properties;
> - Avoid duplicating the "bindings" word in the commit title.
>
> * Patch 8:
> - New patch.
>
> * Patch 9:
> - Fixed some typos in kerneldocs;
> - Add iio_backend_from_indio_dev_parent(). Default way of getting backends
> from IIO devices;
> - Explicitly differentiate frontends and backends ext_info in
> iio_backend_extend_chan_spec().
> - Spell out CW as CONTINUOUS_WAVE;
> - Add _hz suffix in set_sample_rate().
>
> * Patch 10:
> - Rephrase comment in axi_dac_set_sample_rate() when DDS is disabled;
> - Use the new iio_dmaengine_buffer_setup_ext() API;
> - Passed tone as 0,1 value being 1 second tone.
>
> * Patch 11:
> - Fixed mixed spaces with tabs in ABI file and dac -> DAC;
> - Add COMPILE_TEST to kconfig;
> - Dropped operating mode enum. Use defines;
> - Add comments for IIO enum operating mode and the value we need to
> set on the device;
> - Add spaces around {} in the reg_sequence;
> - Always use Mu instead of mixture of Mu and MU;
> - Don't error out if we do not recognize the part id;
> - Make sure to deal with other errors than TIMEOUT in ad9739a_init().
>
> [1]: https://lore.kernel.org/linux-iio/04e2a0569953792673319f7fcab3fe03e6670c03.camel@gmail.com/
>
> ---
> Nuno Sa (7):
> iio: buffer-dma: add iio_dmaengine_buffer_setup()
> dt-bindings: iio: dac: add docs for AXI DAC IP
> dt-bindings: iio: dac: add docs for AD9739A
> iio: core: add get_iio_backend() callback
> iio: backend: add new functionality
> iio: dac: add support for AXI DAC IP core
> iio: dac: support the ad9739a RF DAC
>
> Paul Cercueil (4):
> iio: buffer-dma: Rename iio_dma_buffer_data_available()
> iio: buffer-dma: Enable buffer write support
> iio: buffer-dmaengine: Support specifying buffer direction
> iio: buffer-dmaengine: Enable write support
>
> Documentation/ABI/testing/sysfs-bus-iio-ad9739a | 19 +
> .../devicetree/bindings/iio/dac/adi,ad9739a.yaml | 94 +++
> .../devicetree/bindings/iio/dac/adi,axi-dac.yaml | 62 ++
> MAINTAINERS | 17 +
> drivers/iio/adc/adi-axi-adc.c | 16 +-
> drivers/iio/buffer/industrialio-buffer-dma.c | 100 +++-
> drivers/iio/buffer/industrialio-buffer-dmaengine.c | 83 +--
> drivers/iio/dac/Kconfig | 37 ++
> drivers/iio/dac/Makefile | 2 +
> drivers/iio/dac/ad9739a.c | 454 +++++++++++++++
> drivers/iio/dac/adi-axi-dac.c | 635 +++++++++++++++++++++
> drivers/iio/industrialio-backend.c | 179 ++++++
> include/linux/iio/backend.h | 49 ++
> include/linux/iio/buffer-dma.h | 4 +-
> include/linux/iio/buffer-dmaengine.h | 24 +-
> include/linux/iio/iio.h | 2 +
> 16 files changed, 1698 insertions(+), 79 deletions(-)
> ---
> base-commit: 6020ca4de8e5404b20f15a6d9873cd6eb5f6d8d6
> change-id: 20240405-iio-backend-axi-dac-be99373b036b
> --
>
> Thanks!
> - Nuno Sá
>
^ permalink raw reply
* Re: [PATCH v5 02/10] dt-bindings: mailbox: Add mboxes property for CMDQ secure driver
From: Jason-JH Lin (林睿祥) @ 2024-04-06 16:15 UTC (permalink / raw)
To: conor@kernel.org
Cc: linux-kernel@vger.kernel.org, linux-mediatek@lists.infradead.org,
Houlong Wei (魏厚龙),
devicetree@vger.kernel.org, Shawn Sung (宋孝謙),
CK Hu (胡俊光), conor+dt@kernel.org,
robh@kernel.org, linux-arm-kernel@lists.infradead.org,
krzysztof.kozlowski+dt@linaro.org, matthias.bgg@gmail.com,
jassisinghbrar@gmail.com, angelogioacchino.delregno@collabora.com
In-Reply-To: <20240405-remindful-galley-2dee9eec4f34@spud>
On Fri, 2024-04-05 at 17:13 +0100, Conor Dooley wrote:
> On Fri, Apr 05, 2024 at 02:33:14PM +0000, Jason-JH Lin (林睿祥) wrote:
> > On Thu, 2024-04-04 at 15:52 +0100, Conor Dooley wrote:
> > > On Thu, Apr 04, 2024 at 04:31:06AM +0000, Jason-JH Lin (林睿祥)
> > > wrote:
> > > > Hi Conor,
> > > >
> > > > Thanks for the reviews.
> > > >
> > > > On Wed, 2024-04-03 at 16:46 +0100, Conor Dooley wrote:
> > > > > 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?
> > > > >
> > > >
> > > > I forgot to sync with Shawn about this:
> > > > https://lore.kernel.org/all/20240124011459.12204-1-jason-
> > > > jh.lin@mediatek.com
> > > >
> > > > I'll fix it at the next version.
> > >
> > > When I say "fixed" I don't mean "this is wrong, please fix it", I
> > > mean
> > > "why is the value not static for a particular SoC". This needs to
> > > be
> > > explained in the patch (and the description for the event here
> > > needs
> > > to
> > > explain what the gce-mailbox is reserving an event for).
> > >
> >
> > Oh, I see. Thanks for noticing me.
> >
> > We do want to reserve a static event ID for gce-mailbox to
> > different
> > SoCs. There are 2 mainly reasons to why we set it in DTS:
> > 1. There are 1024 events IDs for GCE to use to execute instructions
> > in
> > the specific event happened. These events could be signaled by HW
> > or SW
> > and their value would be different in different SoC because of HW
> > event
> > IDs distribution range from 0 to 1023.
> > If we set a static event ID: 855 for mt8188, it might be conflict
> > the
> > event ID original set in mt8195.
>
> That's not a problem, we have compatibles for this purpose.
I agree that compatibles can do the same things.
>
> > 2. If we defined the event ID in DTS, we might know how many SW or
> > HW
> > event IDs are used.
> > If someone wants to use a new event ID for a new feature, they
> > could
> > find out the used event IDs in DTS easily and avoid the event ID
> > conflicting.
>
> Are the event IDs not documented in the reference manual for the SoC
> in
> question? Or in documentation for the secure world for these devices?
> A
> DTS should not be the authoritive source for this information for
> developers.
>
The event IDs were defined in:
inculde/dt-bindings/mailbox/mediatek,mt8188-gce.h.
> Additionally, the driver could very easily detect if someone does
> happen
> to put in the reserved ID. That could be generically useful (IOW,
> check
> all of them for re-use) if the ID are to not allowed to be shared.
>
> > The reason why we define a event ID is we want to get a SW signal
> > from
> > secure world. We design a GCE looping thread in gce-mailbox driver
> > to
> > wait for the GCE execute done event for each cmdq secure packets
> > from
> > secure world.
>
> This sort of information needs to be in the commit message, but I
> don't
> think this property is needed at all since it seems to be something
> detectable from the compatible.
I think put this event ID in driver data and distinguish them by
different compatibles can achieve the same thing.
However, I originally thought that align to the existing way like
MUTEX, CCORR, WDMA in
https://lore.kernel.org/all/20240124011459.12204-4-jason-jh.lin@mediatek.com
would be better choice.
I think their usage of gce-events are the same.
What do you think?
Regards,
Jason-JH.Lin
^ permalink raw reply
* Re: [PATCH] ARM: dts: qcom: msm8974-sony-shinano: Enable vibrator
From: Dmitry Baryshkov @ 2024-04-06 16:07 UTC (permalink / raw)
To: Luca Weiss
Cc: ~postmarketos/upstreaming, phone-devel, Bjorn Andersson,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-arm-msm, devicetree, linux-kernel
In-Reply-To: <20240406-shinano-vib-v1-1-fdd02af39d56@z3ntu.xyz>
On Sat, 6 Apr 2024 at 18:36, Luca Weiss <luca@z3ntu.xyz> wrote:
>
> Enable the vibrator connected to PM8941 found on the Sony shinano
> platform.
>
> Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
> ---
> .../arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi | 4 ++++
> 1 file changed, 4 insertions(+)
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
--
With best wishes
Dmitry
^ permalink raw reply
* Re: [PATCH v5 0/2] iio: temperature: ltc2983: small improvements
From: Jonathan Cameron @ 2024-04-06 15:49 UTC (permalink / raw)
To: Nuno Sa
Cc: linux-iio, devicetree, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley,
Krzysztof Kozlowski
In-Reply-To: <20240404-ltc2983-misc-improv-v5-0-c1f58057fcea@analog.com>
On Thu, 4 Apr 2024 10:58:16 +0200
Nuno Sa <nuno.sa@analog.com> wrote:
> Hi Jonathan,
>
> As suggested by you, v5 only has the regulator stuff. I'll send another
> series for the new dev_errp_helper().
Applied.
Thanks,
Jonathan
>
> ---
> Changes in v5:
> - Dropped patches 1,2,5 and 6.
> - Link to v4: https://lore.kernel.org/all/20240328-ltc2983-misc-improv-v4-0-0cc428c07cd5@analog.com/
>
> ---
> Nuno Sa (2):
> dt-bindings: iio: temperature: ltc2983: document power supply
> iio: temperature: ltc2983: support vdd regulator
>
> Documentation/devicetree/bindings/iio/temperature/adi,ltc2983.yaml | 4 ++++
> drivers/iio/temperature/ltc2983.c | 5 +++++
> 2 files changed, 9 insertions(+)
> ---
> base-commit: 6020ca4de8e5404b20f15a6d9873cd6eb5f6d8d6
> change-id: 20240222-ltc2983-misc-improv-1c7a78ece93f
> --
>
> Thanks!
> - Nuno Sá
>
^ permalink raw reply
* [PATCH] ARM: dts: qcom: msm8974-sony-shinano: Enable vibrator
From: Luca Weiss @ 2024-04-06 15:27 UTC (permalink / raw)
To: ~postmarketos/upstreaming, phone-devel, Bjorn Andersson,
Konrad Dybcio, Rob Herring, Krzysztof Kozlowski, Conor Dooley
Cc: linux-arm-msm, devicetree, linux-kernel, Luca Weiss
Enable the vibrator connected to PM8941 found on the Sony shinano
platform.
Signed-off-by: Luca Weiss <luca@z3ntu.xyz>
---
.../arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi b/arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi
index 3a0c0035de09..e129bb1bd6ec 100644
--- a/arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi
+++ b/arch/arm/boot/dts/qcom/qcom-msm8974pro-sony-xperia-shinano-common.dtsi
@@ -202,6 +202,10 @@ led@7 {
};
};
+&pm8941_vib {
+ status = "okay";
+};
+
&remoteproc_adsp {
cx-supply = <&pm8841_s2>;
status = "okay";
---
base-commit: 956abeb75f90eac3d5ba1f4cff7c048f7c079502
change-id: 20240406-shinano-vib-80e27e9bab2c
Best regards,
--
Luca Weiss <luca@z3ntu.xyz>
^ permalink raw reply related
* Re: [PATCH v7 0/8] iio: accel: adxl345: Add spi-3wire feature
From: Jonathan Cameron @ 2024-04-06 15:19 UTC (permalink / raw)
To: Lothar Rubusch
Cc: lars, Michael.Hennerich, robh+dt, krzysztof.kozlowski+dt,
conor+dt, linux-iio, devicetree, linux-kernel, eraretuya
In-Reply-To: <20240401194906.56810-1-l.rubusch@gmail.com>
On Mon, 1 Apr 2024 19:48:58 +0000
Lothar Rubusch <l.rubusch@gmail.com> wrote:
> Pass a function setup() as pointer from SPI/I2C specific modules to the
> core module. Implement setup() to pass the spi-3wire bus option, if
> declared in the device-tree.
>
> In the core module then update data_format register configuration bits
> instead of overwriting it. The changes allow to remove a data_range field.
>
> Signed-off-by: Lothar Rubusch <l.rubusch@gmail.com>
Series applied to the togreg branch of iio.git and pushed out initially
as testing to let 0-day work it's magic (find build issues for us!)
I'll push it out for linux-next to pick up sometime in next week (ish)
Jonathan
> ---
> V1 -> V2: Split into spi-3wire and refactoring
> V2 -> V3: Split further, focus on needed changesets
> V3 -> V4: Drop "Remove single info instances";
> split "Group bus configuration" into separat
> comment patch; reorder patch set
> V4 -> V5: Refrase comments; Align comments to 75; rebuild FORMAT_MASK by
> available flags; fix indention
> V5 -> V6: Remove FORMAT_MASK by a local variable on call site;
> Refrase comments;
> Remove unneeded include
> V6 -> V7: Restructure optional passing the setup() to core's probe()
> Guarantee that initially a regmap_write() was called to init
> all bits to a defined state
> - When a setup() e.g. for 3wire is passed, then call
> regmap_write() inside the setup(). In the following
> core's probe() has to call regmap_update()
> - When NULL is passed, then call regmap_write() in core's
> probe()
> - Refactoring: remove obvious comments and simplify code
>
> Lothar Rubusch (8):
> iio: accel: adxl345: Make data_range obsolete
> iio: accel: adxl345: Group bus configuration
> iio: accel: adxl345: Move defines to header
> dt-bindings: iio: accel: adxl345: Add spi-3wire
> iio: accel: adxl345: Pass function pointer to core
> iio: accel: adxl345: Reorder probe initialization
> iio: accel: adxl345: Add comment to probe
> iio: accel: adxl345: Add spi-3wire option
>
> .../bindings/iio/accel/adi,adxl345.yaml | 2 +
> drivers/iio/accel/adxl345.h | 36 +++++++-
> drivers/iio/accel/adxl345_core.c | 92 ++++++++++---------
> drivers/iio/accel/adxl345_i2c.c | 2 +-
> drivers/iio/accel/adxl345_spi.c | 10 +-
> 5 files changed, 94 insertions(+), 48 deletions(-)
>
^ permalink raw reply
* Re: [PATCH 6/6] iio: adc: ad7173: Add support for AD411x devices
From: Jonathan Cameron @ 2024-04-06 15:10 UTC (permalink / raw)
To: David Lechner
Cc: Ceclan, Dumitru, dumitru.ceclan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel
In-Reply-To: <CAMknhBFZKKim4JyXLpHY=EoyW4ZPp03aHte5xy0ZkDnW16sfeA@mail.gmail.com>
> >
> > >> + case IIO_CURRENT:
> > >> + *val = ad7173_get_ref_voltage_milli(st, ch->cfg.ref_sel);
> > >> + *val /= AD4111_SHUNT_RESISTOR_OHM;
> > >> + *val2 = chan->scan_type.realbits - !!(ch->cfg.bipolar);
> > >
> > > Static analysis tools like to complain about using bool as int.
> > > Probably more clear to write it as (ch->cfg.bipolar ? 1 : 0) anyway.
> > >
> > Maybe it does not apply here, but i followed this advice:
> >
> > Andy Shevchenko V1 of AD7173 (named initially ad717x)
> > "
> > > + return (bool)(value & mask);
> >
> > This is weird. You have int which you get from bool, wouldn't be better
> > to use
> > !!(...) as other GPIO drivers do?
>
> As long as the build bots don't complain, there isn't a reason to
> change it. It is just a matter of personal preference at that point.
>
> I got a sparse warning for something like this recently [1], but maybe
> that case was just because it was inside of a FIELD_PREP() using it as
> bit logic instead of addition and we won't get any warnings here.
>
> [1]: https://lore.kernel.org/linux-iio/20240129195611.701611-3-dlechner@baylibre.com/
It was common to use !! for a number of years, but then it got a
comment from Linus Torvalds making reasonable point that it isn't
easy to read, so slight preference these days is for a ternary.
^ permalink raw reply
* Re: [PATCH 4/6] iio: adc: ad7173: refactor ain and vref selection
From: Jonathan Cameron @ 2024-04-06 15:03 UTC (permalink / raw)
To: Dumitru Ceclan via B4 Relay
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
In-Reply-To: <20240401-ad4111-v1-4-34618a9cc502@analog.com>
On Mon, 01 Apr 2024 18:32:22 +0300
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>
A few line wrapping comments inline.
> ---
> drivers/iio/adc/ad7173.c | 59 +++++++++++++++++++++++++++++++++---------------
> 1 file changed, 41 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index 699bc6970790..bf5a5b384fe2 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -910,6 +910,41 @@ static int ad7173_register_clk_provider(struct iio_dev *indio_dev)
> &st->int_clk_hw);
> }
>
> +static int ad7173_validate_voltage_ain_inputs(struct ad7173_state *st,
> + unsigned int ain[2])
> +{
> + struct device *dev = &st->sd.spi->dev;
> +
> + if (ain[0] >= st->info->num_inputs ||
> + ain[1] >= st->info->num_inputs)
No need to line wrap the above - its under 80 chars on one line with the
new reduced indent due to factoring this out.
> + return dev_err_probe(dev, -EINVAL,
> + "Input pin number out of range for pair (%d %d).\n",
> + ain[0], ain[1]);
> +
> + return 0;
> +}
> +
> +static int ad7173_validate_reference(struct ad7173_state *st, int ref_sel)
> +{
> + struct device *dev = &st->sd.spi->dev;
> + int ret;
> +
> + if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF && !st->info->has_int_ref)
> + return dev_err_probe(dev, -EINVAL,
> + "Internal reference is not available on current model.\n");
> +
> + if (ref_sel == AD7173_SETUP_REF_SEL_EXT_REF2 && !st->info->has_ref2)
> + return dev_err_probe(dev, -EINVAL,
> + "External reference 2 is not available on current model.\n");
> +
> + ret = ad7173_get_ref_voltage_milli(st, ref_sel);
> + if (ret < 0)
> + return dev_err_probe(dev, ret,
> + "Cannot use reference %u\n", ref_sel);
Can pull the string to previous line and then align ref_sel just after (
whilst still remaining under 80 chars and end up a little prettier.
> +
> + return 0;
> +}
> +
> static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> {
> struct ad7173_channel *chans_st_arr, *chan_st_priv;
> @@ -970,11 +1005,9 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> if (ret)
> return ret;
>
> - if (ain[0] >= st->info->num_inputs ||
> - ain[1] >= st->info->num_inputs)
> - return dev_err_probe(dev, -EINVAL,
> - "Input pin number out of range for pair (%d %d).\n",
> - ain[0], ain[1]);
> + ret = ad7173_validate_voltage_ain_inputs(st, ain);
> + if (ret)
> + return ret;
>
> ret = fwnode_property_match_property_string(child,
> "adi,reference-select",
> @@ -985,19 +1018,9 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> else
> ref_sel = ret;
>
> - if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF &&
> - !st->info->has_int_ref)
> - return dev_err_probe(dev, -EINVAL,
> - "Internal reference is not available on current model.\n");
> -
> - if (ref_sel == AD7173_SETUP_REF_SEL_EXT_REF2 && !st->info->has_ref2)
> - return dev_err_probe(dev, -EINVAL,
> - "External reference 2 is not available on current model.\n");
> -
> - ret = ad7173_get_ref_voltage_milli(st, ref_sel);
> - if (ret < 0)
> - return dev_err_probe(dev, ret,
> - "Cannot use reference %u\n", ref_sel);
> + ret = ad7173_validate_reference(st, ref_sel);
> + if (ret)
> + return ret;
>
> if (ref_sel == AD7173_SETUP_REF_SEL_INT_REF)
> st->adc_mode |= AD7173_ADC_MODE_REF_EN;
>
^ permalink raw reply
* Re: [PATCH 2/6] iio: adc: ad7173: fix buffers enablement for ad7176-2
From: Jonathan Cameron @ 2024-04-06 14:56 UTC (permalink / raw)
To: Dumitru Ceclan via B4 Relay
Cc: dumitru.ceclan, Lars-Peter Clausen, Michael Hennerich,
Rob Herring, Krzysztof Kozlowski, Conor Dooley, David Lechner,
linux-iio, devicetree, linux-kernel, Dumitru Ceclan
In-Reply-To: <20240401-ad4111-v1-2-34618a9cc502@analog.com>
On Mon, 01 Apr 2024 18:32:20 +0300
Dumitru Ceclan via B4 Relay <devnull+dumitru.ceclan.analog.com@kernel.org> wrote:
> From: Dumitru Ceclan <dumitru.ceclan@analog.com>
>
> AD7176-2 does not feature input buffers, enable buffers only on
> supported models.
>
> Fixes: cff259bf7274 ("iio: adc: ad7173: fix buffers enablement for ad7176-2")
> Signed-off-by: Dumitru Ceclan <dumitru.ceclan@analog.com>
How bad is this? If you can find out if writing those bits does anything
harmful (they are reserved and datasheet says should be written 0 I think)
That will help people decide whether to backport the fix?
> ---
> drivers/iio/adc/ad7173.c | 11 +++++++++--
> 1 file changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/iio/adc/ad7173.c b/drivers/iio/adc/ad7173.c
> index f6d29abe1d04..8a95b1391826 100644
> --- a/drivers/iio/adc/ad7173.c
> +++ b/drivers/iio/adc/ad7173.c
> @@ -145,6 +145,7 @@ struct ad7173_device_info {
> unsigned int id;
> char *name;
> bool has_temp;
> + bool has_input_buf;
> bool has_int_ref;
> bool has_ref2;
> u8 num_gpios;
> @@ -212,6 +213,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 4,
> .num_gpios = 2,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .clock = 2 * HZ_PER_MHZ,
> .sinc5_data_rates = ad7173_sinc5_data_rates,
> @@ -224,6 +226,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 8,
> .num_gpios = 4,
> .has_temp = false,
> + .has_input_buf = true,
> .has_ref2 = true,
> .clock = 2 * HZ_PER_MHZ,
> .sinc5_data_rates = ad7173_sinc5_data_rates,
> @@ -237,6 +240,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 8,
> .num_gpios = 4,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .has_ref2 = true,
> .clock = 2 * HZ_PER_MHZ,
> @@ -251,6 +255,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 4,
> .num_gpios = 2,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .clock = 16 * HZ_PER_MHZ,
> .sinc5_data_rates = ad7175_sinc5_data_rates,
> @@ -263,6 +268,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 8,
> .num_gpios = 4,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .has_ref2 = true,
> .clock = 16 * HZ_PER_MHZ,
> @@ -289,6 +295,7 @@ static const struct ad7173_device_info ad7173_device_info[] = {
> .num_configs = 4,
> .num_gpios = 2,
> .has_temp = true,
> + .has_input_buf = true,
> .has_int_ref = true,
> .clock = 16 * HZ_PER_MHZ,
> .odr_start_value = AD7177_ODR_START_VALUE,
> @@ -932,7 +939,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
> AD7173_CH_ADDRESS(chan_arr[chan_index].channel,
> chan_arr[chan_index].channel2);
> chan_st_priv->cfg.bipolar = false;
> - chan_st_priv->cfg.input_buf = true;
> + chan_st_priv->cfg.input_buf = st->info->has_input_buf;
> chan_st_priv->cfg.ref_sel = AD7173_SETUP_REF_SEL_INT_REF;
> st->adc_mode |= AD7173_ADC_MODE_REF_EN;
>
> @@ -989,7 +996,7 @@ static int ad7173_fw_parse_channel_config(struct iio_dev *indio_dev)
>
> chan_st_priv->ain = AD7173_CH_ADDRESS(ain[0], ain[1]);
> chan_st_priv->chan_reg = chan_index;
> - chan_st_priv->cfg.input_buf = true;
> + chan_st_priv->cfg.input_buf = st->info->has_input_buf;
> chan_st_priv->cfg.odr = 0;
>
> chan_st_priv->cfg.bipolar = fwnode_property_read_bool(child, "bipolar");
>
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: adc: ad7173: add support for ad411x
From: Jonathan Cameron @ 2024-04-06 14:53 UTC (permalink / raw)
To: David Lechner
Cc: Ceclan, Dumitru, dumitru.ceclan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel
In-Reply-To: <CAMknhBHu8DveBgV3cor8RP2Up4Zs-+QRx7S2aoHZ_3iKiErVjg@mail.gmail.com>
On Wed, 3 Apr 2024 10:40:39 -0500
David Lechner <dlechner@baylibre.com> wrote:
> 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.
Maybe Jonathan was wrong! I was younger then than now :)
However, reg values for child nodes are unique so can't just use a flag these
need to be different values.
> > 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.
The <13 10> option makes sense to me and avoids suggesting a common negative
input.
The 'fun' bit here is that diff-channels doesn't actually tell us anything.
So we could just not provide it and rely on documentation of reg = 16-19 meaning
the current channels?
>
> >
> > >> +
> > >> + 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.
Hmm. That whole approach to pseudo differential does get messy if
we have the common line routed through the main MUX rather than an opt
in only on the negative side.
If I read this right, its almost a trick to support a pseudo differential
wiring with simple registers (I guess reflecting MUX limitations).
So what could we do?
We could assume that VINCOM is used like a conventional pseudo
differential negative signal and have supply-vincom + non diffferential
channels if that's the configuration wanted.
Then for differential channels can support all the VINX VINX+1
and swapped options.
For VIN10 it gets fun as non differential and differential options
I think map to same actual config. Don't see reason we need to express
that in the binding though so let that have VIN10 VINCOM (probably using
a magic channel number) and VIN10 pseudo differential.
Similar setup for ADCIN15 equivalent usage
Code wise this probably won't be particular hard to support in the driver
(obviously I haven't tried though :) is it worth the effort to keep
it inline with other devices that support pseudo differential channesl.
>
> > >> 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.
> >
That's an option as well. In many early drivers we just provided all the
channels. Somewhat depends on whether people buy devices with lots of
channels they don't wire. Mostly I suspect they don't as that's money
wasted!
Jonathan
^ permalink raw reply
* Re: [PATCH v2 0/3] Add support for the IPQ5321 SoC
From: Kathiravan Thirumoorthy @ 2024-04-06 14:48 UTC (permalink / raw)
To: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Ilia Lin, Rafael J. Wysocki, Viresh Kumar
Cc: linux-arm-msm, devicetree, linux-kernel, linux-pm,
Krzysztof Kozlowski, Mukesh Ojha
In-Reply-To: <20240325-ipq5321-sku-support-v2-0-f30ce244732f@quicinc.com>
On 3/25/2024 9:19 PM, Kathiravan Thirumoorthy wrote:
> IPQ5321 SoC belong to IPQ5332 family. Add the SoC ID and the cpufreq
> support. Maximum cpufreq for IPQ5321 is 1.1GHZ, which is determined
> based on the eFuse.
>
> Viresh is okay to merge the cpufreq change via qcom tree[1] and provided
> his Ack.
>
> [1]
> https://lore.kernel.org/linux-arm-msm/20240306053200.6iwrviltwt3pnfnt@vireshk-i7/
Gentle Reminder...
>
> Signed-off-by: Kathiravan Thirumoorthy <quic_kathirav@quicinc.com>
> ---
> Changes in v2:
> - rebase on next-20240325
> - pick up the tags
> - Link to v1:
> https://lore.kernel.org/linux-arm-msm/20240228-ipq5321-sku-support-v1-0-14e4d4715f4b@quicinc.com/
>
> ---
> Kathiravan Thirumoorthy (3):
> dt-bindings: arm: qcom,ids: Add SoC ID for IPQ5321
> soc: qcom: socinfo: Add SoC ID for IPQ5321
> cpufreq: qcom-nvmem: add support for IPQ5321
>
> drivers/cpufreq/qcom-cpufreq-nvmem.c | 1 +
> drivers/soc/qcom/socinfo.c | 1 +
> include/dt-bindings/arm/qcom,ids.h | 1 +
> 3 files changed, 3 insertions(+)
> ---
> base-commit: 1fdad13606e104ff103ca19d2d660830cb36d43e
> change-id: 20240228-ipq5321-sku-support-bd07056d5e01
>
> Best regards,
^ permalink raw reply
* Re: [PATCH 1/6] dt-bindings: adc: ad7173: add support for ad411x
From: Jonathan Cameron @ 2024-04-06 14:26 UTC (permalink / raw)
To: Ceclan, Dumitru
Cc: David Lechner, dumitru.ceclan, Lars-Peter Clausen,
Michael Hennerich, Rob Herring, Krzysztof Kozlowski, Conor Dooley,
linux-iio, devicetree, linux-kernel
In-Reply-To: <aa84a3c5-a3e6-4c76-9b67-624ed8d8c704@gmail.com>
On Thu, 4 Apr 2024 16:08:56 +0300
"Ceclan, Dumitru" <mitrutzceclan@gmail.com> wrote:
> On 03/04/2024 18:22, David Lechner wrote:
> > On Wed, Apr 3, 2024 at 2:50 AM Ceclan, Dumitru <mitrutzceclan@gmail.com> wrote:
> >> On 02/04/2024 00:16, David Lechner wrote:
> >>> On Mon, Apr 1, 2024 at 2:37 PM David Lechner <dlechner@baylibre.com> 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.
> >>>>
> >>>>> 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>
> >>> Thinking about this a bit more...
> >>>
> >>> Since the current inputs have dedicated pins and aren't mix-and-match
> >>> with multiple valid wiring configurations like the voltage inputs, do
> >>> we even need to describe them in the devicetree?
> >>>
> >>> In the driver, the current channels would just be hard-coded like the
> >>> temperature channel since there isn't any application-specific
> >>> variation.
> >> Sure, but we still need to offer the user a way to configure which
> >> current inputs he wants and if they should use bipolar or unipolar coding.
> > From the datasheet, it looks like only positive current input is
> > allowed so I'm not sure bipolar applies here. But, yes, if there is
> > some other variation in wiring or electrical signal that needs to be
> > describe here, then it makes sense to allow a channel configuration
> > node for it.
>
> AD4111 datasheet pg.29:
> When the ADC is configured for bipolar operation, the output
> code is offset binary with a negative full-scale voltage resulting
> in a code of 000 … 000, a zero differential input voltage resulting in
> a code of 100 … 000, and a positive full-scale input voltage
> resulting in a code of 111 … 111. The output code for any
> analog input voltage can be represented as
> Code = 2^(N – 1) × ((V_IN × 0.1/V REF) + 1)
> The output code for any input current is represented as
> Code = 2^(N − 1) × ((I_IN × 50 Ω/V REF) + 1)
>
> I would say bipolar applies here, not a great idea because of the limitation on
> the negative side (Input Current Range min:−0.5 max:+24 mA) so still, the option
> is available.
Just to check I am correct in thinking you 'might' use bipolar if you want
to be able to measure small negative currents, but the range is much larger
in the positive direction?
J
>
^ permalink raw reply
* [PATCH v2] dt-bindings: ti,pcm1681: Convert to dtschema
From: Animesh Agarwal @ 2024-04-06 14:14 UTC (permalink / raw)
Cc: animeshagarwal28, Shenghao Ding, Kevin Lu, Baojun Xu,
Liam Girdwood, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, alsa-devel, devicetree, linux-kernel
Convert the Texas Instruments PCM1681 bindings to DT schema.
Make bindings complete by adding #sound-dai-cells.
Signed-off-by: Animesh Agarwal <animeshagarwal28@gmail.com>
---
Changes in v2:
- Added existing maintainers to the list of maintainers.
- Added #sound-dai-cells, $ref to dai-common and unevaluatedProperties: false.
- Added "audio-codec" in example.
---
.../devicetree/bindings/sound/ti,pcm1681.txt | 15 -------
.../devicetree/bindings/sound/ti,pcm1681.yaml | 43 +++++++++++++++++++
2 files changed, 43 insertions(+), 15 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/sound/ti,pcm1681.txt
create mode 100644 Documentation/devicetree/bindings/sound/ti,pcm1681.yaml
diff --git a/Documentation/devicetree/bindings/sound/ti,pcm1681.txt b/Documentation/devicetree/bindings/sound/ti,pcm1681.txt
deleted file mode 100644
index 4df17185ab80..000000000000
--- a/Documentation/devicetree/bindings/sound/ti,pcm1681.txt
+++ /dev/null
@@ -1,15 +0,0 @@
-Texas Instruments PCM1681 8-channel PWM Processor
-
-Required properties:
-
- - compatible: Should contain "ti,pcm1681".
- - reg: The i2c address. Should contain <0x4c>.
-
-Examples:
-
- i2c_bus {
- pcm1681@4c {
- compatible = "ti,pcm1681";
- reg = <0x4c>;
- };
- };
diff --git a/Documentation/devicetree/bindings/sound/ti,pcm1681.yaml b/Documentation/devicetree/bindings/sound/ti,pcm1681.yaml
new file mode 100644
index 000000000000..5aa00617291c
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/ti,pcm1681.yaml
@@ -0,0 +1,43 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/sound/ti,pcm1681.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Texas Instruments PCM1681 8-channel PWM Processor
+
+maintainers:
+ - Shenghao Ding <shenghao-ding@ti.com>
+ - Kevin Lu <kevin-lu@ti.com>
+ - Baojun Xu <baojun.xu@ti.com>
+
+allOf:
+ - $ref: dai-common.yaml#
+
+properties:
+ compatible:
+ const: ti,pcm1681
+
+ reg:
+ maxItems: 1
+
+ "#sound-dai-cells":
+ const: 0
+
+required:
+ - compatible
+ - reg
+
+unevaluatedProperties: false
+
+examples:
+ - |
+ i2c {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ pcm1681: audio-codec@4c {
+ compatible = "ti,pcm1681";
+ reg = <0x4c>;
+ };
+ };
--
2.44.0
^ permalink raw reply related
* Re: [PATCH v2 1/4] dt-bindings: clock: airoha: add EN7581 binding
From: Krzysztof Kozlowski @ 2024-04-06 11:50 UTC (permalink / raw)
To: Lorenzo Bianconi, 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: <99734deb28889e685a764da94418f68b55ee3bdc.1712399981.git.lorenzo@kernel.org>
On 06/04/2024 12:43, Lorenzo Bianconi wrote:
> Introduce Airoha EN7581 entry in Airoha EN7523 clock binding
>
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
> .../bindings/clock/airoha,en7523-scu.yaml | 31 +++++++++++++++++--
> 1 file changed, 29 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..3f4266637733 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
Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH v2 02/11] spi: dt-bindings: cdns,qspi-nor: sort compatibles alphabetically
From: Krzysztof Kozlowski @ 2024-04-06 11:38 UTC (permalink / raw)
To: Théo Lebrun, Mark Brown, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Vaishnav Achath, Thomas Bogendoerfer, Rob Herring
Cc: linux-spi, devicetree, linux-kernel, linux-mips,
Vladimir Kondratiev, Gregory CLEMENT, Thomas Petazzoni,
Tawfik Bayouk
In-Reply-To: <20240405-cdns-qspi-mbly-v2-2-956679866d6d@bootlin.com>
On 05/04/2024 17:02, Théo Lebrun wrote:
> Compatibles are ordered by date of addition.
> Switch to (deterministic) alphabetical ordering.
>
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
> Documentation/devicetree/bindings/spi/cdns,qspi-nor.yaml | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Best regards,
Krzysztof
^ permalink raw reply
* Re: [PATCH] dt-bindings: extcon: ptn5150: Document the 'port' node
From: Krzysztof Kozlowski @ 2024-04-06 11:26 UTC (permalink / raw)
To: Fabio Estevam
Cc: cw00.choi, myungjoo.ham, robh, conor+dt, devicetree, marex,
Fabio Estevam
In-Reply-To: <CAOMZO5A7yiYdeOjv6BabDSNKk4Gp=n1qwJP0seFvmjYYgjvhOA@mail.gmail.com>
On 05/04/2024 21:58, Fabio Estevam wrote:
> On Fri, Apr 5, 2024 at 6:24 AM Krzysztof Kozlowski <krzk@kernel.org> wrote:
>
>> But more important, what about USB C connector?
>
> In a previous attempt, I tried describing the USB connector:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.9-rc2&id=a4dca89fe8a1585af73e362f5f4e3189a00abf8e
>
> This broke the USB support as reported by Marek, so I had to revert it:
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?h=v6.9-rc2&id=f954785a124e77d4e6bb52cab689a8de447999aa
>
> 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.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v2 3/3] arm64: dts: qcom: msm8916-samsung-rossa: Add LIS2HH12 accelerometer
From: Raymond Hackley @ 2024-04-06 11:15 UTC (permalink / raw)
To: linux-kernel
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stephan Gerhold, Nikita Travkin, linux-arm-msm,
devicetree, ~postmarketos/upstreaming
In-Reply-To: <20240406111348.14358-1-raymondhackley@protonmail.com>
Core Prime LTE uses ST LIS2HH12 accelerometer. Add support for it.
[Stephen: Use common &st_accel definition from common dtsi]
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
Signed-off-by: Raymond Hackley <raymondhackley@protonmail.com>
---
.../arm64/boot/dts/qcom/msm8916-samsung-rossa-common.dtsi | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-rossa-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-rossa-common.dtsi
index b438fa81886c..db95bdbb9f32 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-rossa-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-rossa-common.dtsi
@@ -26,3 +26,11 @@ &clk_pwm {
&clk_pwm_backlight {
status = "disabled";
};
+
+&st_accel {
+ compatible = "st,lis2hh12";
+ mount-matrix = "1", "0", "0",
+ "0", "-1", "0",
+ "0", "0", "1";
+ status = "okay";
+};
--
2.39.2
^ permalink raw reply related
* Re: [PATCH v19 2/9] usb: dwc3: core: Access XHCI address space temporarily to read port info
From: Krzysztof Kozlowski @ 2024-04-06 11:14 UTC (permalink / raw)
To: Dmitry Baryshkov, Bjorn Andersson
Cc: Greg Kroah-Hartman, Johan Hovold, Krishna Kurapati,
Krzysztof Kozlowski, Rob Herring, Bjorn Andersson, Wesley Cheng,
Konrad Dybcio, Conor Dooley, Thinh Nguyen, Felipe Balbi,
devicetree, linux-arm-msm, linux-usb, linux-kernel, quic_ppratap,
quic_jackp, Johan Hovold
In-Reply-To: <CAA8EJpr94_uCHoJyQd2uQyZ35xXixPPMHPVGXciZn+GftUUjnA@mail.gmail.com>
On 05/04/2024 22:36, Dmitry Baryshkov wrote:
>>>>>
>>>>> Also get those internal reviewers to sign-off on the commits and have
>>>>> that show up when you post them next. That way they are also
>>>>> responsible for this patchset, it's not fair that they are making you do
>>>>> all the work here :)
>>>>>
>>>>
>>>> I like this idea and I'm open to us changing our way of handling this.
>>>>
>>>> But unless such internal review brings significant input to the
>>>> development I'd say a s-o-b would take the credit from the actual
>>>> author.
>>>
>>> It does not do that at all. It provides proof that someone else has
>>> reviewed it and agrees with it. Think of it as a "path of blame" for
>>> when things go bad (i.e. there is a bug in the submission.) Putting
>>> your name on it makes you take responsibility if that happens.
>>>
>>
>> Right, this is why I like your idea.
>>
>> But as s-o-b either builds a trail of who handled the patch, or reflects
>> that it was co-authored by multiple people, I don't think either one
>> properly reflects reality.
>>
>>>> We've discussed a few times about carrying Reviewed-by et al from the
>>>> internal reviews, but as maintainer I dislike this because I'd have no
>>>> way to know if a r-b on vN means the patch was reviewed, or if it was
>>>> just "accidentally" carried from v(N-1).
>>>> But it might be worth this risk, is this something you think would be
>>>> appropriate?
>>>
>>> For some companies we REQUIRE this to happen due to low-quality
>>> submissions and waste of reviewer's time. Based on the track record
>>> here for some of these patchsets, hopefully it doesn't become a
>>> requirement for this company as well :)
>>>
>>
>> Interesting, I was under the impression that we (maintainers) didn't
>> want such internally originating tags.
>
> But why? It just means that the patch has been reviewed. In some rare
> cases we explicitly ask a developer to have all the patches reviewed
> before sending them upstream. In such a case having an R-B tag
> fulfills the expectation of the maintainer: it shows that another
> engineer has reviewed the patch.
Wait, there are two types of internal reviews.
Automatic, +1 from Gerrit or from whatever internal processes require,
which are not useful because these internal reviewers do not actually
review. I have seen a lot of such and I complain. It's easy to spot them
- a patchset consisting of few patches, including trivial ones, all of
them carrying one more more review tags. Even fixing a typo: reviewed
tag. Plus then you see that quality of the patchset is actually poor.
Another are real reviews done internally. If they are real, I find them
useful.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH v2 2/3] arm64: dts: qcom: msm8916-samsung-fortuna: Add LSM303C accelerometer/magnetometer
From: Raymond Hackley @ 2024-04-06 11:14 UTC (permalink / raw)
To: linux-kernel
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stephan Gerhold, Nikita Travkin, linux-arm-msm,
devicetree, ~postmarketos/upstreaming, Siddharth Manthan
In-Reply-To: <20240406111348.14358-1-raymondhackley@protonmail.com>
From: Siddharth Manthan <siddharth.manthan@gmail.com>
Some Grand Prime use a ST LSM303C accelerometer/magnetometer combo.
Add support for it.
Signed-off-by: Siddharth Manthan <siddharth.manthan@gmail.com>
[Stephan: Move sensors to common dtsi (disabled by default)]
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[Raymond: Use interrupts-extended]
Signed-off-by: Raymond Hackley <raymondhackley@protonmail.com>
---
.../dts/qcom/msm8216-samsung-fortuna3g.dts | 8 +++++
.../qcom/msm8916-samsung-fortuna-common.dtsi | 33 +++++++++++++++++++
2 files changed, 41 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts b/arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts
index 366914be7d53..e7f6df229f9a 100644
--- a/arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts
+++ b/arch/arm64/boot/dts/qcom/msm8216-samsung-fortuna3g.dts
@@ -9,3 +9,11 @@ / {
compatible = "samsung,fortuna3g", "qcom,msm8916";
chassis-type = "handset";
};
+
+&st_accel {
+ status = "okay";
+};
+
+&st_magn {
+ status = "okay";
+};
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi
index b6e1fe8b0056..17d21949dfec 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi
@@ -152,6 +152,39 @@ bosch_magn: magnetometer@12 {
status = "disabled";
};
+
+ st_accel: accelerometer@1d {
+ compatible = "st,lsm303c-accel";
+ reg = <0x1d>;
+ interrupts-extended = <&tlmm 115 IRQ_TYPE_LEVEL_HIGH>;
+
+ vdd-supply = <&pm8916_l17>;
+ vddio-supply = <&pm8916_l5>;
+
+ pinctrl-0 = <&accel_int_default>;
+ pinctrl-names = "default";
+
+ st,drdy-int-pin = <1>;
+ mount-matrix = "0", "-1", "0",
+ "1", "0", "0",
+ "0", "0", "-1";
+
+ status = "disabled";
+ };
+
+ st_magn: magnetometer@1e {
+ compatible = "st,lsm303c-magn";
+ reg = <0x1e>;
+
+ vdd-supply = <&pm8916_l17>;
+ vddio-supply = <&pm8916_l5>;
+
+ mount-matrix = "0", "-1", "0",
+ "1", "0", "0",
+ "0", "0", "-1";
+
+ status = "disabled";
+ };
};
&blsp_i2c4 {
--
2.39.2
^ permalink raw reply related
* [PATCH v2 1/3] arm64: dts: qcom: msm8916-samsung-fortuna: Add BMC150 accelerometer/magnetometer
From: Raymond Hackley @ 2024-04-06 11:14 UTC (permalink / raw)
To: linux-kernel
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stephan Gerhold, Nikita Travkin, linux-arm-msm,
devicetree, ~postmarketos/upstreaming, Joe Mason
In-Reply-To: <20240406111348.14358-1-raymondhackley@protonmail.com>
From: Joe Mason <buddyjojo06@outlook.com>
Some Grand Prime use a Bosch BMC150 accelerometer/magnetometer combo.
The chip provides two separate I2C devices for the accelerometer
and magnetometer that are already supported by the bmc150-accel
and bmc150-magn driver.
Signed-off-by: Joe Mason <buddyjojo06@outlook.com>
[Stephan: Move sensors to common dtsi, disabled by default]
Signed-off-by: Stephan Gerhold <stephan@gerhold.net>
[Raymond: Add it to grandprimelte. Use interrupts-extended]
Signed-off-by: Raymond Hackley <raymondhackley@protonmail.com>
---
.../qcom/msm8916-samsung-fortuna-common.dtsi | 44 +++++++++++++++++++
.../dts/qcom/msm8916-samsung-gprimeltecan.dts | 8 ++++
.../qcom/msm8916-samsung-grandprimelte.dts | 8 ++++
3 files changed, 60 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi b/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi
index 5e933fb8b363..b6e1fe8b0056 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-fortuna-common.dtsi
@@ -117,6 +117,43 @@ muic: extcon@25 {
};
};
+&blsp_i2c2 {
+ /* Available sensors vary depending on model variant */
+ status = "okay";
+
+ bosch_accel: accelerometer@10 {
+ compatible = "bosch,bmc150_accel";
+ reg = <0x10>;
+ interrupts-extended = <&tlmm 115 IRQ_TYPE_EDGE_RISING>;
+
+ vdd-supply = <&pm8916_l5>;
+ vddio-supply = <&pm8916_l5>;
+
+ pinctrl-0 = <&accel_int_default>;
+ pinctrl-names = "default";
+
+ mount-matrix = "0", "-1", "0",
+ "-1", "0", "0",
+ "0", "0", "1";
+
+ status = "disabled";
+ };
+
+ bosch_magn: magnetometer@12 {
+ compatible = "bosch,bmc150_magn";
+ reg = <0x12>;
+
+ vdd-supply = <&pm8916_l5>;
+ vddio-supply = <&pm8916_l5>;
+
+ mount-matrix = "0", "-1", "0",
+ "-1", "0", "0",
+ "0", "0", "1";
+
+ status = "disabled";
+ };
+};
+
&blsp_i2c4 {
status = "okay";
@@ -223,6 +260,13 @@ &wcnss_mem {
};
&tlmm {
+ accel_int_default: accel-int-default-state {
+ pins = "gpio115";
+ function = "gpio";
+ drive-strength = <2>;
+ bias-disable;
+ };
+
backlight_en_default: backlight-en-default-state {
pins = "gpio98";
function = "gpio";
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-gprimeltecan.dts b/arch/arm64/boot/dts/qcom/msm8916-samsung-gprimeltecan.dts
index 9d65fa58ba92..4dc74e8bf1d8 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-gprimeltecan.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-gprimeltecan.dts
@@ -21,6 +21,14 @@ tz-apps@85500000 {
};
};
+&bosch_accel {
+ status = "okay";
+};
+
+&bosch_magn {
+ status = "okay";
+};
+
&mpss_mem {
/* Firmware for gprimeltecan needs more space */
reg = <0x0 0x86800000 0x0 0x5400000>;
diff --git a/arch/arm64/boot/dts/qcom/msm8916-samsung-grandprimelte.dts b/arch/arm64/boot/dts/qcom/msm8916-samsung-grandprimelte.dts
index a66ce4b13547..cffad734c4df 100644
--- a/arch/arm64/boot/dts/qcom/msm8916-samsung-grandprimelte.dts
+++ b/arch/arm64/boot/dts/qcom/msm8916-samsung-grandprimelte.dts
@@ -10,6 +10,14 @@ / {
chassis-type = "handset";
};
+&bosch_accel {
+ status = "okay";
+};
+
+&bosch_magn {
+ status = "okay";
+};
+
&mpss_mem {
/* Firmware for grandprimelte needs more space */
reg = <0x0 0x86800000 0x0 0x5400000>;
--
2.39.2
^ permalink raw reply related
* [PATCH v2 0/3] arm64: dts: qcom: msm8916-samsung-fortuna: Add accelerometer/magnetometer
From: Raymond Hackley @ 2024-04-06 11:14 UTC (permalink / raw)
To: linux-kernel
Cc: Bjorn Andersson, Konrad Dybcio, Rob Herring, Krzysztof Kozlowski,
Conor Dooley, Stephan Gerhold, Nikita Travkin, linux-arm-msm,
devicetree, ~postmarketos/upstreaming
Some Grand Prime use a Bosch BMC150 accelerometer/magnetometer combo.
The chip provides two separate I2C devices for the accelerometer
and magnetometer that are already supported by the bmc150-accel
and bmc150-magn driver.
Some Grand Prime use a ST LSM303C accelerometer/magnetometer combo.
Core Prime LTE uses ST LIS2HH12 accelerometer.
Add support for them.
---
v2: fix accelerometer@1d: 'interrupt-names' does not match any of the regexes:
'pinctrl-[0-9]+'
^ 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