* Re: [PATCH v3 3/4] media: imx: imx7-media-csi: Lift width constraints for 8bpp formats
From: Alexander Stein @ 2023-04-19 7:01 UTC (permalink / raw)
To: Laurent Pinchart
Cc: Rui Miguel Silva, Mauro Carvalho Chehab, Shawn Guo, Sascha Hauer,
Fabio Estevam, Pengutronix Kernel Team, NXP Linux Team,
linux-media, linux-arm-kernel
In-Reply-To: <20230418155947.GI30837@pendragon.ideasonboard.com>
Hi Laurent,
thanks for the feedback.
Am Dienstag, 18. April 2023, 17:59:47 CEST schrieb Laurent Pinchart:
> Hi Alexander,
>
> Thank you for the patch.
>
> The commit message should state "Lift width constraint for 16bpp
> formats".
To be pedantic it should be called "Lift width constraint for non-8bpp
formats" :)
> I would also phrase is "Relax" instead of "Lift" as it's not
> completely lifted.
That's true, this subtle difference should be accounted for. Thanks.
> On Tue, Apr 18, 2023 at 02:20:40PM +0200, Alexander Stein wrote:
> > For 8-bit formats the image_width just needs to be a multiple of 8 pixels
> > others just a multiple of 4 pixels.
>
> This is a bit terse, and I think a word or two are missing. It could be
> improved:
>
> The driver unconditionally aligns the image width to multiples of 8
> pixels. The real alignment constraint is 8 bytes, as indicated by the
> CSI_IMAG_PARA.IMAGE_WIDTH documentation that calls for 8 pixel alignment
> for 8bpp formats and 4 pixel alignment for other formats.
Thanks for this suggestion. This sounds much better.
> > Signed-off-by: Alexander Stein <alexander.stein@ew.tq-group.com>
> > ---
> > Changes in v3:
> > * Fix commit message (Only 8-bit formats needs multiple of 8 pixels)
> >
> > drivers/media/platform/nxp/imx7-media-csi.c | 9 ++++++++-
> > 1 file changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/media/platform/nxp/imx7-media-csi.c
> > b/drivers/media/platform/nxp/imx7-media-csi.c index
> > 1315f5743b76f..730c9c57bf4bc 100644
> > --- a/drivers/media/platform/nxp/imx7-media-csi.c
> > +++ b/drivers/media/platform/nxp/imx7-media-csi.c
> > @@ -1146,6 +1146,7 @@ __imx7_csi_video_try_fmt(struct v4l2_pix_format
> > *pixfmt,>
> > struct v4l2_rect *compose)
> >
> > {
> >
> > const struct imx7_csi_pixfmt *cc;
> >
> > + u32 walign;
> >
> > if (compose) {
> >
> > compose->width = pixfmt->width;
> >
> > @@ -1162,13 +1163,19 @@ __imx7_csi_video_try_fmt(struct v4l2_pix_format
> > *pixfmt,>
> > cc = imx7_csi_find_pixel_format(pixfmt->pixelformat);
> >
> > }
> >
> > + /* Refer to CSI_IMAG_PARA.IMAGE_WIDTH description */
> > + if (cc->bpp == 8)
> > + walign = 8;
> > + else
> > + walign = 4;
>
> Would the following convey the purpose better ?
>
> /*
> * The width alignment is 8 bytes as indicated by the
> * CSI_IMAG_PARA.IMAGE_WIDTH documentation. Convert it to pixels.
> */
> walign = 8 * 8 / cc->bpp;
I was wondering how to shorten this calculation without using ? operator,
nice.
> > +
> >
> > /*
> >
> > * Round up width for minimum burst size.
> > *
> > * TODO: Implement configurable stride support, and check what the
real
> > * hardware alignment constraint on the width is.
> > */
>
> We can now drop the second part of the sentence :-) The first line is
> actually not very accurate anymore. How about
>
> /*
> * The width alignment is 8 bytes as indicated by the
> * CSI_IMAG_PARA.IMAGE_WIDTH documentation. Convert it to pixels.
> *
> * TODO: Implement configurable stride support.
> */
> walign = 8 * 8 / cc->bpp;
> v4l_bound_align_image(&pixfmt->width, 1, 0xffff, walign,
> &pixfmt->height, 1, 0xffff, 1, 0);
That's neat, thanks. I'll update accordingly.
Best regards,
Alexander
> > - v4l_bound_align_image(&pixfmt->width, 1, 0xffff, 8,
> > + v4l_bound_align_image(&pixfmt->width, 1, 0xffff, walign,
> >
> > &pixfmt->height, 1, 0xffff, 1, 0);
> >
> > pixfmt->bytesperline = pixfmt->width * cc->bpp / 8;
--
TQ-Systems GmbH | Mühlstraße 2, Gut Delling | 82229 Seefeld, Germany
Amtsgericht München, HRB 105018
Geschäftsführer: Detlef Schneider, Rüdiger Stahl, Stefan Schneider
http://www.tq-group.com/
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH V22 2/3] misc: dcc: Add driver support for Data Capture and Compare unit(DCC)
From: Souradeep Chowdhury @ 2023-04-19 7:00 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Andy Gross, Konrad Dybcio, Krzysztof Kozlowski, Bjorn Andersson,
Rob Herring, Alex Elder, Arnd Bergmann, linux-arm-kernel,
linux-kernel, linux-arm-msm, devicetree, Sibi Sankar,
Rajendra Nayak
In-Reply-To: <2023041833-alienate-trash-f4da@gregkh>
On 4/18/2023 9:15 PM, Greg Kroah-Hartman wrote:
> On Tue, Apr 18, 2023 at 08:52:36PM +0530, Souradeep Chowdhury wrote:
>> The DCC is a DMA Engine designed to capture and store data during system
>> crash or software triggers. The DCC operates based on user inputs via
>> the debugfs interface. The user gives addresses as inputs and these
>> addresses are stored in the dcc sram. In case of a system crash or a
>> manual software trigger by the user through the debugfs interface, the
>> dcc captures and stores the values at these addresses. This patch
>> contains the driver which has all the methods pertaining to the debugfs
>> interface, auxiliary functions to support all the four fundamental
>> operations of dcc namely read, write, read/modify/write and loop. The
>> probe method here instantiates all the resources necessary for dcc to
>> operate mainly the dedicated dcc sram where it stores the values. The
>> DCC driver can be used for debugging purposes without going for a reboot
>> since it can perform software triggers as well based on user inputs.
>>
>> Also add the documentation for debugfs entries which explains the
>> functionalities of each debugfs file that has been created for dcc.
>
> I see no documentation entries in this commit :(
My apologies, this patch was given from qcom-next tree which already has
the documentation merged. Will include it from the next versions.
>
>> The following is the justification of using debugfs interface over the
>> other alternatives like sysfs/ioctls
>>
>> i) As can be seen from the debugfs attribute descriptions, some of the
>> debugfs attribute files here contains multiple arguments which needs to
>> be accepted from the user. This goes against the design style of sysfs.
>>
>> ii) The user input patterns have been made simple and convenient in this
>> case with the use of debugfs interface as user doesn't need to shuffle
>> between different files to execute one instruction as was the case on
>> using other alternatives.
>
> Why do you have debugfs and also a misc device? How are they related?
> Why both? Why not just one? What userspace tools are going to use
> either of these interfaces and where are they published to show how this
> all was tested?
DCC has two fundamental steps of usage:-
1.Configuring the register addresses on the dcc_sram which is done by
user through the debugfs interface. For example:-
echo R 0x10c004 > /sys/kernel/debug/dcc/../3/config
Here we are configuring the register addresses for list 3, the 'R'
indicates a read operation, so this register value will be read
in case of a software trigger or kernel panic/watchdog bite and
dumped into the dcc_sram.
2.The dcc_sram content is exposed to the user in the form of a misc
device. The user can parse the content of this dcc_sram to get the
register values. There is an opensource parser available for dcc at
the following location:-
https://git.codelinaro.org/clo/le/platform/vendor/qcom-opensource/tools/-/tree/opensource-tools.lnx.1.0.r176-rel/dcc_parser
>
>> Signed-off-by: Souradeep Chowdhury <quic_schowdhu@quicinc.com>
>> Reviewed-by: Alex Elder <elder@linaro.org>
>> ---
>> drivers/misc/Kconfig | 8 +
>> drivers/misc/Makefile | 1 +
>> drivers/misc/dcc.c | 1300 +++++++++++++++++++++++++++++++++++++++++++++++++
>> 3 files changed, 1309 insertions(+)
>> create mode 100644 drivers/misc/dcc.c
>>
>> diff --git a/drivers/misc/Kconfig b/drivers/misc/Kconfig
>> index 433aa41..e2bc652 100644
>> --- a/drivers/misc/Kconfig
>> +++ b/drivers/misc/Kconfig
>> @@ -276,6 +276,14 @@ config QCOM_COINCELL
>> to maintain PMIC register and RTC state in the absence of
>> external power.
>>
>> +config QCOM_DCC
>> + tristate "Qualcomm Technologies, Inc. Data Capture and Compare(DCC) engine driver"
>> + depends on ARCH_QCOM || COMPILE_TEST
>> + help
>> + This option enables driver for Data Capture and Compare engine. DCC
>> + driver provides interface to configure DCC block and read back
>> + captured data from DCC's internal SRAM.
>
>
> What is the module name?
It's qcom-dcc, will update the name here.
>
>> +
>> config QCOM_FASTRPC
>> tristate "Qualcomm FastRPC"
>> depends on ARCH_QCOM || COMPILE_TEST
>> diff --git a/drivers/misc/Makefile b/drivers/misc/Makefile
>> index 56de439..6fa8efa 100644
>> --- a/drivers/misc/Makefile
>> +++ b/drivers/misc/Makefile
>> @@ -16,6 +16,7 @@ obj-$(CONFIG_TIFM_CORE) += tifm_core.o
>> obj-$(CONFIG_TIFM_7XX1) += tifm_7xx1.o
>> obj-$(CONFIG_PHANTOM) += phantom.o
>> obj-$(CONFIG_QCOM_COINCELL) += qcom-coincell.o
>> +obj-$(CONFIG_QCOM_DCC) += dcc.o
>
> Why such a generic name? Shouldn't it be qcom-dcc?
Ack
>
>
>
>> obj-$(CONFIG_QCOM_FASTRPC) += fastrpc.o
>> obj-$(CONFIG_SENSORS_BH1770) += bh1770glc.o
>> obj-$(CONFIG_SENSORS_APDS990X) += apds990x.o
>> diff --git a/drivers/misc/dcc.c b/drivers/misc/dcc.c
>> new file mode 100644
>> index 0000000..7231ed9
>> --- /dev/null
>> +++ b/drivers/misc/dcc.c
>> @@ -0,0 +1,1300 @@
>> +// SPDX-License-Identifier: GPL-2.0-only
>> +/*
>> + * Copyright (c) 2015-2021, The Linux Foundation. All rights reserved.
>> + * Copyright (c) 2023, Qualcomm Innovation Center, Inc. All rights reserved.
>
> No work happened on this code in 2022? All 22 of these entries were
> only in 2021 and 2023?
Ack
>
>> + */
>> +
>> +#include <linux/bitfield.h>
>> +#include <linux/bitops.h>
>> +#include <linux/debugfs.h>
>> +#include <linux/delay.h>
>> +#include <linux/fs.h>
>> +#include <linux/io.h>
>> +#include <linux/iopoll.h>
>> +#include <linux/miscdevice.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/uaccess.h>
>> +
>> +#define STATUS_READY_TIMEOUT 5000 /* microseconds */
>> +
>> +/* DCC registers */
>> +#define DCC_HW_INFO 0x04
>> +#define DCC_LL_NUM_INFO 0x10
>> +#define DCC_STATUS(vers) ((vers) == 1 ? 0x0c : 0x1c)
>
> Why isn't this just an inline function?
Ack. Will make this inline
>
>> +#define DCC_LL_LOCK 0x00
>> +#define DCC_LL_CFG 0x04
>> +#define DCC_LL_BASE 0x08
>> +#define DCC_FD_BASE 0x0c
>> +#define DCC_LL_TIMEOUT 0x10
>> +#define DCC_LL_INT_ENABLE 0x18
>> +#define DCC_LL_INT_STATUS 0x1c
>> +#define DCC_LL_SW_TRIGGER 0x2c
>> +#define DCC_LL_BUS_ACCESS_STATUS 0x30
>> +
>> +/* Default value used if a bit 6 in the HW_INFO register is set. */
>> +#define DCC_FIX_LOOP_OFFSET 16
>> +
>> +/* Mask to find version info from HW_Info register */
>> +#define DCC_VER_INFO_MASK BIT(9)
>> +
>> +#define MAX_DCC_OFFSET GENMASK(9, 2)
>> +#define MAX_DCC_LEN GENMASK(6, 0)
>> +#define MAX_LOOP_CNT GENMASK(7, 0)
>> +#define MAX_LOOP_ADDR 10
>> +
>> +#define DCC_ADDR_DESCRIPTOR 0x00
>> +#define DCC_ADDR_LIMIT 27
>> +#define DCC_WORD_SIZE sizeof(u32)
>> +#define DCC_ADDR_RANGE_MASK GENMASK(31, 4)
>> +#define DCC_LOOP_DESCRIPTOR BIT(30)
>> +#define DCC_RD_MOD_WR_DESCRIPTOR BIT(31)
>> +#define DCC_LINK_DESCRIPTOR GENMASK(31, 30)
>> +#define DCC_STATUS_MASK GENMASK(1, 0)
>> +#define DCC_LOCK_MASK BIT(0)
>> +#define DCC_LOOP_OFFSET_MASK BIT(6)
>> +#define DCC_TRIGGER_MASK BIT(9)
>> +
>> +#define DCC_WRITE_MASK BIT(15)
>> +#define DCC_WRITE_OFF_MASK GENMASK(7, 0)
>> +#define DCC_WRITE_LEN_MASK GENMASK(14, 8)
>> +
>> +#define DCC_READ_IND 0x00
>> +#define DCC_WRITE_IND (BIT(28))
>> +
>> +#define DCC_AHB_IND 0x00
>> +#define DCC_APB_IND BIT(29)
>> +
>> +#define DCC_MAX_LINK_LIST 8
>> +
>> +#define DCC_VER_MASK2 GENMASK(5, 0)
>> +
>> +#define DCC_SRAM_WORD_LENGTH 4
>> +
>> +#define DCC_RD_MOD_WR_ADDR 0xC105E
>> +
>> +enum dcc_descriptor_type {
>> + DCC_READ_TYPE,
>> + DCC_LOOP_TYPE,
>> + DCC_READ_WRITE_TYPE,
>> + DCC_WRITE_TYPE
>> +};
>> +
>> +struct dcc_config_entry {
>> + u32 base;
>> + u32 offset;
>> + u32 len;
>> + u32 loop_cnt;
>> + u32 write_val;
>> + u32 mask;
>> + bool apb_bus;
>> + enum dcc_descriptor_type desc_type;
>> + struct list_head list;
>> +};
>
> No documentation for this structure?
Ack. Will add documentation to this structure as well.
>
>> +
>> +/**
>> + * struct dcc_drvdata - configuration information related to a dcc device
>> + * @base: Base Address of the dcc device
>> + * @dev: The device attached to the driver data
>> + * @mutex: Lock to protect access and manipulation of dcc_drvdata
>> + * @ram_base: Base address for the SRAM dedicated for the dcc device
>> + * @ram_size: Total size of the SRAM dedicated for the dcc device
>> + * @ram_offset: Offset to the SRAM dedicated for dcc device
>> + * @mem_map_ver: Memory map version of DCC hardware
>> + * @ram_cfg: Used for address limit calculation for dcc
>> + * @ram_start: Starting address of DCC SRAM
>> + * @sram_dev: Miscellaneous device equivalent of dcc SRAM
>> + * @cfg_head: Points to the head of the linked list of addresses
>> + * @dbg_dir: The dcc debugfs directory under which all the debugfs files are placed
>> + * @nr_link_list: Total number of linkedlists supported by the DCC configuration
>> + * @loop_shift: Loop offset bits range for the addresses
>> + * @enable_bitmap: Bitmap to capture the enabled status of each linked list of addresses
>> + */
>> +struct dcc_drvdata {
>> + void __iomem *base;
>> + void __iomem *ram_base;
>> + struct device *dev;
>
> Why do you need this back-pointer here?
This is getting used at multiple places to log
dev_err and also for resource allocation using
devm_kzalloc.
>
>> + struct mutex mutex;
>> + size_t ram_size;
>> + size_t ram_offset;
>> + int mem_map_ver;
>> + unsigned int ram_cfg;
>> + unsigned int ram_start;
>> + struct miscdevice sram_dev;
>
> Wait, this is the struct device, right? Or not?
miscdevice here represents the dcc_sram, an io-memory
dedicated to dcc for configuring/storing register values.
Whereas struct device represents the dcc_device which can
be used to write control signals on the bus to handle dcc
hardware operation sequence(like config_reset,sw_trigger etc.)
>
>> + struct list_head *cfg_head;
>> + struct dentry *dbg_dir;
>
> Why is this needed and not just looked up when necessary?
This needs to be passed while creating sub-directories and also
while removing. Rather than looking up everytime,saving and
re-using this in here.
>
>> + size_t nr_link_list;
>> + u8 loop_shift;
>> + unsigned long *enable_bitmap;
>
> So this is a list of bitmaps? Why "unsigned long"? Why not u64?
Ack
>
>> +};
>> +
>> +struct dcc_cfg_attr {
>> + u32 addr;
>> + u32 prev_addr;
>> + u32 prev_off;
>> + u32 link;
>> + u32 sram_offset;
>> +};
>> +
>> +struct dcc_cfg_loop_attr {
>> + u32 loop_cnt;
>> + u32 loop_len;
>> + u32 loop_off;
>> + bool loop_start;
>> +};
>> +
>> +static inline u32 dcc_list_offset(int version)
>> +{
>> + return version == 1 ? 0x1c : version == 2 ? 0x2c : 0x34;
>> +}
>
> Ah, you do have an inline function for the above mentioned macro.
> Please drop the macro.
>
> And write this inline function out to be readable, single-level ?:
> comments are impossible to read, let alone double-level ones.
>
> Write code for people first, compilers second. You gain nothing by
> making this terse except to confuse people.
Ack. This inline function is different from the previous macro.
Will keep both as inline functions.
>
>> +
>> +static inline void dcc_list_writel(struct dcc_drvdata *drvdata,
>> + u32 ll, u32 val, u32 off)
>> +{
>> + u32 offset = dcc_list_offset(drvdata->mem_map_ver) + off;
>> +
>> + writel(val, drvdata->base + ll * 0x80 + offset);
>
> What is this magic 0x80 for?
This is the list offset needed for address calculation as per the
dcc-hardware specification. Will declare a macro for this.
>
>> +}
>> +
>> +static inline u32 dcc_list_readl(struct dcc_drvdata *drvdata, u32 ll, u32 off)
>> +{
>> + u32 offset = dcc_list_offset(drvdata->mem_map_ver) + off;
>> +
>> + return readl(drvdata->base + ll * 0x80 + offset);
>
> Again, where did 0x80 come from?
Same as above.
>
>> +}
>> +
>> +static void dcc_sram_write_auto(struct dcc_drvdata *drvdata,
>> + u32 val, u32 *off)
>> +{
>> + /* If the overflow condition is met increment the offset
>> + * and return to indicate that overflow has occurred
>> + */
>> + if (unlikely(*off > drvdata->ram_size - 4)) {
>> + *off += 4;
>> + return;
>
> You didn't indicate anything here, all you did was succeed at the call,
> the caller has no way of ever knowing this failed.
>
> Why not return an error?
As per previous discussions it was decided to perform the write
speculatively. So that while writing to the dcc_sram if we exceed
the memory size, dcc will skip the write and keep incrementing
the offset. In the method "dcc_emit_config" we have the check to
finally detect if we have exceeded the sram offset
if (cfg.sram_offset + total_len > drvdata->ram_size) {
cfg.sram_offset += total_len;
goto overstep;
}
>
>> + }
>> +
>> + writel(val, drvdata->ram_base + *off);
>> +
>> + *off += 4;
>
> See, same modification as your "error" above.
>
> How was this tested?
This increment is needed to update the offset for the next memory
position in dcc_sram.
>
>> +static int dcc_emit_config(struct dcc_drvdata *drvdata, unsigned int curr_list)
>> +{
>> + int ret;
>> + u32 total_len, pos;
>> + struct dcc_config_entry *entry;
>> + struct dcc_cfg_attr cfg;
>> + struct dcc_cfg_loop_attr cfg_loop;
>> +
>> + memset(&cfg, 0, sizeof(cfg));
>> + memset(&cfg_loop, 0, sizeof(cfg_loop));
>
> Why are these large structures on the stack?
cfg_loop is needed for offset calculation in case of dcc loop
instructions based on the way it needs to be configured in dcc_sram
for dcc hardware to interpret it. entry, cfg is a generic structure used
across all dcc instructions. All these structures are needed for the
memory checks after we are done with configuring all the dcc instructions.
>
> And if on the stack, why not have the compiler initialize them to 0 for
> you automatically?
Ack
>
> I stopped reviewing here...
>
> greg k-h
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v1 4/5] mtd: rawnand: meson: clear OOB buffer before read
From: Arseniy Krasnov @ 2023-04-19 6:41 UTC (permalink / raw)
To: Liang Yang, Miquel Raynal
Cc: Richard Weinberger, Vignesh Raghavendra, Neil Armstrong,
Kevin Hilman, Jerome Brunet, Martin Blumenstingl, Jianxin Pan,
Yixun Lan, oxffffaa, kernel, linux-mtd, linux-arm-kernel,
linux-amlogic, linux-kernel, yonghui.yu
In-Reply-To: <ee10bdeb-416c-70f0-d323-7107fe0746e8@amlogic.com>
On 19.04.2023 06:05, Liang Yang wrote:
> Hi Arseniy,
>
> On 2023/4/18 22:57, Arseniy Krasnov wrote:
>> [ EXTERNAL EMAIL ]
>>
>>
>>
>> On 18.04.2023 16:25, Miquel Raynal wrote:
>>> Hi Arseniy,
>>>
>>>>>> Hello again @Liang @Miquel!
>>>>>>
>>>>>> One more question about OOB access, as I can see current driver uses the following
>>>>>> callbacks:
>>>>>>
>>>>>> nand->ecc.write_oob_raw = nand_write_oob_std;
>>>>>> nand->ecc.write_oob = nand_write_oob_std;
>>>>>>
>>>>>>
>>>>>> Function 'nand_write_oob_std()' writes data to the end of the page. But as I
>>>>>> can see by dumping 'data_buf' during read, physical layout of each page is the
>>>>>> following (1KB ECC):
>>>>>>
>>>>>> 0x000: [ 1 KB of data ]
>>>>>> 0x400: [ 2B user data] [ 14B ECC code]
>>>>>> 0x410: [ 1 KB of data ] (A)
>>>>>> 0x810: [ 2B user data] [ 14B ECC code]
>>>>>> 0x820: [ 32B unused ]
>>>>>>
>>>>>>
>>>>>>
>>>>>> So, after 'nand_write_oob_std()' (let data be sequence from [0x0 ... 0x3f]),
>>>>>> page will look like this:
>>>>>>
>>>>>> 0x000: [ 0xFF ]
>>>>>> 0x400: [ ........ ]
>>>>>> 0x7f0: [ 0xFF ]
>>>>>> 0x800: [ 00 ....................... ]
>>>>>> 0x830: [ ........................ 3f ]
>>>>>>
>>>>>> Here we have two problems:
>>>>>> 1) Attempt to display raw data by 'nanddump' utility produces a little bit
>>>>>> invalid output, as driver relies on layout (A) from above. E.g. OOB data
>>>>>> is at 0x400 and 0x810. Here is an example (attempt to write 0x11 0x22 0x33 0x44):
>>>>>>
>>>>>> 0x000007f0: 11 22 ff ff ff ff ff ff ff ff ff ff ff ff ff ff |."..............|
>>>>>> OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
>>>>>> OOB Data: 33 44 ff ff ff ff ff ff ff ff ff ff ff ff ff ff |3D..............|
>>>>>> OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
>>>>>> OOB Data: ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff ff |................|
>>>>>>
>>>>> Hi Arseniy,
>>>>>
>>>>> I realized the write_oob_raw() and write_oob() are wrong in meson_nand.c. I suggest both of them should be reworked and follow the format of meson nand controller. i.e. firstly format the data in Layout (A) and then write. reading is firstly reading the data of layout (A) and then compost the layout (B).
>>>>
>>>> IIUC after such writing only OOB (e.g. user bytes) according layout (A), hw will also write ECC codes, so
>>>> it will be impossible to write data to this page later, because we cannot update ECC codes properly for the newly
>>>> written data (we can't update bits from 0 to 1).
>>>>
>>>>>
>>>>>
>>>>>>
>>>>>> 2) Attempt to read data in ECC mode will fail, because IIUC page is in dirty
>>>>>> state (I mean was written at least once) and NAND controller tries to use
>>>>>> ECC codes at 0x400 and 0x810, which are obviously broken in this case. Thus
>>>>>
>>>>> As i said above, write_oob_raw() and write_oob() should be reworked.
>>>>> i don't know what do you mean page was written at least once. anyway the page should be written once, even just write_oob_raw().
>>>>
>>>> Sorry, You mean that after OOB write, we cannot write to the data area (e.g. 0x0 .. 0x810) until page will be erased? For example
>>>> JFFS2 writes to OOB own markers, then it tries to write to the data area of such page.
>>
>> @Liang, I'll describe current test case in details:
>> 1) I have erased page, I can read it in both raw and ecc modes - no problem (it is full of 0xFF).
>> 2) I (JFFS2 for example) want to write only OOB - let it be clean markers.
>> 3) I use raw write to the needed page (please correct me if i'm wrong). Four bytes
>> at 0x400 and 0x810 are updated. All other bytes still 0xff.
>> 4) Now, when i'm trying to read this page in ECC mode, I get ECC errors: IIUC this
>> happens because from controller point of view ECC codes are invalid for current
>> data (all ECCs are 0xff). Is this behaviour is ok?
>
> Yes, it is exactly reported ECC errors.
I see, so if we write OOB (e.g. using raw mode), there is no way to read this page in ECC mode later? And the
only way to make it readable is to write it in ECC mode, but before this write, we need to read it's
user's byte (from previous OOB write) in raw mode, put it to info buf (as user's bytes) and write this page. In this
case NAND controller will generate ECC codes including user's byte and page become readable in ECC mode
again.
>
>> 5) Ok, don't care on these ECC errors, let's go further.
>> 6) I'm going to write same page in ECC mode - how to do it correctly? There is already
>> 4 OOB bytes, considered to be covered by ECC (but in fact now - ECC area is FFed).
>
> If step 4 has excuted "program" command at the page (nand_write_oob_std() does), it can't be written again before erasing the page(block). so we have to read the whole page in the ddr and change the content, erase block, write it again.
>
> I don't think Jffs2 has the same steps (1-6) as you said above. are you sure that happes on Jffs2 or just an example?
I just checked JFFS2 mount/umount again, here is what i see:
0) First attempt to mount JFFS2.
1) It writes OOB to page N (i'm using raw write). It is cleanmarker value 0x85 0x19 0x03 0x20. Mount is done.
2) Umount JFFS2. Done.
3) Second attempt to mount JFFS2.
4) It reads OOB from page N (i'm using raw read). Value is 0x85 0x19 0x03 0x20. Done.
5) It reads page N in ECC mode, and i get:
jffs2: mtd->read(0x100 bytes from N) returned ECC error
6) Mount failed.
We already had problem which looks like this on another device. Solution was to use OOB area which is
not covered by ECC for JFFS2 cleanmarkers.
Thanks, Arseniy
>
>>
>> That's why I asked Your opinion about moving OOB data to nonprotected by ECC area (and
>> leave user's bytes untouched). In this case OOB access is free and not linked with ECC
>> codes which also covers data.
>>
>> Thanks, Arseniy
>>
>>>
>>> A page is written after two steps:
>>> - loading the data into the NAND chip cache (that's when you use the
>>> bus)
>>> - programming the NAND array with the data loaded in cache (that's when
>>> you wait)
>>>
>>> In theory it does not matter where you write in the cache, it's regular
>>> DRAM, you can make random writes there with the appropriate NAND
>>> commands. Of course when using embedded hardware ECC engines, the
>>> controllers usually expect to be fed in a certain way in order to
>>> produce the ECC bytes and put them at the right location in cache.
>>>
>>> And then, when you actually send the "program" command, the NAND cells
>>> actually get programmed based on what has been loaded in cache.
>>
>> Thanks for this details! Very interesting!
>>
>>
>>>
>>> Thanks,
>>> Miquèl
>>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
From: Stefan Wahren @ 2023-04-19 6:37 UTC (permalink / raw)
To: Simon Horman
Cc: Broadcom internal kernel review list, Nathan Chancellor,
Nick Desaulniers, Tom Rix, Greg Kroah-Hartman, linux-rpi-kernel,
linux-arm-kernel, linux-staging, llvm, Florian Fainelli
In-Reply-To: <20230418-vchiq_platform_init-static-v1-1-3c38ba81bc66@kernel.org>
Hi Simon,
Am 18.04.23 um 13:23 schrieb Simon Horman:
> vchiq_platform_init() is only used in this file so it can be static.
>
> clang-16 with W=1 reports:
>
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
> int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> ^
> drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
>
> Signed-off-by: Simon Horman <horms@kernel.org>
this is not the first attempt to fix this [1]. But maybe this has been
fixed in the meantime.
[1] -
https://lore.kernel.org/linux-staging/20221022043548.1671644-1-scottjcrouch@gmail.com/#t
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH] mmc: mtk-sd: reduce CIT for better performance
From: Wenbin Mei @ 2023-04-19 6:30 UTC (permalink / raw)
To: Ulf Hansson
Cc: Chaotian Jing, Matthias Brugger, AngeloGioacchino Del Regno,
linux-mmc, linux-kernel, linux-arm-kernel, linux-mediatek,
Wenbin Mei
CQHCI_SSC1 indicates to CQE the polling period to use when using
periodic SEND_QUEUE_STATUS(CMD13) polling.
The default value is 0x1000, now change it to 0x40, which can
improve the performance of some eMMC devices.
Signed-off-by: Wenbin Mei <wenbin.mei@mediatek.com>
---
drivers/mmc/host/mtk-sd.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/drivers/mmc/host/mtk-sd.c b/drivers/mmc/host/mtk-sd.c
index edade0e54a0c..483353ea3a99 100644
--- a/drivers/mmc/host/mtk-sd.c
+++ b/drivers/mmc/host/mtk-sd.c
@@ -2453,6 +2453,7 @@ static void msdc_hs400_enhanced_strobe(struct mmc_host *mmc,
static void msdc_cqe_enable(struct mmc_host *mmc)
{
struct msdc_host *host = mmc_priv(mmc);
+ struct cqhci_host *cq_host = mmc->cqe_private;
/* enable cmdq irq */
writel(MSDC_INT_CMDQ, host->base + MSDC_INTEN);
@@ -2462,6 +2463,8 @@ static void msdc_cqe_enable(struct mmc_host *mmc)
msdc_set_busy_timeout(host, 20 * 1000000000ULL, 0);
/* default read data timeout 1s */
msdc_set_timeout(host, 1000000000ULL, 0);
+
+ cqhci_writel(cq_host, 0x40, CQHCI_SSC1);
}
static void msdc_cqe_disable(struct mmc_host *mmc, bool recovery)
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 2/4] spi: s3c64xx: add cpu_relax in polling loop
From: Jaewon Kim @ 2023-04-19 6:06 UTC (permalink / raw)
To: Mark Brown, Krzysztof Kozlowski, Andi Shyti, Alim Akhtar
Cc: linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel,
Chanho Park, Jaewon Kim
In-Reply-To: <20230419060639.38853-1-jaewon02.kim@samsung.com>
Adds cpu_relax() to prevent long busy-wait.
There is busy-wait loop to check data transfer completion in polling mode.
Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
---
drivers/spi/spi-s3c64xx.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 273aa02322d9..886722fb40ea 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -568,6 +568,7 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
val = msecs_to_loops(ms);
do {
+ cpu_relax();
status = readl(regs + S3C64XX_SPI_STATUS);
} while (RX_FIFO_LVL(status, sdd) < xfer->len && --val);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 4/4] spi: s3c64xx: support interrupt based pio mode
From: Jaewon Kim @ 2023-04-19 6:06 UTC (permalink / raw)
To: Mark Brown, Krzysztof Kozlowski, Andi Shyti, Alim Akhtar
Cc: linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel,
Chanho Park, Jaewon Kim
In-Reply-To: <20230419060639.38853-1-jaewon02.kim@samsung.com>
Interrupt based pio mode is supported to reduce CPU load.
If transfer size is larger than 32 byte, it is processed using interrupt.
Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
---
drivers/spi/spi-s3c64xx.c | 82 ++++++++++++++++++++++++++++++++-------
1 file changed, 67 insertions(+), 15 deletions(-)
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index cf3060b2639b..ce1afb9a4ed4 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -58,6 +58,8 @@
#define S3C64XX_SPI_MODE_BUS_TSZ_HALFWORD (1<<17)
#define S3C64XX_SPI_MODE_BUS_TSZ_WORD (2<<17)
#define S3C64XX_SPI_MODE_BUS_TSZ_MASK (3<<17)
+#define S3C64XX_SPI_MODE_RX_RDY_LVL GENMASK(16, 11)
+#define S3C64XX_SPI_MODE_RX_RDY_LVL_SHIFT 11
#define S3C64XX_SPI_MODE_SELF_LOOPBACK (1<<3)
#define S3C64XX_SPI_MODE_RXDMA_ON (1<<2)
#define S3C64XX_SPI_MODE_TXDMA_ON (1<<1)
@@ -114,6 +116,8 @@
#define S3C64XX_SPI_TRAILCNT S3C64XX_SPI_MAX_TRAILCNT
+#define S3C64XX_SPI_POLLING_SIZE 32
+
#define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
#define is_polling(x) (x->cntrlr_info->polling)
@@ -552,10 +556,11 @@ static int s3c64xx_wait_for_dma(struct s3c64xx_spi_driver_data *sdd,
}
static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
- struct spi_transfer *xfer)
+ struct spi_transfer *xfer, int use_irq)
{
void __iomem *regs = sdd->regs;
unsigned long val;
+ unsigned long time;
u32 status;
int loops;
u32 cpy_len;
@@ -563,17 +568,24 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
int ms;
u32 tx_time;
- /* sleep during signal transfer time */
- status = readl(regs + S3C64XX_SPI_STATUS);
- if (RX_FIFO_LVL(status, sdd) < xfer->len) {
- tx_time = (xfer->len * 8 * 1000 * 1000) / sdd->cur_speed;
- usleep_range(tx_time / 2, tx_time);
- }
-
/* millisecs to xfer 'len' bytes @ 'cur_speed' */
ms = xfer->len * 8 * 1000 / sdd->cur_speed;
ms += 10; /* some tolerance */
+ if (use_irq) {
+ val = msecs_to_jiffies(ms);
+ time = wait_for_completion_timeout(&sdd->xfer_completion, val);
+ if (!time)
+ return -EIO;
+ } else {
+ /* sleep during signal transfer time */
+ status = readl(regs + S3C64XX_SPI_STATUS);
+ if (RX_FIFO_LVL(status, sdd) < xfer->len) {
+ tx_time = (xfer->len * 8 * 1000 * 1000) / sdd->cur_speed;
+ usleep_range(tx_time / 2, tx_time);
+ }
+ }
+
val = msecs_to_loops(ms);
do {
cpu_relax();
@@ -737,10 +749,13 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
void *rx_buf = NULL;
int target_len = 0, origin_len = 0;
int use_dma = 0;
+ int use_irq = 0;
int status;
u32 speed;
u8 bpw;
unsigned long flags;
+ u32 rdy_lv;
+ u32 val;
reinit_completion(&sdd->xfer_completion);
@@ -761,17 +776,46 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
sdd->rx_dma.ch && sdd->tx_dma.ch) {
use_dma = 1;
- } else if (xfer->len > fifo_len) {
+ } else if (xfer->len >= fifo_len) {
tx_buf = xfer->tx_buf;
rx_buf = xfer->rx_buf;
origin_len = xfer->len;
-
target_len = xfer->len;
- if (xfer->len > fifo_len)
- xfer->len = fifo_len;
+ xfer->len = fifo_len - 1;
}
do {
+ /* transfer size is greater than 32, change to IRQ mode */
+ if (xfer->len > S3C64XX_SPI_POLLING_SIZE)
+ use_irq = 1;
+
+ if (use_irq) {
+ reinit_completion(&sdd->xfer_completion);
+
+ rdy_lv = xfer->len;
+ /* Setup RDY_FIFO trigger Level
+ * RDY_LVL =
+ * fifo_lvl up to 64 byte -> N bytes
+ * 128 byte -> RDY_LVL * 2 bytes
+ * 256 byte -> RDY_LVL * 4 bytes
+ */
+ if (fifo_len == 128)
+ rdy_lv /= 2;
+ else if (fifo_len == 256)
+ rdy_lv /= 4;
+
+ val = readl(sdd->regs + S3C64XX_SPI_MODE_CFG);
+ val &= ~S3C64XX_SPI_MODE_RX_RDY_LVL;
+ val |= (rdy_lv << S3C64XX_SPI_MODE_RX_RDY_LVL_SHIFT);
+ writel(val, sdd->regs + S3C64XX_SPI_MODE_CFG);
+
+ /* Enable FIFO_RDY_EN IRQ */
+ val = readl(sdd->regs + S3C64XX_SPI_INT_EN);
+ writel((val | S3C64XX_SPI_INT_RX_FIFORDY_EN),
+ sdd->regs + S3C64XX_SPI_INT_EN);
+
+ }
+
spin_lock_irqsave(&sdd->lock, flags);
/* Pending only which is to be done */
@@ -793,7 +837,7 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
if (use_dma)
status = s3c64xx_wait_for_dma(sdd, xfer);
else
- status = s3c64xx_wait_for_pio(sdd, xfer);
+ status = s3c64xx_wait_for_pio(sdd, xfer, use_irq);
if (status) {
dev_err(&spi->dev,
@@ -832,8 +876,8 @@ static int s3c64xx_spi_transfer_one(struct spi_master *master,
if (xfer->rx_buf)
xfer->rx_buf += xfer->len;
- if (target_len > fifo_len)
- xfer->len = fifo_len;
+ if (target_len >= fifo_len)
+ xfer->len = fifo_len - 1;
else
xfer->len = target_len;
}
@@ -1003,6 +1047,14 @@ static irqreturn_t s3c64xx_spi_irq(int irq, void *data)
dev_err(&spi->dev, "TX underrun\n");
}
+ if (val & S3C64XX_SPI_ST_RX_FIFORDY) {
+ complete(&sdd->xfer_completion);
+ /* No pending clear irq, turn-off INT_EN_RX_FIFO_RDY */
+ val = readl(sdd->regs + S3C64XX_SPI_INT_EN);
+ writel((val & ~S3C64XX_SPI_INT_RX_FIFORDY_EN),
+ sdd->regs + S3C64XX_SPI_INT_EN);
+ }
+
/* Clear the pending irq by setting and then clearing it */
writel(clr, sdd->regs + S3C64XX_SPI_PENDING_CLR);
writel(0, sdd->regs + S3C64XX_SPI_PENDING_CLR);
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 1/4] spi: s3c64xx: changed to PIO mode if there is no DMA
From: Jaewon Kim @ 2023-04-19 6:06 UTC (permalink / raw)
To: Mark Brown, Krzysztof Kozlowski, Andi Shyti, Alim Akhtar
Cc: linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel,
Chanho Park, Jaewon Kim
In-Reply-To: <20230419060639.38853-1-jaewon02.kim@samsung.com>
Polling mode supported with qurik if there was no DMA in the SOC.
However, there are cased where we cannot or do not want to use DMA.
To support this case, if DMA is not set, it is switched to polling mode.
Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
---
drivers/spi/spi-s3c64xx.c | 8 ++++++--
include/linux/platform_data/spi-s3c64xx.h | 1 +
2 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 71d324ec9a70..273aa02322d9 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -19,7 +19,6 @@
#include <linux/platform_data/spi-s3c64xx.h>
#define MAX_SPI_PORTS 12
-#define S3C64XX_SPI_QUIRK_POLL (1 << 0)
#define S3C64XX_SPI_QUIRK_CS_AUTO (1 << 1)
#define AUTOSUSPEND_TIMEOUT 2000
@@ -116,7 +115,7 @@
#define S3C64XX_SPI_TRAILCNT S3C64XX_SPI_MAX_TRAILCNT
#define msecs_to_loops(t) (loops_per_jiffy / 1000 * HZ * t)
-#define is_polling(x) (x->port_conf->quirks & S3C64XX_SPI_QUIRK_POLL)
+#define is_polling(x) (x->cntrlr_info->polling)
#define RXBUSY (1<<2)
#define TXBUSY (1<<3)
@@ -1067,6 +1066,11 @@ static struct s3c64xx_spi_info *s3c64xx_spi_parse_dt(struct device *dev)
sci->num_cs = temp;
}
+ if (!of_find_property(dev->of_node, "dmas", NULL)) {
+ dev_warn(dev, "cannot find DMA, changed to PIO mode\n");
+ sci->polling = 1;
+ }
+
sci->no_cs = of_property_read_bool(dev->of_node, "no-cs-readback");
return sci;
diff --git a/include/linux/platform_data/spi-s3c64xx.h b/include/linux/platform_data/spi-s3c64xx.h
index 5df1ace6d2c9..cb7b8ddc899f 100644
--- a/include/linux/platform_data/spi-s3c64xx.h
+++ b/include/linux/platform_data/spi-s3c64xx.h
@@ -35,6 +35,7 @@ struct s3c64xx_spi_info {
int src_clk_nr;
int num_cs;
bool no_cs;
+ bool polling;
int (*cfg_gpio)(void);
};
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v2 0/4] Improves polling mode of s3c64xx driver
From: Jaewon Kim @ 2023-04-19 6:06 UTC (permalink / raw)
To: Mark Brown, Krzysztof Kozlowski, Andi Shyti, Alim Akhtar
Cc: linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel,
Chanho Park, Jaewon Kim
In-Reply-To: <CGME20230419062755epcas2p4c3c7c1e0d58e964f6e884f75ae120d91@epcas2p4.samsung.com>
1.
s3cx64xx driver was supporting polling mode using quirk for SOC without DMA.
However, in order to use PIO mode as an optional rather than a quirk, when DMA
is not described, spi operates with pio mode rather than probe fail.
2.
Fixed the problem of high CPU usage in PIO mode.
3.
If the transfer data size is larger than 32-bit, IRQ base PIO mode used.
Jaewon Kim (4):
spi: s3c64xx: changed to PIO mode if there is no DMA
spi: s3c64xx: add cpu_relax in polling loop
spi: s3c64xx: add sleep during transfer
spi: s3c64xx: support interrupt based pio mode
drivers/spi/spi-s3c64xx.c | 85 ++++++++++++++++++++---
include/linux/platform_data/spi-s3c64xx.h | 1 +
2 files changed, 76 insertions(+), 10 deletions(-)
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v2 3/4] spi: s3c64xx: add sleep during transfer
From: Jaewon Kim @ 2023-04-19 6:06 UTC (permalink / raw)
To: Mark Brown, Krzysztof Kozlowski, Andi Shyti, Alim Akhtar
Cc: linux-spi, linux-samsung-soc, linux-arm-kernel, linux-kernel,
Chanho Park, Jaewon Kim
In-Reply-To: <20230419060639.38853-1-jaewon02.kim@samsung.com>
In polling mode, the status register is constantly read to check transfer
completion. It cause excessive CPU usage.
So, it calculates the SPI transfer time and made it sleep.
Signed-off-by: Jaewon Kim <jaewon02.kim@samsung.com>
---
drivers/spi/spi-s3c64xx.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/drivers/spi/spi-s3c64xx.c b/drivers/spi/spi-s3c64xx.c
index 886722fb40ea..cf3060b2639b 100644
--- a/drivers/spi/spi-s3c64xx.c
+++ b/drivers/spi/spi-s3c64xx.c
@@ -561,6 +561,14 @@ static int s3c64xx_wait_for_pio(struct s3c64xx_spi_driver_data *sdd,
u32 cpy_len;
u8 *buf;
int ms;
+ u32 tx_time;
+
+ /* sleep during signal transfer time */
+ status = readl(regs + S3C64XX_SPI_STATUS);
+ if (RX_FIFO_LVL(status, sdd) < xfer->len) {
+ tx_time = (xfer->len * 8 * 1000 * 1000) / sdd->cur_speed;
+ usleep_range(tx_time / 2, tx_time);
+ }
/* millisecs to xfer 'len' bytes @ 'cur_speed' */
ms = xfer->len * 8 * 1000 / sdd->cur_speed;
--
2.17.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] ARM: sti: removal of stih415/stih416 related entries
From: Patrice CHOTARD @ 2023-04-19 6:23 UTC (permalink / raw)
To: Alain Volmat, Russell King; +Cc: linux-arm-kernel, linux-kernel
In-Reply-To: <20230416185939.18497-1-avolmat@me.com>
Hi Alain
On 4/16/23 20:59, Alain Volmat wrote:
> ST's STiH415 and STiH416 platforms have already been removed since
> a while. Remove some remaining bits within the mach-sti.
>
> Signed-off-by: Alain Volmat <avolmat@me.com>
> Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>
> ---
> Patch sent previously as part of serie: https://lore.kernel.org/all/20230209091659.1409-3-avolmat@me.com/
>
> arch/arm/mach-sti/Kconfig | 20 +-------------------
> arch/arm/mach-sti/board-dt.c | 2 --
> 2 files changed, 1 insertion(+), 21 deletions(-)
>
> diff --git a/arch/arm/mach-sti/Kconfig b/arch/arm/mach-sti/Kconfig
> index b2d45cf10a3c..609957dead98 100644
> --- a/arch/arm/mach-sti/Kconfig
> +++ b/arch/arm/mach-sti/Kconfig
> @@ -19,31 +19,13 @@ menuconfig ARCH_STI
> select PL310_ERRATA_769419 if CACHE_L2X0
> select RESET_CONTROLLER
> help
> - Include support for STMicroelectronics' STiH415/416, STiH407/10 and
> + Include support for STMicroelectronics' STiH407/10 and
> STiH418 family SoCs using the Device Tree for discovery. More
> information can be found in Documentation/arm/sti/ and
> Documentation/devicetree.
>
> if ARCH_STI
>
> -config SOC_STIH415
> - bool "STiH415 STMicroelectronics Consumer Electronics family"
> - default y
> - help
> - This enables support for STMicroelectronics Digital Consumer
> - Electronics family StiH415 parts, primarily targeted at set-top-box
> - and other digital audio/video applications using Flattned Device
> - Trees.
> -
> -config SOC_STIH416
> - bool "STiH416 STMicroelectronics Consumer Electronics family"
> - default y
> - help
> - This enables support for STMicroelectronics Digital Consumer
> - Electronics family StiH416 parts, primarily targeted at set-top-box
> - and other digital audio/video applications using Flattened Device
> - Trees.
> -
> config SOC_STIH407
> bool "STiH407 STMicroelectronics Consumer Electronics family"
> default y
> diff --git a/arch/arm/mach-sti/board-dt.c b/arch/arm/mach-sti/board-dt.c
> index ffecbf29646f..8c313f07bd02 100644
> --- a/arch/arm/mach-sti/board-dt.c
> +++ b/arch/arm/mach-sti/board-dt.c
> @@ -12,8 +12,6 @@
> #include "smp.h"
>
> static const char *const stih41x_dt_match[] __initconst = {
> - "st,stih415",
> - "st,stih416",
> "st,stih407",
> "st,stih410",
> "st,stih418",
Reviewed-by: Patrice Chotard <patrice.chotard@foss.st.com>
Thanks
Patrice
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 5/5] arm64: dts: ti: k3-j784s4-evm: Enable DisplayPort-0
From: Jayesh Choudhary @ 2023-04-19 6:17 UTC (permalink / raw)
To: nm, vigneshr, afd
Cc: s-vadapalli, kristo, robh+dt, krzysztof.kozlowski+dt,
linux-arm-kernel, devicetree, linux-kernel, a-bhatia1,
j-choudhary
In-Reply-To: <20230419061710.290068-1-j-choudhary@ti.com>
From: Rahul T R <r-ravikumar@ti.com>
Enable display for J784S4 EVM.
Add assigned clocks for DSS, DT node for DisplayPort PHY and pinmux for
DP HPD. Add the clock frequency for serdes_refclk.
Add the endpoint nodes to describe connection from:
DSS => MHDP => DisplayPort connector.
Also add the GPIO expander-4 node and pinmux for main_i2c4 which is
required for controlling DP power. Set status for all required nodes
for DP-0 as "okay".
Signed-off-by: Rahul T R <r-ravikumar@ti.com>
[j-choudhary@ti.com: move all the changes together to enable DP-0 in EVM]
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---
arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 116 +++++++++++++++++++++++
1 file changed, 116 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
index aef6f53ae8ac..03c9bf34cb1b 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
@@ -102,6 +102,28 @@ vdd_sd_dv: regulator-TLV71033 {
states = <1800000 0x0>,
<3300000 0x1>;
};
+
+ dp0_pwr_3v3: regulator-dp0-prw {
+ compatible = "regulator-fixed";
+ regulator-name = "dp0-pwr";
+ regulator-min-microvolt = <3300000>;
+ regulator-max-microvolt = <3300000>;
+ gpio = <&exp4 0 GPIO_ACTIVE_HIGH>;
+ enable-active-high;
+ };
+
+ dp0: dp0-connector {
+ compatible = "dp-connector";
+ label = "DP0";
+ type = "full-size";
+ dp-pwr-supply = <&dp0_pwr_3v3>;
+
+ port {
+ dp0_connector_in: endpoint {
+ remote-endpoint = <&dp0_out>;
+ };
+ };
+ };
};
&main_pmx0 {
@@ -163,6 +185,19 @@ vdd_sd_dv_pins_default: vdd-sd-dv-pins-default {
J784S4_IOPAD(0x020, PIN_INPUT, 7) /* (AJ35) MCAN15_RX.GPIO0_8 */
>;
};
+
+ dp0_pins_default: dp0-pins-default {
+ pinctrl-single,pins = <
+ J784S4_IOPAD(0x0cc, PIN_INPUT, 12) /* (AM37) SPI0_CS0.DP0_HPD */
+ >;
+ };
+
+ main_i2c4_pins_default: main-i2c4-pins-default {
+ pinctrl-single,pins = <
+ J784S4_IOPAD(0x014, PIN_INPUT_PULLUP, 8) /* (AG33) MCAN14_TX.I2C4_SCL */
+ J784S4_IOPAD(0x010, PIN_INPUT_PULLUP, 8) /* (AH33) MCAN13_RX.I2C4_SDA */
+ >;
+ };
};
&wkup_pmx0 {
@@ -301,3 +336,84 @@ &main_cpsw1_port1 {
phy-mode = "rgmii-rxid";
phy-handle = <&main_phy0>;
};
+
+&serdes_refclk {
+ clock-frequency = <100000000>;
+};
+
+&dss {
+ status = "okay";
+ assigned-clocks = <&k3_clks 218 2>,
+ <&k3_clks 218 5>,
+ <&k3_clks 218 14>,
+ <&k3_clks 218 18>;
+ assigned-clock-parents = <&k3_clks 218 3>,
+ <&k3_clks 218 7>,
+ <&k3_clks 218 16>,
+ <&k3_clks 218 22>;
+};
+
+&serdes_wiz4 {
+ status = "okay";
+};
+
+&serdes4 {
+ status = "okay";
+ serdes4_dp_link: phy@0 {
+ reg = <0>;
+ cdns,num-lanes = <4>;
+ #phy-cells = <0>;
+ cdns,phy-type = <PHY_TYPE_DP>;
+ resets = <&serdes_wiz4 1>, <&serdes_wiz4 2>,
+ <&serdes_wiz4 3>, <&serdes_wiz4 4>;
+ };
+};
+
+&mhdp {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&dp0_pins_default>;
+ phys = <&serdes4_dp_link>;
+ phy-names = "dpphy";
+};
+
+&dss_ports {
+ port {
+ dpi0_out: endpoint {
+ remote-endpoint = <&dp0_in>;
+ };
+ };
+};
+
+&main_i2c4 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_i2c4_pins_default>;
+ clock-frequency = <400000>;
+
+ exp4: gpio@20 {
+ compatible = "ti,tca6408";
+ reg = <0x20>;
+ gpio-controller;
+ #gpio-cells = <2>;
+ };
+};
+
+&dp0_ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ port@0 {
+ reg = <0>;
+ dp0_in: endpoint {
+ remote-endpoint = <&dpi0_out>;
+ };
+ };
+
+ port@4 {
+ reg = <4>;
+ dp0_out: endpoint {
+ remote-endpoint = <&dp0_connector_in>;
+ };
+ };
+};
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 1/5] arm64: dts: ti: k3-j784s4-main: Add system controller and SERDES lane mux
From: Jayesh Choudhary @ 2023-04-19 6:17 UTC (permalink / raw)
To: nm, vigneshr, afd
Cc: s-vadapalli, kristo, robh+dt, krzysztof.kozlowski+dt,
linux-arm-kernel, devicetree, linux-kernel, a-bhatia1,
j-choudhary
In-Reply-To: <20230419061710.290068-1-j-choudhary@ti.com>
From: Siddharth Vadapalli <s-vadapalli@ti.com>
The system controller node manages the CTRL_MMR0 region.
Add serdes_ln_ctrl node which is used for controlling the SERDES lane mux.
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
[j-choudhary@ti.com: Minor cleanup to fix dtc warnings]
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---
arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
index e9169eb358c1..5fb7edf4f5a0 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
@@ -5,6 +5,9 @@
* Copyright (C) 2022 Texas Instruments Incorporated - https://www.ti.com/
*/
+#include <dt-bindings/mux/mux.h>
+#include <dt-bindings/mux/ti-serdes.h>
+
&cbass_main {
msmc_ram: sram@70000000 {
compatible = "mmio-sram";
@@ -26,6 +29,25 @@ l3cache-sram@200000 {
};
};
+ scm_conf: syscon@100000 {
+ compatible = "ti,j721e-system-controller", "syscon", "simple-mfd";
+ reg = <0x00 0x00100000 0x00 0x1c000>;
+ #address-cells = <1>;
+ #size-cells = <1>;
+ ranges = <0x00 0x00 0x00100000 0x1c000>;
+
+ serdes_ln_ctrl: mux-controller-4080 {
+ compatible = "mmio-mux";
+ #mux-control-cells = <1>;
+ mux-reg-masks = <0x4080 0x3>, <0x4084 0x3>, /* SERDES0 lane0/1 select */
+ <0x4088 0x3>, <0x408c 0x3>, /* SERDES0 lane2/3 select */
+ <0x4090 0x3>, <0x4094 0x3>, /* SERDES1 lane0/1 select */
+ <0x4098 0x3>, <0x409c 0x3>, /* SERDES1 lane2/3 select */
+ <0x40a0 0x3>, <0x40a4 0x3>, /* SERDES2 lane0/1 select */
+ <0x40a8 0x3>, <0x40ac 0x3>; /* SERDES2 lane2/3 select */
+ };
+ };
+
gic500: interrupt-controller@1800000 {
compatible = "arm,gic-v3";
#address-cells = <2>;
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 3/5] arm64: dts: ti: k3-j784s4: Add WIZ and SERDES PHY nodes
From: Jayesh Choudhary @ 2023-04-19 6:17 UTC (permalink / raw)
To: nm, vigneshr, afd
Cc: s-vadapalli, kristo, robh+dt, krzysztof.kozlowski+dt,
linux-arm-kernel, devicetree, linux-kernel, a-bhatia1,
j-choudhary
In-Reply-To: <20230419061710.290068-1-j-choudhary@ti.com>
From: Siddharth Vadapalli <s-vadapalli@ti.com>
J784S4 SoC has 4 Serdes instances along with their respective WIZ
instances. Add device-tree nodes for them and disable them by default.
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---
arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 171 +++++++++++++++++++++
1 file changed, 171 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
index 8bd8aebebe1c..51aa476dedba 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
@@ -7,6 +7,15 @@
#include <dt-bindings/mux/mux.h>
#include <dt-bindings/mux/ti-serdes.h>
+#include <dt-bindings/phy/phy.h>
+#include <dt-bindings/phy/phy-ti.h>
+
+/ {
+ serdes_refclk: serdes-refclk {
+ #clock-cells = <0>;
+ compatible = "fixed-clock";
+ };
+};
&cbass_main {
msmc_ram: sram@70000000 {
@@ -440,6 +449,168 @@ main_sdhci1: mmc@4fb0000 {
status = "disabled";
};
+ serdes_wiz0: wiz@5060000 {
+ compatible = "ti,j784s4-wiz-10g";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ power-domains = <&k3_pds 404 TI_SCI_PD_EXCLUSIVE>;
+ clocks = <&k3_clks 404 2>, <&k3_clks 404 6>, <&k3_clks 404 5>, <&serdes_refclk>;
+ clock-names = "fck", "core_ref_clk", "core_ref1_clk", "ext_ref_clk";
+ assigned-clocks = <&k3_clks 404 6>;
+ assigned-clock-parents = <&k3_clks 404 10>;
+ num-lanes = <4>;
+ #reset-cells = <1>;
+ #clock-cells = <1>;
+ ranges = <0x5060000 0x00 0x5060000 0x10000>;
+
+ status = "disabled";
+
+ serdes0: serdes@5060000 {
+ compatible = "ti,j721e-serdes-10g";
+ reg = <0x05060000 0x010000>;
+ reg-names = "torrent_phy";
+ resets = <&serdes_wiz0 0>;
+ reset-names = "torrent_reset";
+ clocks = <&serdes_wiz0 TI_WIZ_PLL0_REFCLK>,
+ <&serdes_wiz0 TI_WIZ_PHY_EN_REFCLK>;
+ clock-names = "refclk", "phy_en_refclk";
+ assigned-clocks = <&serdes_wiz0 TI_WIZ_PLL0_REFCLK>,
+ <&serdes_wiz0 TI_WIZ_PLL1_REFCLK>,
+ <&serdes_wiz0 TI_WIZ_REFCLK_DIG>;
+ assigned-clock-parents = <&k3_clks 404 6>,
+ <&k3_clks 404 6>,
+ <&k3_clks 404 6>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #clock-cells = <1>;
+
+ status = "disabled";
+ };
+ };
+
+ serdes_wiz1: wiz@5070000 {
+ compatible = "ti,j784s4-wiz-10g";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ power-domains = <&k3_pds 405 TI_SCI_PD_EXCLUSIVE>;
+ clocks = <&k3_clks 405 2>, <&k3_clks 405 6>, <&k3_clks 405 5>, <&serdes_refclk>;
+ clock-names = "fck", "core_ref_clk", "core_ref1_clk", "ext_ref_clk";
+ assigned-clocks = <&k3_clks 405 6>;
+ assigned-clock-parents = <&k3_clks 405 10>;
+ num-lanes = <4>;
+ #reset-cells = <1>;
+ #clock-cells = <1>;
+ ranges = <0x05070000 0x00 0x05070000 0x10000>;
+
+ status = "disabled";
+
+ serdes1: serdes@5070000 {
+ compatible = "ti,j721e-serdes-10g";
+ reg = <0x05070000 0x010000>;
+ reg-names = "torrent_phy";
+ resets = <&serdes_wiz1 0>;
+ reset-names = "torrent_reset";
+ clocks = <&serdes_wiz1 TI_WIZ_PLL0_REFCLK>,
+ <&serdes_wiz1 TI_WIZ_PHY_EN_REFCLK>;
+ clock-names = "refclk", "phy_en_refclk";
+ assigned-clocks = <&serdes_wiz1 TI_WIZ_PLL0_REFCLK>,
+ <&serdes_wiz1 TI_WIZ_PLL1_REFCLK>,
+ <&serdes_wiz1 TI_WIZ_REFCLK_DIG>;
+ assigned-clock-parents = <&k3_clks 405 6>,
+ <&k3_clks 405 6>,
+ <&k3_clks 405 6>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #clock-cells = <1>;
+
+ status = "disabled";
+ };
+ };
+
+ serdes_wiz2: wiz@5020000 {
+ compatible = "ti,j784s4-wiz-10g";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ power-domains = <&k3_pds 406 TI_SCI_PD_EXCLUSIVE>;
+ clocks = <&k3_clks 406 2>, <&k3_clks 406 6>, <&k3_clks 406 5>, <&serdes_refclk>;
+ clock-names = "fck", "core_ref_clk", "core_ref1_clk", "ext_ref_clk";
+ assigned-clocks = <&k3_clks 406 6>;
+ assigned-clock-parents = <&k3_clks 406 10>;
+ num-lanes = <4>;
+ #reset-cells = <1>;
+ #clock-cells = <1>;
+ ranges = <0x05020000 0x00 0x05020000 0x10000>;
+
+ status = "disabled";
+
+ serdes2: serdes@5020000 {
+ compatible = "ti,j721e-serdes-10g";
+ reg = <0x05020000 0x010000>;
+ reg-names = "torrent_phy";
+ resets = <&serdes_wiz2 0>;
+ reset-names = "torrent_reset";
+ clocks = <&serdes_wiz2 TI_WIZ_PLL0_REFCLK>,
+ <&serdes_wiz2 TI_WIZ_PHY_EN_REFCLK>;
+ clock-names = "refclk", "phy_en_refclk";
+ assigned-clocks = <&serdes_wiz2 TI_WIZ_PLL0_REFCLK>,
+ <&serdes_wiz2 TI_WIZ_PLL1_REFCLK>,
+ <&serdes_wiz2 TI_WIZ_REFCLK_DIG>;
+ assigned-clock-parents = <&k3_clks 406 6>,
+ <&k3_clks 406 6>,
+ <&k3_clks 406 6>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #clock-cells = <1>;
+
+ status = "disabled";
+ };
+ };
+
+ serdes_wiz4: wiz@5050000 {
+ compatible = "ti,j784s4-wiz-10g";
+ #address-cells = <1>;
+ #size-cells = <1>;
+ power-domains = <&k3_pds 407 TI_SCI_PD_EXCLUSIVE>;
+ clocks = <&k3_clks 407 2>, <&k3_clks 407 6>, <&k3_clks 407 5>, <&serdes_refclk>;
+ clock-names = "fck", "core_ref_clk", "core_ref1_clk", "ext_ref_clk";
+ assigned-clocks = <&k3_clks 407 6>;
+ assigned-clock-parents = <&k3_clks 407 10>;
+ num-lanes = <4>;
+ #reset-cells = <1>;
+ #clock-cells = <1>;
+ ranges = <0x05050000 0x00 0x05050000 0x10000>,
+ <0xa030a00 0x00 0xa030a00 0x40>; /* DPTX PHY */
+
+ status = "disabled";
+
+ serdes4: serdes@5050000 {
+ /*
+ * Note: we also map DPTX PHY registers as the Torrent
+ * needs to manage those.
+ */
+ compatible = "ti,j721e-serdes-10g";
+ reg = <0x05050000 0x010000>,
+ <0x0a030a00 0x40>; /* DPTX PHY */
+ reg-names = "torrent_phy";
+ resets = <&serdes_wiz4 0>;
+ reset-names = "torrent_reset";
+ clocks = <&serdes_wiz4 TI_WIZ_PLL0_REFCLK>,
+ <&serdes_wiz4 TI_WIZ_PHY_EN_REFCLK>;
+ clock-names = "refclk", "phy_en_refclk";
+ assigned-clocks = <&serdes_wiz4 TI_WIZ_PLL0_REFCLK>,
+ <&serdes_wiz4 TI_WIZ_PLL1_REFCLK>,
+ <&serdes_wiz4 TI_WIZ_REFCLK_DIG>;
+ assigned-clock-parents = <&k3_clks 407 6>,
+ <&k3_clks 407 6>,
+ <&k3_clks 407 6>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ #clock-cells = <1>;
+
+ status = "disabled";
+ };
+ };
+
main_navss: bus@30000000 {
compatible = "simple-bus";
#address-cells = <2>;
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 0/5] Add peripherals for J784S4
From: Jayesh Choudhary @ 2023-04-19 6:17 UTC (permalink / raw)
To: nm, vigneshr, afd
Cc: s-vadapalli, kristo, robh+dt, krzysztof.kozlowski+dt,
linux-arm-kernel, devicetree, linux-kernel, a-bhatia1,
j-choudhary
This series adds support for:
- SERDES, WIZ DT nodes, Serdes lane control mux
- MAIN CPSW2G nodes
- DSS and DisplayPort-0 nodes
This series depends on DMA support patches for J784S4[1] which are
applied to linux-next.
DisplayPort has been tested on local J784S4 EVM. Test log:
<https://gist.github.com/Jayesh2000/b1465fc170cd97db4f0956770fafbc50>
Changelog v3->v2:
- fix dtc warnings for 'scm_conf' and 'serdes_ln_ctrl' nodes
(Checked all the changes of the series with W=12 option during build)
- added clock-frequency for serdes_refclk along with other EVM changes
This refclk is being used by all the instances of serdes_wiz which
are disabled by default. So configuring refclk when the serdes nodes
are used for the first time is okay.
Changelog v1->v2:
- Moved J784S4 EVM changes together to the last patch
(Suggested by Andrew)
v2 patch link:
<https://lore.kernel.org/all/20230414151553.339599-1-j-choudhary@ti.com/>
[1]:
<https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=00e34c94987e4fe866f12ad8eac17268c936880c>
<https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=82e6051a48957a89066d15b17bb85d2f662f2bad>
<https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=436b288687176bf4d2c1cd25b86173e5a1649a60>
Rahul T R (2):
arm64: dts: ti: k3-j784s4-main: Add DSS and DP-bridge node
arm64: dts: ti: k3-j784s4-evm: Enable DisplayPort-0
Siddharth Vadapalli (3):
arm64: dts: ti: k3-j784s4-main: Add system controller and SERDES lane
mux
arm64: dts: ti: k3-j784s4: Add Main CPSW2G node
arm64: dts: ti: k3-j784s4: Add WIZ and SERDES PHY nodes
arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 164 ++++++++++
arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 338 +++++++++++++++++++++
2 files changed, 502 insertions(+)
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH v3 4/5] arm64: dts: ti: k3-j784s4-main: Add DSS and DP-bridge node
From: Jayesh Choudhary @ 2023-04-19 6:17 UTC (permalink / raw)
To: nm, vigneshr, afd
Cc: s-vadapalli, kristo, robh+dt, krzysztof.kozlowski+dt,
linux-arm-kernel, devicetree, linux-kernel, a-bhatia1,
j-choudhary
In-Reply-To: <20230419061710.290068-1-j-choudhary@ti.com>
From: Rahul T R <r-ravikumar@ti.com>
Add DSS and DP-bridge node for J784S4 SoC. DSS IP in J784S4 is
same as DSS IP in J721E, so same compatible is being used.
The DP is Cadence MHDP8546.
Signed-off-by: Rahul T R <r-ravikumar@ti.com>
[j-choudhary@ti.com: move all k3-j784s4-main.dtsi changes together]
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---
arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 77 ++++++++++++++++++++++
1 file changed, 77 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
index 51aa476dedba..739741e93bc1 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
@@ -1373,4 +1373,81 @@ main_spi7: spi@2170000 {
clocks = <&k3_clks 383 1>;
status = "disabled";
};
+
+ mhdp: dp-bridge@a000000 {
+ compatible = "ti,j721e-mhdp8546";
+
+ reg = <0x0 0xa000000 0x0 0x30a00>,
+ <0x0 0x4f40000 0x0 0x20>;
+ reg-names = "mhdptx", "j721e-intg";
+
+ clocks = <&k3_clks 217 11>;
+
+ interrupt-parent = <&gic500>;
+ interrupts = <GIC_SPI 614 IRQ_TYPE_LEVEL_HIGH>;
+
+ power-domains = <&k3_pds 217 TI_SCI_PD_EXCLUSIVE>;
+
+ status = "disabled";
+
+ dp0_ports: ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+ };
+ };
+
+ dss: dss@4a00000 {
+ compatible = "ti,j721e-dss";
+ reg =
+ <0x00 0x04a00000 0x00 0x10000>,
+ <0x00 0x04a10000 0x00 0x10000>,
+ <0x00 0x04b00000 0x00 0x10000>,
+ <0x00 0x04b10000 0x00 0x10000>,
+
+ <0x00 0x04a20000 0x00 0x10000>,
+ <0x00 0x04a30000 0x00 0x10000>,
+ <0x00 0x04a50000 0x00 0x10000>,
+ <0x00 0x04a60000 0x00 0x10000>,
+
+ <0x00 0x04a70000 0x00 0x10000>,
+ <0x00 0x04a90000 0x00 0x10000>,
+ <0x00 0x04ab0000 0x00 0x10000>,
+ <0x00 0x04ad0000 0x00 0x10000>,
+
+ <0x00 0x04a80000 0x00 0x10000>,
+ <0x00 0x04aa0000 0x00 0x10000>,
+ <0x00 0x04ac0000 0x00 0x10000>,
+ <0x00 0x04ae0000 0x00 0x10000>,
+ <0x00 0x04af0000 0x00 0x10000>;
+
+ reg-names = "common_m", "common_s0",
+ "common_s1", "common_s2",
+ "vidl1", "vidl2","vid1","vid2",
+ "ovr1", "ovr2", "ovr3", "ovr4",
+ "vp1", "vp2", "vp3", "vp4",
+ "wb";
+
+ clocks = <&k3_clks 218 0>,
+ <&k3_clks 218 2>,
+ <&k3_clks 218 5>,
+ <&k3_clks 218 14>,
+ <&k3_clks 218 18>;
+ clock-names = "fck", "vp1", "vp2", "vp3", "vp4";
+
+ power-domains = <&k3_pds 218 TI_SCI_PD_EXCLUSIVE>;
+
+ interrupts = <GIC_SPI 602 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 603 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 604 IRQ_TYPE_LEVEL_HIGH>,
+ <GIC_SPI 605 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "common_m",
+ "common_s0",
+ "common_s1",
+ "common_s2";
+
+ status = "disabled";
+
+ dss_ports: ports {
+ };
+ };
};
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH v3 2/5] arm64: dts: ti: k3-j784s4: Add Main CPSW2G node
From: Jayesh Choudhary @ 2023-04-19 6:17 UTC (permalink / raw)
To: nm, vigneshr, afd
Cc: s-vadapalli, kristo, robh+dt, krzysztof.kozlowski+dt,
linux-arm-kernel, devicetree, linux-kernel, a-bhatia1,
j-choudhary
In-Reply-To: <20230419061710.290068-1-j-choudhary@ti.com>
From: Siddharth Vadapalli <s-vadapalli@ti.com>
J784S4 SoC has a Main CPSW2G instance of the CPSW Ethernet Switch.
Add the device-tree nodes for the Main CPSW2G instance and enable it.
Signed-off-by: Siddharth Vadapalli <s-vadapalli@ti.com>
Signed-off-by: Jayesh Choudhary <j-choudhary@ti.com>
---
arch/arm64/boot/dts/ti/k3-j784s4-evm.dts | 48 +++++++++++++++
arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi | 68 ++++++++++++++++++++++
2 files changed, 116 insertions(+)
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
index f33815953e77..aef6f53ae8ac 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-evm.dts
@@ -105,6 +105,30 @@ vdd_sd_dv: regulator-TLV71033 {
};
&main_pmx0 {
+ main_cpsw2g_pins_default: main-cpsw2g-pins-default {
+ pinctrl-single,pins = <
+ J784S4_IOPAD(0x0b8, PIN_INPUT, 6) /* (AC34) MCASP1_ACLKX.RGMII1_RD0 */
+ J784S4_IOPAD(0x0a0, PIN_INPUT, 6) /* (AD34) MCASP0_AXR12.RGMII1_RD1 */
+ J784S4_IOPAD(0x0a4, PIN_INPUT, 6) /* (AJ36) MCASP0_AXR13.RGMII1_RD2 */
+ J784S4_IOPAD(0x0a8, PIN_INPUT, 6) /* (AF34) MCASP0_AXR14.RGMII1_RD3 */
+ J784S4_IOPAD(0x0b0, PIN_INPUT, 6) /* (AL33) MCASP1_AXR3.RGMII1_RXC */
+ J784S4_IOPAD(0x0ac, PIN_INPUT, 6) /* (AE34) MCASP0_AXR15.RGMII1_RX_CTL */
+ J784S4_IOPAD(0x08c, PIN_INPUT, 6) /* (AE35) MCASP0_AXR7.RGMII1_TD0 */
+ J784S4_IOPAD(0x090, PIN_INPUT, 6) /* (AC35) MCASP0_AXR8.RGMII1_TD1 */
+ J784S4_IOPAD(0x094, PIN_INPUT, 6) /* (AG35) MCASP0_AXR9.RGMII1_TD2 */
+ J784S4_IOPAD(0x098, PIN_INPUT, 6) /* (AH36) MCASP0_AXR10.RGMII1_TD3 */
+ J784S4_IOPAD(0x0b4, PIN_INPUT, 6) /* (AL34) MCASP1_AXR4.RGMII1_TXC */
+ J784S4_IOPAD(0x09c, PIN_INPUT, 6) /* (AF35) MCASP0_AXR11.RGMII1_TX_CTL */
+ >;
+ };
+
+ main_cpsw2g_mdio_pins_default: main-cpsw2g-mdio-pins-default {
+ pinctrl-single,pins = <
+ J784S4_IOPAD(0x0c0, PIN_INPUT, 6) /* (AD38) MCASP1_AXR0.MDIO0_MDC */
+ J784S4_IOPAD(0x0bc, PIN_INPUT, 6) /* (AD33) MCASP1_AFSX.MDIO0_MDIO */
+ >;
+ };
+
main_uart8_pins_default: main-uart8-pins-default {
pinctrl-single,pins = <
J784S4_IOPAD(0x040, PIN_INPUT, 14) /* (AF37) MCASP0_AXR0.UART8_CTSn */
@@ -253,3 +277,27 @@ &mcu_cpsw_port1 {
phy-mode = "rgmii-rxid";
phy-handle = <&mcu_phy0>;
};
+
+&main_cpsw1 {
+ status = "okay";
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_cpsw2g_pins_default>;
+};
+
+&main_cpsw1_mdio {
+ pinctrl-names = "default";
+ pinctrl-0 = <&main_cpsw2g_mdio_pins_default>;
+
+ main_phy0: ethernet-phy@0 {
+ reg = <0>;
+ ti,rx-internal-delay = <DP83867_RGMIIDCTL_2_00_NS>;
+ ti,fifo-depth = <DP83867_PHYCR_FIFO_DEPTH_4_B_NIB>;
+ ti,min-output-impedance;
+ };
+};
+
+&main_cpsw1_port1 {
+ status = "okay";
+ phy-mode = "rgmii-rxid";
+ phy-handle = <&main_phy0>;
+};
diff --git a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
index 5fb7edf4f5a0..8bd8aebebe1c 100644
--- a/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
+++ b/arch/arm64/boot/dts/ti/k3-j784s4-main.dtsi
@@ -36,6 +36,12 @@ scm_conf: syscon@100000 {
#size-cells = <1>;
ranges = <0x00 0x00 0x00100000 0x1c000>;
+ cpsw1_phy_gmii_sel: phy@4034 {
+ compatible = "ti,am654-phy-gmii-sel";
+ reg = <0x4034 0x4>;
+ #phy-cells = <1>;
+ };
+
serdes_ln_ctrl: mux-controller-4080 {
compatible = "mmio-mux";
#mux-control-cells = <1>;
@@ -777,6 +783,68 @@ cpts@310d0000 {
};
};
+ main_cpsw1: ethernet@c200000 {
+ compatible = "ti,j721e-cpsw-nuss";
+ #address-cells = <2>;
+ #size-cells = <2>;
+ reg = <0x00 0xc200000 0x00 0x200000>;
+ reg-names = "cpsw_nuss";
+ ranges = <0x00 0x00 0x00 0xc200000 0x00 0x200000>;
+ dma-coherent;
+ clocks = <&k3_clks 62 0>;
+ clock-names = "fck";
+ power-domains = <&k3_pds 62 TI_SCI_PD_EXCLUSIVE>;
+
+ dmas = <&main_udmap 0xc640>,
+ <&main_udmap 0xc641>,
+ <&main_udmap 0xc642>,
+ <&main_udmap 0xc643>,
+ <&main_udmap 0xc644>,
+ <&main_udmap 0xc645>,
+ <&main_udmap 0xc646>,
+ <&main_udmap 0xc647>,
+ <&main_udmap 0x4640>;
+ dma-names = "tx0", "tx1", "tx2", "tx3",
+ "tx4", "tx5", "tx6", "tx7",
+ "rx";
+
+ status = "disabled";
+
+ ethernet-ports {
+ #address-cells = <1>;
+ #size-cells = <0>;
+
+ main_cpsw1_port1: port@1 {
+ reg = <1>;
+ label = "port1";
+ phys = <&cpsw1_phy_gmii_sel 1>;
+ ti,mac-only;
+ status = "disabled";
+ };
+ };
+
+ main_cpsw1_mdio: mdio@f00 {
+ compatible = "ti,cpsw-mdio", "ti,davinci_mdio";
+ reg = <0x00 0xf00 0x00 0x100>;
+ #address-cells = <1>;
+ #size-cells = <0>;
+ clocks = <&k3_clks 62 0>;
+ clock-names = "fck";
+ bus_freq = <1000000>;
+ };
+
+ cpts@3d000 {
+ compatible = "ti,am65-cpts";
+ reg = <0x00 0x3d000 0x00 0x400>;
+ clocks = <&k3_clks 62 3>;
+ clock-names = "cpts";
+ interrupts-extended = <&gic500 GIC_SPI 21 IRQ_TYPE_LEVEL_HIGH>;
+ interrupt-names = "cpts";
+ ti,cpts-ext-ts-inputs = <4>;
+ ti,cpts-periodic-outputs = <2>;
+ };
+ };
+
main_mcan0: can@2701000 {
compatible = "bosch,m_can";
reg = <0x00 0x02701000 0x00 0x200>,
--
2.25.1
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* Re: [PATCH] drm/mediatek: Clarify/finish documentation for some driver structures
From: AngeloGioacchino Del Regno @ 2023-04-19 6:16 UTC (permalink / raw)
To: chunkuang.hu
Cc: lee, p.zabel, airlied, daniel, matthias.bgg, dri-devel,
linux-mediatek, linux-kernel, linux-arm-kernel, kernel
In-Reply-To: <20230321111448.270110-1-angelogioacchino.delregno@collabora.com>
Il 21/03/23 12:14, AngeloGioacchino Del Regno ha scritto:
> The documentation for some of the driver structures in mediatek-drm
> was set to be kerneldoc but some code additions didn't actually update
> the comments accordingly and this caused triggering some warnings.
>
> Add comments for the remaining undocumented entries; while at it, also
> clarify some acronyms for various display HW blocks and fix some comment
> blocks to actually get parsed as kerneldoc.
>
> Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
Hello CK,
I just noticed that this patch is getting stale and forgotten.
Please take a look at it, as this solves kerneldoc warnings during kernel build.
Regards,
Angelo
> --- > drivers/gpu/drm/mediatek/mtk_disp_aal.c | 8 +++++---
> drivers/gpu/drm/mediatek/mtk_disp_ccorr.c | 8 +++++---
> drivers/gpu/drm/mediatek/mtk_disp_color.c | 11 +++++++----
> drivers/gpu/drm/mediatek/mtk_disp_gamma.c | 8 ++++++--
> drivers/gpu/drm/mediatek/mtk_disp_ovl.c | 13 +++++++++----
> drivers/gpu/drm/mediatek/mtk_disp_rdma.c | 12 +++++++++---
> 6 files changed, 41 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_aal.c b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> index 434e8a9ce8ab..12d1800c1d34 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_aal.c
> @@ -27,9 +27,11 @@ struct mtk_disp_aal_data {
> };
>
> /**
> - * struct mtk_disp_aal - DISP_AAL driver structure
> - * @ddp_comp - structure containing type enum and hardware resources
> - * @crtc - associated crtc to report irq events to
> + * struct mtk_disp_aal - Display Adaptive Ambient Light driver structure
> + * @clk: clock for DISP_AAL controller
> + * @regs: MMIO registers base
> + * @cmdq_reg: CMDQ Client register
> + * @data: platform specific data for DISP_AAL
> */
> struct mtk_disp_aal {
> struct clk *clk;
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c b/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> index 1773379b2439..b173aa058573 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ccorr.c
> @@ -35,9 +35,11 @@ struct mtk_disp_ccorr_data {
> };
>
> /**
> - * struct mtk_disp_ccorr - DISP_CCORR driver structure
> - * @ddp_comp - structure containing type enum and hardware resources
> - * @crtc - associated crtc to report irq events to
> + * struct mtk_disp_ccorr - Display Color Correction driver structure
> + * @clk: clock for DISP_CCORR block
> + * @regs: MMIO registers base
> + * @cmdq_reg: CMDQ Client register
> + * @data: platform specific data for DISP_CCORR
> */
> struct mtk_disp_ccorr {
> struct clk *clk;
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_color.c b/drivers/gpu/drm/mediatek/mtk_disp_color.c
> index cac9206079e7..7884f4736b7c 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_color.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_color.c
> @@ -31,10 +31,13 @@ struct mtk_disp_color_data {
> unsigned int color_offset;
> };
>
> -/*
> - * struct mtk_disp_color - DISP_COLOR driver structure
> - * @crtc: associated crtc to report irq events to
> - * @data: platform colour driver data
> +/**
> + * struct mtk_disp_color - DISP_COLOR (Display Color) driver structure
> + * @crtc: associated crtc to report irq events to
> + * @clk: clock for DISP_COLOR block
> + * @regs: MMIO registers base
> + * @cmdq_reg: CMDQ Client register
> + * @data: platform specific data for DISP_COLOR
> */
> struct mtk_disp_color {
> struct drm_crtc *crtc;
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> index c844942603f7..7e748613fccb 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_gamma.c
> @@ -31,8 +31,12 @@ struct mtk_disp_gamma_data {
> bool lut_diff;
> };
>
> -/*
> - * struct mtk_disp_gamma - DISP_GAMMA driver structure
> +/**
> + * struct mtk_disp_gamma - Display Gamma driver structure
> + * @clk: clock for DISP_GAMMA block
> + * @regs: MMIO registers base
> + * @cmdq_reg: CMDQ Client register
> + * @data: platform data for DISP_GAMMA
> */
> struct mtk_disp_gamma {
> struct clk *clk;
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> index 9d8c986700ee..00f2871fd1a4 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_ovl.c
> @@ -76,10 +76,15 @@ struct mtk_disp_ovl_data {
> bool supports_afbc;
> };
>
> -/*
> - * struct mtk_disp_ovl - DISP_OVL driver structure
> - * @crtc: associated crtc to report vblank events to
> - * @data: platform data
> +/**
> + * struct mtk_disp_ovl - Display Overlay driver structure
> + * @crtc: associated crtc to report vblank events to
> + * @clk: clock for DISP_OVL block
> + * @regs: MMIO registers base
> + * @cmdq_reg: CMDQ Client register
> + * @data: platform data
> + * @vblank_cb: vblank callback function
> + * @vblank_cb_data: pointer to data that will be passed to vblank_cb()
> */
> struct mtk_disp_ovl {
> struct drm_crtc *crtc;
> diff --git a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> index e8e337903b0d..74f4a0bce5cc 100644
> --- a/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> +++ b/drivers/gpu/drm/mediatek/mtk_disp_rdma.c
> @@ -59,9 +59,15 @@ struct mtk_disp_rdma_data {
> unsigned int fifo_size;
> };
>
> -/*
> - * struct mtk_disp_rdma - DISP_RDMA driver structure
> - * @data: local driver data
> +/**
> + * struct mtk_disp_rdma - Display Read DMA driver structure
> + * @clk: clock for DISP_RDMA block
> + * @regs: MMIO registers base
> + * @cmdq_reg: CMDQ Client register
> + * @data: platform data
> + * @vblank_cb: vblank callback function
> + * @vblank_cb_data: pointer to data that will be passed to vblank_cb()
> + * @fifo_size: size of DISP_RDMA block's FIFO
> */
> struct mtk_disp_rdma {
> struct clk *clk;
--
AngeloGioacchino Del Regno
Software Engineer
Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* [PATCH 2/2] thermal/drivers/mediatek: Add temperature constraints to validate read
From: AngeloGioacchino Del Regno @ 2023-04-19 6:11 UTC (permalink / raw)
To: rafael
Cc: daniel.lezcano, amitk, rui.zhang, matthias.bgg,
angelogioacchino.delregno, aouledameur, bchihi, daniel,
ye.xingchen, hsinyi, michael.kao, linux-pm, linux-kernel,
linux-arm-kernel, linux-mediatek
In-Reply-To: <20230419061146.22246-1-angelogioacchino.delregno@collabora.com>
The AUXADC thermal v1 allows reading temperature range between -20°C to
150°C and any value out of this range is invalid.
Add new definitions for MT8173_TEMP_{MIN_MAX} and a new small helper
mtk_thermal_temp_is_valid() to check if new readings are in range: if
not, we tell to the API that the reading is invalid by returning
THERMAL_TEMP_INVALID.
It was chosen to introduce the helper function because, even though this
temperature range is realistically ok for all, it comes from a downstream
kernel driver for version 1, but here we also support v2 and v3 which may
may have wider constraints.
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/thermal/mediatek/auxadc_thermal.c | 24 +++++++++++++++++------
1 file changed, 18 insertions(+), 6 deletions(-)
diff --git a/drivers/thermal/mediatek/auxadc_thermal.c b/drivers/thermal/mediatek/auxadc_thermal.c
index 3c959a827451..e908c8e9d558 100644
--- a/drivers/thermal/mediatek/auxadc_thermal.c
+++ b/drivers/thermal/mediatek/auxadc_thermal.c
@@ -116,6 +116,10 @@
/* The calibration coefficient of sensor */
#define MT8173_CALIBRATION 165
+/* Valid temperatures range */
+#define MT8173_TEMP_MIN -20000
+#define MT8173_TEMP_MAX 150000
+
/*
* Layout of the fuses providing the calibration data
* These macros could be used for MT8183, MT8173, MT2701, and MT2712.
@@ -689,6 +693,11 @@ static const struct mtk_thermal_data mt7986_thermal_data = {
.version = MTK_THERMAL_V3,
};
+static bool mtk_thermal_temp_is_valid(int temp)
+{
+ return (temp >= MT8173_TEMP_MIN) && (temp <= MT8173_TEMP_MAX);
+}
+
/**
* raw_to_mcelsius_v1 - convert a raw ADC value to mcelsius
* @mt: The thermal controller
@@ -815,14 +824,17 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
temp = mt->raw_to_mcelsius(
mt, conf->bank_data[bank->id].sensors[i], raw);
-
/*
- * The first read of a sensor often contains very high bogus
- * temperature value. Filter these out so that the system does
- * not immediately shut down.
+ * Depending on the filt/sen intervals and ADC polling time,
+ * we may need up to 60 milliseconds after initialization: this
+ * will result in the first reading containing an out of range
+ * temperature value.
+ * Validate the reading to both address the aforementioned issue
+ * and to eventually avoid bogus readings during runtime in the
+ * event that the AUXADC gets unstable due to high EMI, etc.
*/
- if (temp > 200000)
- temp = 0;
+ if (!mtk_thermal_temp_is_valid(temp))
+ temp = THERMAL_TEMP_INVALID;
if (temp > max)
max = temp;
--
2.40.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 1/2] Revert "thermal/drivers/mediatek: Add delay after thermal banks initialization"
From: AngeloGioacchino Del Regno @ 2023-04-19 6:11 UTC (permalink / raw)
To: rafael
Cc: daniel.lezcano, amitk, rui.zhang, matthias.bgg,
angelogioacchino.delregno, aouledameur, bchihi, daniel,
ye.xingchen, hsinyi, michael.kao, linux-pm, linux-kernel,
linux-arm-kernel, linux-mediatek, kernelci.org bot
In-Reply-To: <20230419061146.22246-1-angelogioacchino.delregno@collabora.com>
Some more testing revealed that this commit introduces a regression on some
MT8173 Chromebooks and at least on one MT6795 Sony Xperia M5 smartphone due
to the delay being apparently variable and machine specific.
Another solution would be to delay for a bit more (~70ms) but this is not
feasible for two reasons: first of all, we're adding an even bigger delay
in a probe function; second, some machines need less, some may need even
more, making the msleep at probe solution highly suboptimal.
This reverts commit 10debf8c2da8011c8009dd4b3f6d0ab85891c81b.
Fixes: 10debf8c2da8 ("thermal/drivers/mediatek: Add delay after thermal banks initialization")
Reported-by: "kernelci.org bot" <bot@kernelci.org>
Signed-off-by: AngeloGioacchino Del Regno <angelogioacchino.delregno@collabora.com>
---
drivers/thermal/mediatek/auxadc_thermal.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/drivers/thermal/mediatek/auxadc_thermal.c b/drivers/thermal/mediatek/auxadc_thermal.c
index b6bb9eaafb74..3c959a827451 100644
--- a/drivers/thermal/mediatek/auxadc_thermal.c
+++ b/drivers/thermal/mediatek/auxadc_thermal.c
@@ -816,6 +816,14 @@ static int mtk_thermal_bank_temperature(struct mtk_thermal_bank *bank)
mt, conf->bank_data[bank->id].sensors[i], raw);
+ /*
+ * The first read of a sensor often contains very high bogus
+ * temperature value. Filter these out so that the system does
+ * not immediately shut down.
+ */
+ if (temp > 200000)
+ temp = 0;
+
if (temp > max)
max = temp;
}
@@ -1273,9 +1281,6 @@ static int mtk_thermal_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, mt);
- /* Delay for thermal banks to be ready */
- msleep(30);
-
tzdev = devm_thermal_of_zone_register(&pdev->dev, 0, mt,
&mtk_thermal_ops);
if (IS_ERR(tzdev)) {
--
2.40.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related
* [PATCH 0/2] MediaTek AUXADC thermal: urgent fixes
From: AngeloGioacchino Del Regno @ 2023-04-19 6:11 UTC (permalink / raw)
To: rafael
Cc: daniel.lezcano, amitk, rui.zhang, matthias.bgg,
angelogioacchino.delregno, aouledameur, bchihi, daniel,
ye.xingchen, hsinyi, michael.kao, linux-pm, linux-kernel,
linux-arm-kernel, linux-mediatek
The AUXADC thermal driver unfortunately has issues with a fixed wait
at probe, as this is not only SoC dependent, but actually depends on
the board (and even aging...): for example, that works fine on the
Chromebook that I have here in my hands but not for the ones in our lab.
Some machines are working fine with that 30ms delay at probe, but some
others are not, hence I started digging in downstream sources here and
there, and found that there actually is a valid temperature range for
at least auxadc-thermal *v1* and can be actually found in multiple
downstream kernels for MT8173 and MT6795.
As for v2 and v3 thermal IP, I'm sure that the v1 range works fine but
I've "left room" for adding specific ranges for them later: this fix
is urgent, as many MT8173 and MT8183 Chromebooks are failing tests in
KernelCI due to thermal shutdown during boot.
For the KernelCI logs, you can look at [1] for 8173, [2] for 8183.
[1]: https://storage.kernelci.org/next/master/next-20230405/arm64/defconfig+arm64-chromebook/gcc-10/lab-collabora/igt-kms-mediatek-mt8173-elm-hana.html
[2]: https://storage.kernelci.org/next/master/next-20230405/arm64/defconfig+arm64-chromebook/gcc-10/lab-collabora/cros-ec-mt8183-kukui-jacuzzi-juniper-sku16.html
AngeloGioacchino Del Regno (2):
Revert "thermal/drivers/mediatek: Add delay after thermal banks
initialization"
thermal/drivers/mediatek: Add temperature constraints to validate read
drivers/thermal/mediatek/auxadc_thermal.c | 23 ++++++++++++++++++++---
1 file changed, 20 insertions(+), 3 deletions(-)
--
2.40.0
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] soc: ti: pruss: Avoid cast to incompatible function type
From: Simon Horman @ 2023-04-19 5:45 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Nishanth Menon, Santosh Shilimkar, Nathan Chancellor, Tom Rix,
linux-kernel, linux-arm-kernel, llvm
In-Reply-To: <CAKwvOdnj7rNeh0YeX2NPcbkoJBnRwXb9Dt0SF9mHaMKYVLjCjw@mail.gmail.com>
On Tue, Apr 18, 2023 at 11:44:28AM -0700, Nick Desaulniers wrote:
> On Tue, Apr 18, 2023 at 4:41 AM Simon Horman <horms@kernel.org> wrote:
> >
> > Rather than casting clk_unregister_mux to an incompatible function
> > type provide a trivial wrapper with the correct signature for the
> > use-case.
> >
> > Reported by clang-16 with W=1:
> >
> > drivers/soc/ti/pruss.c:158:38: error: cast from 'void (*)(struct clk *)' to 'void (*)(void *)' converts to incompatible function type [-Werror,-Wcast-function-type-strict]
> > ret = devm_add_action_or_reset(dev, (void(*)(void *))clk_unregister_mux,
> >
> > No functional change intended.
> > Compile tested only.
>
> Thanks for the patch!
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> Here's some more suspects to look at, if you have cycles:
> drivers/base/devres.c:734:int __devm_add_action(struct device *dev,
> void (*action)(void *), void *data, const char *name)
> drivers/i2c/busses/i2c-mchp-pci1xxxx.c:1159: ret =
> devm_add_action(dev, (void (*)(void *))pci1xxxx_i2c_shutdown, i2c);
> drivers/soc/ti/pruss.c:96: ret = devm_add_action_or_reset(dev,
> (void(*)(void *))clk_unregister_mux,
> drivers/mmc/host/meson-mx-sdhc-mmc.c:791: ret =
> devm_add_action_or_reset(dev, (void(*)(void *))mmc_free_host,
> drivers/pci/controller/pcie-microchip-host.c:866:
> devm_add_action_or_reset(dev, (void (*) (void
> *))clk_disable_unprepare,
Thanks, I will take a look as a background task.
Let me know if there is any urgency on your side.
...
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
From: Simon Horman @ 2023-04-19 5:42 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Florian Fainelli, Greg Kroah-Hartman,
Broadcom internal kernel review list, Nick Desaulniers, Tom Rix,
linux-rpi-kernel, linux-arm-kernel, linux-staging, llvm
In-Reply-To: <20230418184419.GB2635379@dev-arch.thelio-3990X>
On Tue, Apr 18, 2023 at 11:44:19AM -0700, Nathan Chancellor wrote:
> On Tue, Apr 18, 2023 at 01:23:11PM +0200, Simon Horman wrote:
> > vchiq_platform_init() is only used in this file so it can be static.
> >
> > clang-16 with W=1 reports:
> >
> > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
> > int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > ^
> > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> >
> > Signed-off-by: Simon Horman <horms@kernel.org>
>
> Introduced by commit 89cc4218f640 ("staging: vchiq_arm: drop unnecessary
> declarations"), not sure it is worth a fixes tag since it is a W=1
> warning.
Thanks. For some reason it didn't occur to me to check which
patch introduced the issue. Perhaps because, as you say, it's a W=1
issue, and I don't think that warrants a fixes tag.
I could include some information in the commit message, like
"Introduced by ..." if desired.
> Reviewed-by: Nathan Chancellor <nathan@kernel.org>
>
> > ---
> > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > index 40dd62cf7399..59312b9f2058 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -462,7 +462,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
> > cleanup_pagelistinfo(instance, pagelistinfo);
> > }
> >
> > -int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > +static int
> > +vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > {
> > struct device *dev = &pdev->dev;
> > struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
> >
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH] staging: vchiq_arm: Make vchiq_platform_init() static
From: Simon Horman @ 2023-04-19 5:39 UTC (permalink / raw)
To: Nick Desaulniers
Cc: Florian Fainelli, Greg Kroah-Hartman,
Broadcom internal kernel review list, Nathan Chancellor, Tom Rix,
linux-rpi-kernel, linux-arm-kernel, linux-staging, llvm
In-Reply-To: <CAKwvOdmuwuSn+4csSCwsNXM01ohsp+ZzcWQ1WfM_kgj_NRugZA@mail.gmail.com>
On Tue, Apr 18, 2023 at 11:39:58AM -0700, Nick Desaulniers wrote:
> On Tue, Apr 18, 2023 at 4:23 AM Simon Horman <horms@kernel.org> wrote:
> >
> > vchiq_platform_init() is only used in this file so it can be static.
> >
> > clang-16 with W=1 reports:
> >
> > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:5: error: no previous prototype for function 'vchiq_platform_init' [-Werror,-Wmissing-prototypes]
> > int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > ^
> > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c:465:1: note: declare 'static' if the function is not intended to be used outside of this translation unit
> >
> > Signed-off-by: Simon Horman <horms@kernel.org>
>
> I would have kept the return type on the original line and just added
> the explicit linkage on top, but it's fine and checkpatch doesn't
> complain. Thanks for the patch!
Thanks, I wasn't sure which was the best way to to on that.
I'll keep this in mind in future.
> Reviewed-by: Nick Desaulniers <ndesaulniers@google.com>
>
> > ---
> > drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > index 40dd62cf7399..59312b9f2058 100644
> > --- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > +++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_arm.c
> > @@ -462,7 +462,8 @@ free_pagelist(struct vchiq_instance *instance, struct vchiq_pagelist_info *pagel
> > cleanup_pagelistinfo(instance, pagelistinfo);
> > }
> >
> > -int vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > +static int
> > +vchiq_platform_init(struct platform_device *pdev, struct vchiq_state *state)
> > {
> > struct device *dev = &pdev->dev;
> > struct vchiq_drvdata *drvdata = platform_get_drvdata(pdev);
> >
>
>
> --
> Thanks,
> ~Nick Desaulniers
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
* Re: [PATCH v6 6/6] KVM: arm64: Refactor writings for PMUVer/CSV2/CSV3
From: Reiji Watanabe @ 2023-04-19 4:59 UTC (permalink / raw)
To: Jing Zhang
Cc: KVM, KVMARM, ARMLinux, Marc Zyngier, Oliver Upton, Will Deacon,
Paolo Bonzini, James Morse, Alexandru Elisei, Suzuki K Poulose,
Fuad Tabba, Raghavendra Rao Ananta
In-Reply-To: <20230404035344.4043856-7-jingzhangos@google.com>
Hi Jing,
On Tue, Apr 04, 2023 at 03:53:44AM +0000, Jing Zhang wrote:
> Refactor writings for ID_AA64PFR0_EL1.[CSV2|CSV3],
> ID_AA64DFR0_EL1.PMUVer and ID_DFR0_ELF.PerfMon based on utilities
> introduced by ID register descriptor array.
>
> Signed-off-by: Jing Zhang <jingzhangos@google.com>
> ---
> arch/arm64/include/asm/cpufeature.h | 1 +
> arch/arm64/kernel/cpufeature.c | 2 +-
> arch/arm64/kvm/id_regs.c | 284 ++++++++++++++++++++--------
> 3 files changed, 203 insertions(+), 84 deletions(-)
>
> diff --git a/arch/arm64/include/asm/cpufeature.h b/arch/arm64/include/asm/cpufeature.h
> index 6bf013fb110d..dc769c2eb7a4 100644
> --- a/arch/arm64/include/asm/cpufeature.h
> +++ b/arch/arm64/include/asm/cpufeature.h
> @@ -915,6 +915,7 @@ static inline unsigned int get_vmid_bits(u64 mmfr1)
> return 8;
> }
>
> +s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp, s64 new, s64 cur);
> struct arm64_ftr_reg *get_arm64_ftr_reg(u32 sys_id);
>
> extern struct arm64_ftr_override id_aa64mmfr1_override;
> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
> index 2e3e55139777..677ec4fe9f6b 100644
> --- a/arch/arm64/kernel/cpufeature.c
> +++ b/arch/arm64/kernel/cpufeature.c
> @@ -791,7 +791,7 @@ static u64 arm64_ftr_set_value(const struct arm64_ftr_bits *ftrp, s64 reg,
> return reg;
> }
>
> -static s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp, s64 new,
> +s64 arm64_ftr_safe_value(const struct arm64_ftr_bits *ftrp, s64 new,
> s64 cur)
> {
> s64 ret = 0;
> diff --git a/arch/arm64/kvm/id_regs.c b/arch/arm64/kvm/id_regs.c
> index fe37b6786b4c..33968ada29bb 100644
> --- a/arch/arm64/kvm/id_regs.c
> +++ b/arch/arm64/kvm/id_regs.c
> @@ -18,6 +18,66 @@
>
> #include "sys_regs.h"
>
> +/**
> + * arm64_check_features() - Check if a feature register value constitutes
> + * a subset of features indicated by the idreg's KVM sanitised limit.
> + *
> + * This function will check if each feature field of @val is the "safe" value
> + * against idreg's KVM sanitised limit return from reset() callback.
> + * If a field value in @val is the same as the one in limit, it is always
> + * considered the safe value regardless For register fields that are not in
> + * writable, only the value in limit is considered the safe value.
> + *
> + * Return: 0 if all the fields are safe. Otherwise, return negative errno.
> + */
> +static int arm64_check_features(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd,
> + u64 val)
> +{
> + const struct arm64_ftr_reg *ftr_reg;
> + const struct arm64_ftr_bits *ftrp = NULL;
> + u32 id = reg_to_encoding(rd);
> + u64 writable_mask = rd->val;
> + u64 limit = 0;
> + u64 mask = 0;
> +
> + /* For hidden and unallocated idregs without reset, only val = 0 is allowed. */
> + if (rd->reset) {
> + limit = rd->reset(vcpu, rd);
> + ftr_reg = get_arm64_ftr_reg(id);
> + if (!ftr_reg)
> + return -EINVAL;
> + ftrp = ftr_reg->ftr_bits;
> + }
> +
> + for (; ftrp && ftrp->width; ftrp++) {
> + s64 f_val, f_lim, safe_val;
> + u64 ftr_mask;
> +
> + ftr_mask = arm64_ftr_mask(ftrp);
> + if ((ftr_mask & writable_mask) != ftr_mask)
> + continue;
> +
> + f_val = arm64_ftr_value(ftrp, val);
> + f_lim = arm64_ftr_value(ftrp, limit);
> + mask |= ftr_mask;
> +
> + if (f_val == f_lim)
> + safe_val = f_val;
> + else
> + safe_val = arm64_ftr_safe_value(ftrp, f_val, f_lim);
Since PMUVer and PerfMon is defined as FTR_EXACT, I believe having lower
value in those two fields than the limit always ends up getting -E2BIG.
Or am I missing something ??
FYI. IIRC, we have some more fields in other ID registers that KVM
shouldn't use as is.
> +
> + if (safe_val != f_val)
> + return -E2BIG;
> + }
> +
> + /* For fields that are not writable, values in limit are the safe values. */
> + if ((val & ~mask) != (limit & ~mask))
> + return -E2BIG;
> +
> + return 0;
> +}
> +
> static u8 vcpu_pmuver(const struct kvm_vcpu *vcpu)
> {
> if (kvm_vcpu_has_pmu(vcpu))
> @@ -68,7 +128,6 @@ u64 kvm_arm_read_id_reg(const struct kvm_vcpu *vcpu, u32 id)
> case SYS_ID_AA64PFR0_EL1:
> if (!vcpu_has_sve(vcpu))
> val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_SVE);
> - val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_AMU);
> if (kvm_vgic_global_state.type == VGIC_V3) {
> val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_GIC);
> val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_GIC), 1);
> @@ -95,15 +154,10 @@ u64 kvm_arm_read_id_reg(const struct kvm_vcpu *vcpu, u32 id)
> val &= ~ARM64_FEATURE_MASK(ID_AA64ISAR2_EL1_WFxT);
> break;
> case SYS_ID_AA64DFR0_EL1:
> - /* Limit debug to ARMv8.0 */
> - val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer);
> - val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer), 6);
> /* Set PMUver to the required version */
> val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer);
> val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer),
> vcpu_pmuver(vcpu));
> - /* Hide SPE from guests */
> - val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMSVer);
> break;
> case SYS_ID_DFR0_EL1:
> val &= ~ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon);
> @@ -162,9 +216,14 @@ static int get_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> static int set_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> u64 val)
> {
> - /* This is what we mean by invariant: you can't change it. */
> - if (val != read_id_reg(vcpu, rd))
> - return -EINVAL;
> + u32 id = reg_to_encoding(rd);
> + int ret;
> +
> + ret = arm64_check_features(vcpu, rd, val);
> + if (ret)
> + return ret;
> +
> + IDREG(vcpu->kvm, id) = val;
>
> return 0;
> }
> @@ -198,12 +257,40 @@ static unsigned int aa32_id_visibility(const struct kvm_vcpu *vcpu,
> return id_visibility(vcpu, r);
> }
>
> +static u64 read_sanitised_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> +{
> + u64 val;
> + u32 id = reg_to_encoding(rd);
> +
> + val = read_sanitised_ftr_reg(id);
> + /*
> + * The default is to expose CSV2 == 1 if the HW isn't affected.
> + * Although this is a per-CPU feature, we make it global because
> + * asymmetric systems are just a nuisance.
> + *
> + * Userspace can override this as long as it doesn't promise
> + * the impossible.
> + */
> + if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED) {
> + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2);
> + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2), 1);
> + }
> + if (arm64_get_meltdown_state() == SPECTRE_UNAFFECTED) {
> + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3);
> + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3), 1);
> + }
> +
> + val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_AMU);
> +
> + return val;
> +}
> +
> static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> const struct sys_reg_desc *rd,
> u64 val)
> {
> u8 csv2, csv3;
> - u64 sval = val;
>
> /*
> * Allow AA64PFR0_EL1.CSV2 to be set from userspace as long as
> @@ -219,16 +306,30 @@ static int set_id_aa64pfr0_el1(struct kvm_vcpu *vcpu,
> if (csv3 > 1 || (csv3 && arm64_get_meltdown_state() != SPECTRE_UNAFFECTED))
> return -EINVAL;
>
> - /* We can only differ with CSV[23], and anything else is an error */
> - val ^= read_id_reg(vcpu, rd);
> - val &= ~(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2) |
> - ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3));
> - if (val)
> - return -EINVAL;
> + return set_id_reg(vcpu, rd, val);
> +}
> +
> +static u64 read_sanitised_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> +{
> + u64 val;
> + u32 id = reg_to_encoding(rd);
>
> - IDREG(vcpu->kvm, reg_to_encoding(rd)) = sval;
> + val = read_sanitised_ftr_reg(id);
> + /* Limit debug to ARMv8.0 */
> + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer);
> + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_DebugVer), 6);
> + /*
> + * Initialise the default PMUver before there is a chance to
> + * create an actual PMU.
> + */
> + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer);
> + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer),
> + kvm_arm_pmu_get_pmuver_limit());
> + /* Hide SPE from guests */
> + val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMSVer);
>
> - return 0;
> + return val;
> }
>
> static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
> @@ -237,6 +338,7 @@ static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
> {
> u8 pmuver, host_pmuver;
> bool valid_pmu;
> + int ret;
>
> host_pmuver = kvm_arm_pmu_get_pmuver_limit();
>
> @@ -256,36 +358,61 @@ static int set_id_aa64dfr0_el1(struct kvm_vcpu *vcpu,
> if (kvm_vcpu_has_pmu(vcpu) != valid_pmu)
> return -EINVAL;
>
> - /* We can only differ with PMUver, and anything else is an error */
> - val ^= read_id_reg(vcpu, rd);
> - val &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer);
> - if (val)
> - return -EINVAL;
> + if (!valid_pmu) {
> + /*
> + * Ignore the PMUVer filed in @val. The PMUVer would be determined
> + * by arch flags bit KVM_ARCH_FLAG_VCPU_HAS_IMP_DEF_PMU,
> + */
> + pmuver = FIELD_GET(ID_AA64DFR0_EL1_PMUVer_MASK, read_id_reg(vcpu, rd));
As vPMU is not configured for this vCPU, I believe pmuver will be
0x0 or 0xf. I think that is not what we want there.
Or am I missing something ?
> + val &= ~ID_AA64DFR0_EL1_PMUVer_MASK;
> + val |= FIELD_PREP(ID_AA64DFR0_EL1_PMUVer_MASK, pmuver);
> + }
>
> - if (valid_pmu) {
> - mutex_lock(&vcpu->kvm->arch.config_lock);
> - IDREG(vcpu->kvm, SYS_ID_AA64DFR0_EL1) &= ~ID_AA64DFR0_EL1_PMUVer_MASK;
> - IDREG(vcpu->kvm, SYS_ID_AA64DFR0_EL1) |= FIELD_PREP(ID_AA64DFR0_EL1_PMUVer_MASK,
> - pmuver);
> + mutex_lock(&vcpu->kvm->arch.config_lock);
>
> - IDREG(vcpu->kvm, SYS_ID_DFR0_EL1) &= ~ID_DFR0_EL1_PerfMon_MASK;
> - IDREG(vcpu->kvm, SYS_ID_DFR0_EL1) |= FIELD_PREP(ID_DFR0_EL1_PerfMon_MASK,
> - pmuver_to_perfmon(pmuver));
> + ret = set_id_reg(vcpu, rd, val);
> + if (ret) {
> mutex_unlock(&vcpu->kvm->arch.config_lock);
> - } else {
> + return ret;
> + }
> +
> + IDREG(vcpu->kvm, SYS_ID_DFR0_EL1) &= ~ID_DFR0_EL1_PerfMon_MASK;
> + IDREG(vcpu->kvm, SYS_ID_DFR0_EL1) |= FIELD_PREP(ID_DFR0_EL1_PerfMon_MASK,
> + pmuver_to_perfmon(pmuver));
> +
> + if (!valid_pmu)
> assign_bit(KVM_ARCH_FLAG_VCPU_HAS_IMP_DEF_PMU, &vcpu->kvm->arch.flags,
> pmuver == ID_AA64DFR0_EL1_PMUVer_IMP_DEF);
> - }
> +
> + mutex_unlock(&vcpu->kvm->arch.config_lock);
>
> return 0;
> }
>
> +static u64 read_sanitised_id_dfr0_el1(struct kvm_vcpu *vcpu,
> + const struct sys_reg_desc *rd)
> +{
> + u64 val;
> + u32 id = reg_to_encoding(rd);
> +
> + val = read_sanitised_ftr_reg(id);
> + /*
> + * Initialise the default PMUver before there is a chance to
> + * create an actual PMU.
> + */
> + val &= ~ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon);
> + val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon), kvm_arm_pmu_get_pmuver_limit());
> +
> + return val;
> +}
> +
> static int set_id_dfr0_el1(struct kvm_vcpu *vcpu,
> const struct sys_reg_desc *rd,
> u64 val)
> {
> u8 perfmon, host_perfmon;
> bool valid_pmu;
> + int ret;
>
> host_perfmon = pmuver_to_perfmon(kvm_arm_pmu_get_pmuver_limit());
>
> @@ -306,25 +433,33 @@ static int set_id_dfr0_el1(struct kvm_vcpu *vcpu,
> if (kvm_vcpu_has_pmu(vcpu) != valid_pmu)
> return -EINVAL;
>
> - /* We can only differ with PerfMon, and anything else is an error */
> - val ^= read_id_reg(vcpu, rd);
> - val &= ~ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon);
> - if (val)
> - return -EINVAL;
> + if (!valid_pmu) {
> + /*
> + * Ignore the PerfMon filed in @val. The PerfMon would be determined
> + * by arch flags bit KVM_ARCH_FLAG_VCPU_HAS_IMP_DEF_PMU,
> + */
I have the same comment as set_id_aa64dfr0_el1().
Thank you,
Reiji
> + perfmon = FIELD_GET(ID_DFR0_EL1_PerfMon_MASK, read_id_reg(vcpu, rd));
> + val &= ~ID_DFR0_EL1_PerfMon_MASK;
> + val |= FIELD_PREP(ID_DFR0_EL1_PerfMon_MASK, perfmon);
> + }
>
> - if (valid_pmu) {
> - mutex_lock(&vcpu->kvm->arch.config_lock);
> - IDREG(vcpu->kvm, SYS_ID_DFR0_EL1) &= ~ID_DFR0_EL1_PerfMon_MASK;
> - IDREG(vcpu->kvm, SYS_ID_DFR0_EL1) |= FIELD_PREP(ID_DFR0_EL1_PerfMon_MASK, perfmon);
> + mutex_lock(&vcpu->kvm->arch.config_lock);
>
> - IDREG(vcpu->kvm, SYS_ID_AA64DFR0_EL1) &= ~ID_AA64DFR0_EL1_PMUVer_MASK;
> - IDREG(vcpu->kvm, SYS_ID_AA64DFR0_EL1) |= FIELD_PREP(ID_AA64DFR0_EL1_PMUVer_MASK,
> - perfmon_to_pmuver(perfmon));
> + ret = set_id_reg(vcpu, rd, val);
> + if (ret) {
> mutex_unlock(&vcpu->kvm->arch.config_lock);
> - } else {
> + return ret;
> + }
> +
> + IDREG(vcpu->kvm, SYS_ID_AA64DFR0_EL1) &= ~ID_AA64DFR0_EL1_PMUVer_MASK;
> + IDREG(vcpu->kvm, SYS_ID_AA64DFR0_EL1) |= FIELD_PREP(ID_AA64DFR0_EL1_PMUVer_MASK,
> + perfmon_to_pmuver(perfmon));
> +
> + if (!valid_pmu)
> assign_bit(KVM_ARCH_FLAG_VCPU_HAS_IMP_DEF_PMU, &vcpu->kvm->arch.flags,
> perfmon == ID_DFR0_EL1_PerfMon_IMPDEF);
> - }
> +
> + mutex_unlock(&vcpu->kvm->arch.config_lock);
>
> return 0;
> }
> @@ -402,9 +537,13 @@ const struct sys_reg_desc id_reg_descs[KVM_ARM_ID_REG_NUM] = {
> /* CRm=1 */
> AA32_ID_SANITISED(ID_PFR0_EL1),
> AA32_ID_SANITISED(ID_PFR1_EL1),
> - { SYS_DESC(SYS_ID_DFR0_EL1), .access = access_id_reg,
> - .get_user = get_id_reg, .set_user = set_id_dfr0_el1,
> - .visibility = aa32_id_visibility, },
> + { SYS_DESC(SYS_ID_DFR0_EL1),
> + .access = access_id_reg,
> + .get_user = get_id_reg,
> + .set_user = set_id_dfr0_el1,
> + .visibility = aa32_id_visibility,
> + .reset = read_sanitised_id_dfr0_el1,
> + .val = ID_DFR0_EL1_PerfMon_MASK, },
> ID_HIDDEN(ID_AFR0_EL1),
> AA32_ID_SANITISED(ID_MMFR0_EL1),
> AA32_ID_SANITISED(ID_MMFR1_EL1),
> @@ -433,8 +572,12 @@ const struct sys_reg_desc id_reg_descs[KVM_ARM_ID_REG_NUM] = {
>
> /* AArch64 ID registers */
> /* CRm=4 */
> - { SYS_DESC(SYS_ID_AA64PFR0_EL1), .access = access_id_reg,
> - .get_user = get_id_reg, .set_user = set_id_aa64pfr0_el1, },
> + { SYS_DESC(SYS_ID_AA64PFR0_EL1),
> + .access = access_id_reg,
> + .get_user = get_id_reg,
> + .set_user = set_id_aa64pfr0_el1,
> + .reset = read_sanitised_id_aa64pfr0_el1,
> + .val = ID_AA64PFR0_EL1_CSV2_MASK | ID_AA64PFR0_EL1_CSV3_MASK, },
> ID_SANITISED(ID_AA64PFR1_EL1),
> ID_UNALLOCATED(4, 2),
> ID_UNALLOCATED(4, 3),
> @@ -444,8 +587,12 @@ const struct sys_reg_desc id_reg_descs[KVM_ARM_ID_REG_NUM] = {
> ID_UNALLOCATED(4, 7),
>
> /* CRm=5 */
> - { SYS_DESC(SYS_ID_AA64DFR0_EL1), .access = access_id_reg,
> - .get_user = get_id_reg, .set_user = set_id_aa64dfr0_el1, },
> + { SYS_DESC(SYS_ID_AA64DFR0_EL1),
> + .access = access_id_reg,
> + .get_user = get_id_reg,
> + .set_user = set_id_aa64dfr0_el1,
> + .reset = read_sanitised_id_aa64dfr0_el1,
> + .val = ID_AA64DFR0_EL1_PMUVer_MASK, },
> ID_SANITISED(ID_AA64DFR1_EL1),
> ID_UNALLOCATED(5, 2),
> ID_UNALLOCATED(5, 3),
> @@ -520,33 +667,4 @@ void kvm_arm_init_id_regs(struct kvm *kvm)
>
> IDREG(kvm, id) = val;
> }
> -
> - /*
> - * The default is to expose CSV2 == 1 if the HW isn't affected.
> - * Although this is a per-CPU feature, we make it global because
> - * asymmetric systems are just a nuisance.
> - *
> - * Userspace can override this as long as it doesn't promise
> - * the impossible.
> - */
> - val = IDREG(kvm, SYS_ID_AA64PFR0_EL1);
> -
> - if (arm64_get_spectre_v2_state() == SPECTRE_UNAFFECTED) {
> - val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2);
> - val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV2), 1);
> - }
> - if (arm64_get_meltdown_state() == SPECTRE_UNAFFECTED) {
> - val &= ~ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3);
> - val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64PFR0_EL1_CSV3), 1);
> - }
> -
> - IDREG(kvm, SYS_ID_AA64PFR0_EL1) = val;
> -
> - /*
> - * Initialise the default PMUver before there is a chance to
> - * create an actual PMU.
> - */
> - IDREG(kvm, SYS_ID_AA64DFR0_EL1) &= ~ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer);
> - IDREG(kvm, SYS_ID_AA64DFR0_EL1) |= FIELD_PREP(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_PMUVer),
> - kvm_arm_pmu_get_pmuver_limit());
> }
> --
> 2.40.0.348.gf938b09366-goog
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox