* Re: [PATCH v2 1/3] ARM: dts: sunxi: remove duplicated entries in makefile
From: Jernej Škrabec @ 2024-03-26 22:23 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Samuel Holland, Pavel Löbl
Cc: Pavel Löbl, devicetree, linux-sunxi
In-Reply-To: <20240320061027.4078852-1-pavel@loebl.cz>
Dne sreda, 20. marec 2024 ob 07:10:19 CET je Pavel Löbl napisal(a):
> During introduction of DTS vendor subdirectories in 724ba6751532, sun8i
> section of the makefile got duplicated. Clean that up.
>
> Fixes: 724ba6751532 ("ARM: dts: Move .dts files to vendor sub-directories")
>
If new revision is needed, remove this empty line, otherwise I can when
applying.
> Signed-off-by: Pavel Löbl <pavel@loebl.cz>
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Best regards,
Jernej
^ permalink raw reply
* Re: [PATCH v6 09/15] media: bcm2835-unicam: Add support for CCP2/CSI2 camera interface
From: Sakari Ailus @ 2024-03-26 22:23 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Dave Stevenson, David Plowman, Jean-Michel Hautbois,
Hans Verkuil, Naushir Patuck, kernel-list, linux-rpi-kernel,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Conor Dooley, Krzysztof Kozlowski,
Rob Herring, devicetree
In-Reply-To: <20240326015028.GB31396@pendragon.ideasonboard.com>
Hi Laurent,
On Tue, Mar 26, 2024 at 03:50:28AM +0200, Laurent Pinchart wrote:
> On Tue, Mar 26, 2024 at 03:37:09AM +0200, Laurent Pinchart wrote:
> > Hi Sakari,
> >
> > On Mon, Mar 25, 2024 at 06:36:49PM +0000, Sakari Ailus wrote:
> > > On Fri, Mar 01, 2024 at 11:32:24PM +0200, Laurent Pinchart wrote:
> > > > From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > >
> > > > Add a driver for the Unicam camera receiver block on BCM283x processors.
> > > > It is represented as two video device nodes: unicam-image and
> > > > unicam-embedded which are connected to an internal subdev (named
> > > > unicam-subdev) in order to manage streams routing.
> > > >
> > > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > > Co-developed-by: Naushir Patuck <naush@raspberrypi.com>
> > > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > > > Co-developed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> > > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> > > > Co-developed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > >
> > > Thanks for submitting this, it's the cleanest and neatest Unicom driver
> >
> > Unicam, or if you insist Unicorn, but not Unicom :-)
A Unicorn driver? Hmm. Maybe Unicam is indeed the best choice here?
> >
> > > I've ever seen!
> > >
> > > Some mostly unimportant comments below, however the bus-type issue needs to
> > > be addressed.
> > >
> > > > ---
> > > > Changes since v5:
> > > >
> > > > - Move to drivers/media/platform/broadcom/
> > > > - Port to the upstream V4L2 streams API
> > > > - Rebase on latest metadata API proposal
> > > > - Add missing error message
> > > > - Drop unneeded documentation block for unicam_isr()
> > > > - Drop unneeded dev_dbg() and dev_err() messages
> > > > - Drop unneeded streams_mask and fmt checks
> > > > - Drop unused unicam_sd_pad_is_sink()
> > > > - Drop unneeded includes
> > > > - Drop v4l2_ctrl_subscribe_event() call
> > > > - Use pm_runtime_resume_and_get()
> > > > - Indentation and line wrap fixes
> > > > - Let the framework set bus_info
> > > > - Use v4l2_fwnode_endpoint_parse()
> > > > - Fix media device cleanup
> > > > - Drop lane reordering checks
> > > > - Fix subdev state locking
> > > > - Drop extra debug messages
> > > > - Move clock handling to runtime PM handlers
> > > > - Reorder functions
> > > > - Rename init functions for more clarity
> > > > - Initialize runtime PM earlier
> > > > - Clarify error messages
> > > > - Simplify subdev init with local variable
> > > > - Fix subdev cleanup
> > > > - Fix typos and indentation
> > > > - Don't initialize local variables needlessly
> > > > - Simplify num lanes check
> > > > - Fix metadata handling in subdev set_fmt
> > > > - Drop manual fallback to .s_stream()
> > > > - Pass v4l2_pix_format to unicam_calc_format_size_bpl()
> > > > - Simplify unicam_set_default_format()
> > > > - Fix default format settings
> > > > - Add busy check in unicam_s_fmt_meta()
> > > > - Add missing \n at end of format strings
> > > > - Fix metadata handling in subdev set_fmt
> > > > - Fix locking when starting streaming
> > > > - Return buffers from start streaming fails
> > > > - Fix format validation for metadata node
> > > > - Use video_device_pipeline_{start,stop}() helpers
> > > > - Simplify format enumeration
> > > > - Drop unset variable
> > > > - Update MAINTAINERS entry
> > > > - Update to the upstream v4l2_async_nf API
> > > > - Update to the latest subdev routing API
> > > > - Update to the latest subdev state API
> > > > - Move from subdev .init_cfg() to .init_state()
> > > > - Update to the latest videobuf2 API
> > > > - Fix v4l2_subdev_enable_streams() error check
> > > > - Use correct pad for the connected subdev
> > > > - Return buffers to vb2 when start streaming fails
> > > > - Improve debugging in start streaming handler
> > > > - Simplify DMA address management
> > > > - Drop comment about bcm2835-camera driver
> > > > - Clarify comments that explain min/max sizes
> > > > - Pass v4l2_pix_format to unicam_try_fmt()
> > > > - Drop unneeded local variables
> > > > - Rename image-related constants and functions
> > > > - Turn unicam_fmt.metadata_fmt into bool
> > > > - Rename unicam_fmt to unicam_format_info
> > > > - Rename unicam_format_info variables to fmtinfo
> > > > - Rename unicam_node.v_fmt to fmt
> > > > - Add metadata formats for RAW10, RAW12 and RAW14
> > > > - Make metadata formats line-based
> > > > - Validate format on metadata video device
> > > > - Add Co-devlopped-by tags
> > > >
> > > > Changes since v3:
> > > >
> > > > - Add the vendor prefix for DT name
> > > > - Use the reg-names in DT parsing
> > > > - Remove MAINTAINERS entry
> > > >
> > > > Changes since v2:
> > > >
> > > > - Change code organization
> > > > - Remove unused variables
> > > > - Correct the fmt_meta functions
> > > > - Rewrite the start/stop streaming
> > > > - You can now start the image node alone, but not the metadata one
> > > > - The buffers are allocated per-node
> > > > - only the required stream is started, if the route exists and is
> > > > enabled
> > > > - Prefix the macros with UNICAM_ to not have too generic names
> > > > - Drop colorspace support
> > > >
> > > > Changes since v1:
> > > >
> > > > - Replace the unicam_{info,debug,error} macros with dev_*()
> > > > ---
> > > > MAINTAINERS | 1 +
> > > > drivers/media/platform/Kconfig | 1 +
> > > > drivers/media/platform/Makefile | 1 +
> > > > drivers/media/platform/broadcom/Kconfig | 23 +
> > > > drivers/media/platform/broadcom/Makefile | 3 +
> > > > .../platform/broadcom/bcm2835-unicam-regs.h | 255 ++
> > > > .../media/platform/broadcom/bcm2835-unicam.c | 2607 +++++++++++++++++
> > > > 7 files changed, 2891 insertions(+)
> > > > create mode 100644 drivers/media/platform/broadcom/Kconfig
> > > > create mode 100644 drivers/media/platform/broadcom/Makefile
> > > > create mode 100644 drivers/media/platform/broadcom/bcm2835-unicam-regs.h
> > > > create mode 100644 drivers/media/platform/broadcom/bcm2835-unicam.c
> > > >
> > > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > > index e50a59654e6e..cc350729f467 100644
> > > > --- a/MAINTAINERS
> > > > +++ b/MAINTAINERS
> > > > @@ -4002,6 +4002,7 @@ M: Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
> > > > L: linux-media@vger.kernel.org
> > > > S: Maintained
> > > > F: Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
> > > > +F: drivers/media/platform/bcm2835/
> > > >
> > > > BROADCOM BCM47XX MIPS ARCHITECTURE
> > > > M: Hauke Mehrtens <hauke@hauke-m.de>
> > > > diff --git a/drivers/media/platform/Kconfig b/drivers/media/platform/Kconfig
> > > > index 91e54215de3a..2d79bfc68c15 100644
> > > > --- a/drivers/media/platform/Kconfig
> > > > +++ b/drivers/media/platform/Kconfig
> > > > @@ -67,6 +67,7 @@ source "drivers/media/platform/amlogic/Kconfig"
> > > > source "drivers/media/platform/amphion/Kconfig"
> > > > source "drivers/media/platform/aspeed/Kconfig"
> > > > source "drivers/media/platform/atmel/Kconfig"
> > > > +source "drivers/media/platform/broadcom/Kconfig"
> > > > source "drivers/media/platform/cadence/Kconfig"
> > > > source "drivers/media/platform/chips-media/Kconfig"
> > > > source "drivers/media/platform/intel/Kconfig"
> > > > diff --git a/drivers/media/platform/Makefile b/drivers/media/platform/Makefile
> > > > index 3296ec1ebe16..da17301f7439 100644
> > > > --- a/drivers/media/platform/Makefile
> > > > +++ b/drivers/media/platform/Makefile
> > > > @@ -10,6 +10,7 @@ obj-y += amlogic/
> > > > obj-y += amphion/
> > > > obj-y += aspeed/
> > > > obj-y += atmel/
> > > > +obj-y += broadcom/
> > > > obj-y += cadence/
> > > > obj-y += chips-media/
> > > > obj-y += intel/
> > > > diff --git a/drivers/media/platform/broadcom/Kconfig b/drivers/media/platform/broadcom/Kconfig
> > > > new file mode 100644
> > > > index 000000000000..cc2c9afcc948
> > > > --- /dev/null
> > > > +++ b/drivers/media/platform/broadcom/Kconfig
> > > > @@ -0,0 +1,23 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +config VIDEO_BCM2835_UNICAM
> > > > + tristate "Broadcom BCM283x/BCM271x Unicam video capture driver"
> > > > + depends on ARCH_BCM2835 || COMPILE_TEST
> > > > + depends on PM
> > > > + depends on VIDEO_DEV
> > > > + select MEDIA_CONTROLLER
> > > > + select V4L2_FWNODE
> > > > + select VIDEO_V4L2_SUBDEV_API
> > > > + select VIDEOBUF2_DMA_CONTIG
> > > > + help
> > > > + Say Y here to enable support for the BCM283x/BCM271x CSI-2 receiver.
> > > > + This is a V4L2 driver that controls the CSI-2 receiver directly,
> > > > + independently from the VC4 firmware.
> > > > +
> > > > + This driver is mutually exclusive with the use of bcm2835-camera. The
> > > > + firmware will disable all access to the peripheral from within the
> > > > + firmware if it finds a DT node using it, and bcm2835-camera will
> > > > + therefore fail to probe.
> > > > +
> > > > + To compile this driver as a module, choose M here. The module will be
> > > > + called bcm2835-unicam.
> > > > diff --git a/drivers/media/platform/broadcom/Makefile b/drivers/media/platform/broadcom/Makefile
> > > > new file mode 100644
> > > > index 000000000000..03d2045aba2e
> > > > --- /dev/null
> > > > +++ b/drivers/media/platform/broadcom/Makefile
> > > > @@ -0,0 +1,3 @@
> > > > +# SPDX-License-Identifier: GPL-2.0
> > > > +
> > > > +obj-$(CONFIG_VIDEO_BCM2835_UNICAM) += bcm2835-unicam.o
> > > > diff --git a/drivers/media/platform/broadcom/bcm2835-unicam-regs.h b/drivers/media/platform/broadcom/bcm2835-unicam-regs.h
> > > > new file mode 100644
> > > > index 000000000000..84775fd2fac5
> > > > --- /dev/null
> > > > +++ b/drivers/media/platform/broadcom/bcm2835-unicam-regs.h
> > > > @@ -0,0 +1,255 @@
> > > > +/* SPDX-License-Identifier: GPL-2.0-only */
> > > > +
> > > > +/*
> > > > + * Copyright (C) 2017-2020 Raspberry Pi Trading.
> > >
> > > Anything up to 2024?
> >
> > Not really. The registers haven't really changed :-) I'll update the
> > copyright in the .c file though.
> >
> > > > + * Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > > + */
> >
> > [snip]
> >
> > > > diff --git a/drivers/media/platform/broadcom/bcm2835-unicam.c b/drivers/media/platform/broadcom/bcm2835-unicam.c
> > > > new file mode 100644
> > > > index 000000000000..716c89b8a217
> > > > --- /dev/null
> > > > +++ b/drivers/media/platform/broadcom/bcm2835-unicam.c
> > > > @@ -0,0 +1,2607 @@
> >
> > [snip]
> >
> > > > +static irqreturn_t unicam_isr(int irq, void *dev)
> > > > +{
> > > > + struct unicam_device *unicam = dev;
> > > > + unsigned int lines_done = unicam_get_lines_done(dev);
> > > > + unsigned int sequence = unicam->sequence;
> > > > + unsigned int i;
> > > > + u32 ista, sta;
> > > > + bool fe;
> > > > + u64 ts;
> > > > +
> > > > + sta = unicam_reg_read(unicam, UNICAM_STA);
> > > > + /* Write value back to clear the interrupts */
> > > > + unicam_reg_write(unicam, UNICAM_STA, sta);
> > > > +
> > > > + ista = unicam_reg_read(unicam, UNICAM_ISTA);
> > > > + /* Write value back to clear the interrupts */
> > > > + unicam_reg_write(unicam, UNICAM_ISTA, ista);
> > > > +
> > > > + dev_dbg(unicam->dev, "ISR: ISTA: 0x%X, STA: 0x%X, sequence %d, lines done %d\n",
> > > > + ista, sta, sequence, lines_done);
> > > > +
> > > > + if (!(sta & (UNICAM_IS | UNICAM_PI0)))
> > > > + return IRQ_HANDLED;
> > > > +
> > > > + /*
> > > > + * Look for either the Frame End interrupt or the Packet Capture status
> > > > + * to signal a frame end.
> > > > + */
> > > > + fe = ista & UNICAM_FEI || sta & UNICAM_PI0;
> > > > +
> > > > + /*
> > > > + * We must run the frame end handler first. If we have a valid next_frm
> > > > + * and we get a simultaneout FE + FS interrupt, running the FS handler
> > > > + * first would null out the next_frm ptr and we would have lost the
> > > > + * buffer forever.
> > > > + */
> > > > + if (fe) {
> > > > + /*
> > > > + * Ensure we have swapped buffers already as we can't
> > > > + * stop the peripheral. If no buffer is available, use a
> > > > + * dummy buffer to dump out frames until we get a new buffer
> > > > + * to use.
> > > > + */
> > > > + for (i = 0; i < ARRAY_SIZE(unicam->node); i++) {
> > > > + if (!unicam->node[i].streaming)
> > > > + continue;
> > > > +
> > > > + /*
> > > > + * If cur_frm == next_frm, it means we have not had
> > > > + * a chance to swap buffers, likely due to having
> > > > + * multiple interrupts occurring simultaneously (like FE
> > > > + * + FS + LS). In this case, we cannot signal the buffer
> > > > + * as complete, as the HW will reuse that buffer.
> > > > + */
> > > > + if (unicam->node[i].cur_frm &&
> > > > + unicam->node[i].cur_frm != unicam->node[i].next_frm)
> > > > + unicam_process_buffer_complete(&unicam->node[i],
> > > > + sequence);
> > > > + unicam->node[i].cur_frm = unicam->node[i].next_frm;
> > > > + }
> > > > + unicam->sequence++;
> > >
> > > Does access to this data need to be serialised somehow.
> >
> > Given that it's only accessed from the interrupt handler (beside
> > start_streaming time, before starting the hardware), I don't think so.
Ack. I guess a memory barrier would be theoretically needed although in
practice other locks will be taken so you might not have issues.
> >
> > > > + }
> > > > +
> > > > + if (ista & UNICAM_FSI) {
> > > > + /*
> > > > + * Timestamp is to be when the first data byte was captured,
> > > > + * aka frame start.
> > > > + */
> > > > + ts = ktime_get_ns();
> > > > + for (i = 0; i < ARRAY_SIZE(unicam->node); i++) {
> > > > + if (!unicam->node[i].streaming)
> > > > + continue;
> > > > +
> > > > + if (unicam->node[i].cur_frm)
> > > > + unicam->node[i].cur_frm->vb.vb2_buf.timestamp =
> > > > + ts;
> > > > + else
> > > > + dev_dbg(unicam->v4l2_dev.dev,
> > > > + "ISR: [%d] Dropping frame, buffer not available at FS\n",
> > > > + i);
> > > > + /*
> > > > + * Set the next frame output to go to a dummy frame
> > > > + * if we have not managed to obtain another frame
> > > > + * from the queue.
> > > > + */
> > > > + unicam_schedule_dummy_buffer(&unicam->node[i]);
> > > > + }
> > > > +
> > > > + unicam_queue_event_sof(unicam);
> > > > + }
> > > > +
> > > > + /*
> > > > + * Cannot swap buffer at frame end, there may be a race condition
> > > > + * where the HW does not actually swap it if the new frame has
> > > > + * already started.
> > > > + */
> > > > + if (ista & (UNICAM_FSI | UNICAM_LCI) && !fe) {
> > > > + for (i = 0; i < ARRAY_SIZE(unicam->node); i++) {
> > > > + if (!unicam->node[i].streaming)
> > > > + continue;
> > > > +
> > > > + spin_lock(&unicam->node[i].dma_queue_lock);
> > > > + if (!list_empty(&unicam->node[i].dma_queue) &&
> > > > + !unicam->node[i].next_frm)
> > > > + unicam_schedule_next_buffer(&unicam->node[i]);
> > > > + spin_unlock(&unicam->node[i].dma_queue_lock);
> > > > + }
> > > > + }
> > > > +
> > > > + if (unicam_reg_read(unicam, UNICAM_ICTL) & UNICAM_FCM) {
> > > > + /* Switch out of trigger mode if selected */
> > > > + unicam_reg_write_field(unicam, UNICAM_ICTL, 1, UNICAM_TFC);
> > > > + unicam_reg_write_field(unicam, UNICAM_ICTL, 0, UNICAM_FCM);
> > > > + }
> > > > + return IRQ_HANDLED;
> > > > +}
> > > > +
> > > > +static void unicam_set_packing_config(struct unicam_device *unicam)
> > > > +{
> > > > + struct unicam_node *node = &unicam->node[UNICAM_IMAGE_NODE];
> > > > + u32 pack, unpack;
> > > > + u32 val;
> > > > +
> > > > + if (node->fmt.fmt.pix.pixelformat == node->fmtinfo->fourcc) {
> > > > + unpack = UNICAM_PUM_NONE;
> > > > + pack = UNICAM_PPM_NONE;
> > > > + } else {
> > > > + switch (node->fmtinfo->depth) {
> > > > + case 8:
> > > > + unpack = UNICAM_PUM_UNPACK8;
> > > > + break;
> > > > + case 10:
> > > > + unpack = UNICAM_PUM_UNPACK10;
> > > > + break;
> > > > + case 12:
> > > > + unpack = UNICAM_PUM_UNPACK12;
> > > > + break;
> > > > + case 14:
> > > > + unpack = UNICAM_PUM_UNPACK14;
> > > > + break;
> > > > + case 16:
> > > > + unpack = UNICAM_PUM_UNPACK16;
> > > > + break;
> > > > + default:
> > > > + unpack = UNICAM_PUM_NONE;
> > > > + break;
> > > > + }
> > > > +
> > > > + /* Repacking is always to 16bpp */
> > > > + pack = UNICAM_PPM_PACK16;
> > >
> > > Also 8-bit data?
> >
> > Not that I know of. The 8-bit entries in unicam_image_formats have no
> > .unpacked_fourcc field, so the condition in the if above will always be
> > true for those as they can only be selected by setting the pixel format
> > to fmtinfo->fourcc.
Ok.
> >
> > > > + }
> > > > +
> > > > + val = 0;
> > >
> > > You could do initialisation in declaration.
> >
> > Yes, but I think it's more readable to keep all the code that affects
> > the 'val' variable together.
If wal was a bit more descriptive name this would be a non-issue. Up to
you.
> >
> > > > + unicam_set_field(&val, unpack, UNICAM_PUM_MASK);
> > > > + unicam_set_field(&val, pack, UNICAM_PPM_MASK);
> > > > + unicam_reg_write(unicam, UNICAM_IPIPE, val);
> > > > +}
> > > > +
> > > > +static void unicam_cfg_image_id(struct unicam_device *unicam)
> > > > +{
> > > > + struct unicam_node *node = &unicam->node[UNICAM_IMAGE_NODE];
> > > > +
> > > > + if (unicam->bus_type == V4L2_MBUS_CSI2_DPHY) {
> > > > + /* CSI2 mode, hardcode VC 0 for now. */
> > > > + unicam_reg_write(unicam, UNICAM_IDI0,
> > > > + (0 << 6) | node->fmtinfo->csi_dt);
> > > > + } else {
> > > > + /* CCP2 mode */
> > > > + unicam_reg_write(unicam, UNICAM_IDI0,
> > > > + 0x80 | node->fmtinfo->csi_dt);
> > > > + }
> > > > +}
> > > > +
> > > > +static void unicam_enable_ed(struct unicam_device *unicam)
> > > > +{
> > > > + u32 val = unicam_reg_read(unicam, UNICAM_DCS);
> > > > +
> > > > + unicam_set_field(&val, 2, UNICAM_EDL_MASK);
> > > > + /* Do not wrap at the end of the embedded data buffer */
> > > > + unicam_set_field(&val, 0, UNICAM_DBOB);
> > > > +
> > > > + unicam_reg_write(unicam, UNICAM_DCS, val);
> > > > +}
> > > > +
> > > > +static void unicam_start_rx(struct unicam_device *unicam,
> > > > + struct unicam_buffer *buf)
> > > > +{
> > > > + struct unicam_node *node = &unicam->node[UNICAM_IMAGE_NODE];
> > > > + int line_int_freq = node->fmt.fmt.pix.height >> 2;
> > > > + unsigned int i;
> > > > + u32 val;
> > > > +
> > > > + if (line_int_freq < 128)
> > > > + line_int_freq = 128;
> > >
> > > line_int_freq = max(line_int_freq, 128);
> > Ack.
> >
> > > > +
> > > > + /* Enable lane clocks */
> > > > + val = 1;
> > >
> > > Initialise in the loop initialisation below, I'd say.
> >
> > How about
> >
> > val = 0x55 & GENMASK(unicam->pipe.num_data_lanes * 2 - 1, 0);
>
> I meant
>
> val = 0x155 & GENMASK(unicam->pipe.num_data_lanes * 2 + 1, 0);
>
> Maybe a comment would be useful ?
>
> /*
> * Enable lane clocks. The register is structured as follows:
> *
> * [9:8] - DAT3
> * [7:6] - DAT2
> * [5:4] - DAT1
> * [3:2] - DAT0
> * [1:0] - CLK
> *
> * Enabled lane must be set to b01, and disabled lanes to b00. The clock
> * lane is always enabled.
> */
> val = 0x155 & GENMASK(unicam->pipe.num_data_lanes * 2 + 1, 0);
Seems good to me.
>
> > > > + for (i = 0; i < unicam->active_data_lanes; i++)
> > > > + val = val << 2 | 1;
> > > > + unicam_clk_write(unicam, val);
> > > > +
> > > > + /* Basic init */
> > > > + unicam_reg_write(unicam, UNICAM_CTRL, UNICAM_MEM);
> > > > +
> > > > + /* Enable analogue control, and leave in reset. */
> > > > + val = UNICAM_AR;
> > > > + unicam_set_field(&val, 7, UNICAM_CTATADJ_MASK);
> > > > + unicam_set_field(&val, 7, UNICAM_PTATADJ_MASK);
> > > > + unicam_reg_write(unicam, UNICAM_ANA, val);
> > > > + usleep_range(1000, 2000);
> > > > +
> > > > + /* Come out of reset */
> > > > + unicam_reg_write_field(unicam, UNICAM_ANA, 0, UNICAM_AR);
> > > > +
> > > > + /* Peripheral reset */
> > > > + unicam_reg_write_field(unicam, UNICAM_CTRL, 1, UNICAM_CPR);
> > > > + unicam_reg_write_field(unicam, UNICAM_CTRL, 0, UNICAM_CPR);
> > > > +
> > > > + unicam_reg_write_field(unicam, UNICAM_CTRL, 0, UNICAM_CPE);
> > > > +
> > > > + /* Enable Rx control. */
> > > > + val = unicam_reg_read(unicam, UNICAM_CTRL);
> > > > + if (unicam->bus_type == V4L2_MBUS_CSI2_DPHY) {
> > > > + unicam_set_field(&val, UNICAM_CPM_CSI2, UNICAM_CPM_MASK);
> > > > + unicam_set_field(&val, UNICAM_DCM_STROBE, UNICAM_DCM_MASK);
> > > > + } else {
> > > > + unicam_set_field(&val, UNICAM_CPM_CCP2, UNICAM_CPM_MASK);
> > > > + unicam_set_field(&val, unicam->bus_flags, UNICAM_DCM_MASK);
> > > > + }
> > > > + /* Packet framer timeout */
> > > > + unicam_set_field(&val, 0xf, UNICAM_PFT_MASK);
> > > > + unicam_set_field(&val, 128, UNICAM_OET_MASK);
> > > > + unicam_reg_write(unicam, UNICAM_CTRL, val);
> > > > +
> > > > + unicam_reg_write(unicam, UNICAM_IHWIN, 0);
> > > > + unicam_reg_write(unicam, UNICAM_IVWIN, 0);
> > > > +
> > > > + /* AXI bus access QoS setup */
> > > > + val = unicam_reg_read(unicam, UNICAM_PRI);
> > > > + unicam_set_field(&val, 0, UNICAM_BL_MASK);
> > > > + unicam_set_field(&val, 0, UNICAM_BS_MASK);
> > > > + unicam_set_field(&val, 0xe, UNICAM_PP_MASK);
> > > > + unicam_set_field(&val, 8, UNICAM_NP_MASK);
> > > > + unicam_set_field(&val, 2, UNICAM_PT_MASK);
> > > > + unicam_set_field(&val, 1, UNICAM_PE);
> > > > + unicam_reg_write(unicam, UNICAM_PRI, val);
> > > > +
> > > > + unicam_reg_write_field(unicam, UNICAM_ANA, 0, UNICAM_DDL);
> > > > +
> > > > + /* Always start in trigger frame capture mode (UNICAM_FCM set) */
> > > > + val = UNICAM_FSIE | UNICAM_FEIE | UNICAM_FCM | UNICAM_IBOB;
> > > > + unicam_set_field(&val, line_int_freq, UNICAM_LCIE_MASK);
> > > > + unicam_reg_write(unicam, UNICAM_ICTL, val);
> > > > + unicam_reg_write(unicam, UNICAM_STA, UNICAM_STA_MASK_ALL);
> > > > + unicam_reg_write(unicam, UNICAM_ISTA, UNICAM_ISTA_MASK_ALL);
> > > > +
> > > > + /* tclk_term_en */
> > > > + unicam_reg_write_field(unicam, UNICAM_CLT, 2, UNICAM_CLT1_MASK);
> > > > + /* tclk_settle */
> > > > + unicam_reg_write_field(unicam, UNICAM_CLT, 6, UNICAM_CLT2_MASK);
> > > > + /* td_term_en */
> > > > + unicam_reg_write_field(unicam, UNICAM_DLT, 2, UNICAM_DLT1_MASK);
> > > > + /* ths_settle */
> > > > + unicam_reg_write_field(unicam, UNICAM_DLT, 6, UNICAM_DLT2_MASK);
> > > > + /* trx_enable */
> > > > + unicam_reg_write_field(unicam, UNICAM_DLT, 0, UNICAM_DLT3_MASK);
> > > > +
> > > > + unicam_reg_write_field(unicam, UNICAM_CTRL, 0, UNICAM_SOE);
> > > > +
> > > > + /* Packet compare setup - required to avoid missing frame ends */
> > > > + val = 0;
> > > > + unicam_set_field(&val, 1, UNICAM_PCE);
> > > > + unicam_set_field(&val, 1, UNICAM_GI);
> > > > + unicam_set_field(&val, 1, UNICAM_CPH);
> > > > + unicam_set_field(&val, 0, UNICAM_PCVC_MASK);
> > > > + unicam_set_field(&val, 1, UNICAM_PCDT_MASK);
> > > > + unicam_reg_write(unicam, UNICAM_CMP0, val);
> > > > +
> > > > + /* Enable clock lane and set up terminations */
> > > > + val = 0;
> > > > + if (unicam->bus_type == V4L2_MBUS_CSI2_DPHY) {
> > > > + /* CSI2 */
> > > > + unicam_set_field(&val, 1, UNICAM_CLE);
> > > > + unicam_set_field(&val, 1, UNICAM_CLLPE);
> > > > + if (!(unicam->bus_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK)) {
> > > > + unicam_set_field(&val, 1, UNICAM_CLTRE);
> > > > + unicam_set_field(&val, 1, UNICAM_CLHSE);
> > > > + }
> > > > + } else {
> > > > + /* CCP2 */
> > > > + unicam_set_field(&val, 1, UNICAM_CLE);
> > > > + unicam_set_field(&val, 1, UNICAM_CLHSE);
> > > > + unicam_set_field(&val, 1, UNICAM_CLTRE);
> > > > + }
> > > > + unicam_reg_write(unicam, UNICAM_CLK, val);
> > > > +
> > > > + /*
> > > > + * Enable required data lanes with appropriate terminations.
> > > > + * The same value needs to be written to UNICAM_DATn registers for
> > > > + * the active lanes, and 0 for inactive ones.
> > > > + */
> > > > + val = 0;
> > > > + if (unicam->bus_type == V4L2_MBUS_CSI2_DPHY) {
> > > > + /* CSI2 */
> > > > + unicam_set_field(&val, 1, UNICAM_DLE);
> > > > + unicam_set_field(&val, 1, UNICAM_DLLPE);
> > > > + if (!(unicam->bus_flags & V4L2_MBUS_CSI2_NONCONTINUOUS_CLOCK)) {
> > > > + unicam_set_field(&val, 1, UNICAM_DLTRE);
> > > > + unicam_set_field(&val, 1, UNICAM_DLHSE);
> > > > + }
> > > > + } else {
> > > > + /* CCP2 */
> > > > + unicam_set_field(&val, 1, UNICAM_DLE);
> > > > + unicam_set_field(&val, 1, UNICAM_DLHSE);
> > > > + unicam_set_field(&val, 1, UNICAM_DLTRE);
> > > > + }
> > > > + unicam_reg_write(unicam, UNICAM_DAT0, val);
> > > > +
> > > > + if (unicam->active_data_lanes == 1)
> > > > + val = 0;
> > > > + unicam_reg_write(unicam, UNICAM_DAT1, val);
> > > > +
> > > > + if (unicam->max_data_lanes > 2) {
> > > > + /*
> > > > + * Registers UNICAM_DAT2 and UNICAM_DAT3 only valid if the
> > > > + * instance supports more than 2 data lanes.
> > > > + */
> > > > + if (unicam->active_data_lanes == 2)
> > > > + val = 0;
> > > > + unicam_reg_write(unicam, UNICAM_DAT2, val);
> > > > +
> > > > + if (unicam->active_data_lanes == 3)
> > > > + val = 0;
> > > > + unicam_reg_write(unicam, UNICAM_DAT3, val);
> > > > + }
> > > > +
> > > > + unicam_reg_write(unicam, UNICAM_IBLS,
> > > > + node->fmt.fmt.pix.bytesperline);
> > > > + unicam_wr_dma_addr(&unicam->node[UNICAM_IMAGE_NODE], buf);
> > > > + unicam_set_packing_config(unicam);
> > > > + unicam_cfg_image_id(unicam);
> > > > +
> > > > + val = unicam_reg_read(unicam, UNICAM_MISC);
> > > > + unicam_set_field(&val, 1, UNICAM_FL0);
> > > > + unicam_set_field(&val, 1, UNICAM_FL1);
> > > > + unicam_reg_write(unicam, UNICAM_MISC, val);
> > > > +
> > > > + /* Enable peripheral */
> > > > + unicam_reg_write_field(unicam, UNICAM_CTRL, 1, UNICAM_CPE);
> > > > +
> > > > + /* Load image pointers */
> > > > + unicam_reg_write_field(unicam, UNICAM_ICTL, 1, UNICAM_LIP_MASK);
> > > > +
> > > > + /*
> > > > + * Enable trigger only for the first frame to
> > > > + * sync correctly to the FS from the source.
> > > > + */
> > > > + unicam_reg_write_field(unicam, UNICAM_ICTL, 1, UNICAM_TFC);
> > > > +}
> >
> > [snip]
> >
> > > > +static int unicam_async_nf_init(struct unicam_device *unicam)
> > > > +{
> > > > + struct v4l2_fwnode_endpoint ep = { };
> > >
> > > If the bus-type property is mandatory and you have no stated defaults
> > > anywhere, this is fine. I.e. all the relevant properties would need to be
Actually this is a non-issue nowadays: bus in struct v4l2_fwnode_endpoint
is no longer a union.
> > > mandatory.
> >
> > They are, as far as I can tell (well, the clock-noncontinuous property
> > is not mandatory, but that's expected as it's a flag).
> >
> > > > + struct fwnode_handle *ep_handle;
> > > > + struct v4l2_async_connection *asc;
> > > > + int ret;
> > > > +
> > > > + ret = of_property_read_u32(unicam->dev->of_node, "brcm,num-data-lanes",
> > > > + &unicam->max_data_lanes);
> > > > + if (ret < 0) {
> > > > + dev_err(unicam->dev, "Missing %s DT property\n",
> > > > + "brcm,num-data-lanes");
> > > > + return -EINVAL;
> > > > + }
> > > > +
> > > > + /* Get and parse the local endpoint. */
> > > > + ep_handle = fwnode_graph_get_endpoint_by_id(dev_fwnode(unicam->dev), 0, 0,
> > > > + FWNODE_GRAPH_ENDPOINT_NEXT);
> > > > + if (!ep_handle) {
> > > > + dev_err(unicam->dev, "No endpoint found\n");
> > > > + return -ENODEV;
> > > > + }
> > > > +
> > > > + ret = v4l2_fwnode_endpoint_parse(ep_handle, &ep);
> > > > + if (ret) {
> > > > + dev_err(unicam->dev, "Failed to parse endpoint: %d\n", ret);
> > > > + goto error;
> > > > + }
> > > > +
> > > > + unicam->bus_type = ep.bus_type;
> > > > +
> > > > + switch (ep.bus_type) {
> > > > + case V4L2_MBUS_CSI2_DPHY: {
> > > > + unsigned int num_data_lanes = ep.bus.mipi_csi2.num_data_lanes;
> > > > +
> > > > + if (num_data_lanes != 1 && num_data_lanes != 2 &&
> > > > + num_data_lanes != 4) {
> > > > + dev_err(unicam->dev, "%u data lanes not supported\n",
> > > > + num_data_lanes);
> > > > + goto error;
> > > > + }
> > > > +
> > > > + if (num_data_lanes > unicam->max_data_lanes) {
> > > > + dev_err(unicam->dev,
> > > > + "Endpoint uses %u data lanes when %u are supported\n",
> > > > + num_data_lanes, unicam->max_data_lanes);
> > > > + goto error;
> > > > + }
> > > > +
> > > > + unicam->active_data_lanes = num_data_lanes;
> > > > + unicam->bus_flags = ep.bus.mipi_csi2.flags;
> > > > + break;
> > > > + }
> > > > +
> > > > + case V4L2_MBUS_CCP2:
> > > > + unicam->max_data_lanes = 1;
> > > > + unicam->active_data_lanes = 1;
> > > > + unicam->bus_flags = ep.bus.mipi_csi1.strobe;
> > > > + break;
> > > > +
> > > > + default:
> > > > + /* Unsupported bus type */
> > > > + dev_err(unicam->dev, "Unsupported bus type %u\n", ep.bus_type);
> > > > + goto error;
> > > > + }
> > > > +
> > > > + /* Initialize and register the async notifier. */
> > > > + v4l2_async_nf_init(&unicam->notifier, &unicam->v4l2_dev);
> > > > +
> > > > + asc = v4l2_async_nf_add_fwnode_remote(&unicam->notifier, ep_handle,
> > > > + struct v4l2_async_connection);
> > > > + fwnode_handle_put(ep_handle);
> > > > + ep_handle = NULL;
> > > > +
> > > > + if (IS_ERR(asc)) {
> > > > + ret = PTR_ERR(asc);
> > > > + dev_err(unicam->dev, "Failed to add entry to notifier: %d\n",
> > > > + ret);
> > > > + goto error;
> > > > + }
> > > > +
> > > > + unicam->notifier.ops = &unicam_async_ops;
> > > > +
> > > > + ret = v4l2_async_nf_register(&unicam->notifier);
> > > > + if (ret) {
> > > > + dev_err(unicam->dev, "Error registering device notifier: %d\n",
> > > > + ret);
> > > > + goto error;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +
> > > > +error:
> > > > + fwnode_handle_put(ep_handle);
> > > > + return ret;
> > > > +}
> > > > +
> > > > +/* -----------------------------------------------------------------------------
> > > > + * Probe & remove
> > > > + */
> > > > +
> > > > +static int unicam_media_init(struct unicam_device *unicam)
> > > > +{
> > > > + int ret;
> > > > +
> > > > + unicam->mdev.dev = unicam->dev;
> > > > + strscpy(unicam->mdev.model, UNICAM_MODULE_NAME,
> > > > + sizeof(unicam->mdev.model));
> > > > + strscpy(unicam->mdev.serial, "", sizeof(unicam->mdev.serial));
> > >
> > > Isn't the field already zeroed?
> >
> > Indeed. I'll drop this.
> >
> > >
> > > > + unicam->mdev.hw_revision = 0;
> > > > +
> > > > + media_device_init(&unicam->mdev);
> > > > +
> > > > + unicam->v4l2_dev.mdev = &unicam->mdev;
> > > > +
> > > > + ret = v4l2_device_register(unicam->dev, &unicam->v4l2_dev);
> > > > + if (ret < 0) {
> > > > + dev_err(unicam->dev, "Unable to register v4l2 device\n");
> > > > + goto err_media_cleanup;
> > > > + }
> > > > +
> > > > + ret = media_device_register(&unicam->mdev);
> > > > + if (ret < 0) {
> > > > + dev_err(unicam->dev,
> > > > + "Unable to register media-controller device\n");
> > > > + goto err_v4l2_unregister;
> > > > + }
> > > > +
> > > > + return 0;
> > > > +
> > > > +err_v4l2_unregister:
> > > > + v4l2_device_unregister(&unicam->v4l2_dev);
> > > > +err_media_cleanup:
> > > > + media_device_cleanup(&unicam->mdev);
> > > > + return ret;
> > > > +}
> >
> > [snip]
>
--
Kind regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH 4/4] arm: dts: allwinner: drop underscore in node names
From: Jernej Škrabec @ 2024-03-26 22:21 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Samuel Holland, Corentin Labbe, Maxime Ripard, devicetree,
linux-arm-kernel, linux-sunxi, linux-kernel, Krzysztof Kozlowski
Cc: Krzysztof Kozlowski
In-Reply-To: <20240317184130.157695-4-krzysztof.kozlowski@linaro.org>
Dne nedelja, 17. marec 2024 ob 19:41:30 CET je Krzysztof Kozlowski napisal(a):
> Underscores should not be used in node names (dtc with W=2 warns about
> them), so replace them with hyphens. Use also generic name for pwrseq
> node, because generic naming is favored by Devicetree spec. All the
> clocks affected by this change use clock-output-names, so resulting
> clock name should not change. Functional impact checked with comparing
> before/after DTBs with dtx_diff and fdtdump.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Best regards,
Jernej
^ permalink raw reply
* Re: [PATCH 3/4] arm64: dts: allwinner: Orange Pi: delete node by phandle
From: Jernej Škrabec @ 2024-03-26 22:19 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Samuel Holland, Corentin Labbe, Maxime Ripard, devicetree,
linux-arm-kernel, linux-sunxi, linux-kernel, Krzysztof Kozlowski
In-Reply-To: <41dcd736-5e58-4b71-ba82-961cd2863a62@linaro.org>
Dne nedelja, 17. marec 2024 ob 19:42:51 CET je Krzysztof Kozlowski napisal(a):
> On 17/03/2024 19:41, Krzysztof Kozlowski wrote:
> > Delete node via phandle, not via full node path, to avoid easy mistakes
> > - if original node name changes, such deletion would be ineffective and
> > not reported by the dtc as error.
> >
> > Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
> > ---
> > arch/arm/boot/dts/allwinner/sun8i-h2-plus-orangepi-r1.dts | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
>
> Eh, copy-paste, the subject prefix should be "ARM:".
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
I can fix it when applying.
Best regards,
Jernej
>
> Best regards,
> Krzysztof
>
>
^ permalink raw reply
* Re: [PATCH 2/4] arm64: dts: allwinner: drop underscore in node names
From: Jernej Škrabec @ 2024-03-26 22:18 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Samuel Holland, Corentin Labbe, Maxime Ripard, devicetree,
linux-arm-kernel, linux-sunxi, linux-kernel, Krzysztof Kozlowski
Cc: Krzysztof Kozlowski
In-Reply-To: <20240317184130.157695-2-krzysztof.kozlowski@linaro.org>
Dne nedelja, 17. marec 2024 ob 19:41:28 CET je Krzysztof Kozlowski napisal(a):
> Underscores should not be used in node names (dtc with W=2 warns about
> them), so replace them with hyphens. Use also generic name for pwrseq
> node, because generic naming is favored by Devicetree spec. All the
> clocks affected by this change use clock-output-names, so resulting
> clock name should not change. Functional impact checked with comparing
> before/after DTBs with dtx_diff and fdtdump.
>
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Best regards,
Jernej
^ permalink raw reply
* Re: [PATCH 1/4] arm64: dts: allwinner: Pine H64: correctly remove reg_gmac_3v3
From: Jernej Škrabec @ 2024-03-26 22:16 UTC (permalink / raw)
To: Rob Herring, Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai,
Samuel Holland, Corentin Labbe, Maxime Ripard, devicetree,
linux-arm-kernel, linux-sunxi, linux-kernel, Krzysztof Kozlowski
Cc: Krzysztof Kozlowski
In-Reply-To: <20240317184130.157695-1-krzysztof.kozlowski@linaro.org>
Dne nedelja, 17. marec 2024 ob 19:41:27 CET je Krzysztof Kozlowski napisal(a):
> There is no "reg_gmac_3v3" device node in sun50i-h6-pine-h64.dts,
> although there is "gmac-3v3" with "reg_gmac_3v3" label, so let's assume
> author wanted to remove that node. Delete node via phandle, not via
> full node path, to fix this.
>
> Fixes: f33a91175029 ("arm64: dts: allwinner: add pineh64 model B")
> Signed-off-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>
Reviewed-by: Jernej Skrabec <jernej.skrabec@gmail.com>
Best regards,
Jernej
> ---
> arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-model-b.dts | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-model-b.dts b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-model-b.dts
> index b710f1a0f53a..1b6e5595ac6e 100644
> --- a/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-model-b.dts
> +++ b/arch/arm64/boot/dts/allwinner/sun50i-h6-pine-h64-model-b.dts
> @@ -5,12 +5,12 @@
>
> #include "sun50i-h6-pine-h64.dts"
>
> +/delete-node/ ®_gmac_3v3;
> +
> / {
> model = "Pine H64 model B";
> compatible = "pine64,pine-h64-model-b", "allwinner,sun50i-h6";
>
> - /delete-node/ reg_gmac_3v3;
> -
> wifi_pwrseq: wifi_pwrseq {
> compatible = "mmc-pwrseq-simple";
> reset-gpios = <&r_pio 1 3 GPIO_ACTIVE_LOW>; /* PM3 */
>
^ permalink raw reply
* Re: [PATCH v2] riscv: dts: starfive: Remove PMIC interrupt info for Visionfive 2 board
From: Bo Gan @ 2024-03-26 22:12 UTC (permalink / raw)
To: Conor Dooley, Bo Gan
Cc: kernel, robh, krzysztof.kozlowski+dt, conor+dt, paul.walmsley,
palmer, aou, devicetree, linux-riscv, linux-kernel, Shengyu Qu,
Conor Dooley, stable
In-Reply-To: <20240326-ladylike-retold-9034734c2445@spud>
On 3/26/24 3:10 PM, Conor Dooley wrote:
> On Tue, Mar 26, 2024 at 03:06:33PM -0700, Bo Gan wrote:
>> On 3/26/24 1:37 PM, Conor Dooley wrote:
>>> From: Conor Dooley <conor.dooley@microchip.com>
>>>
>>> On Thu, 07 Mar 2024 20:21:12 +0800, Shengyu Qu wrote:
>>>> Interrupt line number of the AXP15060 PMIC is not a necessary part of
>>>> its device tree. And this would cause kernel to try to enable interrupt
>>>> line 0, which is not expected. So delete this part from device tree.
>>>>
>>>>
>>>
>>> Applied to riscv-dt-fixes, thanks! And I didn't forget, so I re-wrote
>>> the commit message to add some more information as promised.
>>>
>>> [1/1] riscv: dts: starfive: Remove PMIC interrupt info for Visionfive 2 board
>>> https://git.kernel.org/conor/c/0b163f43920d
>>>
>>> Thanks,
>>> Conor.
>>>
>> Hi Conor,
>>
>> Thank you very much for taking care of this. Actually the PLIC may silently
>> ignore the enablement of interrupt 0, so the upstream openSBI won't notice
>> anything. My modified version, however, will deliberately trigger a fault
>> for all writes to the reserved fields of PLIC, thus catching this issue.
>>
>> Hope it can clarify things a bit more.
>
> https://git.kernel.org/conor/c/0f74c64f0a9f
>
> Better?
>
Great! Thanks again.
Bo
^ permalink raw reply
* Re: [PATCH v6 08/15] dt-bindings: media: Add bindings for bcm2835-unicam
From: Sakari Ailus @ 2024-03-26 22:11 UTC (permalink / raw)
To: Laurent Pinchart
Cc: linux-media, Dave Stevenson, David Plowman, Jean-Michel Hautbois,
Hans Verkuil, Naushir Patuck, kernel-list, linux-rpi-kernel,
Florian Fainelli, Ray Jui, Scott Branden,
bcm-kernel-feedback-list, Conor Dooley, Krzysztof Kozlowski,
Rob Herring, devicetree
In-Reply-To: <20240325230203.GG23988@pendragon.ideasonboard.com>
Hi Laurent,
On Tue, Mar 26, 2024 at 01:02:03AM +0200, Laurent Pinchart wrote:
> Hi Sakari,
>
> On Mon, Mar 25, 2024 at 06:28:11PM +0000, Sakari Ailus wrote:
> > On Fri, Mar 01, 2024 at 11:32:23PM +0200, Laurent Pinchart wrote:
> > > From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > >
> > > Introduce the dt-bindings documentation for bcm2835 CCP2/CSI2 Unicam
> > > camera interface.
> > >
> > > Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> > > Co-developed-by: Naushir Patuck <naush@raspberrypi.com>
> > > Signed-off-by: Naushir Patuck <naush@raspberrypi.com>
> > > Co-developed-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> > > Signed-off-by: Jean-Michel Hautbois <jeanmichel.hautbois@ideasonboard.com>
> > > Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > Reviewed-by: Rob Herring <robh@kernel.org>
> > > Signed-off-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>
> > > ---
> > > Changes since v5:
> > >
> > > - Squash MAINTAINERS changes in
> > >
> > > Changes since v3:
> > >
> > > - Make MAINTAINERS its own patch
> > > - Describe the reg and clocks correctly
> > > - Use a vendor entry for the number of data lanes
> > > ---
> > > .../bindings/media/brcm,bcm2835-unicam.yaml | 117 ++++++++++++++++++
> > > MAINTAINERS | 6 +
> > > 2 files changed, 123 insertions(+)
> > > create mode 100644 Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
> > >
> > > diff --git a/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
> > > new file mode 100644
> > > index 000000000000..1938ace23b3d
> > > --- /dev/null
> > > +++ b/Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
> > > @@ -0,0 +1,117 @@
> > > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > > +%YAML 1.2
> > > +---
> > > +$id: http://devicetree.org/schemas/media/brcm,bcm2835-unicam.yaml#
> > > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > > +
> > > +title: Broadcom BCM283x Camera Interface (Unicam)
> > > +
> > > +maintainers:
> > > + - Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
> > > +
> > > +description: |-
> > > + The Unicam block on BCM283x SoCs is the receiver for either
> > > + CSI-2 or CCP2 data from image sensors or similar devices.
> > > +
> > > + The main platform using this SoC is the Raspberry Pi family of boards. On
> > > + the Pi the VideoCore firmware can also control this hardware block, and
> > > + driving it from two different processors will cause issues. To avoid this,
> > > + the firmware checks the device tree configuration during boot. If it finds
> > > + device tree nodes whose name starts with 'csi' then it will stop the firmware
> > > + accessing the block, and it can then safely be used via the device tree
> > > + binding.
> > > +
> > > +properties:
> > > + compatible:
> > > + const: brcm,bcm2835-unicam
> > > +
> > > + reg:
> > > + items:
> > > + - description: Unicam block.
> > > + - description: Clock Manager Image (CMI) block.
> > > +
> > > + reg-names:
> > > + items:
> > > + - const: unicam
> > > + - const: cmi
> > > +
> > > + interrupts:
> > > + maxItems: 1
> > > +
> > > + clocks:
> > > + items:
> > > + - description: Clock to drive the LP state machine of Unicam.
> > > + - description: Clock for the VPU (core clock).
> > > +
> > > + clock-names:
> > > + items:
> > > + - const: lp
> > > + - const: vpu
> > > +
> > > + power-domains:
> > > + items:
> > > + - description: Unicam power domain
> > > +
> > > + brcm,num-data-lanes:
> > > + $ref: /schemas/types.yaml#/definitions/uint32
> > > + enum: [ 2, 4 ]
> > > + description: |
> > > + Number of CSI-2 data lanes supported by this Unicam instance. The number
> > > + of data lanes actively used is specified with the data-lanes endpoint
> > > + property.
> > > +
> > > + port:
> > > + $ref: /schemas/graph.yaml#/$defs/port-base
> > > + unevaluatedProperties: false
> > > +
> > > + properties:
> > > + endpoint:
> > > + $ref: /schemas/media/video-interfaces.yaml#
> > > + unevaluatedProperties: false
> > > +
> > > + properties:
> > > + data-lanes: true
> > > +
> > > + required:
> > > + - data-lanes
> >
> > As the device supports multiple data interfaces (at least so it seems when
> > looking at the driver code), you need to list the bus-type property here,
> > too.
>
> Good point, I'll add
>
> properties:
> bus-type:
> enum: [ 3, 4 ]
> required:
> - bus-type
>
> Should I also change unevaluatedProperties to additionalProperties for
> the endpoint node, to reject any other property (and the explicitly list
> remote-endpoint as an allowed property) ? The result would be
>
> endpoint:
> $ref: /schemas/media/video-interfaces.yaml#
> additionalProperties: false
>
> properties:
> bus-type:
> enum: [ 3, 4 ]
>
> data-lanes: true
> remote-endpoint: true
>
> required:
> - bus-type
> - data-lanes
> - remote-endpoint
Seems good to me.
>
> > > +
> > > + required:
> > > + - endpoint
> > > +
> > > +required:
> > > + - compatible
> > > + - reg
> > > + - reg-names
> > > + - interrupts
> > > + - clocks
> > > + - clock-names
> > > + - power-domains
> > > + - brcm,num-data-lanes
> > > + - port
> > > +
> > > +additionalProperties: False
> > > +
> > > +examples:
> > > + - |
> > > + #include <dt-bindings/clock/bcm2835.h>
> > > + #include <dt-bindings/interrupt-controller/arm-gic.h>
> > > + #include <dt-bindings/power/raspberrypi-power.h>
> > > + csi1: csi@7e801000 {
> > > + compatible = "brcm,bcm2835-unicam";
> > > + reg = <0x7e801000 0x800>,
> > > + <0x7e802004 0x4>;
> > > + reg-names = "unicam", "cmi";
> > > + interrupts = <GIC_SPI 103 IRQ_TYPE_LEVEL_HIGH>;
> > > + clocks = <&clocks BCM2835_CLOCK_CAM1>,
> > > + <&firmware_clocks 4>;
> > > + clock-names = "lp", "vpu";
> > > + power-domains = <&power RPI_POWER_DOMAIN_UNICAM1>;
> > > + brcm,num-data-lanes = <2>;
> > > + port {
> > > + csi1_ep: endpoint {
> > > + remote-endpoint = <&imx219_0>;
> > > + data-lanes = <1 2>;
> > > + };
> > > + };
> > > + };
> > > +...
> > > diff --git a/MAINTAINERS b/MAINTAINERS
> > > index fada59148cb5..e50a59654e6e 100644
> > > --- a/MAINTAINERS
> > > +++ b/MAINTAINERS
> > > @@ -3997,6 +3997,12 @@ N: bcm113*
> > > N: bcm216*
> > > N: kona
> > >
> > > +BROADCOM BCM2835 CAMERA DRIVERS
> > > +M: Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
> > > +L: linux-media@vger.kernel.org
> > > +S: Maintained
> > > +F: Documentation/devicetree/bindings/media/brcm,bcm2835-unicam.yaml
> > > +
> > > BROADCOM BCM47XX MIPS ARCHITECTURE
> > > M: Hauke Mehrtens <hauke@hauke-m.de>
> > > M: Rafał Miłecki <zajec5@gmail.com>
>
> --
> Regards,
>
> Laurent Pinchart
--
Regards,
Sakari Ailus
^ permalink raw reply
* Re: [PATCH v2] riscv: dts: starfive: Remove PMIC interrupt info for Visionfive 2 board
From: Conor Dooley @ 2024-03-26 22:10 UTC (permalink / raw)
To: Bo Gan
Cc: kernel, robh, krzysztof.kozlowski+dt, conor+dt, paul.walmsley,
palmer, aou, devicetree, linux-riscv, linux-kernel, Shengyu Qu,
Conor Dooley, stable
In-Reply-To: <f472affe-d1ef-cbdb-b5c5-76f6b3ac78b3@gmail.com>
[-- Attachment #1: Type: text/plain, Size: 1228 bytes --]
On Tue, Mar 26, 2024 at 03:06:33PM -0700, Bo Gan wrote:
> On 3/26/24 1:37 PM, Conor Dooley wrote:
> > From: Conor Dooley <conor.dooley@microchip.com>
> >
> > On Thu, 07 Mar 2024 20:21:12 +0800, Shengyu Qu wrote:
> > > Interrupt line number of the AXP15060 PMIC is not a necessary part of
> > > its device tree. And this would cause kernel to try to enable interrupt
> > > line 0, which is not expected. So delete this part from device tree.
> > >
> > >
> >
> > Applied to riscv-dt-fixes, thanks! And I didn't forget, so I re-wrote
> > the commit message to add some more information as promised.
> >
> > [1/1] riscv: dts: starfive: Remove PMIC interrupt info for Visionfive 2 board
> > https://git.kernel.org/conor/c/0b163f43920d
> >
> > Thanks,
> > Conor.
> >
> Hi Conor,
>
> Thank you very much for taking care of this. Actually the PLIC may silently
> ignore the enablement of interrupt 0, so the upstream openSBI won't notice
> anything. My modified version, however, will deliberately trigger a fault
> for all writes to the reserved fields of PLIC, thus catching this issue.
>
> Hope it can clarify things a bit more.
https://git.kernel.org/conor/c/0f74c64f0a9f
Better?
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply
* [PATCH v2 4/4] pinctrl: qcom: spmi-gpio: Add PMIH0108 and PMD8028 support
From: Anjelique Melendez @ 2024-03-26 22:06 UTC (permalink / raw)
To: andersson, konrad.dybcio, linus.walleij, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-arm-msm, linux-gpio, devicetree, linux-kernel,
quic_subbaram, quic_collinsd, quic_jprakash, Anjelique Melendez
In-Reply-To: <20240326220628.2392802-1-quic_amelende@quicinc.com>
Add support for qcom,pmih0108-gpio and qcom,pmd8028-gpio.
Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
---
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index 54ffb7e1189a..4e80c7204e5f 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -1235,10 +1235,12 @@ static const struct of_device_id pmic_gpio_of_match[] = {
{ .compatible = "qcom,pm8994-gpio", .data = (void *) 22 },
{ .compatible = "qcom,pm8998-gpio", .data = (void *) 26 },
{ .compatible = "qcom,pma8084-gpio", .data = (void *) 22 },
+ { .compatible = "qcom,pmd8028-gpio", .data = (void *) 4 },
{ .compatible = "qcom,pmi632-gpio", .data = (void *) 8 },
{ .compatible = "qcom,pmi8950-gpio", .data = (void *) 2 },
{ .compatible = "qcom,pmi8994-gpio", .data = (void *) 10 },
{ .compatible = "qcom,pmi8998-gpio", .data = (void *) 14 },
+ { .compatible = "qcom,pmih0108-gpio", .data = (void *) 18 },
{ .compatible = "qcom,pmk8350-gpio", .data = (void *) 4 },
{ .compatible = "qcom,pmk8550-gpio", .data = (void *) 6 },
{ .compatible = "qcom,pmm8155au-gpio", .data = (void *) 10 },
--
2.34.1
^ permalink raw reply related
* [PATCH v2 3/4] pinctrl: qcom: spmi-gpio: Add PMXR2230 and PM6450 support
From: Anjelique Melendez @ 2024-03-26 22:06 UTC (permalink / raw)
To: andersson, konrad.dybcio, linus.walleij, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-arm-msm, linux-gpio, devicetree, linux-kernel,
quic_subbaram, quic_collinsd, quic_jprakash, Anjelique Melendez
In-Reply-To: <20240326220628.2392802-1-quic_amelende@quicinc.com>
Add support for qcom,pmxr2230-gpio and qcom,pm6450-gpio.
Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
---
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
index f4e2c88a7c82..54ffb7e1189a 100644
--- a/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
+++ b/drivers/pinctrl/qcom/pinctrl-spmi-gpio.c
@@ -1202,6 +1202,7 @@ static const struct of_device_id pmic_gpio_of_match[] = {
{ .compatible = "qcom,pm6150-gpio", .data = (void *) 10 },
{ .compatible = "qcom,pm6150l-gpio", .data = (void *) 12 },
{ .compatible = "qcom,pm6350-gpio", .data = (void *) 9 },
+ { .compatible = "qcom,pm6450-gpio", .data = (void *) 9 },
{ .compatible = "qcom,pm7250b-gpio", .data = (void *) 12 },
{ .compatible = "qcom,pm7325-gpio", .data = (void *) 10 },
{ .compatible = "qcom,pm7550ba-gpio", .data = (void *) 8},
@@ -1253,6 +1254,7 @@ static const struct of_device_id pmic_gpio_of_match[] = {
{ .compatible = "qcom,pmx55-gpio", .data = (void *) 11 },
{ .compatible = "qcom,pmx65-gpio", .data = (void *) 16 },
{ .compatible = "qcom,pmx75-gpio", .data = (void *) 16 },
+ { .compatible = "qcom,pmxr2230-gpio", .data = (void *) 12 },
{ },
};
--
2.34.1
^ permalink raw reply related
* [PATCH v2 2/4] dt-bindings: pinctrl: qcom,pmic-gpio: Add PMIH0108 and PMD8028 support
From: Anjelique Melendez @ 2024-03-26 22:06 UTC (permalink / raw)
To: andersson, konrad.dybcio, linus.walleij, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-arm-msm, linux-gpio, devicetree, linux-kernel,
quic_subbaram, quic_collinsd, quic_jprakash, Anjelique Melendez
In-Reply-To: <20240326220628.2392802-1-quic_amelende@quicinc.com>
Update the Qualcomm Technologies, Inc. PMIC GPIO binding documentation
to include compatible strings for PMIH0108 and PMD8028 PMICs.
Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
---
.../bindings/pinctrl/qcom,pmic-gpio.yaml | 20 +++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
index 2b17d244f051..a786357ed1af 100644
--- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
@@ -57,10 +57,12 @@ properties:
- qcom,pma8084-gpio
- qcom,pmc8180-gpio
- qcom,pmc8180c-gpio
+ - qcom,pmd8028-gpio
- qcom,pmi632-gpio
- qcom,pmi8950-gpio
- qcom,pmi8994-gpio
- qcom,pmi8998-gpio
+ - qcom,pmih0108-gpio
- qcom,pmk8350-gpio
- qcom,pmk8550-gpio
- qcom,pmm8155au-gpio
@@ -143,6 +145,7 @@ allOf:
- qcom,pm8005-gpio
- qcom,pm8450-gpio
- qcom,pm8916-gpio
+ - qcom,pmd8028-gpio
- qcom,pmk8350-gpio
- qcom,pmr735a-gpio
- qcom,pmr735b-gpio
@@ -304,6 +307,21 @@ allOf:
minItems: 1
maxItems: 7
+ - if:
+ properties:
+ comptaible:
+ contains:
+ enum:
+ - qcom,pmih0108-gpio
+ then:
+ properties:
+ gpio-line-names:
+ minItems: 18
+ maxItems: 18
+ gpio-reserved-ranges:
+ minItems: 1
+ maxItems: 9
+
- if:
properties:
compatible:
@@ -452,9 +470,11 @@ $defs:
- gpio1-gpio22 for pm8994
- gpio1-gpio26 for pm8998
- gpio1-gpio22 for pma8084
+ - gpio1-gpio4 for pmd8028
- gpio1-gpio8 for pmi632
- gpio1-gpio2 for pmi8950
- gpio1-gpio10 for pmi8994
+ - gpio1-gpio18 for pmih0108
- gpio1-gpio4 for pmk8350
- gpio1-gpio6 for pmk8550
- gpio1-gpio10 for pmm8155au
--
2.34.1
^ permalink raw reply related
* [PATCH v2 1/4] dt-bindings: pinctrl: qcom,pmic-gpio: Add PMXR2230 and PM6450 support
From: Anjelique Melendez @ 2024-03-26 22:06 UTC (permalink / raw)
To: andersson, konrad.dybcio, linus.walleij, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-arm-msm, linux-gpio, devicetree, linux-kernel,
quic_subbaram, quic_collinsd, quic_jprakash, Anjelique Melendez,
Krzystof Kozlowski
In-Reply-To: <20240326220628.2392802-1-quic_amelende@quicinc.com>
From: David Collins <quic_collinsd@quicinc.com>
Update the Qualcomm Technologies, Inc. PMIC GPIO binding documentation
to include compatible strings for PMXR2230 and PM6450 PMICs.
Signed-off-by: David Collins <quic_collinsd@quicinc.com>
Signed-off-by: Anjelique Melendez <quic_amelende@quicinc.com>
Acked-by: Krzystof Kozlowski <krzystof.kozlowski@linaro.org>
---
.../devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
index 3f8ad07c7cfd..2b17d244f051 100644
--- a/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
+++ b/Documentation/devicetree/bindings/pinctrl/qcom,pmic-gpio.yaml
@@ -24,6 +24,7 @@ properties:
- qcom,pm6150-gpio
- qcom,pm6150l-gpio
- qcom,pm6350-gpio
+ - qcom,pm6450-gpio
- qcom,pm7250b-gpio
- qcom,pm7325-gpio
- qcom,pm7550ba-gpio
@@ -72,6 +73,7 @@ properties:
- qcom,pmx55-gpio
- qcom,pmx65-gpio
- qcom,pmx75-gpio
+ - qcom,pmxr2230-gpio
- enum:
- qcom,spmi-gpio
@@ -198,6 +200,7 @@ allOf:
contains:
enum:
- qcom,pm6350-gpio
+ - qcom,pm6450-gpio
- qcom,pm8350c-gpio
then:
properties:
@@ -261,6 +264,7 @@ allOf:
- qcom,pmc8180c-gpio
- qcom,pmp8074-gpio
- qcom,pms405-gpio
+ - qcom,pmxr2230-gpio
then:
properties:
gpio-line-names:
@@ -417,6 +421,7 @@ $defs:
- gpio1-gpio10 for pm6150
- gpio1-gpio12 for pm6150l
- gpio1-gpio9 for pm6350
+ - gpio1-gpio9 for pm6450
- gpio1-gpio12 for pm7250b
- gpio1-gpio10 for pm7325
- gpio1-gpio8 for pm7550ba
@@ -464,6 +469,7 @@ $defs:
and gpio11)
- gpio1-gpio16 for pmx65
- gpio1-gpio16 for pmx75
+ - gpio1-gpio12 for pmxr2230
items:
pattern: "^gpio([0-9]+)$"
--
2.34.1
^ permalink raw reply related
* [PATCH v2 0/4] Add GPIO support for various PMICs
From: Anjelique Melendez @ 2024-03-26 22:06 UTC (permalink / raw)
To: andersson, konrad.dybcio, linus.walleij, robh+dt,
krzysztof.kozlowski+dt, conor+dt
Cc: linux-arm-msm, linux-gpio, devicetree, linux-kernel,
quic_subbaram, quic_collinsd, quic_jprakash, Anjelique Melendez
Add GPIO support for PMXR2230, PM6450, PMIH0108 and PMD8028
Changes since v1:
- Removed wildcard character from PMIC names
- Combined patch 3/5 and 4/5
- Made subjects and commit messages consistent between changes
Anjelique Melendez (3):
dt-bindings: pinctrl: qcom,pmic-gpio: Add PMIH0108 and PMD8028 support
pinctrl: qcom: spmi-gpio: Add PMXR2230 and PM6450 support
pinctrl: qcom: spmi-gpio: Add PMIH0108 and PMD8028 support
David Collins (1):
dt-bindings: pinctrl: qcom,pmic-gpio: Add PMXR2230 and PM6450 support
.../bindings/pinctrl/qcom,pmic-gpio.yaml | 26 +++++++++++++++++++
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 4 +++
2 files changed, 30 insertions(+)
--
2.34.1
^ permalink raw reply
* Re: [PATCH v2] riscv: dts: starfive: Remove PMIC interrupt info for Visionfive 2 board
From: Bo Gan @ 2024-03-26 22:06 UTC (permalink / raw)
To: Conor Dooley, ganboing, kernel, robh, krzysztof.kozlowski+dt,
conor+dt, paul.walmsley, palmer, aou, devicetree, linux-riscv,
linux-kernel, Shengyu Qu
Cc: Conor Dooley, stable
In-Reply-To: <20240326-create-motivate-2792be1692c5@spud>
On 3/26/24 1:37 PM, Conor Dooley wrote:
> From: Conor Dooley <conor.dooley@microchip.com>
>
> On Thu, 07 Mar 2024 20:21:12 +0800, Shengyu Qu wrote:
>> Interrupt line number of the AXP15060 PMIC is not a necessary part of
>> its device tree. And this would cause kernel to try to enable interrupt
>> line 0, which is not expected. So delete this part from device tree.
>>
>>
>
> Applied to riscv-dt-fixes, thanks! And I didn't forget, so I re-wrote
> the commit message to add some more information as promised.
>
> [1/1] riscv: dts: starfive: Remove PMIC interrupt info for Visionfive 2 board
> https://git.kernel.org/conor/c/0b163f43920d
>
> Thanks,
> Conor.
>
Hi Conor,
Thank you very much for taking care of this. Actually the PLIC may silently
ignore the enablement of interrupt 0, so the upstream openSBI won't notice
anything. My modified version, however, will deliberately trigger a fault
for all writes to the reserved fields of PLIC, thus catching this issue.
Hope it can clarify things a bit more.
Bo
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties
From: Laurent Pinchart @ 2024-03-26 21:53 UTC (permalink / raw)
To: Rob Herring
Cc: devicetree, linux-rpi-kernel, linux-arm-kernel, linux-gpio,
Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Linus Walleij, Bartosz Golaszewski, Eric Anholt, Stefan Wahren
In-Reply-To: <20240326213053.GA3562515-robh@kernel.org>
Hi Rob,
On Tue, Mar 26, 2024 at 04:30:53PM -0500, Rob Herring wrote:
> On Tue, Mar 26, 2024 at 02:49:01AM +0200, Laurent Pinchart wrote:
> > The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> > and, as a result, also needs to specify #address-cells and #size-cells.
> > Those properties have been added to thebcm2835-rpi.dtsi in commits
> > be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> > bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> > limitations"), but the DT bindings haven't been updated, resulting in
> > validation errors:
>
> I don't understand. We treat no dma-ranges the same as empty dma-ranges
> (dma-ranges;). If we didn't, *every* DT would be broken.
>
> We should never have dma-ranges without ranges either.
Please see v2 :-)
https://lore.kernel.org/linux-arm-kernel/20240326195807.15163-3-laurent.pinchart@ideasonboard.com/
https://lore.kernel.org/linux-arm-kernel/20240326195807.15163-4-laurent.pinchart@ideasonboard.com/
--
Regards,
Laurent Pinchart
^ permalink raw reply
* Re: [PATCH] dt-bindings: crypto: ti,omap-sham: Convert to dtschema
From: Rob Herring @ 2024-03-26 21:41 UTC (permalink / raw)
To: Conor Dooley
Cc: Animesh Agarwal, Herbert Xu, David S. Miller, Krzysztof Kozlowski,
Conor Dooley, linux-crypto, devicetree, linux-kernel
In-Reply-To: <20240326-spectrum-talon-0fc977c32c5c@spud>
On Tue, Mar 26, 2024 at 06:20:05PM +0000, Conor Dooley wrote:
> On Tue, Mar 26, 2024 at 05:31:00PM +0530, Animesh Agarwal wrote:
> > Convert the OMAP SoC SHA crypto Module bindings to DT Schema.
> >
> > Signed-off-by: Animesh Agarwal <animeshagarwal28@gmail.com>
> > ---
> > .../devicetree/bindings/crypto/omap-sham.txt | 28 ----------
> > .../bindings/crypto/ti,omap-sham.yaml | 56 +++++++++++++++++++
> > 2 files changed, 56 insertions(+), 28 deletions(-)
> > delete mode 100644 Documentation/devicetree/bindings/crypto/omap-sham.txt
> > create mode 100644 Documentation/devicetree/bindings/crypto/ti,omap-sham.yaml
> >
> > diff --git a/Documentation/devicetree/bindings/crypto/omap-sham.txt b/Documentation/devicetree/bindings/crypto/omap-sham.txt
> > deleted file mode 100644
> > index ad9115569611..000000000000
> > --- a/Documentation/devicetree/bindings/crypto/omap-sham.txt
> > +++ /dev/null
> > @@ -1,28 +0,0 @@
> > -OMAP SoC SHA crypto Module
> > -
> > -Required properties:
> > -
> > -- compatible : Should contain entries for this and backward compatible
> > - SHAM versions:
> > - - "ti,omap2-sham" for OMAP2 & OMAP3.
> > - - "ti,omap4-sham" for OMAP4 and AM33XX.
> > - - "ti,omap5-sham" for OMAP5, DRA7 and AM43XX.
> > -- ti,hwmods: Name of the hwmod associated with the SHAM module
> > -- reg : Offset and length of the register set for the module
> > -- interrupts : the interrupt-specifier for the SHAM module.
> > -
> > -Optional properties:
> > -- dmas: DMA specifiers for the rx dma. See the DMA client binding,
> > - Documentation/devicetree/bindings/dma/dma.txt
> > -- dma-names: DMA request name. Should be "rx" if a dma is present.
> > -
> > -Example:
> > - /* AM335x */
> > - sham: sham@53100000 {
> > - compatible = "ti,omap4-sham";
> > - ti,hwmods = "sham";
> > - reg = <0x53100000 0x200>;
> > - interrupts = <109>;
> > - dmas = <&edma 36>;
> > - dma-names = "rx";
> > - };
> > diff --git a/Documentation/devicetree/bindings/crypto/ti,omap-sham.yaml b/Documentation/devicetree/bindings/crypto/ti,omap-sham.yaml
> > new file mode 100644
> > index 000000000000..7a2529cc4cae
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/crypto/ti,omap-sham.yaml
> > @@ -0,0 +1,56 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/crypto/ti,omap-sham.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: OMAP SoC SHA crypto Module
> > +
> > +maintainers:
> > + - Animesh Agarwal <animeshagarwal28@gmail.com>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - ti,omap2-sham
> > + - ti,omap4-sham
> > + - ti,omap5-sham
> > +
> > + ti,hwmods:
> > + description: Name of the hwmod associated with the SHAM module
> > + $ref: /schemas/types.yaml#/definitions/string
> > + enum: [sham]
>
> Is there really only one value possible here?
> Also, the convention is to put vendor properties like this after more
> common properties like reg, interrupts etc.
>
> > +
> > + reg:
> > + maxItems: 1
> > +
> > + interrupts:
> > + maxItems: 1
> > +
> > + dmas:
> > + maxItems: 1
> > +
> > + dma-names:
> > + const: rx
> > +
> > +dependencies:
> > + dmas: [dma-names]
>
> Is this needed? Unless I'm sorely mistaken dt-schema enforces this itself
> (and same for any $foo-names).
dtschema does not. It does do the other way around. This seems fine.
Rob
^ permalink raw reply
* Re: [PATCH v3 3/8] dt-bindings: opp: Describe H616 OPPs and opp-supported-hw
From: Rob Herring @ 2024-03-26 21:37 UTC (permalink / raw)
To: Andre Przywara
Cc: Yangtao Li, Viresh Kumar, Nishanth Menon, Stephen Boyd,
Krzysztof Kozlowski, Conor Dooley, Chen-Yu Tsai, Jernej Skrabec,
Samuel Holland, Rafael J . Wysocki, linux-pm, devicetree,
linux-sunxi, linux-arm-kernel, Brandon Cheo Fusi, Martin Botka,
Martin Botka, Chris Morgan, Ryan Walklin
In-Reply-To: <20240326114743.712167-4-andre.przywara@arm.com>
On Tue, Mar 26, 2024 at 11:47:38AM +0000, Andre Przywara wrote:
> From: Martin Botka <martin.botka@somainline.org>
>
> Compared to the existing Allwinner H6 OPP scheme, the H616 uses a
> similar NVMEM based mechanism to determine the silicon revision, which
> is required to select the right frequency / voltage pair for the OPPs.
> However it limits the maximum frequency for some speed bins, also seems
> to not support all frequencies in all speed bins, which requires us to
> introduce the opp-supported-hw property.
>
> Add this property to the list of allowed properties, also drop the
> requirement for the revision specific opp-microvolt properties, since
> they might not be needed if using opp-supported-hw.
>
> Also use to opportunity to adjust some wording, and drop a sentence
> referring to the Linux driver and the OPP subsystem.
>
> Shorten the existing example and add another example, showcasing the
> opp-supported-hw property.
>
> Signed-off-by: Martin Botka <martin.botka@somainline.org>
> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
> .../allwinner,sun50i-h6-operating-points.yaml | 86 +++++++++----------
> 1 file changed, 42 insertions(+), 44 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/opp/allwinner,sun50i-h6-operating-points.yaml b/Documentation/devicetree/bindings/opp/allwinner,sun50i-h6-operating-points.yaml
> index 51f62c3ae1947..d679b2e4a7199 100644
> --- a/Documentation/devicetree/bindings/opp/allwinner,sun50i-h6-operating-points.yaml
> +++ b/Documentation/devicetree/bindings/opp/allwinner,sun50i-h6-operating-points.yaml
> @@ -13,25 +13,25 @@ maintainers:
> description: |
> For some SoCs, the CPU frequency subset and voltage value of each
> OPP varies based on the silicon variant in use. Allwinner Process
> - Voltage Scaling Tables defines the voltage and frequency value based
> - on the speedbin blown in the efuse combination. The
> - sun50i-cpufreq-nvmem driver reads the efuse value from the SoC to
> - provide the OPP framework with required information.
> + Voltage Scaling Tables define the voltage and frequency values based
> + on the speedbin blown in the efuse combination.
>
> allOf:
> - $ref: opp-v2-base.yaml#
>
> properties:
> compatible:
> - const: allwinner,sun50i-h6-operating-points
> + enum:
> + - allwinner,sun50i-h6-operating-points
> + - allwinner,sun50i-h616-operating-points
>
> nvmem-cells:
> description: |
> A phandle pointing to a nvmem-cells node representing the efuse
> - registers that has information about the speedbin that is used
> + register that has information about the speedbin that is used
> to select the right frequency/voltage value pair. Please refer
> - the for nvmem-cells bindings
> - Documentation/devicetree/bindings/nvmem/nvmem.txt and also
> + to the nvmem-cells bindings in
> + Documentation/devicetree/bindings/nvmem/nvmem.yaml and also the
> examples below.
>
> opp-shared: true
> @@ -47,15 +47,17 @@ patternProperties:
> properties:
> opp-hz: true
> clock-latency-ns: true
> + opp-microvolt: true
> + opp-supported-hw:
As this is an array,
maxItems: 1
> + description: |
Don't need '|'.
Otherwise,
Reviewed-by: Rob Herring <robh@kernel.org>
^ permalink raw reply
* Re: [PATCH v4 1/1] dt-bindings: net: starfive,jh7110-dwmac: Add StarFive JH8100 support
From: Rob Herring @ 2024-03-26 21:34 UTC (permalink / raw)
To: Tan Chun Hau
Cc: David S . Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Emil Renner Berthing, 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: <20240326052505.197408-2-chunhau.tan@starfivetech.com>
On Mon, Mar 25, 2024 at 10:25:05PM -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>
> ---
> .../devicetree/bindings/net/snps,dwmac.yaml | 1 +
> .../bindings/net/starfive,jh7110-dwmac.yaml | 54 ++++++++++++++-----
> 2 files changed, 41 insertions(+), 14 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/net/snps,dwmac.yaml b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> index 6b0341a8e0ea..a6d596b7dcf4 100644
> --- a/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/snps,dwmac.yaml
> @@ -97,6 +97,7 @@ properties:
> - snps,dwxgmac-2.10
> - starfive,jh7100-dwmac
> - starfive,jh7110-dwmac
> + - starfive,jh8100-dwmac
>
> reg:
> minItems: 1
> diff --git a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> index 0d1962980f57..ce018e9768d2 100644
> --- a/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> +++ b/Documentation/devicetree/bindings/net/starfive,jh7110-dwmac.yaml
> @@ -18,6 +18,7 @@ select:
> enum:
> - starfive,jh7100-dwmac
> - starfive,jh7110-dwmac
> + - starfive,jh8100-dwmac
> required:
> - compatible
>
> @@ -30,6 +31,10 @@ properties:
> - items:
> - const: starfive,jh7110-dwmac
> - const: snps,dwmac-5.20
> + - items:
> + - const: starfive,jh8100-dwmac
> + - const: starfive,jh7110-dwmac
> + - const: snps,dwmac-5.20
>
> reg:
> maxItems: 1
> @@ -107,20 +112,41 @@ allOf:
> contains:
> const: starfive,jh7110-dwmac
> then:
> - properties:
> - interrupts:
> - minItems: 3
> - maxItems: 3
> -
> - interrupt-names:
> - minItems: 3
> - maxItems: 3
interrupts and interrupt-names are the same, so you can leave them here
instead of duplicating them as you have.
> -
> - resets:
> - minItems: 2
> -
> - reset-names:
> - minItems: 2
> + if:
> + properties:
> + compatible:
> + contains:
> + const: starfive,jh8100-dwmac
> + then:
> + properties:
> + interrupts:
> + minItems: 3
> + maxItems: 3
> +
> + interrupt-names:
> + minItems: 3
> + maxItems: 3
> +
> + resets:
> + maxItems: 1
> +
> + reset-names:
> + const: stmmaceth
> + else:
> + properties:
> + interrupts:
> + minItems: 3
> + maxItems: 3
> +
> + interrupt-names:
> + minItems: 3
> + maxItems: 3
> +
> + resets:
> + minItems: 2
> +
> + reset-names:
> + minItems: 2
>
> unevaluatedProperties: false
>
> --
> 2.25.1
>
^ permalink raw reply
* Re: [PATCH 1/2] dt-bindings: arm: bcm: raspberrypi,bcm2835-firmware: Add missing properties
From: Rob Herring @ 2024-03-26 21:30 UTC (permalink / raw)
To: Laurent Pinchart
Cc: devicetree, linux-rpi-kernel, linux-arm-kernel, linux-gpio,
Krzysztof Kozlowski, Conor Dooley, Florian Fainelli,
Broadcom internal kernel review list, Ray Jui, Scott Branden,
Linus Walleij, Bartosz Golaszewski, Eric Anholt, Stefan Wahren
In-Reply-To: <20240326004902.17054-2-laurent.pinchart@ideasonboard.com>
On Tue, Mar 26, 2024 at 02:49:01AM +0200, Laurent Pinchart wrote:
> The raspberrypi,bcm2835-firmware devices requires a dma-ranges property,
> and, as a result, also needs to specify #address-cells and #size-cells.
> Those properties have been added to thebcm2835-rpi.dtsi in commits
> be08d278eb09 ("ARM: dts: bcm283x: Add cells encoding format to firmware
> bus") and 55c7c0621078 ("ARM: dts: bcm283x: Fix vc4's firmware bus DMA
> limitations"), but the DT bindings haven't been updated, resulting in
> validation errors:
I don't understand. We treat no dma-ranges the same as empty dma-ranges
(dma-ranges;). If we didn't, *every* DT would be broken.
We should never have dma-ranges without ranges either.
Rob
^ permalink raw reply
* Re: [PATCH 0/8] block: implement NVMEM provider
From: Daniel Golle @ 2024-03-26 21:28 UTC (permalink / raw)
To: Rob Herring
Cc: Diping Zhang, Jianhui Zhao, Jieying Zeng, Chad Monroe, Adam Fox,
John Crispin, Felix Fietkau, Krzysztof Kozlowski, Conor Dooley,
Ulf Hansson, Jens Axboe, Dave Chinner, Jan Kara,
Thomas Weißschuh, Damien Le Moal, Li Lingfeng,
Christian Brauner, Christian Heusel, Min Li, Adrian Hunter,
Avri Altman, Hannes Reinecke, Christian Loehle, Bean Huo, Yeqi Fu,
Victor Shih, Christophe JAILLET, Dominique Martinet,
Ricardo B. Marliere, devicetree, linux-kernel, linux-mmc,
linux-block
In-Reply-To: <20240326202449.GA3255378-robh@kernel.org>
Hi Rob,
On Tue, Mar 26, 2024 at 03:24:49PM -0500, Rob Herring wrote:
> +boot-architecture list
Good idea, thank you :)
>
> On Mon, Mar 25, 2024 at 03:38:19PM +0000, Daniel Golle wrote:
> > On Mon, Mar 25, 2024 at 10:10:46AM -0500, Rob Herring wrote:
> > > On Thu, Mar 21, 2024 at 07:31:48PM +0000, Daniel Golle wrote:
> > > > On embedded devices using an eMMC it is common that one or more (hw/sw)
> > > > partitions on the eMMC are used to store MAC addresses and Wi-Fi
> > > > calibration EEPROM data.
> > > >
> > > > Implement an NVMEM provider backed by a block device as typically the
> > > > NVMEM framework is used to have kernel drivers read and use binary data
> > > > from EEPROMs, efuses, flash memory (MTD), ...
> > > >
> > > > In order to be able to reference hardware partitions on an eMMC, add code
> > > > to bind each hardware partition to a specific firmware subnode.
> > > >
> > > > Overall, this enables uniform handling across practially all flash
> > > > storage types used for this purpose (MTD, UBI, and now also MMC).
> > > >
> > > > As part of this series it was necessary to define a device tree schema
> > > > for block devices and partitions on them, which (similar to how it now
> > > > works also for UBI volumes) can be matched by one or more properties.
> > > >
> > > > ---
> > > > This series has previously been submitted as RFC on July 19th 2023[1]
> > > > and most of the basic idea did not change since. Another round of RFC
> > > > was submitted on March 5th 2024[2] which has received overall positive
> > > > feedback and only minor corrections have been done since (see
> > > > changelog below).
> > >
> > > I don't recall giving positive feedback.
> > >
> > > I still think this should use offsets rather than partition specific
> > > information. Not wanting to have to update the offsets if they change is
> > > not reason enough to not use them.
> >
> > Using raw offsets on the block device (rather than the partition)
> > won't work for most existing devices and boot firmware out there. They
> > always reference the partition, usually by the name of a GPT
> > partition (but sometimes also PARTUUID or even PARTNO) which is then
> > used in the exact same way as an MTD partition or UBI volume would be
> > on devices with NOR or NAND flash.
>
> MTD normally uses offsets hence why I'd like some alignment. UBI is
> special because raw NAND is, well, special.
I get the point and in a way this is also already intended and
supported by this series. You can already just add an 'nvmem-layout'
node directly to a disk device rather than to a partition and define a
layout in this way.
Making this useful in practice will require some improvements to the
nvmem system in Linux though, because that currently uses signed 32-bit
integers as addresses which is not sufficient for the size of the
user-part of an eMMC. However, that needs to be done then and should
of course not be read as an excuse.
>
> > Just on eMMC we usually use a GPT
> > or MBR partition table rather than defining partitions in DT or cmdline,
> > which is rather rare (for historic reasons, I suppose, but it is what it
> > is now).
>
> Yes, I understand how eMMC works. I don't understand why if you have
> part #, uuid, or name you can't get to the offset or vice-versa. You
> need only 1 piece of identification to map partition table entries to DT
> nodes.
Yes, either of them (or a combination) is fine. In practise I've mostly
seen PARTNAME as identifier used in userland scripts, and only adding
this for now will probably cover most devices (and existing boot firmware)
out there. Notable exceptions are devices which are using MBR partitions
because the BootROM expects the bootloader to be at the same block as
we would usually have the primary GPT. In this case we can only use the
PARTNO, of course, and it stinks.
MediaTek's MT7623A/N is such an example, but it's a slingly outdated
and pretty weird niche SoC I admit.
> Sure, offsets can change, but surely the firmware can handle
> adjusting the DT?
Future firmware may be able to do this, of course. Current existing
firmware already out there on devices such as the quite popular
GL.iNet MT-6000, Netgear's Orbi and Orbi Pro series as well as all
Adtran SmartRG devices does not. Updating or changing the boot
firmware of devices already out there is not intended and quite
challenging, and will make the device incompatible with its vendor
firmware. Hence it would be better to support replacing only the
Linux-based firmware (eg. with OpenWrt or even Debian or any
general-purpose Linux, the eMMC is large enough...) while not having
to touch the boot firmware (and risking to brick the device if that
goes wrong).
Personally, I'm rather burdened and unhappy with vendor attempts to
have the boot firmware mess around too much in (highly customized,
downstream) DT, it may look like a good solution at the moment, but
can totally become an obstacle in an unpredictable future (no offense
ASUS...)
>
> An offset would also work for the case of random firmware data on the
> disk that may or may not have a partition associated with it. There are
> certainly cases of that. I don't think we have much of a solution for
> that other than trying to educate vendors to not do that or OS
> installers only supporting installing to something other than eMMC. This
> is something EBBR[1] is trying to address.
Absolutely. Actually *early* GL-iNet devices did exactly that: Use the
eMMC boot hw-partitions to store boot firmware as well as MAC
addresses and potentially also Wi-Fi calibration data.
The MT-2500 is the example I'm aware of and got sitting on my desk for
testing with this very series (which allows to also reference eMMC
hardware partitions, see "[7/8] mmc: block: set fwnode of disk
devices").
Unfortunately later devices such the the flag-ship MT-6000 moved MAC
addresses and WiFi-EEPROMs into a GPT partition on the user-part of
the eMMC.
>
> > Depending on the eMMC chip used, that partition may not even be at the
> > same offset for different batches of the same device and hence I'd
> > like to just do it in the same way vendor firmware does it as well.
>
> Often vendor firmware is not a model to follow...
I totally agree. However, I don't see a good reason for not supporting
those network-appliance-type embedded devices which even ship with
(outdated, downstream) Linux by default while going through great
lengths for things like broken ACPI tables in many laptops which
require lots of work-arounds to have features like suspend-to-disk
working, or even be able to run Linux at all.
>
> > Chad of Adtran has previously confirmed that [1], which was the
> > positive feedback I was refering to. Other vendors like GL-iNet or
> > Netgear are doing the exact same thing.
> >
> > As of now, we support this in OpenWrt by adding a lot of
> > board-specific knowledge to userland, which is ugly and also prevents
> > using things like PXE-initiated nfsroot on those devices.
> >
> > The purpose of this series is to be able to properly support such devices
> > (ie. practially all consumer-grade routers out there using an eMMC for
> > storing firmware).
> >
> > Also, those devices have enough resources to run a general purpose
> > distribution like Debian instead of OpenWrt, and all the userland
> > hacks to set MAC addresses and extract WiFi-EEPROM-data in a
> > board-specific ways will most certainly never find their way into
> > Debian. It's just not how embedded Linux works, unless you are looking
> > only at the RaspberryPi which got that data stored in a textfile
> > which is shipped by the distribution -- something very weird and very
> > different from literally all of-the-shelf routers, access-points or
> > switches I have ever seen (and I've seen many). Maybe Felix who has
> > seen even more of them can tell us more about that.
>
> General purpose distros want to partition the disk themselves. Adding
> anything to the DT for disk partitions would require the installer to be
> aware of it. There's various distro folks on the boot-arch list, so
> maybe one of them can comment.
Usually the installers are already aware to not touch partitions when
unaware of their purpose. Repartitioning the disk from scratch is not
what (modern) distributions are doing, at least the EFI System
partition is kept, as well as typical rescue/recovery partitions many
vendors put on their (Windows, Mac) laptops to allow to "factory
reset" them.
Installers usually offer to replace (or resize) the "large" partition
used by the currently installed OS instead.
And well, the DT reference to a partition holding e.g. MAC addresses
does make the installer aware of it, obviously.
Thank you for the constructive debate!
Cheers
Daniel
>
> Rob
>
> [1] https://arm-software.github.io/ebbr/index.html#document-chapter4-firmware-media
^ permalink raw reply
* [PATCH v3 5/5] arm64: dts: qcom: qrb2210-rb1: Enable the GPU
From: Konrad Dybcio @ 2024-03-26 21:08 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Marijn Suijten, Konrad Dybcio, linux-arm-msm, linux-clk,
devicetree, linux-kernel, Konrad Dybcio, Dmitry Baryshkov
In-Reply-To: <20240219-topic-rb1_gpu-v3-0-86f67786539a@linaro.org>
Enable the A702 GPU (also marketed as "3D accelerator by qcom [1], lol).
[1] https://docs.qualcomm.com/bundle/publicresource/87-61720-1_REV_A_QUALCOMM_ROBOTICS_RB1_PLATFORM__QUALCOMM_QRB2210__PRODUCT_BRIEF.pdf
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
arch/arm64/boot/dts/qcom/qrb2210-rb1.dts | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
index fca341300521..c54ad9f02e76 100644
--- a/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
+++ b/arch/arm64/boot/dts/qcom/qrb2210-rb1.dts
@@ -199,6 +199,14 @@ &gpi_dma0 {
status = "okay";
};
+&gpu {
+ status = "okay";
+
+ zap-shader {
+ firmware-name = "qcom/qcm2290/a702_zap.mbn";
+ };
+};
+
&i2c2 {
clock-frequency = <400000>;
status = "okay";
--
2.44.0
^ permalink raw reply related
* [PATCH v3 4/5] arm64: dts: qcom: qcm2290: Add GPU nodes
From: Konrad Dybcio @ 2024-03-26 21:08 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Marijn Suijten, Konrad Dybcio, linux-arm-msm, linux-clk,
devicetree, linux-kernel, Konrad Dybcio, Dmitry Baryshkov
In-Reply-To: <20240219-topic-rb1_gpu-v3-0-86f67786539a@linaro.org>
Describe the GPU hardware on the QCM2290.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
arch/arm64/boot/dts/qcom/qcm2290.dtsi | 154 ++++++++++++++++++++++++++++++++++
1 file changed, 154 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcm2290.dtsi b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
index 8221336a8212..913478be6e9d 100644
--- a/arch/arm64/boot/dts/qcom/qcm2290.dtsi
+++ b/arch/arm64/boot/dts/qcom/qcm2290.dtsi
@@ -7,6 +7,7 @@
#include <dt-bindings/clock/qcom,dispcc-qcm2290.h>
#include <dt-bindings/clock/qcom,gcc-qcm2290.h>
+#include <dt-bindings/clock/qcom,qcm2290-gpucc.h>
#include <dt-bindings/clock/qcom,rpmcc.h>
#include <dt-bindings/dma/qcom-gpi.h>
#include <dt-bindings/firmware/qcom,scm.h>
@@ -758,6 +759,11 @@ qusb2_hstx_trim: hstx-trim@25b {
reg = <0x25b 0x1>;
bits = <1 4>;
};
+
+ gpu_speed_bin: gpu-speed-bin@2006 {
+ reg = <0x2006 0x2>;
+ bits = <5 8>;
+ };
};
pmu@1b8e300 {
@@ -1425,6 +1431,154 @@ usb_dwc3_ss: endpoint {
};
};
+ gpu: gpu@5900000 {
+ compatible = "qcom,adreno-07000200", "qcom,adreno";
+ reg = <0x0 0x05900000 0x0 0x40000>;
+ reg-names = "kgsl_3d0_reg_memory";
+
+ interrupts = <GIC_SPI 177 IRQ_TYPE_LEVEL_HIGH>;
+
+ clocks = <&gpucc GPU_CC_GX_GFX3D_CLK>,
+ <&gpucc GPU_CC_AHB_CLK>,
+ <&gcc GCC_BIMC_GPU_AXI_CLK>,
+ <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
+ <&gpucc GPU_CC_CX_GMU_CLK>,
+ <&gpucc GPU_CC_CXO_CLK>;
+ clock-names = "core",
+ "iface",
+ "mem_iface",
+ "alt_mem_iface",
+ "gmu",
+ "xo";
+
+ interconnects = <&bimc MASTER_GFX3D RPM_ALWAYS_TAG
+ &bimc SLAVE_EBI1 RPM_ALWAYS_TAG>;
+ interconnect-names = "gfx-mem";
+
+ iommus = <&adreno_smmu 0 1>,
+ <&adreno_smmu 2 0>;
+ operating-points-v2 = <&gpu_opp_table>;
+ power-domains = <&rpmpd QCM2290_VDDCX>;
+ qcom,gmu = <&gmu_wrapper>;
+
+ nvmem-cells = <&gpu_speed_bin>;
+ nvmem-cell-names = "speed_bin";
+ #cooling-cells = <2>;
+
+ status = "disabled";
+
+ zap-shader {
+ memory-region = <&pil_gpu_mem>;
+ };
+
+ gpu_opp_table: opp-table {
+ compatible = "operating-points-v2";
+
+ /* TODO: Scale RPM_SMD_BIMC_GPU_CLK w/ turbo freqs */
+ opp-1123200000 {
+ opp-hz = /bits/ 64 <1123200000>;
+ required-opps = <&rpmpd_opp_turbo_plus>;
+ opp-peak-kBps = <6881000>;
+ opp-supported-hw = <0x3>;
+ turbo-mode;
+ };
+
+ opp-1017600000 {
+ opp-hz = /bits/ 64 <1017600000>;
+ required-opps = <&rpmpd_opp_turbo>;
+ opp-peak-kBps = <6881000>;
+ opp-supported-hw = <0x3>;
+ turbo-mode;
+ };
+
+ opp-921600000 {
+ opp-hz = /bits/ 64 <921600000>;
+ required-opps = <&rpmpd_opp_nom_plus>;
+ opp-peak-kBps = <6881000>;
+ opp-supported-hw = <0x3>;
+ };
+
+ opp-844800000 {
+ opp-hz = /bits/ 64 <844800000>;
+ required-opps = <&rpmpd_opp_nom>;
+ opp-peak-kBps = <6881000>;
+ opp-supported-hw = <0x7>;
+ };
+
+ opp-672000000 {
+ opp-hz = /bits/ 64 <672000000>;
+ required-opps = <&rpmpd_opp_svs_plus>;
+ opp-peak-kBps = <3879000>;
+ opp-supported-hw = <0xf>;
+ };
+
+ opp-537600000 {
+ opp-hz = /bits/ 64 <537600000>;
+ required-opps = <&rpmpd_opp_svs>;
+ opp-peak-kBps = <2929000>;
+ opp-supported-hw = <0xf>;
+ };
+
+ opp-355200000 {
+ opp-hz = /bits/ 64 <355200000>;
+ required-opps = <&rpmpd_opp_low_svs>;
+ opp-peak-kBps = <1720000>;
+ opp-supported-hw = <0xf>;
+ };
+ };
+ };
+
+ gmu_wrapper: gmu@596a000 {
+ compatible = "qcom,adreno-gmu-wrapper";
+ reg = <0x0 0x0596a000 0x0 0x30000>;
+ reg-names = "gmu";
+ power-domains = <&gpucc GPU_CX_GDSC>,
+ <&gpucc GPU_GX_GDSC>;
+ power-domain-names = "cx",
+ "gx";
+ };
+
+ gpucc: clock-controller@5990000 {
+ compatible = "qcom,qcm2290-gpucc";
+ reg = <0x0 0x05990000 0x0 0x9000>;
+ clocks = <&gcc GCC_GPU_CFG_AHB_CLK>,
+ <&rpmcc RPM_SMD_XO_CLK_SRC>,
+ <&gcc GCC_GPU_GPLL0_CLK_SRC>,
+ <&gcc GCC_GPU_GPLL0_DIV_CLK_SRC>;
+ power-domains = <&rpmpd QCM2290_VDDCX>;
+ required-opps = <&rpmpd_opp_low_svs>;
+ #clock-cells = <1>;
+ #reset-cells = <1>;
+ #power-domain-cells = <1>;
+ };
+
+ adreno_smmu: iommu@59a0000 {
+ compatible = "qcom,qcm2290-smmu-500", "qcom,adreno-smmu",
+ "qcom,smmu-500", "arm,mmu-500";
+ reg = <0x0 0x059a0000 0x0 0x10000>;
+ interrupts = <GIC_SPI 163 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 168 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 169 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 170 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 171 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 172 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 173 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 174 IRQ_TYPE_LEVEL_HIGH>;
+
+ clocks = <&gcc GCC_GPU_MEMNOC_GFX_CLK>,
+ <&gpucc GPU_CC_HLOS1_VOTE_GPU_SMMU_CLK>,
+ <&gcc GCC_GPU_SNOC_DVM_GFX_CLK>;
+ clock-names = "mem",
+ "hlos",
+ "iface";
+
+ power-domains = <&gpucc GPU_CX_GDSC>;
+
+ #global-interrupts = <1>;
+ #iommu-cells = <2>;
+ };
+
mdss: display-subsystem@5e00000 {
compatible = "qcom,qcm2290-mdss";
reg = <0x0 0x05e00000 0x0 0x1000>;
--
2.44.0
^ permalink raw reply related
* [PATCH v3 3/5] clk: qcom: Add QCM2290 GPU clock controller driver
From: Konrad Dybcio @ 2024-03-26 21:08 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Marijn Suijten, Konrad Dybcio, linux-arm-msm, linux-clk,
devicetree, linux-kernel, Konrad Dybcio, Dmitry Baryshkov
In-Reply-To: <20240219-topic-rb1_gpu-v3-0-86f67786539a@linaro.org>
Add a driver for the GPU clock controller block found on the QCM2290 SoC.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/clk/qcom/Kconfig | 9 +
drivers/clk/qcom/Makefile | 1 +
drivers/clk/qcom/gpucc-qcm2290.c | 423 +++++++++++++++++++++++++++++++++++++++
3 files changed, 433 insertions(+)
diff --git a/drivers/clk/qcom/Kconfig b/drivers/clk/qcom/Kconfig
index 8ab08e7b5b6c..c7ec41a6bd7f 100644
--- a/drivers/clk/qcom/Kconfig
+++ b/drivers/clk/qcom/Kconfig
@@ -65,6 +65,15 @@ config CLK_X1E80100_TCSRCC
Support for the TCSR clock controller on X1E80100 devices.
Say Y if you want to use peripheral devices such as SD/UFS.
+config CLK_QCM2290_GPUCC
+ tristate "QCM2290 Graphics Clock Controller"
+ depends on ARM64 || COMPILE_TEST
+ select CLK_QCM2290_GCC
+ help
+ Support for the graphics clock controller on QCM2290 devices.
+ Say Y if you want to support graphics controller devices and
+ functionality such as 3D graphics.
+
config QCOM_A53PLL
tristate "MSM8916 A53 PLL"
help
diff --git a/drivers/clk/qcom/Makefile b/drivers/clk/qcom/Makefile
index dec5b6db6860..cee261eb5144 100644
--- a/drivers/clk/qcom/Makefile
+++ b/drivers/clk/qcom/Makefile
@@ -26,6 +26,7 @@ obj-$(CONFIG_CLK_X1E80100_DISPCC) += dispcc-x1e80100.o
obj-$(CONFIG_CLK_X1E80100_GCC) += gcc-x1e80100.o
obj-$(CONFIG_CLK_X1E80100_GPUCC) += gpucc-x1e80100.o
obj-$(CONFIG_CLK_X1E80100_TCSRCC) += tcsrcc-x1e80100.o
+obj-$(CONFIG_CLK_QCM2290_GPUCC) += gpucc-qcm2290.o
obj-$(CONFIG_IPQ_APSS_PLL) += apss-ipq-pll.o
obj-$(CONFIG_IPQ_APSS_6018) += apss-ipq6018.o
obj-$(CONFIG_IPQ_GCC_4019) += gcc-ipq4019.o
diff --git a/drivers/clk/qcom/gpucc-qcm2290.c b/drivers/clk/qcom/gpucc-qcm2290.c
new file mode 100644
index 000000000000..b6e20d63ac85
--- /dev/null
+++ b/drivers/clk/qcom/gpucc-qcm2290.c
@@ -0,0 +1,423 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (c) 2020, The Linux Foundation. All rights reserved.
+ * Copyright (c) 2024, Linaro Limited
+ */
+
+#include <linux/clk-provider.h>
+#include <linux/mod_devicetable.h>
+#include <linux/module.h>
+#include <linux/platform_device.h>
+#include <linux/pm_clock.h>
+#include <linux/pm_runtime.h>
+#include <linux/regmap.h>
+
+#include <dt-bindings/clock/qcom,qcm2290-gpucc.h>
+
+#include "clk-alpha-pll.h"
+#include "clk-branch.h"
+#include "clk-rcg.h"
+#include "clk-regmap.h"
+#include "clk-regmap-divider.h"
+#include "clk-regmap-mux.h"
+#include "clk-regmap-phy-mux.h"
+#include "gdsc.h"
+#include "reset.h"
+
+enum {
+ DT_GCC_AHB_CLK,
+ DT_BI_TCXO,
+ DT_GCC_GPU_GPLL0_CLK_SRC,
+ DT_GCC_GPU_GPLL0_DIV_CLK_SRC,
+};
+
+enum {
+ P_BI_TCXO,
+ P_GPLL0_OUT_MAIN,
+ P_GPLL0_OUT_MAIN_DIV,
+ P_GPU_CC_PLL0_2X_DIV_CLK_SRC,
+ P_GPU_CC_PLL0_OUT_AUX,
+ P_GPU_CC_PLL0_OUT_AUX2,
+ P_GPU_CC_PLL0_OUT_MAIN,
+};
+
+static const struct pll_vco huayra_vco[] = {
+ { 600000000, 3300000000, 0 },
+ { 600000000, 2200000000, 1 },
+};
+
+static const struct alpha_pll_config gpu_cc_pll0_config = {
+ .l = 0x25,
+ .config_ctl_val = 0x200d4828,
+ .config_ctl_hi_val = 0x6,
+ .test_ctl_val = GENMASK(28, 26),
+ .test_ctl_hi_val = BIT(14),
+ .user_ctl_val = 0xf,
+};
+
+static struct clk_alpha_pll gpu_cc_pll0 = {
+ .offset = 0x0,
+ .vco_table = huayra_vco,
+ .num_vco = ARRAY_SIZE(huayra_vco),
+ .regs = clk_alpha_pll_regs[CLK_ALPHA_PLL_TYPE_HUAYRA_2290],
+ .clkr = {
+ .hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_pll0",
+ .parent_data = &(const struct clk_parent_data) {
+ .index = DT_BI_TCXO,
+ },
+ .num_parents = 1,
+ .ops = &clk_alpha_pll_huayra_ops,
+ },
+ },
+};
+
+static const struct parent_map gpu_cc_parent_map_0[] = {
+ { P_BI_TCXO, 0 },
+ { P_GPU_CC_PLL0_OUT_MAIN, 1 },
+ { P_GPLL0_OUT_MAIN, 5 },
+ { P_GPLL0_OUT_MAIN_DIV, 6 },
+};
+
+static const struct clk_parent_data gpu_cc_parent_data_0[] = {
+ { .index = DT_BI_TCXO, },
+ { .hw = &gpu_cc_pll0.clkr.hw, },
+ { .index = DT_GCC_GPU_GPLL0_CLK_SRC, },
+ { .index = DT_GCC_GPU_GPLL0_DIV_CLK_SRC, },
+};
+
+static const struct parent_map gpu_cc_parent_map_1[] = {
+ { P_BI_TCXO, 0 },
+ { P_GPU_CC_PLL0_2X_DIV_CLK_SRC, 1 },
+ { P_GPU_CC_PLL0_OUT_AUX2, 2 },
+ { P_GPU_CC_PLL0_OUT_AUX, 3 },
+ { P_GPLL0_OUT_MAIN, 5 },
+};
+
+static const struct clk_parent_data gpu_cc_parent_data_1[] = {
+ { .index = DT_BI_TCXO, },
+ { .hw = &gpu_cc_pll0.clkr.hw, },
+ { .hw = &gpu_cc_pll0.clkr.hw, },
+ { .hw = &gpu_cc_pll0.clkr.hw, },
+ { .index = DT_GCC_GPU_GPLL0_CLK_SRC, },
+};
+
+static const struct freq_tbl ftbl_gpu_cc_gmu_clk_src[] = {
+ F(200000000, P_GPLL0_OUT_MAIN, 3, 0, 0),
+ { }
+};
+
+static struct clk_rcg2 gpu_cc_gmu_clk_src = {
+ .cmd_rcgr = 0x1120,
+ .mnd_width = 0,
+ .hid_width = 5,
+ .parent_map = gpu_cc_parent_map_0,
+ .freq_tbl = ftbl_gpu_cc_gmu_clk_src,
+ .clkr.hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_gmu_clk_src",
+ .parent_data = gpu_cc_parent_data_0,
+ .num_parents = ARRAY_SIZE(gpu_cc_parent_data_0),
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_rcg2_shared_ops,
+ },
+};
+
+static const struct freq_tbl ftbl_gpu_cc_gx_gfx3d_clk_src[] = {
+ F(355200000, P_GPU_CC_PLL0_OUT_AUX, 2, 0, 0),
+ F(537600000, P_GPU_CC_PLL0_OUT_AUX2, 2, 0, 0),
+ F(672000000, P_GPU_CC_PLL0_OUT_AUX2, 2, 0, 0),
+ F(844800000, P_GPU_CC_PLL0_OUT_AUX2, 2, 0, 0),
+ F(921600000, P_GPU_CC_PLL0_OUT_AUX2, 2, 0, 0),
+ F(1017600000, P_GPU_CC_PLL0_OUT_AUX2, 2, 0, 0),
+ F(1123200000, P_GPU_CC_PLL0_OUT_AUX2, 2, 0, 0),
+ { }
+};
+
+static struct clk_rcg2 gpu_cc_gx_gfx3d_clk_src = {
+ .cmd_rcgr = 0x101c,
+ .mnd_width = 0,
+ .hid_width = 5,
+ .parent_map = gpu_cc_parent_map_1,
+ .freq_tbl = ftbl_gpu_cc_gx_gfx3d_clk_src,
+ .clkr.hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_gx_gfx3d_clk_src",
+ .parent_data = gpu_cc_parent_data_1,
+ .num_parents = ARRAY_SIZE(gpu_cc_parent_data_1),
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_rcg2_ops,
+ },
+};
+
+static struct clk_branch gpu_cc_ahb_clk = {
+ .halt_reg = 0x1078,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x1078,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_ahb_clk",
+ .flags = CLK_IS_CRITICAL,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch gpu_cc_crc_ahb_clk = {
+ .halt_reg = 0x107c,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x107c,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_crc_ahb_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch gpu_cc_cx_gfx3d_clk = {
+ .halt_reg = 0x10a4,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x10a4,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_cx_gfx3d_clk",
+ .parent_data = &(const struct clk_parent_data){
+ .hw = &gpu_cc_gx_gfx3d_clk_src.clkr.hw,
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch gpu_cc_cx_gmu_clk = {
+ .halt_reg = 0x1098,
+ .halt_check = BRANCH_HALT,
+ .clkr = {
+ .enable_reg = 0x1098,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_cx_gmu_clk",
+ .parent_data = &(const struct clk_parent_data){
+ .hw = &gpu_cc_gmu_clk_src.clkr.hw,
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch gpu_cc_cx_snoc_dvm_clk = {
+ .halt_reg = 0x108c,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x108c,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_cx_snoc_dvm_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch gpu_cc_cxo_aon_clk = {
+ .halt_reg = 0x1004,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x1004,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_cxo_aon_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch gpu_cc_cxo_clk = {
+ .halt_reg = 0x109c,
+ .halt_check = BRANCH_HALT,
+ .clkr = {
+ .enable_reg = 0x109c,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_cxo_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch gpu_cc_gx_gfx3d_clk = {
+ .halt_reg = 0x1054,
+ .halt_check = BRANCH_HALT_DELAY,
+ .clkr = {
+ .enable_reg = 0x1054,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_gx_gfx3d_clk",
+ .parent_data = &(const struct clk_parent_data){
+ .hw = &gpu_cc_gx_gfx3d_clk_src.clkr.hw,
+ },
+ .num_parents = 1,
+ .flags = CLK_SET_RATE_PARENT,
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch gpu_cc_sleep_clk = {
+ .halt_reg = 0x1090,
+ .halt_check = BRANCH_VOTED,
+ .clkr = {
+ .enable_reg = 0x1090,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_sleep_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct clk_branch gpu_cc_hlos1_vote_gpu_smmu_clk = {
+ .halt_reg = 0x5000,
+ .halt_check = BRANCH_VOTED,
+ .clkr = {
+ .enable_reg = 0x5000,
+ .enable_mask = BIT(0),
+ .hw.init = &(struct clk_init_data){
+ .name = "gpu_cc_hlos1_vote_gpu_smmu_clk",
+ .ops = &clk_branch2_ops,
+ },
+ },
+};
+
+static struct gdsc gpu_cx_gdsc = {
+ .gdscr = 0x106c,
+ .gds_hw_ctrl = 0x1540,
+ .pd = {
+ .name = "gpu_cx_gdsc",
+ },
+ .pwrsts = PWRSTS_OFF_ON,
+ .flags = VOTABLE,
+};
+
+static struct gdsc gpu_gx_gdsc = {
+ .gdscr = 0x100c,
+ .clamp_io_ctrl = 0x1508,
+ .resets = (unsigned int []){ GPU_GX_BCR },
+ .reset_count = 1,
+ .pd = {
+ .name = "gpu_gx_gdsc",
+ },
+ .parent = &gpu_cx_gdsc.pd,
+ .pwrsts = PWRSTS_OFF_ON,
+ .flags = CLAMP_IO | AON_RESET | SW_RESET,
+};
+
+static struct clk_regmap *gpu_cc_qcm2290_clocks[] = {
+ [GPU_CC_AHB_CLK] = &gpu_cc_ahb_clk.clkr,
+ [GPU_CC_CRC_AHB_CLK] = &gpu_cc_crc_ahb_clk.clkr,
+ [GPU_CC_CX_GFX3D_CLK] = &gpu_cc_cx_gfx3d_clk.clkr,
+ [GPU_CC_CX_GMU_CLK] = &gpu_cc_cx_gmu_clk.clkr,
+ [GPU_CC_CX_SNOC_DVM_CLK] = &gpu_cc_cx_snoc_dvm_clk.clkr,
+ [GPU_CC_CXO_AON_CLK] = &gpu_cc_cxo_aon_clk.clkr,
+ [GPU_CC_CXO_CLK] = &gpu_cc_cxo_clk.clkr,
+ [GPU_CC_GMU_CLK_SRC] = &gpu_cc_gmu_clk_src.clkr,
+ [GPU_CC_GX_GFX3D_CLK] = &gpu_cc_gx_gfx3d_clk.clkr,
+ [GPU_CC_GX_GFX3D_CLK_SRC] = &gpu_cc_gx_gfx3d_clk_src.clkr,
+ [GPU_CC_PLL0] = &gpu_cc_pll0.clkr,
+ [GPU_CC_SLEEP_CLK] = &gpu_cc_sleep_clk.clkr,
+ [GPU_CC_HLOS1_VOTE_GPU_SMMU_CLK] = &gpu_cc_hlos1_vote_gpu_smmu_clk.clkr,
+};
+
+static const struct qcom_reset_map gpu_cc_qcm2290_resets[] = {
+ [GPU_GX_BCR] = { 0x1008 },
+};
+
+static struct gdsc *gpu_cc_qcm2290_gdscs[] = {
+ [GPU_CX_GDSC] = &gpu_cx_gdsc,
+ [GPU_GX_GDSC] = &gpu_gx_gdsc,
+};
+
+static const struct regmap_config gpu_cc_qcm2290_regmap_config = {
+ .reg_bits = 32,
+ .reg_stride = 4,
+ .val_bits = 32,
+ .max_register = 0x9000,
+ .fast_io = true,
+};
+
+
+static const struct qcom_cc_desc gpu_cc_qcm2290_desc = {
+ .config = &gpu_cc_qcm2290_regmap_config,
+ .clks = gpu_cc_qcm2290_clocks,
+ .num_clks = ARRAY_SIZE(gpu_cc_qcm2290_clocks),
+ .resets = gpu_cc_qcm2290_resets,
+ .num_resets = ARRAY_SIZE(gpu_cc_qcm2290_resets),
+ .gdscs = gpu_cc_qcm2290_gdscs,
+ .num_gdscs = ARRAY_SIZE(gpu_cc_qcm2290_gdscs),
+};
+
+static const struct of_device_id gpu_cc_qcm2290_match_table[] = {
+ { .compatible = "qcom,qcm2290-gpucc" },
+ { }
+};
+MODULE_DEVICE_TABLE(of, gpu_cc_qcm2290_match_table);
+
+static int gpu_cc_qcm2290_probe(struct platform_device *pdev)
+{
+ struct regmap *regmap;
+ int ret;
+
+ regmap = qcom_cc_map(pdev, &gpu_cc_qcm2290_desc);
+ if (IS_ERR(regmap))
+ return PTR_ERR(regmap);
+
+ ret = devm_pm_runtime_enable(&pdev->dev);
+ if (ret)
+ return ret;
+
+ ret = devm_pm_clk_create(&pdev->dev);
+ if (ret)
+ return ret;
+
+ ret = pm_clk_add(&pdev->dev, NULL);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "failed to acquire ahb clock\n");
+ return ret;
+ }
+
+ ret = pm_runtime_resume_and_get(&pdev->dev);
+ if (ret)
+ return ret;
+
+ clk_huayra_2290_pll_configure(&gpu_cc_pll0, regmap, &gpu_cc_pll0_config);
+
+ regmap_update_bits(regmap, 0x1060, BIT(0), BIT(0)); /* GPU_CC_GX_CXO_CLK */
+
+ ret = qcom_cc_really_probe(pdev, &gpu_cc_qcm2290_desc, regmap);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register display clock controller\n");
+ goto out_pm_runtime_put;
+ }
+
+out_pm_runtime_put:
+ pm_runtime_put_sync(&pdev->dev);
+
+ return 0;
+}
+
+static struct platform_driver gpu_cc_qcm2290_driver = {
+ .probe = gpu_cc_qcm2290_probe,
+ .driver = {
+ .name = "gpucc-qcm2290",
+ .of_match_table = gpu_cc_qcm2290_match_table,
+ },
+};
+module_platform_driver(gpu_cc_qcm2290_driver);
+
+MODULE_DESCRIPTION("QTI QCM2290 GPU clock controller driver");
+MODULE_LICENSE("GPL");
--
2.44.0
^ permalink raw reply related
* [PATCH v3 2/5] clk: qcom: clk-alpha-pll: Add HUAYRA_2290 support
From: Konrad Dybcio @ 2024-03-26 21:08 UTC (permalink / raw)
To: Bjorn Andersson, Michael Turquette, Stephen Boyd, Rob Herring,
Krzysztof Kozlowski, Conor Dooley
Cc: Marijn Suijten, Konrad Dybcio, linux-arm-msm, linux-clk,
devicetree, linux-kernel, Konrad Dybcio, Dmitry Baryshkov
In-Reply-To: <20240219-topic-rb1_gpu-v3-0-86f67786539a@linaro.org>
Commit 134b55b7e19f ("clk: qcom: support Huayra type Alpha PLL")
introduced an entry to the alpha offsets array, but diving into QCM2290
downstream and some documentation, it turned out that the name Huayra
apparently has been used quite liberally across many chips, even with
noticeably different hardware.
Introduce another set of offsets and a new configure function for the
Huayra PLL found on QCM2290. This is required e.g. for the consumers
of GPUCC_PLL0 to properly start.
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Signed-off-by: Konrad Dybcio <konrad.dybcio@linaro.org>
---
drivers/clk/qcom/clk-alpha-pll.c | 47 ++++++++++++++++++++++++++++++++++++++++
drivers/clk/qcom/clk-alpha-pll.h | 3 +++
2 files changed, 50 insertions(+)
diff --git a/drivers/clk/qcom/clk-alpha-pll.c b/drivers/clk/qcom/clk-alpha-pll.c
index 8a412ef47e16..27ba8aa3e577 100644
--- a/drivers/clk/qcom/clk-alpha-pll.c
+++ b/drivers/clk/qcom/clk-alpha-pll.c
@@ -83,6 +83,19 @@ const u8 clk_alpha_pll_regs[][PLL_OFF_MAX_REGS] = {
[PLL_OFF_TEST_CTL_U] = 0x20,
[PLL_OFF_STATUS] = 0x24,
},
+ [CLK_ALPHA_PLL_TYPE_HUAYRA_2290] = {
+ [PLL_OFF_L_VAL] = 0x04,
+ [PLL_OFF_ALPHA_VAL] = 0x08,
+ [PLL_OFF_USER_CTL] = 0x0c,
+ [PLL_OFF_CONFIG_CTL] = 0x10,
+ [PLL_OFF_CONFIG_CTL_U] = 0x14,
+ [PLL_OFF_CONFIG_CTL_U1] = 0x18,
+ [PLL_OFF_TEST_CTL] = 0x1c,
+ [PLL_OFF_TEST_CTL_U] = 0x20,
+ [PLL_OFF_TEST_CTL_U1] = 0x24,
+ [PLL_OFF_OPMODE] = 0x28,
+ [PLL_OFF_STATUS] = 0x38,
+ },
[CLK_ALPHA_PLL_TYPE_BRAMMO] = {
[PLL_OFF_L_VAL] = 0x04,
[PLL_OFF_ALPHA_VAL] = 0x08,
@@ -779,6 +792,40 @@ static long clk_alpha_pll_round_rate(struct clk_hw *hw, unsigned long rate,
return clamp(rate, min_freq, max_freq);
}
+void clk_huayra_2290_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
+ const struct alpha_pll_config *config)
+{
+ u32 val;
+
+ clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL(pll), config->config_ctl_val);
+ clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U(pll), config->config_ctl_hi_val);
+ clk_alpha_pll_write_config(regmap, PLL_CONFIG_CTL_U1(pll), config->config_ctl_hi1_val);
+ clk_alpha_pll_write_config(regmap, PLL_TEST_CTL(pll), config->test_ctl_val);
+ clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U(pll), config->test_ctl_hi_val);
+ clk_alpha_pll_write_config(regmap, PLL_TEST_CTL_U1(pll), config->test_ctl_hi1_val);
+ clk_alpha_pll_write_config(regmap, PLL_L_VAL(pll), config->l);
+ clk_alpha_pll_write_config(regmap, PLL_ALPHA_VAL(pll), config->alpha);
+ clk_alpha_pll_write_config(regmap, PLL_USER_CTL(pll), config->user_ctl_val);
+
+ /* Set PLL_BYPASSNL */
+ regmap_update_bits(regmap, PLL_MODE(pll), PLL_BYPASSNL, PLL_BYPASSNL);
+ regmap_read(regmap, PLL_MODE(pll), &val);
+
+ /* Wait 5 us between setting BYPASS and deasserting reset */
+ udelay(5);
+
+ /* Take PLL out from reset state */
+ regmap_update_bits(regmap, PLL_MODE(pll), PLL_RESET_N, PLL_RESET_N);
+ regmap_read(regmap, PLL_MODE(pll), &val);
+
+ /* Wait 50us for PLL_LOCK_DET bit to go high */
+ usleep_range(50, 55);
+
+ /* Enable PLL output */
+ regmap_update_bits(regmap, PLL_MODE(pll), PLL_OUTCTRL, PLL_OUTCTRL);
+}
+EXPORT_SYMBOL_GPL(clk_huayra_2290_pll_configure);
+
static unsigned long
alpha_huayra_pll_calc_rate(u64 prate, u32 l, u32 a)
{
diff --git a/drivers/clk/qcom/clk-alpha-pll.h b/drivers/clk/qcom/clk-alpha-pll.h
index fb6d50263bb9..d1cd52158c17 100644
--- a/drivers/clk/qcom/clk-alpha-pll.h
+++ b/drivers/clk/qcom/clk-alpha-pll.h
@@ -15,6 +15,7 @@
enum {
CLK_ALPHA_PLL_TYPE_DEFAULT,
CLK_ALPHA_PLL_TYPE_HUAYRA,
+ CLK_ALPHA_PLL_TYPE_HUAYRA_2290,
CLK_ALPHA_PLL_TYPE_BRAMMO,
CLK_ALPHA_PLL_TYPE_FABIA,
CLK_ALPHA_PLL_TYPE_TRION,
@@ -191,6 +192,8 @@ extern const struct clk_ops clk_alpha_pll_rivian_evo_ops;
void clk_alpha_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
const struct alpha_pll_config *config);
+void clk_huayra_2290_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
+ const struct alpha_pll_config *config);
void clk_fabia_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
const struct alpha_pll_config *config);
void clk_trion_pll_configure(struct clk_alpha_pll *pll, struct regmap *regmap,
--
2.44.0
^ permalink raw reply related
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