Re: [PATCH v7 5/5] powerpc/hv-24x7: Update post_mobility_fixup() to handle migration
On 4/29/20 5:07 PM, Michael Ellerman wrote: > Kajol Jain writes: >> Function 'read_sys_info_pseries()' is added to get system parameter >> values like number of sockets and chips per socket. >> and it gets these details via rtas_call with token >> "PROCESSOR_MODULE_INFO". >> >> Incase lpar migrate from one system to another, system >> parameter details like chips per sockets or number of sockets might >> change. So, it needs to be re-initialized otherwise, these values >> corresponds to previous system values. >> This patch adds a call to 'read_sys_info_pseries()' from >> 'post-mobility_fixup()' to re-init the physsockets and physchips values. >> >> Signed-off-by: Kajol Jain >> --- >> arch/powerpc/platforms/pseries/mobility.c | 12 >> 1 file changed, 12 insertions(+) >> >> diff --git a/arch/powerpc/platforms/pseries/mobility.c >> b/arch/powerpc/platforms/pseries/mobility.c >> index b571285f6c14..226accd6218b 100644 >> --- a/arch/powerpc/platforms/pseries/mobility.c >> +++ b/arch/powerpc/platforms/pseries/mobility.c >> @@ -371,6 +371,18 @@ void post_mobility_fixup(void) >> /* Possibly switch to a new RFI flush type */ >> pseries_setup_rfi_flush(); >> >> +/* >> + * Incase lpar migrate from one system to another, system > > In case an LPAR migrates > >> + * parameter details like chips per sockets and number of sockets >> + * might change. So, it needs to be re-initialized otherwise these > ^ ^ > they need the >> + * values corresponds to previous system. > ^ > will correspond to the > >> + * Here, adding a call to read_sys_info_pseries() declared in > > Adding is the wrong tense in a comment. When someone reads the comment > the code has already been added. Past tense would be right, but really > the comment shouldn't say what you did, it should say why. > >> + * platforms/pseries/pseries.h to re-init the physsockets and >> + * physchips value. > > Call read_sys_info_pseries() to reinitialise the values. > >> + */ >> +if (IS_ENABLED(CONFIG_HV_PERF_CTRS) && IS_ENABLED(CONFIG_PPC_RTAS)) >> +read_sys_info_pseries(); > > The RTAS check is not needed. pseries always selects RTAS. > > You shouldn't need the IS_ENABLED() check here though, do it with an > empty version in the header when CONFIG_HV_PERF_CTRS is not enabled. > Hi Michael, Thanks for reviewing the patch. Is something like this you are suggesting. Please let me know if my understanding is fine. +#ifndef CONFIG_HV_PERF_CTRS +#define read_sys_info_pseries() +#endif Thanks, Kajol Jain > cheers >
Re: [PATCH V3 02/16] arm64/cpufeature: Drop TraceFilt feature exposure from ID_DFR0 register
On 05/05/2020 01:54 AM, Will Deacon wrote:
> On Sat, May 02, 2020 at 07:03:51PM +0530, Anshuman Khandual wrote:
>> ID_DFR0 based TraceFilt feature should not be exposed to guests. Hence lets
>> drop it.
>>
>> Cc: Catalin Marinas
>> Cc: Will Deacon
>> Cc: Marc Zyngier
>> Cc: Mark Rutland
>> Cc: James Morse
>> Cc: Suzuki K Poulose
>> Cc: linux-arm-ker...@lists.infradead.org
>> Cc: linux-kernel@vger.kernel.org
>>
>> Suggested-by: Mark Rutland
>> Reviewed-by: Suzuki K Poulose
>> Signed-off-by: Anshuman Khandual
>> ---
>> arch/arm64/kernel/cpufeature.c | 1 -
>> 1 file changed, 1 deletion(-)
>>
>> diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
>> index 6d032fbe416f..51386dade423 100644
>> --- a/arch/arm64/kernel/cpufeature.c
>> +++ b/arch/arm64/kernel/cpufeature.c
>> @@ -435,7 +435,6 @@ static const struct arm64_ftr_bits ftr_id_pfr1[] = {
>> };
>>
>> static const struct arm64_ftr_bits ftr_id_dfr0[] = {
>> -ARM64_FTR_BITS(FTR_HIDDEN, FTR_STRICT, FTR_LOWER_SAFE, 28, 4, 0),
>
> Hmm, this still confuses me. Is this not now FTR_NONSTRICT? Why is that ok?
Mark had mentioned about it earlier
(https://patchwork.kernel.org/patch/11287805/)
Did I misinterpret the first part ? Could not figure "capping the emulated debug
features" part. Probably, Mark could give some more details.
>From the earlier discussion:
* ID_DFR0 fields need more thought; we should limit what we expose here.
I don't think it's valid for us to expose TraceFilt, and I suspect we
need to add capping for debug features we currently emulate.
Re: [PATCH net v1] net: broadcom: fix a mistake about ioremap resource
On Mon, May 04, 2020 at 08:28:52PM -0700, Florian Fainelli wrote:
>
>
> On 5/4/2020 7:03 PM, Dejin Zheng wrote:
> > Commit d7a5502b0bb8b ("net: broadcom: convert to
> > devm_platform_ioremap_resource_byname()") will broke this driver.
> > idm_base and nicpm_base were optional, after this change, they are
> > mandatory. it will probe fails with -22 when the dtb doesn't have them
> > defined. so revert part of this commit and make idm_base and nicpm_base
> > as optional.
> >
> > Fixes: d7a5502b0bb8bde ("net: broadcom: convert to
> > devm_platform_ioremap_resource_byname()")
> > Reported-by: Jonathan Richardson
> > Cc: Scott Branden
> > Cc: Ray Jui
> > Cc: Florian Fainelli
> > Cc: David S. Miller
> > Signed-off-by: Dejin Zheng
>
> Acked-by: Florian Fainelli
Florian, Thank you very much for helping me as always!
BR,
Dejin
> --
> Florian
Re: [PATCH] uprobes: ensure that uprobe->offset and ->ref_ctr_offset are properly aligned
Hi, On Mon, May 04, 2020 at 08:40:44PM +0200, Christian Borntraeger wrote: > > > On 04.05.20 18:47, Oleg Nesterov wrote: > > uprobe_write_opcode() must not cross page boundary; prepare_uprobe() > > relies on arch_uprobe_analyze_insn() which should validate "vaddr" but > > some architectures (csky, s390, and sparc) don't do this. > > I think the idea was that the uprobe instruction is 2 bytes and instructions > are always aligned to 2 bytes on s390. (we can have 2,4 or 6 bytes). > > > > > We can remove the BUG_ON() check in prepare_uprobe() and validate the > > offset early in __uprobe_register(). The new IS_ALIGNED() check matches > > the alignment check in arch_prepare_kprobe() on supported architectures, > > so I think that all insns must be aligned to UPROBE_SWBP_INSN_SIZE. > > Not sure if it would have been possible to try to create a uprobe on an > odd address. If yes, then the new IS_ALIGNED check certainly makes this > better for s390, so the patch looks sane. Adding Vasily and Sven to double > check. I did a quick test, and without this patch it is possible to place a uprobe at an odd address. With the patch it fails with EINVAL, which is more reasonable. Regards Sven
Re: [PATCH v5 0/4] Charge loop device i/o to issuing cgroup
On Wed, Apr 29, 2020 at 12:25:40PM +0200, Jan Kara wrote: > On Wed 29-04-20 07:47:34, Dave Chinner wrote: > > On Tue, Apr 28, 2020 at 12:13:46PM -0400, Dan Schatzberg wrote: > > > The loop device runs all i/o to the backing file on a separate kworker > > > thread which results in all i/o being charged to the root cgroup. This > > > allows a loop device to be used to trivially bypass resource limits > > > and other policy. This patch series fixes this gap in accounting. > > > > How is this specific to the loop device? Isn't every block device > > that offloads work to a kthread or single worker thread susceptible > > to the same "exploit"? > > > > Or is the problem simply that the loop worker thread is simply not > > taking the IO's associated cgroup and submitting the IO with that > > cgroup associated with it? That seems kinda simple to fix > > > > > Naively charging cgroups could result in priority inversions through > > > the single kworker thread in the case where multiple cgroups are > > > reading/writing to the same loop device. > > > > And that's where all the complexity and serialisation comes from, > > right? > > > > So, again: how is this unique to the loop device? Other block > > devices also offload IO to kthreads to do blocking work and IO > > submission to lower layers. Hence this seems to me like a generic > > "block device does IO submission from different task" issue that > > should be handled by generic infrastructure and not need to be > > reimplemented multiple times in every block device driver that > > offloads work to other threads... > > Yeah, I was thinking about the same when reading the patch series > description. We already have some cgroup workarounds for btrfs kthreads if > I remember correctly, we have cgroup handling for flush workers, now we are > adding cgroup handling for loopback device workers, and soon I'd expect > someone comes with a need for DM/MD worker processes and IMHO it's getting > out of hands because the complexity spreads through the kernel with every > subsystem comming with slightly different solution to the problem and also > the number of kthreads gets multiplied by the number of cgroups. So I > agree some generic solution how to approach IO throttling of kthreads / > workers would be desirable. Yup, that's pretty much what I was thinking: it's yet another special snowflake for cgroup-aware IO > OTOH I don't have a great idea how the generic infrastructure should look > like... I haven't given it any thought - it's not something I have any bandwidth to spend time on. I'll happily review a unified generic cgroup-aware kthread-based IO dispatch mechanism, but I don't have the time to design and implement that myself OTOH, I will make time to stop people screwing up filesystems and block devices with questionable complexity and unique, storage device dependent userspace visible error behaviour. This sort of change is objectively worse for users than not supporting the functionality in the first place. Cheers, Dave. -- Dave Chinner da...@fromorbit.com
[PATCH v3] riscv: force __cpu_up_ variables to put in data section
Put __cpu_up_stack_pointer and __cpu_up_task_pointer in data section. Currently, these two variables are put in bss section, there is a potential risk that secondary harts get the uninitialized value before main hart finishing the bss clearing. In this case, all secondary harts would pass the waiting loop and enable the MMU before main hart set up the page table. This issue happened on random booting of multiple harts, which means it will manifest for BBL and OpenSBI which older than v0.6. In OpenSBI v0.7 (or higher version), we have HSM extension so all the secondary harts are brought-up by Linux kernel in an orderly fashion. This means we don't need this change for OpenSBI v0.7 (or higher version). Changes in v2 and v3: - Add commit description about random booting. Signed-off-by: Zong Li Reviewed-by: Greentime Hu Reviewed-by: Anup Patel Reviewed-by: Atish Patra --- arch/riscv/kernel/cpu_ops.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/riscv/kernel/cpu_ops.c b/arch/riscv/kernel/cpu_ops.c index c4c33bf02369..0ec22354018c 100644 --- a/arch/riscv/kernel/cpu_ops.c +++ b/arch/riscv/kernel/cpu_ops.c @@ -15,8 +15,8 @@ const struct cpu_operations *cpu_ops[NR_CPUS] __ro_after_init; -void *__cpu_up_stack_pointer[NR_CPUS]; -void *__cpu_up_task_pointer[NR_CPUS]; +void *__cpu_up_stack_pointer[NR_CPUS] __section(.data); +void *__cpu_up_task_pointer[NR_CPUS] __section(.data); extern const struct cpu_operations cpu_ops_sbi; extern const struct cpu_operations cpu_ops_spinwait; -- 2.26.1
Re: iT X - the fair pay initiative - cursive, bold latin script examples
Some examples of cursive and bold features of the latin script here. https://www.youtube.com/watch?v=-r3GLmUzuds Den 02.05.2020 18:00, skrev Ywe Cærlyn: So peeps, I think I have the best system ever here. Gather industry support, and we can finally have the success it deserves. Serene Greetings, Ywe Cærlyn. iT X project lead. https://www.youtube.com/channel/UCR3gmLVjHS5A702wo4bol_Q Den 02.05.2020 05:02, skrev Ywe Cærlyn: Update: Calibri should be the standard font for this fair pay initiative, which fits this "OS", or hardware/software interface system. Then one can also use cursive for direct arabic words like 𝐴𝐿𝐿𝐴𝐻, and it mixes well with already established monotheism. For instance: 𝐴𝐿𝐿𝐴𝐻 is boss of the cosmoses and the earth. For direct chinese words, one can use bold. for instance, 𝐂𝐡𝐚𝐧 philosophy. And I do believe scientific monotheism, 𝐴𝐿𝐿𝐴𝐻 and 𝐂𝐡𝐚𝐧 is all really compatible when done well. Den 01.05.2020 11:02, skrev Ywe Cærlyn: There has at times been a certain spirituality in the *nix community. We support scientific monotheism, good interpretation of Islam, and good chan philosophy. I have just called this iT-Transcendence. I think actually "uni" in unix might be a referrance to this. Calling it iT-Transcendence instead, we universalize it, and include a broader group, strengthening this, and the fair pay background in monotheism. Serene Greetings! Ywe Cærlyn Researcher, Multi-Artist. iT X & Mascot Projects.
Re: [PATCH v2 1/3] soc: qcom: rpmh-rsc: Remove tcs_is_free() and find_free_tcs() APIs
Quoting Doug Anderson (2020-04-27 17:13:04)
> On Sat, Apr 25, 2020 at 10:53 AM Stephen Boyd wrote:
> > diff --git a/drivers/soc/qcom/rpmh-rsc.c b/drivers/soc/qcom/rpmh-rsc.c
> > index 571aa1012f23..3f4951840365 100644
> > --- a/drivers/soc/qcom/rpmh-rsc.c
> > +++ b/drivers/soc/qcom/rpmh-rsc.c
> > @@ -492,33 +476,37 @@ static void __tcs_buffer_write(struct rsc_drv *drv,
> > int tcs_id, int cmd_id,
> > *
> > * This will walk through the TCSes in the group and check if any of them
> > * appear to be sending to addresses referenced in the message. If it finds
> > - * one it'll return -EBUSY.
> > + * one it'll return -EBUSY because the hardware can't handle more than
> > + * one of the same address being changed at the same time.
> > *
> > - * Only for use for active-only transfers.
> > + * Only for use with active-only transfers.
> > *
> > * Must be called with the drv->lock held since that protects tcs_in_use.
> > *
> > - * Return: 0 if nothing in flight or -EBUSY if we should try again later.
> > + * Return: offset` of free slot if nothing in flight and a free slot is
> > found
>
> Why the back tick after "offset"?
Must be some copy/paste bug!
>
>
> > + * or -EBUSY if we should try again later.
> > * The caller must re-enable interrupts between tries since that's
> > - * the only way tcs_is_free() will ever return true and the only
> > way
> > + * the only way tcs_in_use will ever be updated and the only way
> > * RSC_DRV_CMD_ENABLE will ever be cleared.
> > */
> > -static int check_for_req_inflight(struct rsc_drv *drv, struct tcs_group
> > *tcs,
> > - const struct tcs_request *msg)
> > +static int check_for_req_inflight_and_find_free(struct rsc_drv *drv,
> > + const struct tcs_group *tcs, const struct tcs_request *msg)
> > {
> > unsigned long curr_enabled;
> > u32 addr;
> > - int i, j, k;
> > - int tcs_id = tcs->offset;
> > -
> > - for (i = 0; i < tcs->num_tcs; i++, tcs_id++) {
> > - if (tcs_is_free(drv, tcs_id))
> > - continue;
> > + int j, k;
> > + int i = tcs->offset;
> > + unsigned long max = tcs->offset + tcs->num_tcs;
> > + int first_free = i;
>
> The way "first_free" is calculated definitely adds complexity to this
> function. Are we sure it's justified compared to just calling
> find_next_zero_bit() if the function doesn't return -EBUSY? If you
> really like it this way I won't object too strongly, but I'm not
> convinced that it makes the code size smaller (vs. jumping to a common
> implementation in the kernel) and it seems unlikely to have any
> real-world speed impact.
I was trying to coalesce the double loop over the free bits here by
adding a couple more lines to keep track of the first free bit and to
set the bit when it's found. It almost feels like it would be better to
inline this whole function into the one call site too.
>
>
> > - curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE,
> > tcs_id);
> > + for_each_set_bit_from(i, drv->tcs_in_use, max) {
> > + /* Find a free tcs to use in this group */
> > + if (first_free == i)
> > + first_free = i + 1; /* Maybe the next one is free?
> > */
> >
> > + curr_enabled = read_tcs_reg(drv, RSC_DRV_CMD_ENABLE, i);
> > for_each_set_bit(j, &curr_enabled, MAX_CMDS_PER_TCS) {
> > - addr = read_tcs_cmd(drv, RSC_DRV_CMD_ADDR, tcs_id,
> > j);
> > + addr = read_tcs_cmd(drv, RSC_DRV_CMD_ADDR, i, j);
> > for (k = 0; k < msg->num_cmds; k++) {
> > if (addr == msg->cmds[k].addr)
> > return -EBUSY;
> > @@ -526,28 +514,11 @@ static int check_for_req_inflight(struct rsc_drv
> > *drv, struct tcs_group *tcs,
> > }
> > }
> >
> > - return 0;
> > -}
> > + if (first_free >= max)
> > + return -EBUSY;
> >
> > -/**
> > - * find_free_tcs() - Find free tcs in the given tcs_group; only for active.
> > - * @tcs: A pointer to the active-only tcs_group (or the wake tcs_group if
> > - * we borrowed it because there are zero active-only ones).
> > - *
> > - * Must be called with the drv->lock held since that protects tcs_in_use.
> > - *
> > - * Return: The first tcs that's free.
> > - */
> > -static int find_free_tcs(struct tcs_group *tcs)
> > -{
> > - int i;
> > -
> > - for (i = 0; i < tcs->num_tcs; i++) {
> > - if (tcs_is_free(tcs->drv, tcs->offset + i))
> > - return tcs->offset + i;
> > - }
> > -
> > - return -EBUSY;
> > + set_bit(first_free, drv->tcs_in_use);
>
> Function is not documented to also set the bit. Do we really gain
> anything by setting it in this function, or can it just stay with the
> caller? I
[PATCH net-next v6 0/2] provide support for PHY master/slave configuration
changes v6: - use NL_SET_ERR_MSG_ATTR in ethnl_update_linkmodes - add sanity checks in the ioctl interface - use bool for ethnl_validate_master_slave_cfg() changes v5: - set MASTER_SLAVE_CFG_UNSUPPORTED as default value - send a netlink error message on validation error - more code fixes changes v4: - rename port_mode to master_slave - move validation code to net/ethtool/linkmodes.c - add UNSUPPORTED state and avoid sending unsupported fields - more formatting and naming fixes - tja11xx: support only force mode - tja11xx: mark state as unsupported changes v3: - provide separate field for config and state. - make state rejected on set - add validation changes v2: - change names. Use MASTER_PREFERRED instead of MULTIPORT - configure master/slave only on request. Default configuration can be provided by PHY or eeprom - status and configuration to the user space. Oleksij Rempel (2): ethtool: provide UAPI for PHY master/slave configuration. net: phy: tja11xx: add support for master-slave configuration Documentation/networking/ethtool-netlink.rst | 35 drivers/net/phy/nxp-tja11xx.c| 43 + drivers/net/phy/phy.c| 4 +- drivers/net/phy/phy_device.c | 94 include/linux/phy.h | 3 + include/uapi/linux/ethtool.h | 16 +++- include/uapi/linux/ethtool_netlink.h | 2 + include/uapi/linux/mii.h | 2 + net/ethtool/ioctl.c | 6 ++ net/ethtool/linkmodes.c | 53 +++ 10 files changed, 240 insertions(+), 18 deletions(-) -- 2.26.2
[PATCH net-next v6 2/2] net: phy: tja11xx: add support for master-slave configuration
The TJA11xx PHYs have a vendor specific Master/Slave configuration bit,
which is not compatible with IEEE 803.2-2018 spec for 100Base-T1
devices. So, provide a custom config_ange call back to solve this
problem.
Signed-off-by: Oleksij Rempel
---
drivers/net/phy/nxp-tja11xx.c | 43 +++
1 file changed, 43 insertions(+)
diff --git a/drivers/net/phy/nxp-tja11xx.c b/drivers/net/phy/nxp-tja11xx.c
index cc766b2d4136e..ca5f9d4dc57ed 100644
--- a/drivers/net/phy/nxp-tja11xx.c
+++ b/drivers/net/phy/nxp-tja11xx.c
@@ -30,6 +30,7 @@
#define MII_ECTRL_WAKE_REQUEST BIT(0)
#define MII_CFG1 18
+#define MII_CFG1_MASTER_SLAVE BIT(15)
#define MII_CFG1_AUTO_OP BIT(14)
#define MII_CFG1_SLEEP_CONFIRM BIT(6)
#define MII_CFG1_LED_MODE_MASK GENMASK(5, 4)
@@ -167,6 +168,32 @@ static int tja11xx_soft_reset(struct phy_device *phydev)
return genphy_soft_reset(phydev);
}
+static int tja11xx_config_aneg(struct phy_device *phydev)
+{
+ u16 ctl = 0;
+ int ret;
+
+ switch (phydev->master_slave_set) {
+ case MASTER_SLAVE_CFG_MASTER_FORCE:
+ ctl |= MII_CFG1_MASTER_SLAVE;
+ break;
+ case MASTER_SLAVE_CFG_SLAVE_FORCE:
+ break;
+ case MASTER_SLAVE_CFG_UNKNOWN:
+ case MASTER_SLAVE_CFG_UNSUPPORTED:
+ return 0;
+ default:
+ phydev_warn(phydev, "Unsupported Master/Slave mode\n");
+ return -ENOTSUPP;
+ }
+
+ ret = phy_modify_changed(phydev, MII_CFG1, MII_CFG1_MASTER_SLAVE, ctl);
+ if (ret < 0)
+ return ret;
+
+ return __genphy_config_aneg(phydev, ret);
+}
+
static int tja11xx_config_init(struct phy_device *phydev)
{
int ret;
@@ -224,10 +251,22 @@ static int tja11xx_read_status(struct phy_device *phydev)
{
int ret;
+ phydev->master_slave_get = MASTER_SLAVE_CFG_UNKNOWN;
+ phydev->master_slave_state = MASTER_SLAVE_STATE_UNSUPPORTED;
+
ret = genphy_update_link(phydev);
if (ret)
return ret;
+ ret = phy_read(phydev, MII_CFG1);
+ if (ret < 0)
+ return ret;
+
+ if (ret & MII_CFG1_MASTER_SLAVE)
+ phydev->master_slave_get = MASTER_SLAVE_CFG_MASTER_FORCE;
+ else
+ phydev->master_slave_get = MASTER_SLAVE_CFG_SLAVE_FORCE;
+
if (phydev->link) {
ret = phy_read(phydev, MII_COMMSTAT);
if (ret < 0)
@@ -504,6 +543,7 @@ static struct phy_driver tja11xx_driver[] = {
.features = PHY_BASIC_T1_FEATURES,
.probe = tja11xx_probe,
.soft_reset = tja11xx_soft_reset,
+ .config_aneg= tja11xx_config_aneg,
.config_init= tja11xx_config_init,
.read_status= tja11xx_read_status,
.suspend= genphy_suspend,
@@ -519,6 +559,7 @@ static struct phy_driver tja11xx_driver[] = {
.features = PHY_BASIC_T1_FEATURES,
.probe = tja11xx_probe,
.soft_reset = tja11xx_soft_reset,
+ .config_aneg= tja11xx_config_aneg,
.config_init= tja11xx_config_init,
.read_status= tja11xx_read_status,
.suspend= genphy_suspend,
@@ -533,6 +574,7 @@ static struct phy_driver tja11xx_driver[] = {
.features = PHY_BASIC_T1_FEATURES,
.probe = tja1102_p0_probe,
.soft_reset = tja11xx_soft_reset,
+ .config_aneg= tja11xx_config_aneg,
.config_init= tja11xx_config_init,
.read_status= tja11xx_read_status,
.match_phy_device = tja1102_p0_match_phy_device,
@@ -551,6 +593,7 @@ static struct phy_driver tja11xx_driver[] = {
.features = PHY_BASIC_T1_FEATURES,
/* currently no probe for Port 1 is need */
.soft_reset = tja11xx_soft_reset,
+ .config_aneg= tja11xx_config_aneg,
.config_init= tja11xx_config_init,
.read_status= tja11xx_read_status,
.match_phy_device = tja1102_p1_match_phy_device,
--
2.26.2
Re: [PATCH] sysctl: Make sure proc handlers can't expose heap memory
On Mon, May 04, 2020 at 09:59:03PM +, Luis Chamberlain wrote:
> On Mon, May 04, 2020 at 01:32:07PM -0700, Kees Cook wrote:
> > On Mon, May 04, 2020 at 07:59:37PM +, Luis Chamberlain wrote:
> > > On Mon, May 04, 2020 at 12:08:55PM -0700, Kees Cook wrote:
> > > > Just as a precaution, make sure that proc handlers don't accidentally
> > > > grow "count" beyond the allocated kbuf size.
> > > >
> > > > Signed-off-by: Kees Cook
> > > > ---
> > > > This applies to hch's sysctl cleanup tree...
> > > > ---
> > > > fs/proc/proc_sysctl.c | 3 +++
> > > > 1 file changed, 3 insertions(+)
> > > >
> > > > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> > > > index 15030784566c..535ab26473af 100644
> > > > --- a/fs/proc/proc_sysctl.c
> > > > +++ b/fs/proc/proc_sysctl.c
> > > > @@ -546,6 +546,7 @@ static ssize_t proc_sys_call_handler(struct file
> > > > *filp, void __user *ubuf,
> > > > struct inode *inode = file_inode(filp);
> > > > struct ctl_table_header *head = grab_header(inode);
> > > > struct ctl_table *table = PROC_I(inode)->sysctl_entry;
> > > > + size_t count_max = count;
> > > > void *kbuf;
> > > > ssize_t error;
> > > >
> > > > @@ -590,6 +591,8 @@ static ssize_t proc_sys_call_handler(struct file
> > > > *filp, void __user *ubuf,
> > > >
> > > > if (!write) {
> > > > error = -EFAULT;
> > > > + if (WARN_ON(count > count_max))
> > > > + count = count_max;
> > >
> > > That would crash a system with panic-on-warn. I don't think we want that?
> >
> > Eh? None of the handlers should be making this mistake currently and
> > it's not a mistake that can be controlled from userspace. WARN() is
> > absolutely what's wanted here: report an impossible situation (and
> > handle it gracefully for the bulk of users that don't have
> > panic_on_warn set).
>
> Alrighty, Greg are you OK with this type of WARN_ON()? You recently
> expressed concerns over its use due to panic-on-warn on another patch.
We should never call WARN() on any path that a user can trigger.
If it is just a "the developer called this api in a foolish way" then we
could use a WARN_ON() to have them realize their mistake, but in my
personal experience, foolish developers don't even notice that kind of
mistake :(
thanks,
greg k-h
[PATCH net-next v6 1/2] ethtool: provide UAPI for PHY master/slave configuration.
This UAPI is needed for BroadR-Reach 100BASE-T1 devices. Due to lack of auto-negotiation support, we needed to be able to configure the MASTER-SLAVE role of the port manually or from an application in user space. The same UAPI can be used for 1000BASE-T or MultiGBASE-T devices to force MASTER or SLAVE role. See IEEE 802.3-2018: 22.2.4.3.7 MASTER-SLAVE control register (Register 9) 22.2.4.3.8 MASTER-SLAVE status register (Register 10) 40.5.2 MASTER-SLAVE configuration resolution 45.2.1.185.1 MASTER-SLAVE config value (1.2100.14) 45.2.7.10 MultiGBASE-T AN control 1 register (Register 7.32) The MASTER-SLAVE role affects the clock configuration: --- When the PHY is configured as MASTER, the PMA Transmit function shall source TX_TCLK from a local clock source. When configured as SLAVE, the PMA Transmit function shall source TX_TCLK from the clock recovered from data stream provided by MASTER. iMX6Q KSZ9031XXX --\/---\/\ || ||| MAC || PHY Slave |<-->| PHY Master | |<--- 125 MHz ---+-<--/ || \ | --/\---/\/ ^ \-TX_TCLK --- Since some clock or link related issues are only reproducible in a specific MASTER-SLAVE-role, MAC and PHY configuration, it is beneficial to provide generic (not 100BASE-T1 specific) interface to the user space for configuration flexibility and trouble shooting. Signed-off-by: Oleksij Rempel --- Documentation/networking/ethtool-netlink.rst | 35 drivers/net/phy/phy.c| 4 +- drivers/net/phy/phy_device.c | 94 include/linux/phy.h | 3 + include/uapi/linux/ethtool.h | 16 +++- include/uapi/linux/ethtool_netlink.h | 2 + include/uapi/linux/mii.h | 2 + net/ethtool/ioctl.c | 6 ++ net/ethtool/linkmodes.c | 53 +++ 9 files changed, 197 insertions(+), 18 deletions(-) diff --git a/Documentation/networking/ethtool-netlink.rst b/Documentation/networking/ethtool-netlink.rst index 567326491f80b..8f5cefc539cf1 100644 --- a/Documentation/networking/ethtool-netlink.rst +++ b/Documentation/networking/ethtool-netlink.rst @@ -392,14 +392,16 @@ Request contents: Kernel response contents: - == == - ``ETHTOOL_A_LINKMODES_HEADER``nested reply header - ``ETHTOOL_A_LINKMODES_AUTONEG`` u8 autonegotiation status - ``ETHTOOL_A_LINKMODES_OURS`` bitset advertised link modes - ``ETHTOOL_A_LINKMODES_PEER`` bitset partner link modes - ``ETHTOOL_A_LINKMODES_SPEED`` u32 link speed (Mb/s) - ``ETHTOOL_A_LINKMODES_DUPLEX``u8 duplex mode - == == + == == == + ``ETHTOOL_A_LINKMODES_HEADER`` nested reply header + ``ETHTOOL_A_LINKMODES_AUTONEG`` u8 autonegotiation status + ``ETHTOOL_A_LINKMODES_OURS``bitset advertised link modes + ``ETHTOOL_A_LINKMODES_PEER``bitset partner link modes + ``ETHTOOL_A_LINKMODES_SPEED`` u32 link speed (Mb/s) + ``ETHTOOL_A_LINKMODES_DUPLEX`` u8 duplex mode + ``ETHTOOL_A_LINKMODES_MASTER_SLAVE_CFG``u8 Master/slave port mode + ``ETHTOOL_A_LINKMODES_MASTER_SLAVE_STATE`` u8 Master/slave port state + == == == For ``ETHTOOL_A_LINKMODES_OURS``, value represents advertised modes and mask represents supported modes. ``ETHTOOL_A_LINKMODES_PEER`` in the reply is a bit @@ -414,14 +416,15 @@ LINKMODES_SET Request contents: - == == - ``ETHTOOL_A_LINKMODES_HEADER``nested request header - ``ETHTOOL_A_LINKMODES_AUTONEG`` u8 autonegotiation status - ``ETHTOOL_A_LINKMODES_OURS`` bitset advertised link modes - ``ETHTOOL_A_LINKMODES_PEER`` bitset partner link modes - ``ETHTOOL_A_LINKMODES_SPEED`` u32 link speed (Mb/s) - ``ETHTOOL_A_LINKMODES_DUPLEX``u8 duplex mode - == == + == == == + ``ETHTOOL_A_LINKMODES_HEADER`` nested request header + ``ETHTOOL_A_LINKMODES_AUTONEG``
Re: [PATCH v6 1/5] soc: qcom: rpmh-rsc: Correctly ignore CPU_CLUSTER_PM notifications
Quoting Douglas Anderson (2020-05-04 10:50:15)
> Our switch statement doesn't have entries for CPU_CLUSTER_PM_ENTER,
> CPU_CLUSTER_PM_ENTER_FAILED, and CPU_CLUSTER_PM_EXIT and doesn't have
> a default. This means that we'll try to do a flush in those cases but
> we won't necessarily be the last CPU down. That's not so ideal since
> our (lack of) locking assumes we're on the last CPU.
>
> Luckily this isn't as big a problem as you'd think since (at least on
> the SoC I tested) we don't get these notifications except on full
> system suspend. ...and on full system suspend we get them on the last
> CPU down. That means that the worst problem we hit is flushing twice.
> Still, it's good to make it correct.
>
> Fixes: 985427f997b6 ("soc: qcom: rpmh: Invoke rpmh_flush() for dirty caches")
> Reported-by: Stephen Boyd
> Signed-off-by: Douglas Anderson
> ---
Reviewed-by: Stephen Boyd
Re: [PATCH v5 0/4] Charge loop device i/o to issuing cgroup
On Tue, Apr 28, 2020 at 10:27:32PM -0400, Johannes Weiner wrote: > On Wed, Apr 29, 2020 at 07:47:34AM +1000, Dave Chinner wrote: > > On Tue, Apr 28, 2020 at 12:13:46PM -0400, Dan Schatzberg wrote: > > > This patch series does some > > > minor modification to the loop driver so that each cgroup can make > > > forward progress independently to avoid this inversion. > > > > > > With this patch series applied, the above script triggers OOM kills > > > when writing through the loop device as expected. > > > > NACK! > > > > The IO that is disallowed should fail with ENOMEM or some similar > > error, not trigger an OOM kill that shoots some innocent bystander > > in the head. That's worse than using BUG() to report errors... > > Did you actually read the script? Of course I did. You specifically mean this bit: echo 64M > $CGROUP/memory.max; mount -t tmpfs -o size=512m tmpfs /tmp; truncate -s 512m /tmp/backing_file losetup $LOOP_DEV /tmp/backing_file dd if=/dev/zero of=$LOOP_DEV bs=1M count=256; And with this patch set the dd gets OOM killed because the /tmp/backing_file usage accounted to the cgroup goes over 64MB and so tmpfs OOM kills the IO... As I said: that's very broken behaviour from a storage stack perspective. > It's OOMing because it's creating 256M worth of tmpfs pages inside a > 64M cgroup. It's not killing an innocent bystander, it's killing in > the cgroup that is allocating all that memory - after Dan makes sure > that memory is accounted to its rightful owner. What this example does is turn /tmp into thinly provisioned storage for $CGROUP via a restricted quota. It has a device size of 512MB, but only has physical storage capacity of 64MB, as constrained by the cgroup memory limit. You're dealing with management of -storage devices- and -IO error reporting- here, not memory management. When thin provisioned storage runs out of space - for whatever reason - and it cannot resolve the issue by blocking, then it *must* return ENOSPC to the IO submitter to tell it the IO has failed. This is no different to if we set a quota on /tmp/backing_file and it is exhausted at 64MB of data written - we fail the IO with ENOSPC or EDQUOT, depending on which quota we used. IOWs, we have solid expectations on how block devices report unsolvable resource shortages during IO because those errors have to be handled correctly by whatever is submitting the IO. We do not use the OOM-killer to report or resolve ENOSPC errors. Indeed, once you've killed the dd, that CGROUP still consumes 64MB of tmpfs space in /tmp/backing_file, in which case any further IO to $LOOP_DEV is also going to OOM kill. These are horrible semantics for reporting errors to userspace. And if the OOM-killer actually frees the 64MB of data written to /tmp/backing_file through the $LOOP_DEV, then you're actually corrupting the storage and ensuring that nobody can read the data that was written to $LOOP_DEV. So now lets put a filesystem on $LOOP_DEV in the above example, and write out data to the filesystem which does IO to $LOOP_DEV. Just by chance, the filesystem does some metadata writes iin the context of the user process doing the writes (because journalling, etc) and that metadata IO is what pushes the loop device over the cgroup's memory limit. You kill the user application even though it wasn't directly responsible for going over the 64MB limit of space in $LOOP_DEV. What happens now to the filesystem's metadata IO? Did $LOOP_DEV return an error, or after the OOM kill did the IO succeed? What happens if all the IO being generated from the user application is metadata and that starts failing - killing the user application doesn't help anything - the filesystem IO is failing and that's where the errors need to be reported. And if the answer is "metadata IO isn't accounted to the $CGROUP" then what happens when the tmpfs actually runs out of it's 512MB of space because of all the metadata the filesystem wrote (e.g. create lots of zero length files)? That's an ENOSPC error, and we'll get that from $LOOP_DEV just fine. So why should the same error - running out of tmpfs space vs running out of CGROUP quota space on tmpfs be handled differently? Either they are both ENOSPC IO errors, or they are both OOM kill vectors because tmpfs space has run out... See the problem here? We report storage resource shortages (whatever the cause) by IO errors, not by killing userspace processes. Userspace may be able to handle the IO error sanely; it has no warning or choice when you use OOM kill to report ENOSPC errors... > As opposed to before this series, where all this memory isn't > accounted properly and goes to the root cgroup - where, ironically, it > could cause OOM and kill an actually innocent bystander. Johannes, I didn't question the need for the functionality. I questioned the implementation and pointed out fundamental problems it has as well as the architectural questions raised by needing special kthread-based handling for corre
Re: [PATCH 4.19 STABLE 2/2] KVM: VMX: Mark RCX, RDX and RSI as clobbered in vmx_vcpu_run()'s asm blob
On Tue, May 05, 2020 at 08:15:02AM +0200, Greg Kroah-Hartman wrote:
> On Mon, May 04, 2020 at 06:23:48PM -0700, Sean Christopherson wrote:
> > Save RCX, RDX and RSI to fake outputs to coerce the compiler into
> > treating them as clobbered. RCX in particular is likely to be reused by
> > the compiler to dereference the 'struct vcpu_vmx' pointer, which will
> > result in a null pointer dereference now that RCX is zeroed by the asm
> > blob.
> >
> > Add ASM_CALL_CONSTRAINT to fudge around an issue where
> > during modpost can't find vmx_return when specifying output constraints.
> >
> > Reported-by: Tobias Urdin
> > Fixes: b4be98039a92 ("KVM: VMX: Zero out *all* general purpose registers
> > after VM-Exit")
> > Signed-off-by: Sean Christopherson
> > ---
> > arch/x86/kvm/vmx.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> > index 5b06a98ffd4c..54c8b4dc750d 100644
> > --- a/arch/x86/kvm/vmx.c
> > +++ b/arch/x86/kvm/vmx.c
> > @@ -10882,7 +10882,8 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu
> > *vcpu)
> > ".global vmx_return \n\t"
> > "vmx_return: " _ASM_PTR " 2b \n\t"
> > ".popsection"
> > - : : "c"(vmx), "d"((unsigned long)HOST_RSP), "S"(evmcs_rsp),
> > + : ASM_CALL_CONSTRAINT, "=c"((int){0}), "=d"((int){0}),
> > "=S"((int){0})
> > + : "c"(vmx), "d"((unsigned long)HOST_RSP), "S"(evmcs_rsp),
> > [launched]"i"(offsetof(struct vcpu_vmx, __launched)),
> > [fail]"i"(offsetof(struct vcpu_vmx, fail)),
> > [host_rsp]"i"(offsetof(struct vcpu_vmx, host_rsp)),
> > --
> > 2.26.0
> >
>
> What is the git commit id of this patch in Linus's tree?
There is none. In upstream at the time of the offending commit (b4be98039a92
in 4.19, 0e0ab73c9a024 upstream), the inline asm blob had previously been
moved to a dedicated helper, __vmx_vcpu_run(), that was intentionally put
into a separate compilation unit, i.e. consuming the clobbered register
was effectively impossible because %rcx is volatile and __vmx_vcpu_run()
couldn't itself be inlined.
To make things more confusing, the inline asm blob got moved into a proper
asm subroutine shortly thereafter. Things really start to diverge from
current upstream right around the time of this commit.
Re: [PATCH v2 0/3] arm64: dts: rockchip: misc. cleanups and improvements
Hi, On Mon, Apr 27, 2020 at 3:31 PM Chen-Yu Tsai wrote: > > From: Chen-Yu Tsai > > Hi, > > Here is the remaining patch for roc-rk3399-pc, along with a few other > new patches. > > - Based on discussions from v1, patch one, newly added, drops the list > of valid values for linux,default-triggers. > > - Patch two is the same as in v1 > > - Patch three, new, sets dr_mode to "host" to the dwc2 OTG controller, > matching what the board uses it for, a host port. This gets rid of > a warning from the kernel. > > Please have a look. Are there any comments from the maintainers? I feel like this is getting needlessly sidetracked by the endless discussion about GPIO LED node names which I left untouched on purpose in this series. ChenYu > Regards > ChenYu > > > Chen-Yu Tsai (3): > dt-bindings: leds: common: Drop enumeration for linux,default-triggers > arm64: dts: rockchip: rk3399-roc-pc: Fix MMC numbering for LED > triggers > arm64: dts: rockchip: rk3328-roc-cc: Set dr_mode to "host" for OTG > > .../devicetree/bindings/leds/common.yaml | 21 +-- > .../arm64/boot/dts/rockchip/rk3328-roc-cc.dts | 1 + > .../dts/rockchip/rk3399-roc-pc-mezzanine.dts | 8 +++ > .../boot/dts/rockchip/rk3399-roc-pc.dtsi | 4 ++-- > 4 files changed, 12 insertions(+), 22 deletions(-) > > -- > 2.26.0 >
Re: [PATCH] slub: limit count of partial slabs scanned to gather statistics
On 05/05/2020 00.19, David Rientjes wrote:
On Mon, 4 May 2020, Konstantin Khlebnikov wrote:
To get exact count of free and used objects slub have to scan list of
partial slabs. This may take at long time. Scanning holds spinlock and
blocks allocations which move partial slabs to per-cpu lists and back.
Example found in the wild:
# cat /sys/kernel/slab/dentry/partial
14478538 N0=7329569 N1=7148969
# time cat /sys/kernel/slab/dentry/objects
286225471 N0=136967768 N1=149257703
real0m1.722s
user0m0.001s
sys 0m1.721s
The same problem in slab was addressed in commit f728b0a5d72a ("mm, slab:
faster active and free stats") by adding more kmem cache statistics.
For slub same approach requires atomic op on fast path when object frees.
Let's simply limit count of scanned slabs and print warning.
Limit set in /sys/module/slub/parameters/max_partial_to_count.
Default is 1 which should be enough for most sane cases.
Return linear approximation if list of partials is longer than limit.
Nobody should notice difference.
Hi Konstantin,
Do you only exhibit this on slub for SO_ALL|SO_OBJECTS? I notice the
timing in the changelog is only looking at "objects" and not "partial".
"partial" is a count of partial slabs which simply sums per-numa counters.
Affected only "objects" and "objects_partial" which walk the list.
If so, it seems this is also a problem for get_slabinfo() since it also
uses the count_free() callback for count_partial().
Yep, /proc/slabinfo also affected.
Actually it's more affected than sysfs - it walks all cgroups while sysfs shows
only root.
Concern would be that the kernel has now drastically changed a statistic
that it exports to userspace. There was some discussion about this back
in 2016[*] and one idea was that slabinfo would truncate its scanning and
append a '+' to the end of the value to indicate it exceeds the max, i.e.
1+. I think that '+' actually caused the problem itself for userspace
processes.
Yep, "+" will break everything for sure.
I thought about returning "-1" or INT_MAX without counting,
but approximation gives almost correct result without breaking anything.
Each partial slab has at least one used and free object thus approximated
result will be somewhere between nr_partial_slabs and nr_partial_objects.
I think the patch is too far reaching, however, since it impacts all
count_partial() counting and not only for the case cited in the changelog.
Are there examples for things other than the count_free() callback?
Nope, this is just a statistics for used/free objects.
Total count of objects and slabs are counted precisely.
[*] https://lore.kernel.org/patchwork/patch/708427/
Signed-off-by: Konstantin Khlebnikov
---
mm/slub.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/mm/slub.c b/mm/slub.c
index 9bf44955c4f1..86a366f7acb6 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2407,16 +2407,29 @@ static inline unsigned long node_nr_objs(struct
kmem_cache_node *n)
#endif /* CONFIG_SLUB_DEBUG */
#if defined(CONFIG_SLUB_DEBUG) || defined(CONFIG_SYSFS)
+
+static unsigned long max_partial_to_count __read_mostly = 1;
+module_param(max_partial_to_count, ulong, 0644);
+
static unsigned long count_partial(struct kmem_cache_node *n,
int (*get_count)(struct page *))
{
+ unsigned long counted = 0;
unsigned long flags;
unsigned long x = 0;
struct page *page;
spin_lock_irqsave(&n->list_lock, flags);
- list_for_each_entry(page, &n->partial, slab_list)
+ list_for_each_entry(page, &n->partial, slab_list) {
x += get_count(page);
+
+ if (++counted > max_partial_to_count) {
+ pr_warn_once("SLUB: too much partial slabs to count all
objects, increase max_partial_to_count.\n");
+ /* Approximate total count of objects */
+ x = mult_frac(x, n->nr_partial, counted);
+ break;
+ }
+ }
spin_unlock_irqrestore(&n->list_lock, flags);
return x;
}
[PATCH] kobject: documentation: Fix erroneous function example in kobject doc.
Update the definitions of some functions listed in the kobject
document, since they have been changed.
Signed-off-by: Qi Zheng
---
Documentation/core-api/kobject.rst | 26 ++
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/Documentation/core-api/kobject.rst
b/Documentation/core-api/kobject.rst
index 1f62d4d7d966..c3ac2963283b 100644
--- a/Documentation/core-api/kobject.rst
+++ b/Documentation/core-api/kobject.rst
@@ -80,11 +80,11 @@ what is the pointer to the containing structure? You must
avoid tricks
(such as assuming that the kobject is at the beginning of the structure)
and, instead, use the container_of() macro, found in ::
-container_of(pointer, type, member)
+container_of(ptr, type, member)
where:
- * ``pointer`` is the pointer to the embedded kobject,
+ * ``ptr`` is the pointer to the embedded kobject,
* ``type`` is the type of the containing structure, and
* ``member`` is the name of the structure field to which ``pointer`` points.
@@ -140,7 +140,7 @@ the name of the kobject, call kobject_rename()::
int kobject_rename(struct kobject *kobj, const char *new_name);
-kobject_rename does not perform any locking or have a solid notion of
+kobject_rename() does not perform any locking or have a solid notion of
what names are valid so the caller must provide their own sanity checking
and serialization.
@@ -222,17 +222,17 @@ ksets, show and store functions, and other details. This
is the one
exception where a single kobject should be created. To create such an
entry, use the function::
-struct kobject *kobject_create_and_add(char *name, struct kobject *parent);
+struct kobject *kobject_create_and_add(const char *name, struct kobject
*parent);
This function will create a kobject and place it in sysfs in the location
underneath the specified parent kobject. To create simple attributes
associated with this kobject, use::
-int sysfs_create_file(struct kobject *kobj, struct attribute *attr);
+int sysfs_create_file(struct kobject *kobj, const struct attribute *attr);
or::
-int sysfs_create_group(struct kobject *kobj, struct attribute_group *grp);
+int sysfs_create_group(struct kobject *kobj, const struct attribute_group
*grp);
Both types of attributes used here, with a kobject that has been created
with the kobject_create_and_add(), can be of type kobj_attribute, so no
@@ -300,8 +300,10 @@ kobj_type::
void (*release)(struct kobject *kobj);
const struct sysfs_ops *sysfs_ops;
struct attribute **default_attrs;
+const struct attribute_group **default_groups;
const struct kobj_ns_type_operations *(*child_ns_type)(struct
kobject *kobj);
const void *(*namespace)(struct kobject *kobj);
+void (*get_ownership)(struct kobject *kobj, kuid_t *uid, kgid_t
*gid);
};
This structure is used to describe a particular type of kobject (or, more
@@ -352,12 +354,12 @@ created and never declared statically or on the stack.
To create a new
kset use::
struct kset *kset_create_and_add(const char *name,
- struct kset_uevent_ops *u,
- struct kobject *parent);
+ const struct kset_uevent_ops *uevent_ops,
+ struct kobject *parent_kobj);
When you are finished with the kset, call::
- void kset_unregister(struct kset *kset);
+ void kset_unregister(struct kset *k);
to destroy it. This removes the kset from sysfs and decrements its reference
count. When the reference count goes to zero, the kset will be released.
@@ -371,9 +373,9 @@ If a kset wishes to control the uevent operations of the
kobjects
associated with it, it can use the struct kset_uevent_ops to handle it::
struct kset_uevent_ops {
- int (*filter)(struct kset *kset, struct kobject *kobj);
- const char *(*name)(struct kset *kset, struct kobject *kobj);
- int (*uevent)(struct kset *kset, struct kobject *kobj,
+ int (* const filter)(struct kset *kset, struct kobject *kobj);
+ const char *(* const name)(struct kset *kset, struct kobject *kobj);
+ int (* const uevent)(struct kset *kset, struct kobject *kobj,
struct kobj_uevent_env *env);
};
--
2.25.1
Re: [PATCH/RFC] clk: gate: Add some kunit test suites
On Mon, 2020-05-04 at 13:19 -0700, Brendan Higgins wrote: > On Sun, May 3, 2020 at 10:54 PM Vaittinen, Matti > wrote: > > On Wed, 2020-04-29 at 12:15 +0800, David Gow wrote: > > > On Tue, Apr 14, 2020 at 7:46 PM Vaittinen, Matti > > > wrote: > > > > Hello Stephen & All, > > > > > > > > Prologue: > > > > > > > > I have been traumatized in the past - by unit tests :) Thus I > > > > am > > > > always > > > > a bit jumpy when I see people adding UTs. I always see the > > > > inertia > > > > UTs > > > > add to development - when people change anything they must also > > > > change > > > > impacted UTs - and every unnecessary UT case is actually > > > > burden. I > > > > was > > > > once buried under such burden.. Back at those times I quite > > > > often > > > > found > > > > myself wondering why I spend two days fixing UT cases after I > > > > did a > > > > necessary code change which took maybe 10 minutes. Please see > > > > my > > > > comments knowing this history and do your decisions based on > > > > less > > > > biased - brighter understanding :] > > > > > > > > > > > > > > Hey Matti, > > > > > > As someone who's definitely wasted a lot of time fixing > > > poorly-thought-through tests, but who is nevertheless working > > > enthusiastically on KUnit, I wanted to respond quickly here. > > > > I appreciate your reply. And I appreciate your (and others) work on > > KUinit. I don't think UTs are bad. I believe UTs are great and > > powerfull tool - when used on specific occasions. But UTs > > definitely > > are "double-edged sword" - you can hit your own leg. > > Sure, I saw you just added a unit test, so I would assume that you > don't think that UTs are inherently bad :-) > > > > Certainly, the goal is not to reduce development velocity, though > > > it > > > is redistributed a bit: hopefully the extra time spent updating > > > tests > > > will be more than paid back in identifying and fixing bugs > > > earlier, > > > as > > > well as making testing one's own changes simpler. Only time will > > > tell > > > if we're right or not, but if they turn out to cause more > > > problems > > > than they solve we can always adjust or remove them. I remain > > > optimistic, though. > > > > Fixing and identifying bugs is definitely "the thing" in UTs. But > > what > > comes to weighing the benefits of UTs Vs. downsides - that's hard. > > First thing on that front is to understand the cost of UTs. And in > > my > > experience - many people underestimate that. It's easy too see > > things > > black & white and think UTs are only good - which is not true. > > > > UT cases are code which must be maintained as any code. And we must > > never add code which is not beneficial as maintaining it is work. > > We do > > constantly work to decrease amount of code to be maintaned - UTs > > are no > > exception - all code is a burden. Unnecessary code is burden with > > no > > purpose. And UTs have no other purpose but to point out mistakes. > > Only > > I am in agreement with everything you said so far. > > > good test is a test that is failing and pointing out a bug which > > would > > have otherwise gone unnoticed. Passing test is a waste. > > Not to be uncharitable in how I read this, but I don't think this is > quite right. I think we would all agree that a test that *cannot* > fail > is a waste, but on the other hand, you should never submit code that > doesn't pass *good tests*, right? Like you wrote tests for your > linear > range stuff to verify that it worked as expected; I don't know if > some > of the tests didn't pass on the first try, but you did ultimately fix > all the issues, and submitted only tests that pass, right? I am > pretty > sure no one would accept code accompanied with tests that don't pass. > > So would a better way to phrase this be: "tests that can only pass > are > a waste"? Or maybe, "tests that don't provide useful information on a > failure are a waste"? What I mean is really that a test that is not failing is a waste. To make it more accurate though - a test that is not failing at some point during development is a waste. > > So key thing when considering if adding a test is beneficial is > > whether > > the test is likely to point out a bug (in early phase). A bug that > > would otherwise have gone through unnoticed. > > Why not a bug later on? On another project, I noticed a piece of code > that kept breaking, so I added a test for that piece of code, and > people stopped breaking it: mission accomplished, right? You are right. "Later on" is fine :) As is "on another project". > > > I do think that these tests are unlikely to increase the burden > > > on > > > developers much: > > > > All code which uses kernel APIs is increasing burden for someone > > who > > needs to change the API. Much can be discussed ;) > > > > > they seem mostly to test interfaces and behaviour > > > which is used quite a bit elsewhere in the kernel: changes that > > > break > > > them seem likel
Re: net: rtw88: fix an issue about leak system resources
> Brian, Thanks very much for your reminder, Reminders can hopefully trigger positive effects. > These comments have always bothered me. Thanks for such information. > Now I can put it on my blacklist. I find it unfortunate that you choose to adjust your communication preferences in this direction. Thus I am curious if other contributors will get more chances to integrate another bit of advice into your software development attention. Regards, Markus
Re: [PATCH 4.19 STABLE 2/2] KVM: VMX: Mark RCX, RDX and RSI as clobbered in vmx_vcpu_run()'s asm blob
On Mon, May 04, 2020 at 06:23:48PM -0700, Sean Christopherson wrote:
> Save RCX, RDX and RSI to fake outputs to coerce the compiler into
> treating them as clobbered. RCX in particular is likely to be reused by
> the compiler to dereference the 'struct vcpu_vmx' pointer, which will
> result in a null pointer dereference now that RCX is zeroed by the asm
> blob.
>
> Add ASM_CALL_CONSTRAINT to fudge around an issue where
> during modpost can't find vmx_return when specifying output constraints.
>
> Reported-by: Tobias Urdin
> Fixes: b4be98039a92 ("KVM: VMX: Zero out *all* general purpose registers
> after VM-Exit")
> Signed-off-by: Sean Christopherson
> ---
> arch/x86/kvm/vmx.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/arch/x86/kvm/vmx.c b/arch/x86/kvm/vmx.c
> index 5b06a98ffd4c..54c8b4dc750d 100644
> --- a/arch/x86/kvm/vmx.c
> +++ b/arch/x86/kvm/vmx.c
> @@ -10882,7 +10882,8 @@ static void __noclone vmx_vcpu_run(struct kvm_vcpu
> *vcpu)
> ".global vmx_return \n\t"
> "vmx_return: " _ASM_PTR " 2b \n\t"
> ".popsection"
> - : : "c"(vmx), "d"((unsigned long)HOST_RSP), "S"(evmcs_rsp),
> + : ASM_CALL_CONSTRAINT, "=c"((int){0}), "=d"((int){0}),
> "=S"((int){0})
> + : "c"(vmx), "d"((unsigned long)HOST_RSP), "S"(evmcs_rsp),
> [launched]"i"(offsetof(struct vcpu_vmx, __launched)),
> [fail]"i"(offsetof(struct vcpu_vmx, fail)),
> [host_rsp]"i"(offsetof(struct vcpu_vmx, host_rsp)),
> --
> 2.26.0
>
What is the git commit id of this patch in Linus's tree?
thanks,
greg k-h
Re: [PATCH v2 2/2] clk: qoriq: add cpufreq platform device
Quoting Mian Yousaf Kaukab (2020-04-21 01:30:00) > Add a platform device for qoirq-cpufreq driver for the compatible > clockgen blocks. > > Reviewed-by: Yuantian Tang > Acked-by: Viresh Kumar > Signed-off-by: Mian Yousaf Kaukab > --- Acked-by: Stephen Boyd
Re: [PATCH] net/mlx5: reduce stack usage in qp_read_field
On Mon, May 04, 2020 at 05:41:56PM +0200, Arnd Bergmann wrote: > On Sun, May 3, 2020 at 7:30 AM Leon Romanovsky wrote: > > On Thu, Apr 30, 2020 at 04:37:14PM +0200, Arnd Bergmann wrote: > > > On Thu, Apr 30, 2020 at 7:22 AM Leon Romanovsky wrote: > > > > > > While warning limit is generally 1024 bytes for 32-bit architectures, > > > and 2048 bytes fro 64-bit architectures, we should probably > > > reduce the latter to something like 1280 bytes and fix up the > > > warnings that introduces. > > > > It a chicken and an egg problem, I tried to use default frame size, but > > the output of my kernel build was constantly flooded with those warnings > > and made hard to spot real issues in the code I developed. > > > > When did you last try? I usually send patches whenever I see a new > warning, so there really shouldn't be any such warnings in the mainline > kernel except for cases where patches are still under discussion. > > If you have a configuration in which you see lots of frame size warnings, > can you send me that .config file for that? It is possible that one or > more of the patches in my backlog fix a bunch of those issues > and need to be resent. I tried now (your patch is not in that branch yet). ➜ kernel git:(m/ece) mkt build Start kernel compilation in silent mode arch/x86/events/amd/ibs.c: In function ‘perf_ibs_handle_irq’: arch/x86/events/amd/ibs.c:681:1: warning: the frame size of 1152 bytes is larger than 1024 bytes [-Wframe-larger-than=] 681 | } | ^ fs/select.c: In function ‘do_select’: fs/select.c:611:1: warning: the frame size of 1136 bytes is larger than 1024 bytes [-Wframe-larger-than=] 611 | } | ^ fs/select.c: In function ‘do_sys_poll’: fs/select.c:1023:1: warning: the frame size of 1312 bytes is larger than 1024 bytes [-Wframe-larger-than=] 1023 | } | ^ drivers/video/fbdev/cirrusfb.o: warning: objtool: cirrusfb_set_par_foo.cold()+0x25f: sibling call from callable instruction with modified stack frame net/core/rtnetlink.c: In function ‘__rtnl_newlink’: net/core/rtnetlink.c:3379:1: warning: the frame size of 1280 bytes is larger than 1024 bytes [-Wframe-larger-than=] 3379 | } | ^ q drivers/acpi/processor_thermal.c: In function ‘cpu_has_cpufreq’: drivers/acpi/processor_thermal.c:66:1: warning: the frame size of 1472 bytes is larger than 1024 bytes [-Wframe-larger-than=] 66 | } | ^ drivers/tty/serial/8250/8250_core.c: In function ‘serial8250_probe’: drivers/tty/serial/8250/8250_core.c:849:1: warning: the frame size of 1144 bytes is larger than 1024 bytes [-Wframe-larger-than=] 849 | } | ^ drivers/tty/serial/8250/8250_pnp.c: In function ‘serial_pnp_probe’: drivers/tty/serial/8250/8250_pnp.c:490:1: warning: the frame size of 1120 bytes is larger than 1024 bytes [-Wframe-larger-than=] 490 | } | ^ drivers/tty/serial/8250/8250_pci.c: In function ‘pciserial_init_ports’: drivers/tty/serial/8250/8250_pci.c:3952:1: warning: the frame size of 1160 bytes is larger than 1024 bytes [-Wframe-larger-than=] 3952 | } | ^ drivers/tty/serial/8250/8250_exar.c: In function ‘exar_pci_probe’: drivers/tty/serial/8250/8250_exar.c:641:1: warning: the frame size of 1168 bytes is larger than 1024 bytes [-Wframe-larger-than=] 641 | } | ^ drivers/tty/serial/8250/8250_lpss.c: In function ‘lpss8250_probe.part.0’: drivers/tty/serial/8250/8250_lpss.c:342:1: warning: the frame size of 1152 bytes is larger than 1024 bytes [-Wframe-larger-than=] 342 | } | ^ drivers/tty/serial/8250/8250_mid.c: In function ‘mid8250_probe.part.0’: drivers/tty/serial/8250/8250_mid.c:337:1: warning: the frame size of 1128 bytes is larger than 1024 bytes [-Wframe-larger-than=] 337 | } | ^ net/ipv6/ipv6_sockglue.c: In function ‘do_ipv6_setsockopt.isra.0’: net/ipv6/ipv6_sockglue.c:937:1: warning: the frame size of 1088 bytes is larger than 1024 bytes [-Wframe-larger-than=] net/ipv6/ipv6_sockglue.c:937:1: warning: the frame size of 1088 bytes is larger than 1024 bytes [-Wframe-larger-than=] 937 | } | ^ drivers/net/tun.o: warning: objtool: tun_chr_ioctl.cold()+0x107: sibling call from callable instruction with modified stack frame drivers/net/ethernet/mellanox/mlx5/core/debugfs.c: In function ‘qp_read_field.isra.0’: drivers/net/ethernet/mellanox/mlx5/core/debugfs.c:274:1: warning: the frame size of 1312 bytes is larger than 1024 bytes [-Wframe-larger-than=] 274 | } | ^ drivers/infiniband/core/nldev.c: In function ‘nldev_newlink’: drivers/infiniband/core/nldev.c:1547:1: warning: the frame size of 1056 bytes is larger than 1024 bytes [-Wframe-larger-than=] 1547 | } | ^ drivers/infiniband/core/nldev.c: In function ‘nldev_get_chardev’: drivers/infiniband/core/nldev.c:1658:1: warning: the frame size of 1080 bytes is larger than 1024 bytes [-Wframe-larger-than=] 1658 | } | ^ drivers/infiniband/core/nldev.c: In function ‘res_get_common_dumpit’: drivers/infiniband/core/nldev.c:1443:1: warning: the frame size of 1040 b
Re: [PATCH] iommu: Relax ACS requirement for RCiEP devices.
Hi Alex
+ Joerg, accidently missed in the Cc.
On Mon, May 04, 2020 at 11:19:36PM -0600, Alex Williamson wrote:
> On Mon, 4 May 2020 21:42:16 -0700
> Ashok Raj wrote:
>
> > PCIe Spec recommends we can relax ACS requirement for RCIEP devices.
> >
> > PCIe 5.0 Specification.
> > 6.12 Access Control Services (ACS)
> > Implementation of ACS in RCiEPs is permitted but not required. It is
> > explicitly permitted that, within a single Root Complex, some RCiEPs
> > implement ACS and some do not. It is strongly recommended that Root Complex
> > implementations ensure that all accesses originating from RCiEPs
> > (PFs and VFs) without ACS capability are first subjected to processing by
> > the Translation Agent (TA) in the Root Complex before further decoding and
> > processing. The details of such Root Complex handling are outside the scope
> > of this specification.
> >
> > Since Linux didn't give special treatment to allow this exception, certain
> > RCiEP MFD devices are getting grouped in a single iommu group. This
> > doesn't permit a single device to be assigned to a guest for instance.
> >
> > In one vendor system: Device 14.x were grouped in a single IOMMU group.
> >
> > /sys/kernel/iommu_groups/5/devices/:00:14.0
> > /sys/kernel/iommu_groups/5/devices/:00:14.2
> > /sys/kernel/iommu_groups/5/devices/:00:14.3
> >
> > After the patch:
> > /sys/kernel/iommu_groups/5/devices/:00:14.0
> > /sys/kernel/iommu_groups/5/devices/:00:14.2
> > /sys/kernel/iommu_groups/6/devices/:00:14.3 <<< new group
> >
> > 14.0 and 14.2 are integrated devices, but legacy end points.
> > Whereas 14.3 was a PCIe compliant RCiEP.
> >
> > 00:14.3 Network controller: Intel Corporation Device 9df0 (rev 30)
> > Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00
> >
> > This permits assigning this device to a guest VM.
> >
> > Fixes: f096c061f552 ("iommu: Rework iommu_group_get_for_pci_dev()")
> > Signed-off-by: Ashok Raj
> > To: Joerg Roedel
> > To: Bjorn Helgaas
> > Cc: linux-kernel@vger.kernel.org
> > Cc: io...@lists.linux-foundation.org
> > Cc: Lu Baolu
> > Cc: Alex Williamson
> > Cc: Darrel Goeddel
> > Cc: Mark Scott ,
> > Cc: Romil Sharma
> > Cc: Ashok Raj
> > ---
> > drivers/iommu/iommu.c | 15 ++-
> > 1 file changed, 14 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> > index 2b471419e26c..5744bd65f3e2 100644
> > --- a/drivers/iommu/iommu.c
> > +++ b/drivers/iommu/iommu.c
> > @@ -1187,7 +1187,20 @@ static struct iommu_group
> > *get_pci_function_alias_group(struct pci_dev *pdev,
> > struct pci_dev *tmp = NULL;
> > struct iommu_group *group;
> >
> > - if (!pdev->multifunction || pci_acs_enabled(pdev, REQ_ACS_FLAGS))
> > + /*
> > +* PCI Spec 5.0, Section 6.12 Access Control Service
> > +* Implementation of ACS in RCiEPs is permitted but not required.
> > +* It is explicitly permitted that, within a single Root
> > +* Complex, some RCiEPs implement ACS and some do not. It is
> > +* strongly recommended that Root Complex implementations ensure
> > +* that all accesses originating from RCiEPs (PFs and VFs) without
> > +* ACS capability are first subjected to processing by the Translation
> > +* Agent (TA) in the Root Complex before further decoding and
> > +* processing.
> > +*/
>
> Is the language here really strong enough to make this change? ACS is
> an optional feature, so being permitted but not required is rather
> meaningless. The spec is also specifically avoiding the words "must"
> or "shall" and even when emphasized with "strongly", we still only have
> a recommendation that may or may not be honored. This seems like a
> weak basis for assuming that RCiEPs universally honor this
> recommendation. Thanks,
>
We are speaking about PCIe spec, where people write it about 5 years ahead
and every vendor tries to massage their product behavior with vague
words like this.. :)
But honestly for any any RCiEP, or even integrated endpoints, there
is no way to send them except up north. These aren't behind a RP.
I did check with couple folks who are part of the SIG, and seem to agree
that ACS treatment for RCiEP's doesn't mean much.
I understand the language isn't strong, but it doesn't seem like ACS should
be a strong requirement for RCiEP's and reasonable to relax.
What are your thoughts?
Cheers,
Ashok
>
> > + if (!pdev->multifunction ||
> > + (pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END) ||
> > +pci_acs_enabled(pdev, REQ_ACS_FLAGS))
> > return NULL;
> >
> > for_each_pci_dev(tmp) {
>
Re: [PATCH] vfio-pci: Mask cap zero
On Mon, 4 May 2020 17:03:54 -0600
Alex Williamson wrote:
> On Mon, 4 May 2020 15:08:08 -0700
> Neo Jia wrote:
>
> > On Mon, May 04, 2020 at 12:52:53PM -0600, Alex Williamson wrote:
> > > External email: Use caution opening links or attachments
> > >
> > >
> > > On Mon, 4 May 2020 18:09:16 +0200
> > > Cornelia Huck wrote:
> > >
> > > > On Fri, 01 May 2020 15:41:24 -0600
> > > > Alex Williamson wrote:
> > > >
> > > > > There is no PCI spec defined capability with ID 0, therefore we don't
> > > > > expect to find it in a capability chain and we use this index in an
> > > > > internal array for tracking the sizes of various capabilities to
> > > > > handle
> > > > > standard config space. Therefore if a device does present us with a
> > > > > capability ID 0, we mark our capability map with nonsense that can
> > > > > trigger conflicts with other capabilities in the chain. Ignore ID 0
> > > > > when walking the capability chain, handling it as a hidden capability.
> > > > >
> > > > > Seen on an NVIDIA Tesla T4.
> > > > >
> > > > > Signed-off-by: Alex Williamson
> > > > > ---
> > > > > drivers/vfio/pci/vfio_pci_config.c |2 +-
> > > > > 1 file changed, 1 insertion(+), 1 deletion(-)
> > > > >
> > > > > diff --git a/drivers/vfio/pci/vfio_pci_config.c
> > > > > b/drivers/vfio/pci/vfio_pci_config.c
> > > > > index 87d0cc8c86ad..5935a804cb88 100644
> > > > > --- a/drivers/vfio/pci/vfio_pci_config.c
> > > > > +++ b/drivers/vfio/pci/vfio_pci_config.c
> > > > > @@ -1487,7 +1487,7 @@ static int vfio_cap_init(struct vfio_pci_device
> > > > > *vdev)
> > > > > if (ret)
> > > > > return ret;
> > > > >
> > > > > - if (cap <= PCI_CAP_ID_MAX) {
> > > >
> > > > Maybe add a comment:
> > > >
> > > > /* no PCI spec defined capability with ID 0: hide it */
> >
> > Hi Alex,
> >
> > I think this is NULL Capability defined in Codes and IDs spec, probably we
> > should just add a new enum to represent that?
>
> Yes, it looks like the 1.1 version of that specification from June 2015
> changed ID 0 from reserved to a NULL capability. So my description and
> this comment are wrong, but I wonder if we should did anything
> different with the handling of this capability. It's specified to
> contain only the ID and next pointer, so I'd expect it's primarily a
> mechanism for hardware vendors to blow fuses in config space to
> maintain a capability chain while maybe hiding a feature not supported
> by the product sku. Hiding the capability in vfio is trivial, exposing
> it implies some changes to our config space map that might be more
> subtle. I'm inclined to stick with this solution for now. Thanks,
>
> Alex
From this description, I also think that we should simply hide these
NULL capabilities.
Re: [PATCH net] net: dsa: Do not leave DSA master with NULL netdev_ops
When ndo_get_phys_port_name() for the CPU port was added we introduced
an early check for when the DSA master network device in
dsa_master_ndo_setup() already implements ndo_get_phys_port_name(). When
we perform the teardown operation in dsa_master_ndo_teardown() we would
not be checking that cpu_dp->orig_ndo_ops was successfully allocated and
non-NULL initialized.
With network device drivers such as virtio_net, this leads to a NPD as
soon as the DSA switch hanging off of it gets torn down because we are
now assigning the virtio_net device's netdev_ops a NULL pointer.
Fixes: da7b9e9b00d4 ("net: dsa: Add ndo_get_phys_port_name() for CPU port")
Reported-by: Allen Pais
Signed-off-by: Florian Fainelli
Tested-by: Allen Pais
Thank you Florain.
---
net/dsa/master.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/net/dsa/master.c b/net/dsa/master.c
index b5c535af63a3..a621367c6e8c 100644
--- a/net/dsa/master.c
+++ b/net/dsa/master.c
@@ -289,7 +289,8 @@ static void dsa_master_ndo_teardown(struct net_device *dev)
{
struct dsa_port *cpu_dp = dev->dsa_ptr;
- dev->netdev_ops = cpu_dp->orig_ndo_ops;
+ if (cpu_dp->orig_ndo_ops)
+ dev->netdev_ops = cpu_dp->orig_ndo_ops;
cpu_dp->orig_ndo_ops = NULL;
}
Re: [RFC PATCH 00/61] fscache, cachefiles: Rewrite the I/O interface in terms of kiocb/iov_iter
Can you split this into a few smaller series? > 60 patches is beyond reviewer comprehension.
Re: [PATCH V4 1/5] dt-bindings: clock: Convert i.MX6Q clock to json-schema
Quoting Anson Huang (2020-04-29 17:51:57) > Convert the i.MX6Q clock binding to DT schema format using json-schema. > > Signed-off-by: Anson Huang > --- Please send a cover letter next time. Who is going to apply these patches? I hope Rob? If you need an acked-by feel free to have mine! Acked-by: Stephen Boyd
Re: [PATCH] sysctl: Make sure proc handlers can't expose heap memory
On Mon, May 04, 2020 at 12:08:55PM -0700, Kees Cook wrote: > Just as a precaution, make sure that proc handlers don't accidentally > grow "count" beyond the allocated kbuf size. > > Signed-off-by: Kees Cook > --- > This applies to hch's sysctl cleanup tree... This looks ok o me. You should probably add Al to the Cc list as he has picked up my series into a branch of vfs.git. Acked-by: Christoph Hellwig
Re: [PATCH 5/5] sysctl: pass kernel pointers to ->proc_handler
On Mon, May 04, 2020 at 12:01:11PM -0700, Kees Cook wrote: > > if (error) > > - goto out; > > + goto out_free_buf; > > > > /* careful: calling conventions are nasty here */ > > Is this comment still valid after doing these cleanups? The comment is pretty old so I decided to keep it. That being said I'm not sure it really is very helpful.
Re: [PATCH] drm: Replace drm_modeset_lock/unlock_all with DRM_MODESET_LOCK_ALL_* helpers
On 04.05.2020 13:53, Daniel Vetter wrote:
> On Fri, May 01, 2020 at 05:49:33PM +0200, Michał Orzeł wrote:
>>
>>
>> On 30.04.2020 20:30, Daniel Vetter wrote:
>>> On Thu, Apr 30, 2020 at 5:38 PM Sean Paul wrote:
On Wed, Apr 29, 2020 at 4:57 AM Jani Nikula
wrote:
>
> On Tue, 28 Apr 2020, Michal Orzel wrote:
>> As suggested by the TODO list for the kernel DRM subsystem, replace
>> the deprecated functions that take/drop modeset locks with new helpers.
>>
>> Signed-off-by: Michal Orzel
>> ---
>> drivers/gpu/drm/drm_mode_object.c | 10 ++
>> 1 file changed, 6 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_mode_object.c
>> b/drivers/gpu/drm/drm_mode_object.c
>> index 35c2719..901b078 100644
>> --- a/drivers/gpu/drm/drm_mode_object.c
>> +++ b/drivers/gpu/drm/drm_mode_object.c
>> @@ -402,12 +402,13 @@ int drm_mode_obj_get_properties_ioctl(struct
>> drm_device *dev, void *data,
>> {
>> struct drm_mode_obj_get_properties *arg = data;
>> struct drm_mode_object *obj;
>> + struct drm_modeset_acquire_ctx ctx;
>> int ret = 0;
>>
>> if (!drm_core_check_feature(dev, DRIVER_MODESET))
>> return -EOPNOTSUPP;
>>
>> - drm_modeset_lock_all(dev);
>> + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>
> I cry a little every time I look at the DRM_MODESET_LOCK_ALL_BEGIN and
> DRM_MODESET_LOCK_ALL_END macros. :(
>
> Currently only six users... but there are ~60 calls to
> drm_modeset_lock_all{,_ctx} that I presume are to be replaced. I wonder
> if this will come back and haunt us.
>
What's the alternative? Seems like the options without the macros is
to use incorrect scope or have a bunch of retry/backoff cargo-cult
everywhere (and hope the copy source is done correctly).
>>>
>>> Yeah Sean & me had a bunch of bikesheds and this is the least worst
>>> option we could come up with. You can't make it a function because of
>>> the control flow. You don't want to open code this because it's tricky
>>> to get right, if all you want is to just grab all locks. But it is
>>> magic hidden behind a macro, which occasionally ends up hurting.
>>> -Daniel
>> So what are we doing with this problem? Should we replace at once approx. 60
>> calls?
>
> I'm confused by your question - dradual conversion is entirely orthogonal
> to what exactly we're converting too. All I added here is that we've
> discussed this at length, and the macro is the best thing we've come up
> with. I still think it's the best compromise.
>
> Flag-day conversion for over 60 calls doesn't work, no matter what.
> -Daniel
>
I agree with that. All I wanted to ask was whether I should add something
additional to this patch or not.
Thanks,
Michal
>>
>> Michal
>>>
Sean
> BR,
> Jani.
>
>
>>
>> obj = drm_mode_object_find(dev, file_priv, arg->obj_id,
>> arg->obj_type);
>> if (!obj) {
>> @@ -427,7 +428,7 @@ int drm_mode_obj_get_properties_ioctl(struct
>> drm_device *dev, void *data,
>> out_unref:
>> drm_mode_object_put(obj);
>> out:
>> - drm_modeset_unlock_all(dev);
>> + DRM_MODESET_LOCK_ALL_END(ctx, ret);
>> return ret;
>> }
>>
>> @@ -449,12 +450,13 @@ static int set_property_legacy(struct
>> drm_mode_object *obj,
>> {
>> struct drm_device *dev = prop->dev;
>> struct drm_mode_object *ref;
>> + struct drm_modeset_acquire_ctx ctx;
>> int ret = -EINVAL;
>>
>> if (!drm_property_change_valid_get(prop, prop_value, &ref))
>> return -EINVAL;
>>
>> - drm_modeset_lock_all(dev);
>> + DRM_MODESET_LOCK_ALL_BEGIN(dev, ctx, 0, ret);
>> switch (obj->type) {
>> case DRM_MODE_OBJECT_CONNECTOR:
>> ret = drm_connector_set_obj_prop(obj, prop, prop_value);
>> @@ -468,7 +470,7 @@ static int set_property_legacy(struct
>> drm_mode_object *obj,
>> break;
>> }
>> drm_property_change_valid_put(prop, ref);
>> - drm_modeset_unlock_all(dev);
>> + DRM_MODESET_LOCK_ALL_END(ctx, ret);
>>
>> return ret;
>> }
>
> --
> Jani Nikula, Intel Open Source Graphics Center
>>>
>>>
>>>
>
[PATCH 3/4] dt-bindings: i2c-stm32: add SMBus Alert bindings
Add a new binding of the i2c-stm32f7 driver to enable the handling of the SMBUS-Alert Signed-off-by: Alain Volmat --- Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml | 4 1 file changed, 4 insertions(+) diff --git a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml index b50a2f420b36..04c0882c3661 100644 --- a/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml +++ b/Documentation/devicetree/bindings/i2c/st,stm32-i2c.yaml @@ -36,6 +36,10 @@ allOf: minItems: 3 maxItems: 3 +st,smbus-alert: + description: Enable the SMBus Alert feature + $ref: /schemas/types.yaml#/definitions/flag + - if: properties: compatible: -- 2.17.1
[PATCH 2/4] i2c: addition of client reg/unreg callbacks
Addition of two callbacks reg_client and unreg_client that can be
implemented by adapter drivers in order to take action whenever a
client is being registered to it.
Signed-off-by: Alain Volmat
---
drivers/i2c/i2c-core-base.c | 11 +++
include/linux/i2c.h | 6 ++
2 files changed, 17 insertions(+)
diff --git a/drivers/i2c/i2c-core-base.c b/drivers/i2c/i2c-core-base.c
index 2e4560671183..4c84c6264314 100644
--- a/drivers/i2c/i2c-core-base.c
+++ b/drivers/i2c/i2c-core-base.c
@@ -319,6 +319,12 @@ static int i2c_device_probe(struct device *dev)
if (!client)
return 0;
+ if (client->adapter->algo->reg_client) {
+ status = client->adapter->algo->reg_client(client);
+ if (status)
+ return status;
+ }
+
driver = to_i2c_driver(dev->driver);
client->irq = client->init_irq;
@@ -417,6 +423,8 @@ static int i2c_device_probe(struct device *dev)
put_sync_adapter:
if (client->flags & I2C_CLIENT_HOST_NOTIFY)
pm_runtime_put_sync(&client->adapter->dev);
+ if (client->adapter->algo->reg_client)
+ client->adapter->algo->unreg_client(client);
return status;
}
@@ -445,6 +453,9 @@ static int i2c_device_remove(struct device *dev)
if (client->flags & I2C_CLIENT_HOST_NOTIFY)
pm_runtime_put(&client->adapter->dev);
+ if (client->adapter->algo->unreg_client)
+ client->adapter->algo->unreg_client(client);
+
return status;
}
diff --git a/include/linux/i2c.h b/include/linux/i2c.h
index 45d36ba4826b..61b838caf454 100644
--- a/include/linux/i2c.h
+++ b/include/linux/i2c.h
@@ -509,6 +509,8 @@ i2c_register_board_info(int busnum, struct i2c_board_info
const *info,
* so e.g. PMICs can be accessed very late before shutdown. Optional.
* @functionality: Return the flags that this algorithm/adapter pair supports
* from the ``I2C_FUNC_*`` flags.
+ * @reg_client: Callback informing that a new client is being registered
+ * @unreg_client: Callback informing that a client is being removed
* @reg_slave: Register given client to I2C slave mode of this adapter
* @unreg_slave: Unregister given client from I2C slave mode of this adapter
*
@@ -545,6 +547,10 @@ struct i2c_algorithm {
/* To determine what the adapter supports */
u32 (*functionality)(struct i2c_adapter *adap);
+ /* To inform the adapter of the probe/remove of a client */
+ int (*reg_client)(struct i2c_client *client);
+ void (*unreg_client)(struct i2c_client *client);
+
#if IS_ENABLED(CONFIG_I2C_SLAVE)
int (*reg_slave)(struct i2c_client *client);
int (*unreg_slave)(struct i2c_client *client);
--
2.17.1
[PATCH 1/4] i2c: smbus: add core function handling SMBus host-notify
SMBus Host-Notify protocol, from the adapter point of view
consist of receiving a message from a client, including the
client address and some other data.
It can be simply handled by creating a new slave device
and registering a callback performing the parsing of the
message received from the client.
This commit introduces two new core functions
* i2c_new_smbus_host_notify_device
* i2c_free_smbus_host_notify_device
that take care of registration of the new slave device and
callback and will call i2c_handle_smbus_host_notify once a
Host-Notify event is received.
Signed-off-by: Alain Volmat
---
drivers/i2c/i2c-core-smbus.c | 105 +++
include/linux/i2c-smbus.h| 2 +
2 files changed, 107 insertions(+)
diff --git a/drivers/i2c/i2c-core-smbus.c b/drivers/i2c/i2c-core-smbus.c
index b34d2ff06931..0c7e135c73e1 100644
--- a/drivers/i2c/i2c-core-smbus.c
+++ b/drivers/i2c/i2c-core-smbus.c
@@ -708,3 +708,108 @@ int of_i2c_setup_smbus_alert(struct i2c_adapter *adapter)
}
EXPORT_SYMBOL_GPL(of_i2c_setup_smbus_alert);
#endif
+
+struct i2c_smbus_host_notify_status {
+ bool notify_start;
+ u8 addr;
+};
+
+static int i2c_smbus_host_notify_cb(struct i2c_client *client,
+ enum i2c_slave_event event, u8 *val)
+{
+ struct i2c_smbus_host_notify_status *status = client->dev.platform_data;
+ int ret;
+
+ switch (event) {
+ case I2C_SLAVE_WRITE_REQUESTED:
+ status->notify_start = true;
+ break;
+ case I2C_SLAVE_WRITE_RECEIVED:
+ /* We only retrieve the first byte received (addr)
+* since there is currently no way to retrieve the data
+* parameter from the client.
+*/
+ if (!status->notify_start)
+ break;
+ status->addr = *val;
+ status->notify_start = false;
+ break;
+ case I2C_SLAVE_STOP:
+ ret = i2c_handle_smbus_host_notify(client->adapter,
+ status->addr);
+ if (ret < 0) {
+ dev_warn(&client->adapter->dev, "failed to handle
host_notify (%d)\n",
+ ret);
+ return ret;
+ }
+ break;
+ default:
+ /* Only handle necessary events */
+ break;
+ }
+
+ return 0;
+}
+
+/**
+ * i2c_new_smbus_host_notify_device - get a client for SMBus host-notify
support
+ * @adapter: the target adapter
+ * Context: can sleep
+ *
+ * Setup handling of the SMBus host-notify protocol on a given I2C bus segment.
+ *
+ * Handling is done by creating a device and its callback and handling data
+ * received via the SMBus host-notify address (0x8)
+ *
+ * This returns the client, which should be ultimately freed using
+ * i2c_free_smbus_host_notify_device(); or an ERRPTR to indicate an error.
+ */
+struct i2c_client *i2c_new_smbus_host_notify_device(struct i2c_adapter
*adapter)
+{
+ struct i2c_board_info host_notify_board_info = {
+ I2C_BOARD_INFO("smbus_host_notify", 0x08),
+ .flags = I2C_CLIENT_SLAVE,
+ };
+ struct i2c_smbus_host_notify_status *status;
+ struct i2c_client *client;
+ int ret;
+
+ status = kzalloc(sizeof(struct i2c_smbus_host_notify_status),
+GFP_KERNEL);
+ if (!status)
+ return ERR_PTR(-ENOMEM);
+
+ host_notify_board_info.platform_data = status;
+
+ client = i2c_new_client_device(adapter, &host_notify_board_info);
+ if (IS_ERR(client)) {
+ kfree(status);
+ return client;
+ }
+
+ ret = i2c_slave_register(client, i2c_smbus_host_notify_cb);
+ if (ret) {
+ i2c_unregister_device(client);
+ kfree(status);
+ return ERR_PTR(ret);
+ }
+
+ return client;
+}
+EXPORT_SYMBOL_GPL(i2c_new_smbus_host_notify_device);
+
+/**
+ * i2c_free_smbus_host_notify_device - free the client for SMBus host-notify
+ * support
+ * @client: the client to free
+ * Context: can sleep
+ *
+ * Free the i2c_client allocated via i2c_new_smbus_host_notify_device
+ */
+void i2c_free_smbus_host_notify_device(struct i2c_client *client)
+{
+ i2c_slave_unregister(client);
+ kfree(client->dev.platform_data);
+ i2c_unregister_device(client);
+}
+EXPORT_SYMBOL_GPL(i2c_free_smbus_host_notify_device);
diff --git a/include/linux/i2c-smbus.h b/include/linux/i2c-smbus.h
index 8c5459034f92..926f6d8ae30d 100644
--- a/include/linux/i2c-smbus.h
+++ b/include/linux/i2c-smbus.h
@@ -38,5 +38,7 @@ static inline int of_i2c_setup_smbus_alert(struct i2c_adapter
*adap)
return 0;
}
#endif
+struct i2c_client *i2c_new_smbus_host_notify_device(struct i2c_adapter
*adapter);
+void i2c_free_smbus_host_notify_device(struct i2c_client *client);
#e
[PATCH 0/4] stm32-f7: Addition of SMBus Alert / Host-notify features
This serie adds SMBus Alert and SMBus Host-Notify features for the i2c-stm32f7. For that purpore, I propose two enhancements to the i2c framework. 1. Addition of host-notify client handling as part of the i2c-core-smbus so that any other i2c adapter can benefit from it, even those without specific HW support for Host-Notify. 2. Addition of two i2c_algorithm reg_client and unreg_client allowing adapter to take some actions when a client is being probed. Indeed, in case of host-notify, the binding/flag is related to the client and the adapter might want to enable the host-notify feature only when there is a client requesting the host-notify. (Since SMBus host-notify is using the address 0x08 which is a valid I2C address, keeping host-notify always enabled would make the 0x08 I2C slave address non usable). Alain Volmat (4): i2c: smbus: add core function handling SMBus host-notify i2c: addition of client reg/unreg callbacks dt-bindings: i2c-stm32: add SMBus Alert bindings i2c: stm32f7: Add SMBus-specific protocols support .../devicetree/bindings/i2c/st,stm32-i2c.yaml | 4 + drivers/i2c/busses/Kconfig| 1 + drivers/i2c/busses/i2c-stm32f7.c | 198 +- drivers/i2c/i2c-core-base.c | 11 + drivers/i2c/i2c-core-smbus.c | 105 ++ include/linux/i2c-smbus.h | 2 + include/linux/i2c.h | 6 + 7 files changed, 317 insertions(+), 10 deletions(-) -- 2.17.1
[PATCH 4/4] i2c: stm32f7: Add SMBus-specific protocols support
This patch adds the support for SMBus Host notify and SMBus Alert
extensions protocols
Signed-off-by: Alain Volmat
---
drivers/i2c/busses/Kconfig | 1 +
drivers/i2c/busses/i2c-stm32f7.c | 198 +--
2 files changed, 189 insertions(+), 10 deletions(-)
diff --git a/drivers/i2c/busses/Kconfig b/drivers/i2c/busses/Kconfig
index 2f6e39b41e6c..b82c2f7d7d50 100644
--- a/drivers/i2c/busses/Kconfig
+++ b/drivers/i2c/busses/Kconfig
@@ -1024,6 +1024,7 @@ config I2C_STM32F7
tristate "STMicroelectronics STM32F7 I2C support"
depends on ARCH_STM32 || COMPILE_TEST
select I2C_SLAVE
+ select I2C_SMBUS
help
Enable this option to add support for STM32 I2C controller embedded
in STM32F7 SoCs.
diff --git a/drivers/i2c/busses/i2c-stm32f7.c b/drivers/i2c/busses/i2c-stm32f7.c
index 9c9e10ea9199..6d02ddbc1ab4 100644
--- a/drivers/i2c/busses/i2c-stm32f7.c
+++ b/drivers/i2c/busses/i2c-stm32f7.c
@@ -14,10 +14,12 @@
* This driver is based on i2c-stm32f4.c
*
*/
+#include
#include
#include
#include
#include
+#include
#include
#include
#include
@@ -50,6 +52,8 @@
/* STM32F7 I2C control 1 */
#define STM32F7_I2C_CR1_PECEN BIT(23)
+#define STM32F7_I2C_CR1_ALERTENBIT(22)
+#define STM32F7_I2C_CR1_SMBHEN BIT(20)
#define STM32F7_I2C_CR1_WUPEN BIT(18)
#define STM32F7_I2C_CR1_SBCBIT(16)
#define STM32F7_I2C_CR1_RXDMAENBIT(15)
@@ -121,6 +125,7 @@
(((n) & STM32F7_I2C_ISR_ADDCODE_MASK) >> 17)
#define STM32F7_I2C_ISR_DIRBIT(16)
#define STM32F7_I2C_ISR_BUSY BIT(15)
+#define STM32F7_I2C_ISR_ALERT BIT(13)
#define STM32F7_I2C_ISR_PECERR BIT(11)
#define STM32F7_I2C_ISR_ARLO BIT(9)
#define STM32F7_I2C_ISR_BERR BIT(8)
@@ -134,6 +139,7 @@
#define STM32F7_I2C_ISR_TXEBIT(0)
/* STM32F7 I2C Interrupt Clear */
+#define STM32F7_I2C_ICR_ALERTCFBIT(13)
#define STM32F7_I2C_ICR_PECCF BIT(11)
#define STM32F7_I2C_ICR_ARLOCF BIT(9)
#define STM32F7_I2C_ICR_BERRCF BIT(8)
@@ -150,7 +156,7 @@
#define STM32F7_I2C_MAX_LEN0xff
#define STM32F7_I2C_DMA_LEN_MIN0x16
-#define STM32F7_I2C_MAX_SLAVE 0x2
+#define STM32F7_I2C_MAX_SLAVE 0x3
#define STM32F7_I2C_DNF_DEFAULT0
#define STM32F7_I2C_DNF_MAX16
@@ -274,6 +280,29 @@ struct stm32f7_i2c_msg {
u8 smbus_buf[I2C_SMBUS_BLOCK_MAX + 3] __aligned(4);
};
+/**
+ * struct stm32f7_i2c_host - SMBus host specific data
+ * @client: I2C slave device that represents SMBus host
+ * @notify_start: indicate that this is the start of the notify transaction
+ * @addr: device address of SMBus device that initiate SMBus host protocol
+ */
+struct stm32f7_i2c_host {
+ struct i2c_client *client;
+ bool notify_start;
+ u8 addr;
+};
+
+/**
+ * struct stm32f7_i2c_alert - SMBus alert specific data
+ * @setup: platform data for the smbus_alert i2c client
+ * @ara: I2C slave device used to respond to the SMBus Alert with Alert
+ * Response Address
+ */
+struct stm32f7_i2c_alert {
+ struct i2c_smbus_alert_setup setup;
+ struct i2c_client *ara;
+};
+
/**
* struct stm32f7_i2c_dev - private data of the controller
* @adap: I2C adapter for this controller
@@ -301,6 +330,9 @@ struct stm32f7_i2c_msg {
* @fmp_creg: register address for clearing Fast Mode Plus bits
* @fmp_mask: mask for Fast Mode Plus bits in set register
* @wakeup_src: boolean to know if the device is a wakeup source
+ * @host_notify_cnt: atomic to know number of host_notify enabled clients
+ * @host_notify_client: SMBus host-notify client
+ * @alert: SMBus alert specific data
*/
struct stm32f7_i2c_dev {
struct i2c_adapter adap;
@@ -327,6 +359,9 @@ struct stm32f7_i2c_dev {
u32 fmp_creg;
u32 fmp_mask;
bool wakeup_src;
+ atomic_t host_notify_cnt;
+ struct i2c_client *host_notify_client;
+ struct stm32f7_i2c_alert *alert;
};
/*
@@ -1321,10 +1356,20 @@ static int stm32f7_i2c_get_free_slave_id(struct
stm32f7_i2c_dev *i2c_dev,
int i;
/*
-* slave[0] supports 7-bit and 10-bit slave address
-* slave[1] supports 7-bit slave address only
+* slave[0] support only SMBus Host address (0x8)
+* slave[1] supports 7-bit and 10-bit slave address
+* slave[2] supports 7-bit slave address only
*/
- for (i = STM32F7_I2C_MAX_SLAVE - 1; i >= 0; i--) {
+ if (atomic_read(&i2c_dev->host_notify_cnt)) {
+ if (slave->addr == 0x08) {
+ if (i2c_dev->slave[0])
+ goto fail;
+
Re: [v3] nfp: abm: Fix incomplete release of system resources in nfp_abm_vnic_set_mac()
> Thanks for your feedback, and yes, I'd like to further adjust the description > details > to make the patch more clear and better. Thanks for such a positive response. > But because Jakub seems to prefer v1, so I'm somehow confused Such a view can be reasonable. The change acceptance varies between involved contributors. > if I should submit a new version based on v1 Such a choice depends also on your willingness to clarify and improve the software situation by better commit messages. > or based on the version that has an addition of a jump target? I propose that you can offer patch series with two update steps (for example) according to the affected function implementation. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=47cf1b422e6093aee2a3e55d5e162112a2c69870#n102 1. Add a missing function call for the completion of the desired resource clean-up. 2. Adjust a bit of exception handling code so that it can be better reused at the end of this function. https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=47cf1b422e6093aee2a3e55d5e162112a2c69870#n450 I am curious if you are going to answer (my) previous questions and suggestions (in constructive ways). Regards, Markus
Re: [PATCH] Fix all coding-style warnings on lm75 driver
On 05.05.2020 03:58, Guenter Roeck wrote:
> On Thu, Apr 30, 2020 at 04:05:34PM +0200, Michal Orzel wrote:
>> Check/fix all warnings generated by checkpatch.pl script on LM75 driver.
>>
>> Signed-off-by: Michal Orzel
>
> Applied, but for the future please prepend your patches with something like
> "subsystem: driver:", or for hwmon "hwmon: (driver)".
>
> Also, please keep in mind that such cleanups are not encouraged unless you
> also provide functional changes.
>
> Thanks,
> Guenter
>
I will keep it in mind.
Thanks,
Michal
>> ---
>> drivers/hwmon/lm75.c | 8 ++--
>> drivers/hwmon/lm75.h | 31 +--
>> 2 files changed, 23 insertions(+), 16 deletions(-)
>>
>> diff --git a/drivers/hwmon/lm75.c b/drivers/hwmon/lm75.c
>> index 5e63922..ba0be48 100644
>> --- a/drivers/hwmon/lm75.c
>> +++ b/drivers/hwmon/lm75.c
>> @@ -797,8 +797,10 @@ static int lm75_detect(struct i2c_client *new_client,
>>
>> /* First check for LM75A */
>> if (i2c_smbus_read_byte_data(new_client, 7) == LM75A_ID) {
>> -/* LM75A returns 0xff on unused registers so
>> - just to be sure we check for that too. */
>> +/*
>> + * LM75A returns 0xff on unused registers so
>> + * just to be sure we check for that too.
>> + */
>> if (i2c_smbus_read_byte_data(new_client, 4) != 0xff
>> || i2c_smbus_read_byte_data(new_client, 5) != 0xff
>> || i2c_smbus_read_byte_data(new_client, 6) != 0xff)
>> @@ -849,6 +851,7 @@ static int lm75_suspend(struct device *dev)
>> {
>> int status;
>> struct i2c_client *client = to_i2c_client(dev);
>> +
>> status = i2c_smbus_read_byte_data(client, LM75_REG_CONF);
>> if (status < 0) {
>> dev_dbg(&client->dev, "Can't read config? %d\n", status);
>> @@ -863,6 +866,7 @@ static int lm75_resume(struct device *dev)
>> {
>> int status;
>> struct i2c_client *client = to_i2c_client(dev);
>> +
>> status = i2c_smbus_read_byte_data(client, LM75_REG_CONF);
>> if (status < 0) {
>> dev_dbg(&client->dev, "Can't read config? %d\n", status);
>> diff --git a/drivers/hwmon/lm75.h b/drivers/hwmon/lm75.h
>> index b614e63..a398171 100644
>> --- a/drivers/hwmon/lm75.h
>> +++ b/drivers/hwmon/lm75.h
>> @@ -1,17 +1,15 @@
>> /* SPDX-License-Identifier: GPL-2.0-or-later */
>> /*
>> -lm75.h - Part of lm_sensors, Linux kernel modules for hardware
>> - monitoring
>> -Copyright (c) 2003 Mark M. Hoffman
>> -
>> -*/
>> + * lm75.h - Part of lm_sensors, Linux kernel modules for hardware monitoring
>> + * Copyright (c) 2003 Mark M. Hoffman
>> + */
>>
>> /*
>> -This file contains common code for encoding/decoding LM75 type
>> -temperature readings, which are emulated by many of the chips
>> -we support. As the user is unlikely to load more than one driver
>> -which contains this code, we don't worry about the wasted space.
>> -*/
>> + * This file contains common code for encoding/decoding LM75 type
>> + * temperature readings, which are emulated by many of the chips
>> + * we support. As the user is unlikely to load more than one driver
>> + * which contains this code, we don't worry about the wasted space.
>> + */
>>
>> #include
>>
>> @@ -20,18 +18,23 @@
>> #define LM75_TEMP_MAX 125000
>> #define LM75_SHUTDOWN 0x01
>>
>> -/* TEMP: 0.001C/bit (-55C to +125C)
>> - REG: (0.5C/bit, two's complement) << 7 */
>> +/*
>> + * TEMP: 0.001C/bit (-55C to +125C)
>> + * REG: (0.5C/bit, two's complement) << 7
>> + */
>> static inline u16 LM75_TEMP_TO_REG(long temp)
>> {
>> int ntemp = clamp_val(temp, LM75_TEMP_MIN, LM75_TEMP_MAX);
>> +
>> ntemp += (ntemp < 0 ? -250 : 250);
>> return (u16)((ntemp / 500) << 7);
>> }
>>
>> static inline int LM75_TEMP_FROM_REG(u16 reg)
>> {
>> -/* use integer division instead of equivalent right shift to
>> - guarantee arithmetic shift and preserve the sign */
>> +/*
>> + * use integer division instead of equivalent right shift to
>> + * guarantee arithmetic shift and preserve the sign
>> + */
>> return ((s16)reg / 128) * 500;
>> }
>> --
>> 2.7.4
>>
Re: [PATCH] slub: limit count of partial slabs scanned to gather statistics
On 04/05/2020 22.56, Andrew Morton wrote:
On Mon, 04 May 2020 19:07:39 +0300 Konstantin Khlebnikov
wrote:
To get exact count of free and used objects slub have to scan list of
partial slabs. This may take at long time. Scanning holds spinlock and
blocks allocations which move partial slabs to per-cpu lists and back.
Example found in the wild:
# cat /sys/kernel/slab/dentry/partial
14478538 N0=7329569 N1=7148969
# time cat /sys/kernel/slab/dentry/objects
286225471 N0=136967768 N1=149257703
real0m1.722s
user0m0.001s
sys 0m1.721s
I assume this could trigger the softlockup detector or even NMI
watchdog in some situations?
Yes, irqs are disabled here. But loop itself is pretty fast.
It requires terabytes of ram to reach common thresholds for watchdogs.
The same problem in slab was addressed in commit f728b0a5d72a ("mm, slab:
faster active and free stats") by adding more kmem cache statistics.
For slub same approach requires atomic op on fast path when object frees.
Let's simply limit count of scanned slabs and print warning.
Limit set in /sys/module/slub/parameters/max_partial_to_count.
Default is 1 which should be enough for most sane cases.
Return linear approximation if list of partials is longer than limit.
Nobody should notice difference.
That's a pretty sad "solution" :(
But I guess it's better than nothing at all, unless there are
alternative ideas?
Running this loop till the end adds more problems than gives information.
Adding new percpu or atomic counters to fast paths seems redundant even for
debugging.
Actually there is no much sense in accurate statistics for count of objects,
when there are millions of them.
Memory consumption here is defined by count and size of slabs.
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -2407,16 +2407,29 @@ static inline unsigned long node_nr_objs(struct
kmem_cache_node *n)
#endif /* CONFIG_SLUB_DEBUG */
#if defined(CONFIG_SLUB_DEBUG) || defined(CONFIG_SYSFS)
+
+static unsigned long max_partial_to_count __read_mostly = 1;
+module_param(max_partial_to_count, ulong, 0644);
+
static unsigned long count_partial(struct kmem_cache_node *n,
int (*get_count)(struct page *))
{
+ unsigned long counted = 0;
unsigned long flags;
unsigned long x = 0;
struct page *page;
spin_lock_irqsave(&n->list_lock, flags);
- list_for_each_entry(page, &n->partial, slab_list)
+ list_for_each_entry(page, &n->partial, slab_list) {
x += get_count(page);
+
+ if (++counted > max_partial_to_count) {
+ pr_warn_once("SLUB: too much partial slabs to count all
objects, increase max_partial_to_count.\n");
+ /* Approximate total count of objects */
+ x = mult_frac(x, n->nr_partial, counted);
+ break;
+ }
+ }
spin_unlock_irqrestore(&n->list_lock, flags);
return x;
}
Re: [PATCH] drm/bridge: ti-sn65dsi86: Implement lane reordering + polarity
Quoting Douglas Anderson (2020-05-04 21:36:31)
> The ti-sn65dsi86 MIPI DSI to eDP bridge chip supports arbitrary
> remapping of eDP lanes and also polarity inversion. Both of these
> features have been described in the device tree bindings for the
> device since the beginning but were never implemented in the driver.
> Implement both of them.
>
> Part of this change also allows you to (via the same device tree
> bindings) specify to use fewer than the max number of DP lanes that
> the panel reports. This could be useful if your display supports more
> lanes but only a few are hooked up on your board.
>
> Signed-off-by: Douglas Anderson
> ---
Except for one thing below:
Reviewed-by: Stephen Boyd
> diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> index 1a125423eb07..52cca54b525f 100644
> --- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> +++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
> @@ -707,26 +716,20 @@ static void ti_sn_bridge_enable(struct drm_bridge
> *bridge)
> int dp_rate_idx;
> unsigned int val;
> int ret = -EINVAL;
> + int max_dp_lanes;
>
> - /*
> -* Run with the maximum number of lanes that the DP sink supports.
> -*
> -* Depending use cases, we might want to revisit this later because:
> -* - It's plausible that someone may have run fewer lines to the
> -* sink than the sink actually supports, assuming that the lines
> -* will just be driven at a higher rate.
> -* - The DP spec seems to indicate that it's more important to
> minimize
> -* the number of lanes than the link rate.
> -*
> -* If we do revisit, it would be important to measure the power
> impact.
> -*/
> - pdata->dp_lanes = ti_sn_get_max_lanes(pdata);
> + max_dp_lanes = ti_sn_get_max_lanes(pdata);
> + pdata->dp_lanes = min(pdata->dp_lanes, max_dp_lanes);
>
> /* DSI_A lane config */
> val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
Not a problem in this patch, but maybe this can be SN_MAX_DP_LANES -
pdata->dsi->lanes now.
> regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
>CHA_DSI_LANES_MASK, val);
>
> + regmap_write(pdata->regmap, SN_LN_ASSIGN_REG, pdata->ln_assign);
> + regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, LN_POLRS_MASK,
> + pdata->ln_polrs << LN_POLRS_OFFSET);
> +
> /* set dsi clk frequency value */
> ti_sn_bridge_set_dsi_rate(pdata);
>
> @@ -1063,6 +1066,50 @@ static int ti_sn_setup_gpio_controller(struct
> ti_sn_bridge *pdata)
> return ret;
> }
>
> +static void ti_sn_bridge_parse_lanes(struct ti_sn_bridge *pdata,
> +struct device_node *np)
> +{
> + u32 lane_assignments[SN_MAX_DP_LANES] = { 0, 1, 2, 3 };
> + u32 lane_polarities[SN_MAX_DP_LANES] = { };
> + struct device_node *endpoint;
> + u8 ln_assign = 0;
> + u8 ln_polrs = 0;
Do we need to assign to 0 to start? Seems like no?
> + int dp_lanes;
> + int i;
> +
> + /*
> +* Read config from the device tree about lane remapping and lane
> +* polarities. These are optional and we assume identity map and
> +* normal polarity if nothing is specified. It's OK to specify just
> +* data-lanes but not lane-polarities but not vice versa.
> +*/
> + endpoint = of_graph_get_endpoint_by_regs(np, 1, -1);
> + dp_lanes = of_property_count_u32_elems(endpoint, "data-lanes");
> + if (dp_lanes > 0) {
> + of_property_read_u32_array(endpoint, "data-lanes",
> + lane_assignments, dp_lanes);
> + of_property_read_u32_array(endpoint, "lane-polarities",
> + lane_polarities, dp_lanes);
> + } else {
> + dp_lanes = SN_MAX_DP_LANES;
> + }
Needs an of_node_put(endpoint) here for the
of_graph_get_endpoint_by_regs() above.
> +
> + /*
> +* Convert into register format. Loop over all lanes even if
> +* data-lanes had fewer elements so that we nicely initialize
> +* the LN_ASSIGN register.
> +*/
> + for (i = SN_MAX_DP_LANES - 1; i >= 0; i--) {
> + ln_assign = ln_assign << LN_ASSIGN_WIDTH |
> lane_assignments[i];
> + ln_polrs = ln_polrs << 1 | lane_polarities[i];
> + }
> +
> + /* Stash in our struct for when we power on */
> + pdata->dp_lanes = dp_lanes;
> + pdata->ln_assign = ln_assign;
> + pdata->ln_polrs = ln_polrs;
> +}
Re: [PATCH] ASoC: qcom: Use devm_platform_ioremap_resource_byname() to simplify code
Hi Tang,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on asoc/for-next]
[also build test ERROR on v5.7-rc4 next-20200504]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Tang-Bin/ASoC-qcom-Use-devm_platform_ioremap_resource_byname-to-simplify-code/20200505-051041
base: https://git.kernel.org/pub/scm/linux/kernel/git/broonie/sound.git
for-next
config: sh-allmodconfig (attached as .config)
compiler: sh4-linux-gcc (GCC) 9.3.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=sh
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
sound/soc/qcom/lpass-cpu.c: In function 'asoc_qcom_lpass_cpu_platform_probe':
>> sound/soc/qcom/lpass-cpu.c:445:21: error: expected ';' before 'if'
445 | "lpass-lpaif")
| ^
| ;
446 | if (IS_ERR((void const __force *)drvdata->lpaif)) {
| ~~
vim +445 sound/soc/qcom/lpass-cpu.c
415
416 int asoc_qcom_lpass_cpu_platform_probe(struct platform_device *pdev)
417 {
418 struct lpass_data *drvdata;
419 struct device_node *dsp_of_node;
420 struct lpass_variant *variant;
421 struct device *dev = &pdev->dev;
422 const struct of_device_id *match;
423 int ret, i, dai_id;
424
425 dsp_of_node = of_parse_phandle(pdev->dev.of_node, "qcom,adsp",
0);
426 if (dsp_of_node) {
427 dev_err(&pdev->dev, "DSP exists and holds audio
resources\n");
428 return -EBUSY;
429 }
430
431 drvdata = devm_kzalloc(&pdev->dev, sizeof(struct lpass_data),
432 GFP_KERNEL);
433 if (!drvdata)
434 return -ENOMEM;
435 platform_set_drvdata(pdev, drvdata);
436
437 match = of_match_device(dev->driver->of_match_table, dev);
438 if (!match || !match->data)
439 return -EINVAL;
440
441 drvdata->variant = (struct lpass_variant *)match->data;
442 variant = drvdata->variant;
443
444 drvdata->lpaif = devm_platform_ioremap_resource_byname(pdev,
> 445 "lpass-lpaif")
446 if (IS_ERR((void const __force *)drvdata->lpaif)) {
447 dev_err(&pdev->dev, "error mapping reg resource: %ld\n",
448 PTR_ERR((void const __force
*)drvdata->lpaif));
449 return PTR_ERR((void const __force *)drvdata->lpaif);
450 }
451
452 lpass_cpu_regmap_config.max_register =
LPAIF_WRDMAPER_REG(variant,
453 variant->wrdma_channels
+
454
variant->wrdma_channel_start);
455
456 drvdata->lpaif_map = devm_regmap_init_mmio(&pdev->dev,
drvdata->lpaif,
457 &lpass_cpu_regmap_config);
458 if (IS_ERR(drvdata->lpaif_map)) {
459 dev_err(&pdev->dev, "error initializing regmap: %ld\n",
460 PTR_ERR(drvdata->lpaif_map));
461 return PTR_ERR(drvdata->lpaif_map);
462 }
463
464 if (variant->init)
465 variant->init(pdev);
466
467 for (i = 0; i < variant->num_dai; i++) {
468 dai_id = variant->dai_driver[i].id;
469 drvdata->mi2s_osr_clk[dai_id] = devm_clk_get(&pdev->dev,
470
variant->dai_osr_clk_names[i]);
471 if (IS_ERR(drvdata->mi2s_osr_clk[dai_id])) {
472 dev_warn(&pdev->dev,
473 "%s() error getting optional %s: %ld\n",
474 __func__,
475 variant->dai_osr_clk_names[i],
476 PTR_ERR(drvdata->mi2s_osr_clk[dai_id]));
477
478 drvdata->m
Re: [PATCH] drm/bridge: ti-sn65dsi86: Fix off-by-one error in clock choice
Quoting Douglas Anderson (2020-05-04 21:32:29)
> If the rate in our table is _equal_ to the rate we want then it's OK
> to pick it. It doesn't need to be greater than the one we want.
>
> Fixes: a095f15c00e2 ("drm/bridge: add support for sn65dsi86 bridge driver")
> Signed-off-by: Douglas Anderson
> ---
Reviewed-by: Stephen Boyd
Re: [PATCH v4 2/2] mtd: rawnand: Add NAND controller support on Intel LGM SoC
Hi Boris,
On 4/5/2020 4:58 pm, Boris Brezillon wrote:
On Mon, 4 May 2020 16:50:08 +0800
"Ramuthevar, Vadivel MuruganX"
wrote:
Hi Boris,
On 4/5/2020 3:17 pm, Boris Brezillon wrote:
On Mon, 4 May 2020 15:15:08 +0800
"Ramuthevar, Vadivel MuruganX"
wrote:
Hi Boris,
Thank you very much for the prompt review and suggestions...
On 4/5/2020 3:08 pm, Boris Brezillon wrote:
On Mon, 4 May 2020 10:02:35 +0800
"Ramuthevar, Vadivel MuruganX"
wrote:
Hi Boris,
On 30/4/2020 9:01 pm, Boris Brezillon wrote:
On Thu, 30 Apr 2020 14:36:00 +0200
Boris Brezillon wrote:
On Thu, 30 Apr 2020 17:07:03 +0800
"Ramuthevar, Vadivel MuruganX"
wrote:
The question is, is it the same value we have in nand_pa or it is
different?
Different address which is 0xE140 NAND_BASE_PHY address.
Then why didn't you tell me they didn't match when I suggested to pass
sorry, because you keep asking nand_pa after that only I realized that.
nand_pa? So now the question is, what does this address represent?
EBU-MODULE
_ ___
| | ||NAND CTRL |
| FPI BUS |==>| CS0(0x174) | 0xE100( 0xE14/0xE1C) NAND_PHY_BASE
|_| |_CS1(0x17C)_|__ |
EBU_CONRTROLLER_BASE : 0xE0F0_
HSNAND_BASE: 0xE100_
NAND_CS0: 0xE140_
NAND_CS1: 0xE1C0_
MEM_REGION_BASE_CS0: 0x17400 (internal to ebu controller )
MEM_REGION_BASE_CS1: 0x17C00
Hm, I wonder if we shouldn't use a 'ranges' property to describe this
address translation. Something like
ebu@xxx {
ranges = <0x1740 0xe140 0x1000>,
<0x17c0 0xe1c0 0x1000>;
reg = <0x1740>, <0x17c0>;
reg-names = "cs-0", "cs-1";
}
The translated address (0xE1X0) will be available in res->start,
and the non-translated one (0x17X0) can be retrieved with
of_get_address(). All you'd have to do then would be calculate the
mask:
mask = (translated_address & original_address) >> 22;
num_comp_bits = fls(mask);
WARN_ON(mask != GENMASK(num_comp_bits - 1, 0));
Which allows you to properly set the ADDR_SEL() register without
relying on some hardcoded values:
writel(original_address | EBU_ADDR_SEL_REGEN |
EBU_ADDR_COMP_BITS(num_comp_bits),
ebu_host->ebu + EBU_ADDR_SEL(csid));
That's quite important if we want to merge the xway NAND driver with
this one.
Looks like the translation is done at the FPI bus declaration level (see
[1]). We really need to see the big picture to take a wise decision
about the bindings. Would you mind pasting your dsti/dts files
somewhere? It feels like the NAND controller is a sub-part of a more
generic 'memory' controller, in which case the NAND controller should be
declared as a child of this generic memory bus (called localbus in [1],
but maybe EBU is more accurate).
[1]https://github.com/xieyaxiongfly/Atheros_CSI_tool_OpenWRT_src/blob/master/target/linux/lantiq/files-4.14/arch/mips/boot/dts/vr9.dtsi#L162
ebu_nand: ebu_nand@e0f0 {
compatible = "intel,lgm-ebu-nand";
reg = <0xe0f0 0x100
0xe100 0x300
0xe140 0x8
0xe1c0 0x1>;
reg-names = "ebunand", "hsnand", "nand_cs0", nand_cs1";
dmas = <&dma0 8>, <&dma0 9>;
dma-names = "ebu_rx", "ebu_tx";
clocks = <&cgu0 LGM_GCLK_EBU>;
};
&ebu_nand {
status = "disabled";
nand,cs = <1>;
nand-ecc-mode = "hw";
pinctrl-names = "default";
pinctrl-0 = <&ebu_nand_base &ebu_cs1>;
};
Ok. If I understand the SoC topology correctly it should actually be
something like that:
{
...
fpi@x {
compatible = "intel,lgm-fpi", "simple-bus";
/* You might have other ranges to define here */
ranges = <0x1600 0xe000 0x100>;
...
Sorry, we do not have fpi tree node in our dts/dtsi file instead we have
the below one.. , that also not included the major peripherals
controllers node.
/* Special part from CPU core */
core: core {
compatible = "intel,core", "simple-bus";
#address-cells = <1>;
#size-cells = <1>;
ranges;
ioapic1: interrupt-controller@fec0 {
#interrupt-cells = <2>;
#address-cells = <0>;
compatible = "intel,ce4100-ioapic";
interrupt-controller;
reg = <0xfec0 0x1000>;
nr_entries = <256>;
Re: [PATCH v7 3/3] phy: intel: Add driver support for ComboPhy
On 04-05-20, 17:32, Dilip Kota wrote:
>
> On 5/4/2020 5:20 PM, Vinod Koul wrote:
> > On 04-05-20, 16:26, Dilip Kota wrote:
> > > On 5/4/2020 3:29 PM, Vinod Koul wrote:
> > > > On 30-04-20, 15:15, Dilip Kota wrote:
> > > >
> > > > > + u32 mask, u32 val)
> > > > > +{
> > > > > + u32 reg_val;
> > > > > +
> > > > > + reg_val = readl(base + reg);
> > > > > + reg_val &= ~mask;
> > > > > + reg_val |= FIELD_PREP(mask, val);
> > > > > + writel(reg_val, base + reg);
> > > > bypassing regmap here... why?
> > > It is not regmap address, one of the below two addresses are passed to
> > > this
> > > function.
> > okay, perhaps add a comment somewhere that regmap is not used for this
> > base?
> I dont see a need of adding a comment, describing don't do regmap here.
Driver uses regmap except here, which seems odd hence explanation
required for this.
> >
> > > struct intel_combo_phy {
> > > ...
> > > void __iomem *app_base;
> > > void __iomem *cr_base;
> > > ...
> > > }
> >
> > > > > +static int intel_cbphy_calibrate(struct phy *phy)
> > > > > +{
> > > > > + struct intel_cbphy_iphy *iphy = phy_get_drvdata(phy);
> > > > > + struct intel_combo_phy *cbphy = iphy->parent;
> > > > > + void __iomem *cr_base = cbphy->cr_base;
> > > > > + int val, ret, id;
> > > > > +
> > > > > + if (cbphy->phy_mode != PHY_XPCS_MODE)
> > > > > + return 0;
> > > > > +
> > > > > + id = PHY_ID(iphy);
> > > > > +
> > > > > + /* trigger auto RX adaptation */
> > > > > + combo_phy_w32_off_mask(cr_base, CR_ADDR(PCS_XF_ATE_OVRD_IN_2,
> > > > > id),
> > > > > +ADAPT_REQ_MSK, 3);
> > > > > + /* Wait RX adaptation to finish */
> > > > > + ret = readl_poll_timeout(cr_base + CR_ADDR(PCS_XF_RX_ADAPT_ACK,
> > > > > id),
> > > > > + val, val & RX_ADAPT_ACK_BIT, 10, 5000);
> > > > > + if (ret)
> > > > > + dev_err(cbphy->dev, "RX Adaptation failed!\n");
> > > > you want to continue her and not return error?
> > > Next step is stopping the Adaptation, it should be done in both error and
> > > success case.
> > Again documenting this helps, pls add some comments on this behaviour
> Comments are already in place, mentioning Start and Stop of Rx Adaptation.
> And Stop is being is done as Start is triggered, so not needed to mention
> error and success.
Ok
--
~Vinod
Re: [PATCH] iommu: Relax ACS requirement for RCiEP devices.
On Mon, 4 May 2020 21:42:16 -0700
Ashok Raj wrote:
> PCIe Spec recommends we can relax ACS requirement for RCIEP devices.
>
> PCIe 5.0 Specification.
> 6.12 Access Control Services (ACS)
> Implementation of ACS in RCiEPs is permitted but not required. It is
> explicitly permitted that, within a single Root Complex, some RCiEPs
> implement ACS and some do not. It is strongly recommended that Root Complex
> implementations ensure that all accesses originating from RCiEPs
> (PFs and VFs) without ACS capability are first subjected to processing by
> the Translation Agent (TA) in the Root Complex before further decoding and
> processing. The details of such Root Complex handling are outside the scope
> of this specification.
>
> Since Linux didn't give special treatment to allow this exception, certain
> RCiEP MFD devices are getting grouped in a single iommu group. This
> doesn't permit a single device to be assigned to a guest for instance.
>
> In one vendor system: Device 14.x were grouped in a single IOMMU group.
>
> /sys/kernel/iommu_groups/5/devices/:00:14.0
> /sys/kernel/iommu_groups/5/devices/:00:14.2
> /sys/kernel/iommu_groups/5/devices/:00:14.3
>
> After the patch:
> /sys/kernel/iommu_groups/5/devices/:00:14.0
> /sys/kernel/iommu_groups/5/devices/:00:14.2
> /sys/kernel/iommu_groups/6/devices/:00:14.3 <<< new group
>
> 14.0 and 14.2 are integrated devices, but legacy end points.
> Whereas 14.3 was a PCIe compliant RCiEP.
>
> 00:14.3 Network controller: Intel Corporation Device 9df0 (rev 30)
> Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00
>
> This permits assigning this device to a guest VM.
>
> Fixes: f096c061f552 ("iommu: Rework iommu_group_get_for_pci_dev()")
> Signed-off-by: Ashok Raj
> To: Joerg Roedel
> To: Bjorn Helgaas
> Cc: linux-kernel@vger.kernel.org
> Cc: io...@lists.linux-foundation.org
> Cc: Lu Baolu
> Cc: Alex Williamson
> Cc: Darrel Goeddel
> Cc: Mark Scott ,
> Cc: Romil Sharma
> Cc: Ashok Raj
> ---
> drivers/iommu/iommu.c | 15 ++-
> 1 file changed, 14 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
> index 2b471419e26c..5744bd65f3e2 100644
> --- a/drivers/iommu/iommu.c
> +++ b/drivers/iommu/iommu.c
> @@ -1187,7 +1187,20 @@ static struct iommu_group
> *get_pci_function_alias_group(struct pci_dev *pdev,
> struct pci_dev *tmp = NULL;
> struct iommu_group *group;
>
> - if (!pdev->multifunction || pci_acs_enabled(pdev, REQ_ACS_FLAGS))
> + /*
> + * PCI Spec 5.0, Section 6.12 Access Control Service
> + * Implementation of ACS in RCiEPs is permitted but not required.
> + * It is explicitly permitted that, within a single Root
> + * Complex, some RCiEPs implement ACS and some do not. It is
> + * strongly recommended that Root Complex implementations ensure
> + * that all accesses originating from RCiEPs (PFs and VFs) without
> + * ACS capability are first subjected to processing by the Translation
> + * Agent (TA) in the Root Complex before further decoding and
> + * processing.
> + */
Is the language here really strong enough to make this change? ACS is
an optional feature, so being permitted but not required is rather
meaningless. The spec is also specifically avoiding the words "must"
or "shall" and even when emphasized with "strongly", we still only have
a recommendation that may or may not be honored. This seems like a
weak basis for assuming that RCiEPs universally honor this
recommendation. Thanks,
Alex
> + if (!pdev->multifunction ||
> + (pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END) ||
> + pci_acs_enabled(pdev, REQ_ACS_FLAGS))
> return NULL;
>
> for_each_pci_dev(tmp) {
Re: [PATCH v8 0/5] Add SS/HS-USB changes for Qualcomm SM8150 chipset
On 04-05-20, 16:54, Wesley Cheng wrote: > This series adds support for the Synopsis 7nm HSPHY USB driver being > used in QCOM chipsets. The HSPHY register map differs compared to > other PHY revisions. In addition, modifications and updates are done > to the QMP driver to add new registers/offsets, and to update the > initialization sequence for enabling the SSUSB path on SM8150. Applied all, thanks. Pls note the dt-binding patch had a trailing line at the end of file, I have stripped that while applying. -- ~Vinod
Re: fs: jfs: fix a possible data race in txBegin()
> I am not sure how to add the tag "Fixes"... How helpful do you find the available software documentation? https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=47cf1b422e6093aee2a3e55d5e162112a2c69870#n183 > I need to find which previous commit add the code about txBegin()? I suggest to take another look at corresponding source code places by a command like “git blame”. https://git-scm.com/book/en/v2/Git-Tools-Debugging-with-Gits Regards, Markus
Re: [PATCH 1/6] dt-bindings: phy: meson8b-usb2: Convert to json-schema
On 04-05-20, 10:31, Joe Perches wrote:
> On Mon, 2020-05-04 at 12:55 +0200, hex dump wrote:
> > On Sat, May 2, 2020 at 1:48 PM Martin Blumenstingl
> > wrote:
> > > Now that we have the DT validation in place, let's convert the device
> > > tree bindings for the Amlogic Meson8, Meson8b, Meson8m2 and GXBB USB2
> > > PHY over to a YAML schema.
> > >
> > > While here, also add the fallback compatible string
> > > "amlogic,meson-gxbb-usb2-phy" which is already used in
> > > arch/arm/boot/dts/meson{,8,8b}.dtsi.
> > >
> > > Signed-off-by: Martin Blumenstingl
> >
> > Tested-by: hexdump
>
> Is the kernel now accepting "Tested-by" lines from robots?
Should we not? bots have been given Reported-by...
>
> If hexdump0...@googlemail.com is not a robot, can you please use
> your full legal name instead?
Looking at emails, this doesnt seem a bot, so I am asking for full legal
name for these to be added.
Thanks
--
~Vinod
Re: [PATCH] kcsan: Add test suite
On Mon, Apr 27, 2020 at 11:23 PM 'Marco Elver' via kasan-dev wrote: > > On Mon, 27 Apr 2020 at 16:35, Marco Elver wrote: > > > > This adds KCSAN test focusing on behaviour of the integrated runtime. > > Tests various race scenarios, and verifies the reports generated to > > console. Makes use of KUnit for test organization, and the Torture > > framework for test thread control. > > > > Signed-off-by: Marco Elver > > --- > > +KUnit devs > We had some discussions on how to best test sanitizer runtimes, and we > believe that this test is what testing sanitizer runtimes should > roughly look like. Note that, for KCSAN there are various additional > complexities like multiple threads, and report generation isn't > entirely deterministic (need to run some number of iterations to get > reports, may get multiple reports, etc.). Thanks very much for writing the test. I do think that it goes a little outside what we'd normally expect of a unit test (notably with the issues around determinism and threading), but it's good to see KUnit being pushed in new directions a bit. The biggest issue in my mind is the possibility that the non-determinism of the tests could cause false positives. If we're trying to run as many KUnit tests as possible as part of continuous integration systems or as a condition for accepting patches, having flaky tests could be annoying. The KCSAN tests seem to break/fail as-is when run on single-core machines (at least, under qemu), so some way of documenting this as a requirement would probably be necessary, too. One possibility would be to add support for "skipped" tests to KUnit (the TAP specification allows for it), so that the KCSAN test could detect cases where it's not reliable, and skip itself (leaving a note as to why). In the short term, though, we'd absolutely need some documentation around the dependencies for the test. (For the record, the failures I saw were all due to running under qemu emulating as a uniprocessor/single-core machine: with CONFIG_PREEMPT_VOLUNTARY, it would just hang after creating the first couple of threads. With CONFIG_PREEMPT, the tests completed, but the majority of them failed.) > The main thing, however, is that we want to verify the actual output > (or absence of it) to console. This is what the KCSAN test does using > the 'console' tracepoint. Could KUnit provide some generic > infrastructure to check console output, like is done in the test here? > Right now I couldn't say what the most useful generalization of this > would be (without it just being a wrapper around the console > tracepoint), because the way I've decided to capture and then match > console output is quite test-specific. For now we can replicate this > logic on a per-test basis, but it would be extremely useful if there > was a generic interface that KUnit could provide in future. This is something we've discussed here a couple of times as well. While I'll confess to being a little bit wary of having tests rely too heavily on console output: it risks being a bit fragile if the exact contents or formatting of messages change, or ends up having a lot of string formatting and/or parsing code in the tests. I do agree, though, that it probably needs to be at least a part of testing things like sanitizers where the ultimate goal is to produce console output. I'm not exactly sure how we'd implement it yet, so it's probably not going to happen extremely soon, but what you have here looks to me like a good example we can generalise as needed. Cheers, -- David
[PATCH -next] drm/radeon: fix unsigned comparison with 0
Fixes warning because pipe is unsigned long and can never be negtative
vers/gpu/drm/radeon/radeon_kms.c:831:11: warning:
comparison of unsigned expression < 0 is always false [-Wtype-limits]
if (pipe < 0 || pipe >= rdev->num_crtc) {
drivers/gpu/drm/radeon/radeon_kms.c:857:11: warning:
comparison of unsigned expression < 0 is always false [-Wtype-limits]
if (pipe < 0 || pipe >= rdev->num_crtc) {
Reported-by: Hulk Robot
Signed-off-by: ChenTao
---
drivers/gpu/drm/radeon/radeon_kms.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/radeon/radeon_kms.c
b/drivers/gpu/drm/radeon/radeon_kms.c
index 372962358a18..c5d1dc9618a4 100644
--- a/drivers/gpu/drm/radeon/radeon_kms.c
+++ b/drivers/gpu/drm/radeon/radeon_kms.c
@@ -828,7 +828,7 @@ int radeon_enable_vblank_kms(struct drm_crtc *crtc)
unsigned long irqflags;
int r;
- if (pipe < 0 || pipe >= rdev->num_crtc) {
+ if (pipe >= rdev->num_crtc) {
DRM_ERROR("Invalid crtc %d\n", pipe);
return -EINVAL;
}
@@ -854,7 +854,7 @@ void radeon_disable_vblank_kms(struct drm_crtc *crtc)
struct radeon_device *rdev = dev->dev_private;
unsigned long irqflags;
- if (pipe < 0 || pipe >= rdev->num_crtc) {
+ if (pipe >= rdev->num_crtc) {
DRM_ERROR("Invalid crtc %d\n", pipe);
return;
}
--
2.22.0
Re: [PATCH v4 10/12] OPP: Add support for setting interconnect-tags
On 05-05-20, 01:52, Sibi Sankar wrote: > Add support for setting tags on icc paths associated with > the opp_table. > > Signed-off-by: Sibi Sankar > --- > drivers/opp/of.c | 25 +++-- > 1 file changed, 19 insertions(+), 6 deletions(-) Maybe this should be part of Georgi's series ? -- viresh
Re: [PATCH 04/15] ath10k: fix gcc-10 zero-length-bounds warnings
"Gustavo A. R. Silva" writes:
> On 5/4/20 06:54, Kalle Valo wrote:
>> "Gustavo A. R. Silva" writes:
>>
>>> Hi Arnd,
>>>
>>> On 4/30/20 16:30, Arnd Bergmann wrote:
gcc-10 started warning about out-of-bounds access for zero-length
arrays:
In file included from drivers/net/wireless/ath/ath10k/core.h:18,
from drivers/net/wireless/ath/ath10k/htt_rx.c:8:
drivers/net/wireless/ath/ath10k/htt_rx.c: In function
'ath10k_htt_rx_tx_fetch_ind':
drivers/net/wireless/ath/ath10k/htt.h:1683:17: warning: array subscript
65535 is outside the bounds of an interior zero-length array 'struct
htt_tx_fetch_record[0]' [-Wzero-length-bounds]
1683 | return (void *)&ind->records[le16_to_cpu(ind->num_records)];
| ^~~~
drivers/net/wireless/ath/ath10k/htt.h:1676:29: note: while referencing
'records'
1676 | struct htt_tx_fetch_record records[0];
| ^~~
The structure was already converted to have a flexible-array member in
the past, but there are two zero-length members in the end and only
one of them can be a flexible-array member.
Swap the two around to avoid the warning, as 'resp_ids' is not accessed
in a way that causes a warning.
Fixes: 3ba225b506a2 ("treewide: Replace zero-length array with
flexible-array member")
Fixes: 22e6b3bc5d96 ("ath10k: add new htt definitions")
Signed-off-by: Arnd Bergmann
---
drivers/net/wireless/ath/ath10k/htt.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/ath/ath10k/htt.h
b/drivers/net/wireless/ath/ath10k/htt.h
index e7096a73c6ca..7621f0a3dc77 100644
--- a/drivers/net/wireless/ath/ath10k/htt.h
+++ b/drivers/net/wireless/ath/ath10k/htt.h
@@ -1673,8 +1673,8 @@ struct htt_tx_fetch_ind {
__le32 token;
__le16 num_resp_ids;
__le16 num_records;
- struct htt_tx_fetch_record records[0];
- __le32 resp_ids[]; /* ath10k_htt_get_tx_fetch_ind_resp_ids() */
+ __le32 resp_ids[0]; /* ath10k_htt_get_tx_fetch_ind_resp_ids() */
+ struct htt_tx_fetch_record records[];
} __packed;
static inline void *
>>>
>>> The treewide patch is an experimental change and, as this change only
>>> applies
>>> to my -next tree, I will carry this patch in it, so other people don't have
>>> to worry about this at all.
>>
>> Gustavo, why do you have ath10k patches in your tree? I prefer that
>> ath10k patches go through my ath.git tree so that they are reviewed and
>> tested.
>>
>
> I just wanted to test out a mechanical change. I will remove it from my tree
> now and will send a patch to you so you can apply it to your ath.git tree.
Great, thanks.
--
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
Re: [PATCH V3 10/16] arm64/cpufeature: Add remaining feature bits in ID_AA64PFR0 register
On 05/02/2020 02:33 PM, Anshuman Khandual wrote:
Enable MPAM and SEL2 features bits in ID_AA64PFR0 register as per ARM DDI
0487F.a specification.
Cc: Catalin Marinas
Cc: Will Deacon
Cc: Mark Rutland
Cc: Suzuki K Poulose
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Suggested-by: Will Deacon
Signed-off-by: Anshuman Khandual
---
arch/arm64/include/asm/sysreg.h | 2 ++
arch/arm64/kernel/cpufeature.c | 2 ++
2 files changed, 4 insertions(+)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 40eaf89f1032..c93ea6613f51 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -641,6 +641,8 @@
#define ID_AA64PFR0_CSV2_SHIFT56
#define ID_AA64PFR0_DIT_SHIFT 48
#define ID_AA64PFR0_AMU_SHIFT 44
+#define ID_AA64PFR0_MPAM_SHIFT 40
+#define ID_AA64PFR0_SEL2_SHIFT 36
#define ID_AA64PFR0_SVE_SHIFT 32
#define ID_AA64PFR0_RAS_SHIFT 28
#define ID_AA64PFR0_GIC_SHIFT 24
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index dbedcae28061..f5a39e040804 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -217,6 +217,8 @@ static const struct arm64_ftr_bits ftr_id_aa64pfr0[] = {
ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE,
ID_AA64PFR0_CSV2_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE,
ID_AA64PFR0_DIT_SHIFT, 4, 0),
ARM64_FTR_BITS(FTR_HIDDEN, FTR_NONSTRICT, FTR_LOWER_SAFE,
ID_AA64PFR0_AMU_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE,
ID_AA64PFR0_MPAM_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE,
ID_AA64PFR0_SEL2_SHIFT, 4, 0),
Both of them must be hidden. And also need to check the impact of
exposing this to the guests, especially MPAM.
Suzuki
Re: [PATCH v4 06/12] cpufreq: qcom: Update the bandwidth levels on frequency change
On 05-05-20, 01:52, Sibi Sankar wrote: > Add support to parse optional OPP table attached to the cpu node when > the OPP bandwidth values are populated. This allows for scaling of > DDR/L3 bandwidth levels with frequency change. > > Signed-off-by: Sibi Sankar What about using opp_set_rate instead ? -- viresh
Re: [PATCH V3 09/16] arm64/cpufeature: Add remaining feature bits in ID_AA64ISAR0 register
On 05/02/2020 02:33 PM, Anshuman Khandual wrote:
Enable TLB features bit in ID_AA64ISAR0 register as per ARM DDI 0487F.a
specification.
Cc: Catalin Marinas
Cc: Will Deacon
Cc: Mark Rutland
Cc: Suzuki K Poulose
Cc: linux-arm-ker...@lists.infradead.org
Cc: linux-kernel@vger.kernel.org
Suggested-by: Will Deacon
Signed-off-by: Anshuman Khandual
---
arch/arm64/include/asm/sysreg.h | 1 +
arch/arm64/kernel/cpufeature.c | 1 +
2 files changed, 2 insertions(+)
diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 0f34927f52b9..40eaf89f1032 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -597,6 +597,7 @@
/* id_aa64isar0 */
#define ID_AA64ISAR0_RNDR_SHIFT 60
+#define ID_AA64ISAR0_TLB_SHIFT 56
#define ID_AA64ISAR0_TS_SHIFT 52
#define ID_AA64ISAR0_FHM_SHIFT48
#define ID_AA64ISAR0_DP_SHIFT 44
diff --git a/arch/arm64/kernel/cpufeature.c b/arch/arm64/kernel/cpufeature.c
index f4e15e355aee..dbedcae28061 100644
--- a/arch/arm64/kernel/cpufeature.c
+++ b/arch/arm64/kernel/cpufeature.c
@@ -174,6 +174,7 @@ static bool __system_matches_cap(unsigned int n);
*/
static const struct arm64_ftr_bits ftr_id_aa64isar0[] = {
ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE,
ID_AA64ISAR0_RNDR_SHIFT, 4, 0),
+ ARM64_FTR_BITS(FTR_VISIBLE, FTR_STRICT, FTR_LOWER_SAFE,
ID_AA64ISAR0_TLB_SHIFT, 4, 0),
I don't see any reason why this should be VISIBLE to the userspace.
Suzuki
[PATCH v3] KVM: nVMX: Skip IBPB when switching between vmcs01 and vmcs02
Skip the Indirect Branch Prediction Barrier that is triggered on a VMCS
switch when running with spectre_v2_user=on/auto if the switch is
guaranteed to be between two VMCSes in the same guest, i.e. between
vmcs01 and vmcs02. The IBPB is intended to prevent one guest from
attacking another, which is unnecessary in the nested case as it's the
same guest from KVM's perspective.
This all but eliminates the overhead observed for nested VMX transitions
when running with CONFIG_RETPOLINE=y and spectre_v2_user=on/auto, which
can be significant, e.g. roughly 3x on current systems.
Reported-by: Alexander Graf
Cc: KarimAllah Raslan
Cc: sta...@vger.kernel.org
Fixes: 15d45071523d ("KVM/x86: Add IBPB support")
Signed-off-by: Sean Christopherson
---
v3: Switch back to passing 'struct loaded_vmcs buddy', but keep the WARN
if the buddy VMCS isn't already loaded as well as the comment as to
why skipping IBPB in the nested case is ok. [Paolo]
v2: Pass a boolean to indicate a nested VMCS switch and instead WARN if
the buddy VMCS is not already loaded. [Alex]
arch/x86/kvm/vmx/nested.c | 2 +-
arch/x86/kvm/vmx/vmx.c| 18 ++
arch/x86/kvm/vmx/vmx.h| 3 ++-
3 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/arch/x86/kvm/vmx/nested.c b/arch/x86/kvm/vmx/nested.c
index 2c36f3f53108..1a02bdfeeb2b 100644
--- a/arch/x86/kvm/vmx/nested.c
+++ b/arch/x86/kvm/vmx/nested.c
@@ -303,7 +303,7 @@ static void vmx_switch_vmcs(struct kvm_vcpu *vcpu, struct
loaded_vmcs *vmcs)
cpu = get_cpu();
prev = vmx->loaded_vmcs;
vmx->loaded_vmcs = vmcs;
- vmx_vcpu_load_vmcs(vcpu, cpu);
+ vmx_vcpu_load_vmcs(vcpu, cpu, prev);
vmx_sync_vmcs_host_state(vmx, prev);
put_cpu();
diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
index 3ab6ca6062ce..06ee0572b929 100644
--- a/arch/x86/kvm/vmx/vmx.c
+++ b/arch/x86/kvm/vmx/vmx.c
@@ -1311,10 +1311,12 @@ static void vmx_vcpu_pi_load(struct kvm_vcpu *vcpu, int
cpu)
pi_set_on(pi_desc);
}
-void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu)
+void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu,
+ struct loaded_vmcs *buddy)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
bool already_loaded = vmx->loaded_vmcs->cpu == cpu;
+ struct vmcs *prev;
if (!already_loaded) {
loaded_vmcs_clear(vmx->loaded_vmcs);
@@ -1333,10 +1335,18 @@ void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu)
local_irq_enable();
}
- if (per_cpu(current_vmcs, cpu) != vmx->loaded_vmcs->vmcs) {
+ prev = per_cpu(current_vmcs, cpu);
+ if (prev != vmx->loaded_vmcs->vmcs) {
per_cpu(current_vmcs, cpu) = vmx->loaded_vmcs->vmcs;
vmcs_load(vmx->loaded_vmcs->vmcs);
- indirect_branch_prediction_barrier();
+
+ /*
+* No indirect branch prediction barrier needed when switching
+* the active VMCS within a guest, e.g. on nested VM-Enter.
+* The L1 VMM can protect itself with retpolines, IBPB or IBRS.
+*/
+ if (!buddy || WARN_ON_ONCE(buddy->vmcs != prev))
+ indirect_branch_prediction_barrier();
}
if (!already_loaded) {
@@ -1377,7 +1387,7 @@ void vmx_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
{
struct vcpu_vmx *vmx = to_vmx(vcpu);
- vmx_vcpu_load_vmcs(vcpu, cpu);
+ vmx_vcpu_load_vmcs(vcpu, cpu, NULL);
vmx_vcpu_pi_load(vcpu, cpu);
diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
index b5e773267abe..d3d48acc6bd9 100644
--- a/arch/x86/kvm/vmx/vmx.h
+++ b/arch/x86/kvm/vmx/vmx.h
@@ -320,7 +320,8 @@ struct kvm_vmx {
};
bool nested_vmx_allowed(struct kvm_vcpu *vcpu);
-void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu);
+void vmx_vcpu_load_vmcs(struct kvm_vcpu *vcpu, int cpu,
+ struct loaded_vmcs *buddy);
void vmx_vcpu_load(struct kvm_vcpu *vcpu, int cpu);
int allocate_vpid(void);
void free_vpid(int vpid);
--
2.26.0
Re: [PATCH v4 04/12] OPP: Add and export helper to update voltage
On 05-05-20, 01:52, Sibi Sankar wrote: > Add and export 'dev_pm_opp_update_voltage' to update the voltage of an > opp for a given frequency. This will be useful to update the opps with > voltages read back from firmware. > > Signed-off-by: Sibi Sankar Have a look at dev_pm_opp_adjust_voltage(). -- viresh
[PATCH] iommu: Relax ACS requirement for RCiEP devices.
PCIe Spec recommends we can relax ACS requirement for RCIEP devices.
PCIe 5.0 Specification.
6.12 Access Control Services (ACS)
Implementation of ACS in RCiEPs is permitted but not required. It is
explicitly permitted that, within a single Root Complex, some RCiEPs
implement ACS and some do not. It is strongly recommended that Root Complex
implementations ensure that all accesses originating from RCiEPs
(PFs and VFs) without ACS capability are first subjected to processing by
the Translation Agent (TA) in the Root Complex before further decoding and
processing. The details of such Root Complex handling are outside the scope
of this specification.
Since Linux didn't give special treatment to allow this exception, certain
RCiEP MFD devices are getting grouped in a single iommu group. This
doesn't permit a single device to be assigned to a guest for instance.
In one vendor system: Device 14.x were grouped in a single IOMMU group.
/sys/kernel/iommu_groups/5/devices/:00:14.0
/sys/kernel/iommu_groups/5/devices/:00:14.2
/sys/kernel/iommu_groups/5/devices/:00:14.3
After the patch:
/sys/kernel/iommu_groups/5/devices/:00:14.0
/sys/kernel/iommu_groups/5/devices/:00:14.2
/sys/kernel/iommu_groups/6/devices/:00:14.3 <<< new group
14.0 and 14.2 are integrated devices, but legacy end points.
Whereas 14.3 was a PCIe compliant RCiEP.
00:14.3 Network controller: Intel Corporation Device 9df0 (rev 30)
Capabilities: [40] Express (v2) Root Complex Integrated Endpoint, MSI 00
This permits assigning this device to a guest VM.
Fixes: f096c061f552 ("iommu: Rework iommu_group_get_for_pci_dev()")
Signed-off-by: Ashok Raj
To: Joerg Roedel
To: Bjorn Helgaas
Cc: linux-kernel@vger.kernel.org
Cc: io...@lists.linux-foundation.org
Cc: Lu Baolu
Cc: Alex Williamson
Cc: Darrel Goeddel
Cc: Mark Scott ,
Cc: Romil Sharma
Cc: Ashok Raj
---
drivers/iommu/iommu.c | 15 ++-
1 file changed, 14 insertions(+), 1 deletion(-)
diff --git a/drivers/iommu/iommu.c b/drivers/iommu/iommu.c
index 2b471419e26c..5744bd65f3e2 100644
--- a/drivers/iommu/iommu.c
+++ b/drivers/iommu/iommu.c
@@ -1187,7 +1187,20 @@ static struct iommu_group
*get_pci_function_alias_group(struct pci_dev *pdev,
struct pci_dev *tmp = NULL;
struct iommu_group *group;
- if (!pdev->multifunction || pci_acs_enabled(pdev, REQ_ACS_FLAGS))
+ /*
+* PCI Spec 5.0, Section 6.12 Access Control Service
+* Implementation of ACS in RCiEPs is permitted but not required.
+* It is explicitly permitted that, within a single Root
+* Complex, some RCiEPs implement ACS and some do not. It is
+* strongly recommended that Root Complex implementations ensure
+* that all accesses originating from RCiEPs (PFs and VFs) without
+* ACS capability are first subjected to processing by the Translation
+* Agent (TA) in the Root Complex before further decoding and
+* processing.
+*/
+ if (!pdev->multifunction ||
+ (pci_pcie_type(pdev) == PCI_EXP_TYPE_RC_END) ||
+pci_acs_enabled(pdev, REQ_ACS_FLAGS))
return NULL;
for_each_pci_dev(tmp) {
--
2.7.4
[PATCH] drm/bridge: ti-sn65dsi86: Implement lane reordering + polarity
The ti-sn65dsi86 MIPI DSI to eDP bridge chip supports arbitrary
remapping of eDP lanes and also polarity inversion. Both of these
features have been described in the device tree bindings for the
device since the beginning but were never implemented in the driver.
Implement both of them.
Part of this change also allows you to (via the same device tree
bindings) specify to use fewer than the max number of DP lanes that
the panel reports. This could be useful if your display supports more
lanes but only a few are hooked up on your board.
Signed-off-by: Douglas Anderson
---
This patch is based upon my my outstanding series[1] not because there
is any real requirement but simply to avoid merge conflicts. I
believe that my previous series is ready to land. If, however, you'd
prefer that I rebase this patch somewhere atop something else then
please shout.
[1] https://lore.kernel.org/r/20200430194617.197510-1-diand...@chromium.org
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 75 ++-
1 file changed, 62 insertions(+), 13 deletions(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 1a125423eb07..52cca54b525f 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -50,8 +50,12 @@
#define SN_CHA_VERTICAL_BACK_PORCH_REG 0x36
#define SN_CHA_HORIZONTAL_FRONT_PORCH_REG 0x38
#define SN_CHA_VERTICAL_FRONT_PORCH_REG0x3A
+#define SN_LN_ASSIGN_REG 0x59
+#define LN_ASSIGN_WIDTH 2
#define SN_ENH_FRAME_REG 0x5A
#define VSTREAM_ENABLEBIT(3)
+#define LN_POLRS_OFFSET 4
+#define LN_POLRS_MASK 0xf0
#define SN_DATA_FORMAT_REG 0x5B
#define BPP_18_RGBBIT(0)
#define SN_HPD_DISABLE_REG 0x5C
@@ -98,6 +102,7 @@
#define SN_REGULATOR_SUPPLY_NUM4
+#define SN_MAX_DP_LANES4
#define SN_NUM_GPIOS 4
/**
@@ -115,6 +120,8 @@
* @enable_gpio: The GPIO we toggle to enable the bridge.
* @supplies: Data for bulk enabling/disabling our regulators.
* @dp_lanes: Count of dp_lanes we're using.
+ * @ln_assign:Value to program to the LN_ASSIGN register.
+ * @ln_polr: Value for the 4-bit LN_POLRS field of SN_ENH_FRAME_REG.
*
* @gchip:If we expose our GPIOs, this is used.
* @gchip_output: A cache of whether we've set GPIOs to output. This
@@ -140,6 +147,8 @@ struct ti_sn_bridge {
struct gpio_desc*enable_gpio;
struct regulator_bulk_data supplies[SN_REGULATOR_SUPPLY_NUM];
int dp_lanes;
+ u8 ln_assign;
+ u8 ln_polrs;
struct gpio_chipgchip;
DECLARE_BITMAP(gchip_output, SN_NUM_GPIOS);
@@ -707,26 +716,20 @@ static void ti_sn_bridge_enable(struct drm_bridge *bridge)
int dp_rate_idx;
unsigned int val;
int ret = -EINVAL;
+ int max_dp_lanes;
- /*
-* Run with the maximum number of lanes that the DP sink supports.
-*
-* Depending use cases, we might want to revisit this later because:
-* - It's plausible that someone may have run fewer lines to the
-* sink than the sink actually supports, assuming that the lines
-* will just be driven at a higher rate.
-* - The DP spec seems to indicate that it's more important to minimize
-* the number of lanes than the link rate.
-*
-* If we do revisit, it would be important to measure the power impact.
-*/
- pdata->dp_lanes = ti_sn_get_max_lanes(pdata);
+ max_dp_lanes = ti_sn_get_max_lanes(pdata);
+ pdata->dp_lanes = min(pdata->dp_lanes, max_dp_lanes);
/* DSI_A lane config */
val = CHA_DSI_LANES(4 - pdata->dsi->lanes);
regmap_update_bits(pdata->regmap, SN_DSI_LANES_REG,
CHA_DSI_LANES_MASK, val);
+ regmap_write(pdata->regmap, SN_LN_ASSIGN_REG, pdata->ln_assign);
+ regmap_update_bits(pdata->regmap, SN_ENH_FRAME_REG, LN_POLRS_MASK,
+ pdata->ln_polrs << LN_POLRS_OFFSET);
+
/* set dsi clk frequency value */
ti_sn_bridge_set_dsi_rate(pdata);
@@ -1063,6 +1066,50 @@ static int ti_sn_setup_gpio_controller(struct
ti_sn_bridge *pdata)
return ret;
}
+static void ti_sn_bridge_parse_lanes(struct ti_sn_bridge *pdata,
+struct device_node *np)
+{
+ u32 lane_assignments[SN_MAX_DP_LANES] = { 0, 1, 2, 3 };
+ u32 lane_polarities[SN_MAX_DP_LANES] = { };
+ struct device_node *endpoint;
+ u8 ln_assign = 0;
+ u8 ln_polrs = 0;
+ int dp_lanes;
+ int i;
+
+ /*
+* R
[PATCH] drm/bridge: ti-sn65dsi86: Fix off-by-one error in clock choice
If the rate in our table is _equal_ to the rate we want then it's OK
to pick it. It doesn't need to be greater than the one we want.
Fixes: a095f15c00e2 ("drm/bridge: add support for sn65dsi86 bridge driver")
Signed-off-by: Douglas Anderson
---
drivers/gpu/drm/bridge/ti-sn65dsi86.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
index 6ad688b320ae..be000b0ca56b 100644
--- a/drivers/gpu/drm/bridge/ti-sn65dsi86.c
+++ b/drivers/gpu/drm/bridge/ti-sn65dsi86.c
@@ -475,7 +475,7 @@ static int ti_sn_bridge_calc_min_dp_rate_idx(struct
ti_sn_bridge *pdata)
1000 * pdata->dp_lanes * DP_CLK_FUDGE_DEN);
for (i = 1; i < ARRAY_SIZE(ti_sn_bridge_dp_rate_lut) - 1; i++)
- if (ti_sn_bridge_dp_rate_lut[i] > dp_rate_mhz)
+ if (ti_sn_bridge_dp_rate_lut[i] >= dp_rate_mhz)
break;
return i;
--
2.26.2.526.g744177e7f7-goog
Re: [PATCH v1 1/3] printk: honor the max_reason field in kmsg_dumper
> > I changed it to make code cleaner: for such basic operation there are > > too many conditions if we will keep it inside the kmsg_dump(). > > However, if being able to set always_kmsg_dump dynamically during > > runtime is deemed important, I can change it back to be checked in > > kmsg_dump. > > If you agree that we do not have to modify this variable dynamically, > I will also change the permission here: > module_param_named(always_kmsg_dump, always_kmsg_dump, bool, S_IRUGO | > S_IWUSR); Hi Sergey, After thinking about this. I will move this logic back to kmsg_dump(), to keep the current behavior where kmsg_dump can be modified during runtime. Thank you, Pasha
Re: [PATCH 2/2] x86/resctrl: Support CPUID enumeration of MBM counter width
Hi Borislav,
On 5/4/2020 12:56 AM, Borislav Petkov wrote:
> Hi,
>
> On Sun, May 03, 2020 at 11:51:00AM -0700, Reinette Chatre wrote:
>> I am struggling with what should follow ...
>
> Since a diff is better than a thousand words :-) see below.
>
Thank you so much for providing the details. Your explanation is clear
to me but I do have one clarification question ...
> @@ -597,6 +598,8 @@ static void bsp_init_amd(struct cpuinfo_x86 *c)
> x86_amd_ls_cfg_ssbd_mask = 1ULL << bit;
> }
> }
> +
> + resctrl_cpu_detect(c);
> }
>
...
> @@ -322,6 +323,11 @@ static void early_init_intel(struct cpuinfo_x86 *c)
> detect_ht_early(c);
> }
>
> +static void bsp_init_intel(struct cpuinfo_x86 *c)
> +{
> + resctrl_cpu_detect(c);
> +}
> +
> #ifdef CONFIG_X86_32
> /*
> * Early probe support logic for ppro memory erratum #50
> @@ -961,6 +967,7 @@ static const struct cpu_dev intel_cpu_dev = {
> #endif
> .c_detect_tlb = intel_detect_tlb,
> .c_early_init = early_init_intel,
> + .c_bsp_init = bsp_init_intel,
> .c_init = init_intel,
> .c_x86_vendor = X86_VENDOR_INTEL,
> };
> diff --git a/arch/x86/kernel/cpu/resctrl/core.c
> b/arch/x86/kernel/cpu/resctrl/core.c
> index d8cc5223b7ce..5e5955aa6593 100644
> --- a/arch/x86/kernel/cpu/resctrl/core.c
> +++ b/arch/x86/kernel/cpu/resctrl/core.c
> @@ -22,7 +22,7 @@
> #include
>
> #include
> -#include
> +#include
> #include "internal.h"
>
> /* Mutex to protect rdtgroup access. */
> @@ -958,6 +958,35 @@ static __init void rdt_init_res_defs(void)
>
> static enum cpuhp_state rdt_online;
>
> +/* Runs once on the BSP during boot. */
> +void resctrl_cpu_detect(struct cpuinfo_x86 *c)
> +{
> + if (!cpu_has(c, X86_FEATURE_CQM_LLC)) {
> + c->x86_cache_max_rmid = -1;
> + c->x86_cache_occ_scale = -1;
> + c->x86_cache_mbm_width_offset = -1;
> + return;
> + }
> +
> + /* will be overridden if occupancy monitoring exists */
> + c->x86_cache_max_rmid = cpuid_ebx(0xf);
> +
> + if (cpu_has(c, X86_FEATURE_CQM_OCCUP_LLC) ||
> + cpu_has(c, X86_FEATURE_CQM_MBM_TOTAL) ||
> + cpu_has(c, X86_FEATURE_CQM_MBM_LOCAL)) {
> + u32 eax, ebx, ecx, edx;
> +
> + /* QoS sub-leaf, EAX=0Fh, ECX=1 */
> + cpuid_count(0xf, 1, &eax, &ebx, &ecx, &edx);
> +
> + c->x86_cache_max_rmid = ecx;
> + c->x86_cache_occ_scale = ebx;
> +
> + if (c->x86_vendor == X86_VENDOR_INTEL)
> + c->x86_cache_mbm_width_offset = eax & 0xff;
> + }
> +}
> +
resctrl_cpu_detect() is now identical among vendors. Do we still need
the c_bsp_init helpers? Could we not perhaps call resctrl_cpu_detect()
directly from early_identify_cpu()?
Thank you
Reinette
Re: [PATCH 1/4] dma-mapping: move the remaining DMA API calls out of line
On 17/04/2020 17:58, Christoph Hellwig wrote: > On Wed, Apr 15, 2020 at 09:21:37PM +1000, Alexey Kardashevskiy wrote: >> And the fact they were exported leaves possibility that there is a >> driver somewhere relying on these symbols or distro kernel won't build >> because the symbol disappeared from exports (I do not know what KABI >> guarantees or if mainline kernel cares). > > We absolutely do not care. In fact for abuses of APIs that drivers > should not use we almost care to make them private and break people > abusing them. ok :) >> I do not care in particular but >> some might, a line separated with empty lines in the commit log would do. > > I'll add a blurb for the next version. Has it gone anywhere? Thanks, -- Alexey
Re: [PATCH v4 0/7] clean up redundant 'kvm_run' parameters
Paolo Bonzini, any opinion on this? Thanks and best, Tianjia On 2020/4/27 12:35, Tianjia Zhang wrote: In the current kvm version, 'kvm_run' has been included in the 'kvm_vcpu' structure. For historical reasons, many kvm-related function parameters retain the 'kvm_run' and 'kvm_vcpu' parameters at the same time. This patch does a unified cleanup of these remaining redundant parameters. This series of patches has completely cleaned the architecture of arm64, mips, ppc, and s390 (no such redundant code on x86). Due to the large number of modified codes, a separate patch is made for each platform. On the ppc platform, there is also a redundant structure pointer of 'kvm_run' in 'vcpu_arch', which has also been cleaned separately. --- v4 change: mips: fixes two errors in entry.c. v3 change: Keep the existing `vcpu->run` in the function body unchanged. v2 change: s390 retains the original variable name and minimizes modification. Tianjia Zhang (7): KVM: s390: clean up redundant 'kvm_run' parameters KVM: arm64: clean up redundant 'kvm_run' parameters KVM: PPC: Remove redundant kvm_run from vcpu_arch KVM: PPC: clean up redundant 'kvm_run' parameters KVM: PPC: clean up redundant kvm_run parameters in assembly KVM: MIPS: clean up redundant 'kvm_run' parameters KVM: MIPS: clean up redundant kvm_run parameters in assembly arch/arm64/include/asm/kvm_coproc.h | 12 +-- arch/arm64/include/asm/kvm_host.h| 11 +-- arch/arm64/include/asm/kvm_mmu.h | 2 +- arch/arm64/kvm/handle_exit.c | 36 +++ arch/arm64/kvm/sys_regs.c| 13 ++- arch/mips/include/asm/kvm_host.h | 32 +-- arch/mips/kvm/emulate.c | 59 arch/mips/kvm/entry.c| 21 ++--- arch/mips/kvm/mips.c | 14 +-- arch/mips/kvm/trap_emul.c| 114 ++- arch/mips/kvm/vz.c | 26 ++ arch/powerpc/include/asm/kvm_book3s.h| 16 ++-- arch/powerpc/include/asm/kvm_host.h | 1 - arch/powerpc/include/asm/kvm_ppc.h | 27 +++--- arch/powerpc/kvm/book3s.c| 4 +- arch/powerpc/kvm/book3s.h| 2 +- arch/powerpc/kvm/book3s_64_mmu_hv.c | 12 +-- arch/powerpc/kvm/book3s_64_mmu_radix.c | 4 +- arch/powerpc/kvm/book3s_emulate.c| 10 +- arch/powerpc/kvm/book3s_hv.c | 64 ++--- arch/powerpc/kvm/book3s_hv_nested.c | 12 +-- arch/powerpc/kvm/book3s_interrupts.S | 17 ++-- arch/powerpc/kvm/book3s_paired_singles.c | 72 +++--- arch/powerpc/kvm/book3s_pr.c | 33 --- arch/powerpc/kvm/booke.c | 39 arch/powerpc/kvm/booke.h | 8 +- arch/powerpc/kvm/booke_emulate.c | 2 +- arch/powerpc/kvm/booke_interrupts.S | 9 +- arch/powerpc/kvm/bookehv_interrupts.S| 10 +- arch/powerpc/kvm/e500_emulate.c | 15 ++- arch/powerpc/kvm/emulate.c | 10 +- arch/powerpc/kvm/emulate_loadstore.c | 32 +++ arch/powerpc/kvm/powerpc.c | 72 +++--- arch/powerpc/kvm/trace_hv.h | 6 +- arch/s390/kvm/kvm-s390.c | 23 +++-- virt/kvm/arm/arm.c | 6 +- virt/kvm/arm/mmio.c | 11 ++- virt/kvm/arm/mmu.c | 5 +- 38 files changed, 392 insertions(+), 470 deletions(-)
Re: perf build error with gcc 10 on arm and aarch64
Hi Thomas,
[ + Mathieu/Mike/Suzuki ]
On Mon, May 04, 2020 at 10:22:27PM +0300, Thomas Backlund wrote:
> This is building perf from kernel-5.6.10 on armv7hl and aarch64:
>
> Compiler is gcc 10.1.0-RC
>
>
> LD perf-in.o
> ld: arch/perf-in.o: in function `.LANCHOR0':
> /home/iurt/rpmbuild/BUILD/kernel-arm/linux-5.6/tools/perf/util/include/../../util/cs-etm.h:118:
> multiple definition of `traceid_list';
> util/perf-in.o:/home/iurt/rpmbuild/BUILD/kernel-arm/linux-5.6/tools/perf/util/cs-etm.h:118:
> first defined here
> make[3]: ***
> [/home/iurt/rpmbuild/BUILD/kernel-arm/linux-5.6/tools/build/Makefile.build:145:
> perf-in.o] Error 1
>
> LD perf-in.o
> ld:
> arch/perf-in.o:/home/iurt/rpmbuild/BUILD/kernel-aarch64/linux-5.6/tools/perf/util/include/../../util/cs-etm.h:118:
> multiple definition of `traceid_list';
> util/perf-in.o:/home/iurt/rpmbuild/BUILD/kernel-aarch64/linux-5.6/tools/perf/util/cs-etm.h:118:
> first defined here
> make[3]: ***
> [/home/iurt/rpmbuild/BUILD/kernel-aarch64/linux-5.6/tools/build/Makefile.build:145:
> perf-in.o] Error 1
> make[2]: *** [Makefile.perf:616: perf-in.o] Error 2
> make[1]: *** [Makefile.perf:225: sub-make] Error 2
> make: *** [Makefile:70: all] Error 2
>
>
> The same build succeeds with gcc 9.3.0
Thanks for reporting the issue.
Could you help confirm if below change can resolve this issue?
Thanks,
Leo
---8<---
Subject: [PATCH] perf cs-etm: Move defined of traceid_list
The variable 'traceid_list' is defined in the header file cs-etm.h,
if multiple C files include cs-etm.h the compiler might complaint for
multiple definition of 'traceid_list'.
To fix multiple definition error, move the definition of 'traceid_list'
into cs-etm.c.
Signed-off-by: Leo Yan
---
tools/perf/util/cs-etm.c | 3 +++
tools/perf/util/cs-etm.h | 3 ---
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/tools/perf/util/cs-etm.c b/tools/perf/util/cs-etm.c
index 62d2f9b9ce1b..381d9708e9bd 100644
--- a/tools/perf/util/cs-etm.c
+++ b/tools/perf/util/cs-etm.c
@@ -94,6 +94,9 @@ struct cs_etm_queue {
struct cs_etm_traceid_queue **traceid_queues;
};
+/* RB tree for quick conversion between traceID and metadata pointers */
+static struct intlist *traceid_list;
+
static int cs_etm__update_queues(struct cs_etm_auxtrace *etm);
static int cs_etm__process_queues(struct cs_etm_auxtrace *etm);
static int cs_etm__process_timeless_queues(struct cs_etm_auxtrace *etm,
diff --git a/tools/perf/util/cs-etm.h b/tools/perf/util/cs-etm.h
index 650ecc2a6349..4ad925d6d799 100644
--- a/tools/perf/util/cs-etm.h
+++ b/tools/perf/util/cs-etm.h
@@ -114,9 +114,6 @@ enum cs_etm_isa {
CS_ETM_ISA_T32,
};
-/* RB tree for quick conversion between traceID and metadata pointers */
-struct intlist *traceid_list;
-
struct cs_etm_queue;
struct cs_etm_packet {
--
2.17.1
Re: [PATCH] fs: jfs: fix a possible data race in txBegin()
On 2020/5/5 0:15, Markus Elfring wrote: Thus, a data race can occur for tblk->flag. To fix this data race, the spinlock log->gclock is used in txBegin(). This data race is found by our concurrency fuzzer. How do you think about a wording variant like the following? Change description: A data race can occur for the data structure member “flag”. This data race was found by our concurrency fuzzer. Thus use the spin lock “gclock” for the resetting of five data structure members in this function implementation. Would you like to add the tag “Fixes” to the commit message? Thanks, Markus. I am not sure how to add the tag "Fixes"... I need to find which previous commit add the code about txBegin()? Best wishes, Jia-Ju Bai
Re: [RFC Patch v1 2/4] irqchip/gic-v3: Add support to handle SGI as pseudo NMI
On Fri, 1 May 2020 at 18:33, Sumit Garg wrote: > > Hi Marc, > > On Thu, 30 Apr 2020 at 17:43, Sumit Garg wrote: > > > > On Thu, 30 Apr 2020 at 14:43, Marc Zyngier wrote: > > > > > > On Thu, 30 Apr 2020 12:50:28 +0530 > > > Sumit Garg wrote: > > > > > > > Hi Marc, > > > > > > > > On Wed, 29 Apr 2020 at 13:53, Marc Zyngier wrote: > > > > > > [...] > > > > > > > > What I would like is for the arch code to request these interrupts as > > > > > normal interrupts, for which there is one problem to solve: how do you > > > > > find out about the Linux IRQ number for a given IPI. Or rather, how > > > > > do you get rid of the requirement to have IPI numbers at all and just > > > > > say "give me a per-cpu interrupt that I can use as an IPI, and by the > > > > > way here's the handler you can call". > > > > > > > > I think what you are looking for here is something that could be > > > > sufficed via enabling "CONFIG_GENERIC_IRQ_IPI" framework for arm64/arm > > > > architectures. It's currently used for mips architecture. Looking at > > > > its implementation, I think it should be possible to hook up SGIs > > > > under new IPI irq_domain for GICv2/v3. > > > > > > > > So with this framework we should be able to dynamically allocate IPIs > > > > and use common APIs such as request_irq()/request_nmi() to tell IPI > > > > specific handlers. > > > > > > > > If this approach looks sane to you then I can start working on a PoC > > > > implementation for arm64. > > > > > > I can't say I'm keen. This IPI framework doesn't really work for the > > > GIC: > > > > > > - it requires a separate irqdomain to be able to guarantee that you > > > allocate the hwirq in the SGI range. What is the point? > > > - it creates yet another level of indirection on IPI injection > > > > > > This framework was created to deal with two cases: > > > - systems that can't represent their IPI with a single hwirq spanning > > > all the CPUs > > > - "accelerator cores" that don't run Linux > > > > > > The GIC architecture avoids the first point, and I don't even want to > > > think of the second one. > > > > > > Also, it doesn't solve the initial problem, which is to bootstrap the > > > whole thing. The IPI framework relies on an irqdomain to be created the > > > first place. This would mean teaching the arch code about the > > > intricacies of irqdomains, FW nodes and other terrible things. All > > > things which are better hidden in the GIC drivers (not to mention the > > > other horror stories that are the RPi-2/3 irqchip and the Huawei GIC > > > knock-off). > > > > > > What I have in mind is to replace the set_smp_cross_call() with > > > something that passes the required set of information (interrupt range, > > > at the very least). The only thing that I plan to reuse from the IPI > > > framework is the chip->ipi_send_mask() callback. > > > > > > > Fair enough, I will just pass the allocated interrupt range base > > instead of set_smp_cross_call() and use __ipi_send_mask() to invoke a > > particular IPI. > > Thinking more about this, there seems to be multiple irqchip drivers > registering softirq API via set_smp_cross_call(). So we need to > introduce a new API instead of replacing set_smp_cross_call() under > "CONFIG_GENERIC_IRQ_IPI" macro until all drivers switch to IPIs as > full interrupts. > > BTW, could we take up this generalization as follow-up work as it > seems to be independent of current IPI NMI work? > > > > > And to request an arch specific IPI as NMI, will use > > arch_get_ipinr_nmi() and in turn use request_percpu_nmi() to turn that > > particular IPI as NMI. > > I have updated the second patch [1] in my tree to incorporate these > changes. The updated commit log is as follows: > > commit 25c96663126264ec758c49a4a01a9c285f4ccc61 > Author: Sumit Garg > Date: Wed Apr 22 16:29:59 2020 +0530 > > irqchip/gic-v3: Setup arch specific IPI as pseudo NMI > > Add support to mark arch specific IPI as pseudo NMI. Currently its used > to allow arm64 specific IPI_CALL_NMI_FUNC to be marked as pseudo NMI. > > Brief description of changes: > - Update NMI setup/teardown routines for SGIs. > - Enable NMI support prior to gic_smp_init(). > - Setup custom flow handler for SGI setup as NMI. > - Request, prepare and enable arch specific IPI as per CPU NMI using > common APIs. > > Signed-off-by: Sumit Garg > > Please have a look and let me know if this is something you were looking for. > In case there are no major objections to this approach, I will post complete v2 patch-set (alongwith Marc's patches) for detailed review. -Sumit > [1] > https://git.linaro.org/people/sumit.garg/linux.git/commit/?h=kgdb-nmi&id=25c96663126264ec758c49a4a01a9c285f4ccc61 > > -Sumit > > > > Thanks, > > > > > > M. > > > -- > > > Jazz is not dead. It just smells funny...
Re: [PATCH] docs: dt: fix broken links due to txt->yaml renames
On Mon, May 4, 2020 at 12:56 PM Uwe Kleine-König wrote: > > Hi Sam, > > On Mon, May 04, 2020 at 07:45:22PM +0200, Sam Ravnborg wrote: > > On Mon, May 04, 2020 at 11:30:20AM +0200, Mauro Carvalho Chehab wrote: > > > There are some new broken doc links due to yaml renames > > > at DT. Developers should really run: > > > > > > ./scripts/documentation-file-ref-check > > > > > > in order to solve those issues while submitting patches. > > Would love if some bot could do this for me on any patches that creates > > .yaml files or so. > > I know I will forget this and it can be automated. > > If I get a bot mail that my patch would broke a link I would > > have it fixed before it hits any tree. I can probably add this to what I'm already checking. Not completely automated though as it depends on me to review before sending. > What about adding a check to check_patch? That would be the best way to get submitters to do this. Rob
Re: [PATCH v6 6/7] dt-bindings: input: Add docs for ADC driven joystick.
On Sun, 3 May 2020 19:14:50 +0200, Artur Rojek wrote: > Add documentation for the adc-joystick driver, used to provide support > for joysticks connected over ADC. > > Signed-off-by: Artur Rojek > Tested-by: Paul Cercueil > Reviewed-by: Rob Herring > --- > > Changes: > > v2: - Add `reg` property to axis subnode in order to enumerate the axes, > - rename `linux,abs-code` property to `linux,code`, > - drop `linux,` prefix from the remaining properties of axis subnode > > v3: no change > > v4: - remove "bindings" from the unique identifier string, > - replace `|` with `>` for all description properties, > - specify the number of items for `io-channels`, > - correct the regex pattern of `axis` property, > - specify the value range of `reg` property for each axis, > - put `abs-range` properties under `allOf` > > v5: add `a-f` to the regex pattern of `axis` property > > v6: no change > > .../bindings/input/adc-joystick.yaml | 121 ++ > 1 file changed, 121 insertions(+) > create mode 100644 Documentation/devicetree/bindings/input/adc-joystick.yaml > My bot found errors running 'make dt_binding_check' on your patch: Error: Documentation/devicetree/bindings/input/adc-joystick.example.dts:24.31-32 syntax error FATAL ERROR: Unable to parse input tree scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/input/adc-joystick.example.dt.yaml' failed make[1]: *** [Documentation/devicetree/bindings/input/adc-joystick.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs Makefile:1300: recipe for target 'dt_binding_check' failed make: *** [dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1282045 If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade Please check and re-submit.
Re: [PATCH net-next 1/7] dt-binding: ti: am65x: document common platform time sync cpts module
On Fri, 1 May 2020 23:50:05 +0300, Grygorii Strashko wrote: > Document device tree bindings for TI AM654/J721E SoC The Common Platform > Time Sync (CPTS) module. The CPTS module is used to facilitate host control > of time sync operations. Main features of CPTS module are: > - selection of multiple external clock sources > - 64-bit timestamp mode in ns with ppm and nudge adjustment. > - control of time sync events via interrupt or polling > - hardware timestamp of ext. events (HWx_TS_PUSH) > - periodic generator function outputs (TS_GENFx) > - PPS in combination with timesync router > - Depending on integration it enables compliance with the IEEE 1588-2008 > standard for a precision clock synchronization protocol, Ethernet Enhanced > Scheduled Traffic Operations (CPTS_ESTFn) and PCIe Subsystem Precision Time > Measurement (PTM). > > Signed-off-by: Grygorii Strashko > --- > .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 7 + > .../bindings/net/ti,k3-am654-cpts.yaml| 152 ++ > 2 files changed, 159 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/net/ti,k3-am654-cpts.yaml > My bot found errors running 'make dt_binding_check' on your patch: Documentation/devicetree/bindings/net/ti,k3-am654-cpts.yaml: $id: relative path/filename doesn't match actual path or filename expected: http://devicetree.org/schemas/net/ti,k3-am654-cpts.yaml# Unknown file referenced: [Errno 2] No such file or directory: '/usr/local/lib/python3.6/dist-packages/dtschema/schemas/net/ti,am654-cpts.yaml' Documentation/devicetree/bindings/Makefile:12: recipe for target 'Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dts' failed make[1]: *** [Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.example.dts] Error 255 make[1]: *** Waiting for unfinished jobs Makefile:1300: recipe for target 'dt_binding_check' failed make: *** [dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1281460 If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade Please check and re-submit.
Re: [PATCH net-next 1/7] dt-binding: ti: am65x: document common platform time sync cpts module
On Fri, May 01, 2020 at 11:50:05PM +0300, Grygorii Strashko wrote: > Document device tree bindings for TI AM654/J721E SoC The Common Platform > Time Sync (CPTS) module. The CPTS module is used to facilitate host control > of time sync operations. Main features of CPTS module are: > - selection of multiple external clock sources > - 64-bit timestamp mode in ns with ppm and nudge adjustment. > - control of time sync events via interrupt or polling > - hardware timestamp of ext. events (HWx_TS_PUSH) > - periodic generator function outputs (TS_GENFx) > - PPS in combination with timesync router > - Depending on integration it enables compliance with the IEEE 1588-2008 > standard for a precision clock synchronization protocol, Ethernet Enhanced > Scheduled Traffic Operations (CPTS_ESTFn) and PCIe Subsystem Precision Time > Measurement (PTM). > > Signed-off-by: Grygorii Strashko > --- > .../bindings/net/ti,k3-am654-cpsw-nuss.yaml | 7 + > .../bindings/net/ti,k3-am654-cpts.yaml| 152 ++ > 2 files changed, 159 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/net/ti,k3-am654-cpts.yaml > > diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml > b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml > index 78bf511e2892..0f3fde45e200 100644 > --- a/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml > +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpsw-nuss.yaml > @@ -144,6 +144,13 @@ patternProperties: > description: >CPSW MDIO bus. > > + "^cpts$": Fixed strings go under 'properties'. > +type: object > +allOf: > + - $ref: "ti,am654-cpts.yaml#" > +description: > + CPSW Common Platform Time Sync (CPTS) module. > + > required: >- compatible >- reg > diff --git a/Documentation/devicetree/bindings/net/ti,k3-am654-cpts.yaml > b/Documentation/devicetree/bindings/net/ti,k3-am654-cpts.yaml > new file mode 100644 > index ..1b535d41e5c6 > --- /dev/null > +++ b/Documentation/devicetree/bindings/net/ti,k3-am654-cpts.yaml > @@ -0,0 +1,152 @@ > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause) > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/net/ti,am654-cpts.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: The TI AM654x/J721E Common Platform Time Sync (CPTS) module Device > Tree Bindings > + > +maintainers: > + - Grygorii Strashko > + - Sekhar Nori > + > +description: |+ > + The TI AM654x/J721E CPTS module is used to facilitate host control of time > + sync operations. > + Main features of CPTS module are > + - selection of multiple external clock sources > + - Software control of time sync events via interrupt or polling > + - 64-bit timestamp mode in ns with PPM and nudge adjustment. > + - hardware timestamp push inputs (HWx_TS_PUSH) > + - timestamp counter compare output (TS_COMP) > + - timestamp counter bit output (TS_SYNC) > + - periodic Generator function outputs (TS_GENFx) > + - Ethernet Enhanced Scheduled Traffic Operations (CPTS_ESTFn) (TSN) > + - external hardware timestamp push inputs (HWx_TS_PUSH) timestamping > + > + Depending on integration it enables compliance with the IEEE 1588-2008 > + standard for a precision clock synchronization protocol, Ethernet Enhanced > + Scheduled Traffic Operations (CPTS_ESTFn) and PCIe Subsystem Precision > Time > + Measurement (PTM). > + > + TI AM654x/J721E SoCs has several similar CPTS modules integrated into the > + different parts of the system which could be synchronized with each other > + - Main CPTS > + - MCU CPSW CPTS with IEEE 1588-2008 support > + - PCIe subsystem CPTS for PTM support > + > + Depending on CPTS module integration and when CPTS is integral part of > + another module (MCU CPSW for example) "compatible" and "reg" can > + be omitted - parent module is fully responsible for CPTS enabling and > + configuration. That's fine, but you should still have compatible and reg. > + > +properties: > + $nodename: > +pattern: "^cpts(@.*|-[0-9a-f])*$" > + > + compatible: > +oneOf: > + - const: ti,am65-cpts > + - const: ti,j721e-cpts > + > + reg: > +maxItems: 1 > +description: > + The physical base address and size of CPTS IO range Wrong indentation. > + > + reg-names: > +items: > + - const: cpts Don't really need *-names when there's only one and you haven't picked very meaningful names. > + > + clocks: > +description: CPTS reference clock > + > + clock-names: > +items: > + - const: cpts > + > + interrupts-extended: Use 'interrupts' here, the tooling will fixup things to allow both. > +items: > + - description: CPTS events interrupt > + > + interrupt-names: > +items: > + - const: "cpts" Don't need quotes. > + > + ti,cpts-ext-ts-inputs: > +allOf: > + - $ref: /schemas/types.yaml#/definitions/uint32 > +maximum
[PATCH v1 3/5] [media] mtk-mdp: handle vpu_wdt_reg_handler() errors during probe
This is a cleanup to better handle errors during MDP probe.
Signed-off-by: ei...@chromium.org
Signed-off-by: Eizan Miyamoto
---
drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
index c20ac7681c6f..f974242663dc 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
@@ -188,8 +188,12 @@ static int mtk_mdp_probe(struct platform_device *pdev)
}
mdp->vpu_dev = vpu_get_plat_device(pdev);
- vpu_wdt_reg_handler(mdp->vpu_dev, mtk_mdp_reset_handler, mdp,
- VPU_RST_MDP);
+ ret = vpu_wdt_reg_handler(mdp->vpu_dev, mtk_mdp_reset_handler, mdp,
+ VPU_RST_MDP);
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to register reset handler\n");
+ goto err_wdt_reg;
+ }
platform_set_drvdata(pdev, mdp);
@@ -206,6 +210,8 @@ static int mtk_mdp_probe(struct platform_device *pdev)
err_set_max_seg_size:
+err_wdt_reg:
+
err_m2m_register:
v4l2_device_unregister(&mdp->v4l2_dev);
--
2.26.2.526.g744177e7f7-goog
Re: [RFC net-next 1/3] net: marvell: prestera: Add Switchdev driver for Prestera family ASIC device 98DX325x (AC3x)
Hi Ido,
On Sat, May 02, 2020 at 06:20:49PM +0300, Vadym Kochan wrote:
> Hi Ido,
>
> On Thu, Mar 05, 2020 at 04:49:37PM +0200, Ido Schimmel wrote:
> > On Tue, Feb 25, 2020 at 04:30:54PM +, Vadym Kochan wrote:
> > > +int mvsw_pr_port_learning_set(struct mvsw_pr_port *port, bool learn)
> > > +{
> > > + return mvsw_pr_hw_port_learning_set(port, learn);
> > > +}
> > > +
> > > +int mvsw_pr_port_flood_set(struct mvsw_pr_port *port, bool flood)
> > > +{
> > > + return mvsw_pr_hw_port_flood_set(port, flood);
> > > +}
> >
> > Flooding and learning are per-port attributes? Not per-{port, VLAN} ?
> > If so, you need to have various restrictions in the driver in case
> > someone configures multiple vlan devices on top of a port and enslaves
> > them to different bridges.
Yes, and there is no support for vlan device on top of the port.
> >
> > > +
> > > +
> > > + INIT_LIST_HEAD(&port->vlans_list);
> > > + port->pvid = MVSW_PR_DEFAULT_VID;
> >
> > If you're using VID 1, then you need to make sure that user cannot
> > configure a VLAN device with with this VID. If possible, I suggest that
> > you use VID 4095, as it cannot be configured from user space.
> >
> > I'm actually not entirely sure why you need a default VID.
> >
>
> > > +mvsw_pr_port_vlan_bridge_join(struct mvsw_pr_port_vlan *port_vlan,
> > > + struct mvsw_pr_bridge_port *br_port,
> > > + struct netlink_ext_ack *extack)
> > > +{
> > > + struct mvsw_pr_port *port = port_vlan->mvsw_pr_port;
> > > + struct mvsw_pr_bridge_vlan *br_vlan;
> > > + u16 vid = port_vlan->vid;
> > > + int err;
> > > +
> > > + if (port_vlan->bridge_port)
> > > + return 0;
> > > +
> > > + err = mvsw_pr_port_flood_set(port, br_port->flags & BR_FLOOD);
> > > + if (err)
> > > + return err;
> > > +
> > > + err = mvsw_pr_port_learning_set(port, br_port->flags & BR_LEARNING);
> > > + if (err)
> > > + goto err_port_learning_set;
> >
> > It seems that learning and flooding are not per-{port, VLAN} attributes,
> > so I'm not sure why you have this here.
> >
> > The fact that you don't undo this in mvsw_pr_port_vlan_bridge_leave()
> > tells me it should not be here.
> >
>
> > +
> > > +void
> > > +mvsw_pr_port_vlan_bridge_leave(struct mvsw_pr_port_vlan *port_vlan)
> > > +{
> > > + struct mvsw_pr_port *port = port_vlan->mvsw_pr_port;
> > > + struct mvsw_pr_bridge_vlan *br_vlan;
> > > + struct mvsw_pr_bridge_port *br_port;
> > > + int port_count;
> > > + u16 vid = port_vlan->vid;
> > > + bool last_port, last_vlan;
> > > +
> > > + br_port = port_vlan->bridge_port;
> > > + last_vlan = list_is_singular(&br_port->vlan_list);
> > > + port_count =
> > > + mvsw_pr_bridge_vlan_port_count_get(br_port->bridge_device, vid);
> > > + br_vlan = mvsw_pr_bridge_vlan_find(br_port, vid);
> > > + last_port = port_count == 1;
> > > + if (last_vlan) {
> > > + mvsw_pr_fdb_flush_port(port, MVSW_PR_FDB_FLUSH_MODE_DYNAMIC);
> > > + } else if (last_port) {
> > > + mvsw_pr_fdb_flush_vlan(port->sw, vid,
> > > +MVSW_PR_FDB_FLUSH_MODE_DYNAMIC);
> > > + } else {
> > > + mvsw_pr_fdb_flush_port_vlan(port, vid,
> > > + MVSW_PR_FDB_FLUSH_MODE_DYNAMIC);
> >
> > If you always flush based on {port, VID}, then why do you need the other
> > two?
> >
>
> > +
> > > +static int mvsw_pr_port_obj_attr_set(struct net_device *dev,
> > > + const struct switchdev_attr *attr,
> > > + struct switchdev_trans *trans)
> > > +{
> > > + int err = 0;
> > > + struct mvsw_pr_port *port = netdev_priv(dev);
> > > +
> > > + switch (attr->id) {
> > > + case SWITCHDEV_ATTR_ID_PORT_STP_STATE:
> > > + err = -EOPNOTSUPP;
> >
> > You don't support STP?
>
> Not, yet. But it will be in the next submission or official patch.
> >
> > > + break;
>
> > > + default:
> > > + kfree(switchdev_work);
> > > + return NOTIFY_DONE;
> > > + }
> > > +
> > > + queue_work(mvsw_owq, &switchdev_work->work);
> >
> > Once you defer the operation you cannot return an error, which is
> > problematic. Do you have a way to know if the operation will succeed or
> > not? That is, if the hardware has enough space for this new FDB entry?
> >
> Right, fdb configuration on via fw is blocking operation I still need to
> think on it if it is possible by current design.
>
>
> >
> > Why do you need both 'struct mvsw_pr_switchdev' and 'struct
> > mvsw_pr_bridge'? I think the second is enough. Also, I assume
> > 'switchdev' naming is inspired by mlxsw, but 'bridge' is better.
> >
> I changed to use bridge for bridge object, because having bridge_device
> may confuse.
>
> Thank you for your comments they were very useful, sorry for so late
> answer, I decided to re-implement this version a bit. Regarding flooding
> and default vid I still need to check it.
>
> Regards,
> Vadym Kochan
[PATCH v1 5/5] [media] mtk-mdp: Remove mtk_mdp_comp.id and supporting functionality
Since components are registered in a list, the numeric component id that
specified a location in an array is not necessary.
Signed-off-by: ei...@chromium.org
Signed-off-by: Eizan Miyamoto
---
drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 60 +++
drivers/media/platform/mtk-mdp/mtk_mdp_comp.h | 19 +-
drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 10 +---
3 files changed, 11 insertions(+), 78 deletions(-)
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
index d4afed1363d5..362fff924aef 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
@@ -14,46 +14,6 @@
#include "mtk_mdp_comp.h"
-static const char * const mtk_mdp_comp_stem[MTK_MDP_COMP_TYPE_MAX] = {
- "mdp_rdma",
- "mdp_rsz",
- "mdp_wdma",
- "mdp_wrot",
-};
-
-struct mtk_mdp_comp_match {
- enum mtk_mdp_comp_type type;
- int alias_id;
-};
-
-static const struct mtk_mdp_comp_match mtk_mdp_matches[MTK_MDP_COMP_ID_MAX] = {
- { MTK_MDP_RDMA, 0 },
- { MTK_MDP_RDMA, 1 },
- { MTK_MDP_RSZ, 0 },
- { MTK_MDP_RSZ, 1 },
- { MTK_MDP_RSZ, 2 },
- { MTK_MDP_WDMA, 0 },
- { MTK_MDP_WROT, 0 },
- { MTK_MDP_WROT, 1 },
-};
-
-int mtk_mdp_comp_get_id(struct device *dev, struct device_node *node,
- enum mtk_mdp_comp_type comp_type)
-{
- int id = of_alias_get_id(node, mtk_mdp_comp_stem[comp_type]);
- int i;
-
- for (i = 0; i < ARRAY_SIZE(mtk_mdp_matches); i++) {
- if (comp_type == mtk_mdp_matches[i].type &&
- id == mtk_mdp_matches[i].alias_id)
- return i;
- }
-
- dev_err(dev, "Failed to get id. type: %d, id: %d\n", comp_type, id);
-
- return -EINVAL;
-}
-
void mtk_mdp_comp_clock_on(struct device *dev, struct mtk_mdp_comp *comp)
{
int i, err;
@@ -62,8 +22,8 @@ void mtk_mdp_comp_clock_on(struct device *dev, struct
mtk_mdp_comp *comp)
err = mtk_smi_larb_get(comp->larb_dev);
if (err)
dev_err(dev,
- "failed to get larb, err %d. type:%d id:%d\n",
- err, comp->type, comp->id);
+ "failed to get larb, err %d. type:%d\n",
+ err, comp->type);
}
for (i = 0; i < ARRAY_SIZE(comp->clk); i++) {
@@ -72,8 +32,8 @@ void mtk_mdp_comp_clock_on(struct device *dev, struct
mtk_mdp_comp *comp)
err = clk_prepare_enable(comp->clk[i]);
if (err)
dev_err(dev,
- "failed to enable clock, err %d. type:%d id:%d i:%d\n",
- err, comp->type, comp->id, i);
+ "failed to enable clock, err %d. type:%d i:%d\n",
+ err, comp->type, i);
}
}
@@ -92,21 +52,15 @@ void mtk_mdp_comp_clock_off(struct device *dev, struct
mtk_mdp_comp *comp)
}
int mtk_mdp_comp_init(struct device *dev, struct device_node *node,
- struct mtk_mdp_comp *comp, enum mtk_mdp_comp_id comp_id)
+ struct mtk_mdp_comp *comp,
+ enum mtk_mdp_comp_type comp_type)
{
struct device_node *larb_node;
struct platform_device *larb_pdev;
int i;
- if (comp_id < 0 || comp_id >= MTK_MDP_COMP_ID_MAX) {
- dev_err(dev, "Invalid comp_id %d\n", comp_id);
- return -EINVAL;
- }
-
- INIT_LIST_HEAD(&comp->node);
comp->dev_node = of_node_get(node);
- comp->id = comp_id;
- comp->type = mtk_mdp_matches[comp_id].type;
+ comp->type = comp_type;
for (i = 0; i < ARRAY_SIZE(comp->clk); i++) {
comp->clk[i] = of_clk_get(node, i);
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h
b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h
index 1f745891c6c3..1bf0242cce46 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h
@@ -22,18 +22,6 @@ enum mtk_mdp_comp_type {
MTK_MDP_COMP_TYPE_MAX,
};
-enum mtk_mdp_comp_id {
- MTK_MDP_COMP_RDMA0,
- MTK_MDP_COMP_RDMA1,
- MTK_MDP_COMP_RSZ0,
- MTK_MDP_COMP_RSZ1,
- MTK_MDP_COMP_RSZ2,
- MTK_MDP_COMP_WDMA,
- MTK_MDP_COMP_WROT0,
- MTK_MDP_COMP_WROT1,
- MTK_MDP_COMP_ID_MAX,
-};
-
/**
* struct mtk_mdp_comp - the MDP's function component data
* @node: list node to track sibing MDP components
@@ -41,7 +29,6 @@ enum mtk_mdp_comp_id {
* @clk: clocks required for component
* @larb_dev: SMI device required for component
* @type: component type
- * @id:component ID
*/
struct mtk_mdp_comp {
struct list_headnode;
@@ -49,14 +36,12 @@ struct mtk_mdp_comp {
struct clk *clk[2
[PATCH v1 4/5] [media] mtk-mdp: convert mtk_mdp_dev.comp array to list
The functions mtk_mdp_register/unregister_component have been created to
add / remove items from the list of components.
This will eventually enable us to specify a list of components in the
device tree instead of hardcoding them into this driver.
The list is modified by a single thread at driver probe time, and will
not be traversed by another thread until the call to pm_runtime_enable
at the end of probing.
Signed-off-by: ei...@chromium.org
Signed-off-by: Eizan Miyamoto
---
drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 1 +
drivers/media/platform/mtk-mdp/mtk_mdp_comp.h | 2 +
drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 46 +--
drivers/media/platform/mtk-mdp/mtk_mdp_core.h | 10 +++-
4 files changed, 43 insertions(+), 16 deletions(-)
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
index facc6104b91f..d4afed1363d5 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
@@ -103,6 +103,7 @@ int mtk_mdp_comp_init(struct device *dev, struct
device_node *node,
return -EINVAL;
}
+ INIT_LIST_HEAD(&comp->node);
comp->dev_node = of_node_get(node);
comp->id = comp_id;
comp->type = mtk_mdp_matches[comp_id].type;
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h
b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h
index 3b83bd6e0d8b..1f745891c6c3 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h
@@ -36,6 +36,7 @@ enum mtk_mdp_comp_id {
/**
* struct mtk_mdp_comp - the MDP's function component data
+ * @node: list node to track sibing MDP components
* @dev_node: component device node
* @clk: clocks required for component
* @larb_dev: SMI device required for component
@@ -43,6 +44,7 @@ enum mtk_mdp_comp_id {
* @id:component ID
*/
struct mtk_mdp_comp {
+ struct list_headnode;
struct device_node *dev_node;
struct clk *clk[2];
struct device *larb_dev;
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
index f974242663dc..e6e702d9cb69 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
@@ -55,19 +55,19 @@ MODULE_DEVICE_TABLE(of, mtk_mdp_of_ids);
static void mtk_mdp_clock_on(struct mtk_mdp_dev *mdp)
{
struct device *dev = &mdp->pdev->dev;
- int i;
+ struct mtk_mdp_comp *comp_node;
- for (i = 0; i < ARRAY_SIZE(mdp->comp); i++)
- mtk_mdp_comp_clock_on(dev, mdp->comp[i]);
+ list_for_each_entry(comp_node, &mdp->comp_list, node)
+ mtk_mdp_comp_clock_on(dev, comp_node);
}
static void mtk_mdp_clock_off(struct mtk_mdp_dev *mdp)
{
struct device *dev = &mdp->pdev->dev;
- int i;
+ struct mtk_mdp_comp *comp_node;
- for (i = 0; i < ARRAY_SIZE(mdp->comp); i++)
- mtk_mdp_comp_clock_off(dev, mdp->comp[i]);
+ list_for_each_entry(comp_node, &mdp->comp_list, node)
+ mtk_mdp_comp_clock_off(dev, comp_node);
}
static void mtk_mdp_wdt_worker(struct work_struct *work)
@@ -91,12 +91,25 @@ static void mtk_mdp_reset_handler(void *priv)
queue_work(mdp->wdt_wq, &mdp->wdt_work);
}
+void mtk_mdp_register_component(struct mtk_mdp_dev *mdp,
+ struct mtk_mdp_comp *comp)
+{
+ list_add(&mdp->comp_list, &comp->node);
+}
+
+void mtk_mdp_unregister_component(struct mtk_mdp_dev *mdp,
+ struct mtk_mdp_comp *comp)
+{
+ list_del(&comp->node);
+}
+
static int mtk_mdp_probe(struct platform_device *pdev)
{
struct mtk_mdp_dev *mdp;
struct device *dev = &pdev->dev;
struct device_node *node, *parent;
- int i, ret = 0;
+ struct mtk_mdp_comp *comp, *comp_temp;
+ int ret = 0;
mdp = devm_kzalloc(dev, sizeof(*mdp), GFP_KERNEL);
if (!mdp)
@@ -104,6 +117,7 @@ static int mtk_mdp_probe(struct platform_device *pdev)
mdp->id = pdev->id;
mdp->pdev = pdev;
+ INIT_LIST_HEAD(&mdp->comp_list);
INIT_LIST_HEAD(&mdp->ctx_list);
mutex_init(&mdp->lock);
@@ -124,7 +138,6 @@ static int mtk_mdp_probe(struct platform_device *pdev)
const struct of_device_id *of_id;
enum mtk_mdp_comp_type comp_type;
int comp_id;
- struct mtk_mdp_comp *comp;
of_id = of_match_node(mtk_mdp_comp_dt_ids, node);
if (!of_id)
@@ -150,13 +163,14 @@ static int mtk_mdp_probe(struct platform_device *pdev)
of_node_put(node);
goto err_comp;
}
- mdp->comp[comp_id] = comp;
ret = mtk_mdp_comp_init(dev, node, comp, comp_id);
i
[PATCH v2] igb: Report speed and duplex as unknown when device is runtime suspended
igb device gets runtime suspended when there's no link partner. We can't
get correct speed under that state:
$ cat /sys/class/net/enp3s0/speed
1000
In addition to that, an error can also be spotted in dmesg:
[ 385.991957] igb :03:00.0 enp3s0: PCIe link lost
Since device can only be runtime suspended when there's no link partner,
we can skip reading register and let the following logic set speed and
duplex with correct status.
The more generic approach will be wrap get_link_ksettings() with begin()
and complete() callbacks. However, for this particular issue, begin()
calls igb_runtime_resume() , which tries to rtnl_lock() while the lock
is already hold by upper ethtool layer.
So let's take this approach until the igb_runtime_resume() no longer
needs to hold rtnl_lock.
Cc: Jeff Kirsher
Cc: Aaron Brown
Suggested-by: Alexander Duyck
Signed-off-by: Kai-Heng Feng
---
v2:
- Don't early return the routine so other info can be set.
drivers/net/ethernet/intel/igb/igb_ethtool.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c
b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index 39d3b76a6f5d..2cd003c5ad43 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -143,7 +143,8 @@ static int igb_get_link_ksettings(struct net_device *netdev,
u32 speed;
u32 supported, advertising;
- status = rd32(E1000_STATUS);
+ status = pm_runtime_suspended(&adapter->pdev->dev) ?
+0 : rd32(E1000_STATUS);
if (hw->phy.media_type == e1000_media_type_copper) {
supported = (SUPPORTED_10baseT_Half |
--
2.17.1
[PATCH v1 0/5] MTK MDP driver cleanups to prep for futher work
From: Eizan Miyamoto It most notably converts an array of MDP components to a list instead, but also removes some unused fields. This series of patches does some cleanup in preparation for futher work so that hardware video decode works on 4.19 and later kernels. We are planning on adding a dummy driver for the relevant MDP components that will be bound together using the component framework, which will enable calls to set up IOMMUs and LARBs, and make calls into pm_runtime. Eizan Miyamoto (5): [media] mtk-mdp: remove mtk_mdp_comp.regs from mtk_mdp_comp.h [media] mtk-mdp: handle vb2_dma_contig_set_max_seg_size errors during probe [media] mtk-mdp: handle vpu_wdt_reg_handler() errors during probe [media] mtk-mdp: convert mtk_mdp_dev.comp array to list [media] mtk-mdp: Remove mtk_mdp_comp.id and supporting functionality drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 60 ++- drivers/media/platform/mtk-mdp/mtk_mdp_comp.h | 23 +- drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 74 --- drivers/media/platform/mtk-mdp/mtk_mdp_core.h | 10 ++- 4 files changed, 68 insertions(+), 99 deletions(-) -- 2.26.2.526.g744177e7f7-goog
[PATCH v1 2/5] [media] mtk-mdp: handle vb2_dma_contig_set_max_seg_size errors during probe
This is a cleanup to better handle errors during MDP probe.
Signed-off-by: ei...@chromium.org
Signed-off-by: Eizan Miyamoto
---
drivers/media/platform/mtk-mdp/mtk_mdp_core.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
index aeaed2cf4458..c20ac7681c6f 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_core.c
@@ -193,13 +193,19 @@ static int mtk_mdp_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, mdp);
- vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
+ ret = vb2_dma_contig_set_max_seg_size(&pdev->dev, DMA_BIT_MASK(32));
+ if (ret) {
+ dev_err(&pdev->dev, "Failed to set vb2 dma mag seg size\n");
+ goto err_set_max_seg_size;
+ }
pm_runtime_enable(dev);
dev_dbg(dev, "mdp-%d registered successfully\n", mdp->id);
return 0;
+err_set_max_seg_size:
+
err_m2m_register:
v4l2_device_unregister(&mdp->v4l2_dev);
--
2.26.2.526.g744177e7f7-goog
[PATCH v1 1/5] [media] mtk-mdp: remove mtk_mdp_comp.regs from mtk_mdp_comp.h
These fields are not used and can be removed.
Signed-off-by: ei...@chromium.org
Signed-off-by: Eizan Miyamoto
---
drivers/media/platform/mtk-mdp/mtk_mdp_comp.c | 1 -
drivers/media/platform/mtk-mdp/mtk_mdp_comp.h | 2 --
2 files changed, 3 deletions(-)
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
index 14991685adb7..facc6104b91f 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.c
@@ -106,7 +106,6 @@ int mtk_mdp_comp_init(struct device *dev, struct
device_node *node,
comp->dev_node = of_node_get(node);
comp->id = comp_id;
comp->type = mtk_mdp_matches[comp_id].type;
- comp->regs = of_iomap(node, 0);
for (i = 0; i < ARRAY_SIZE(comp->clk); i++) {
comp->clk[i] = of_clk_get(node, i);
diff --git a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h
b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h
index 998a4b953025..3b83bd6e0d8b 100644
--- a/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h
+++ b/drivers/media/platform/mtk-mdp/mtk_mdp_comp.h
@@ -38,7 +38,6 @@ enum mtk_mdp_comp_id {
* struct mtk_mdp_comp - the MDP's function component data
* @dev_node: component device node
* @clk: clocks required for component
- * @regs: Mapped address of component registers.
* @larb_dev: SMI device required for component
* @type: component type
* @id:component ID
@@ -46,7 +45,6 @@ enum mtk_mdp_comp_id {
struct mtk_mdp_comp {
struct device_node *dev_node;
struct clk *clk[2];
- void __iomem*regs;
struct device *larb_dev;
enum mtk_mdp_comp_type type;
enum mtk_mdp_comp_idid;
--
2.26.2.526.g744177e7f7-goog
Re: [PATCH 2/2] dt-bindings: media: Document MSM8939 Venus
On Fri, 1 May 2020 22:35:03 +0200, Konrad Dybcio wrote: > Signed-off-by: Konrad Dybcio > --- > .../bindings/media/qcom,msm8939-venus.yaml| 119 ++ > 1 file changed, 119 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/media/qcom,msm8939-venus.yaml > My bot found errors running 'make dt_binding_check' on your patch: Documentation/devicetree/bindings/media/qcom,msm8939-venus.example.dts:20:18: fatal error: dt-bindings/clock/qcom,gcc-msm8939.h: No such file or directory #include ^~ compilation terminated. scripts/Makefile.lib:312: recipe for target 'Documentation/devicetree/bindings/media/qcom,msm8939-venus.example.dt.yaml' failed make[1]: *** [Documentation/devicetree/bindings/media/qcom,msm8939-venus.example.dt.yaml] Error 1 make[1]: *** Waiting for unfinished jobs Makefile:1300: recipe for target 'dt_binding_check' failed make: *** [dt_binding_check] Error 2 See https://patchwork.ozlabs.org/patch/1281452 If you already ran 'make dt_binding_check' and didn't see the above error(s), then make sure dt-schema is up to date: pip3 install git+https://github.com/devicetree-org/dt-schema.git@master --upgrade Please check and re-submit.
Re: [PATCH v3 00/12] Convert some DT documentation files to ReST
On Wed, Apr 15, 2020 at 9:45 AM Mauro Carvalho Chehab wrote: > > While most of the devicetree stuff has its own format (with is now being > converted to YAML format), some documents there are actually > describing the DT concepts and how to contribute to it. > > IMHO, those documents would fit perfectly as part of the documentation > body, as part of the firmare documents set. > > This patch series manually converts some DT documents that, on my > opinion, would belong to it. > > If you want to see how this would show at the documentation body, > a sneak peak of this series (together with the other pending > doc patches from me) is available at: > > https://www.infradead.org/~mchehab/kernel_docs/devicetree/index.html > > - > > v3: >- rebased on the top of next-20200414 > > Mauro Carvalho Chehab (12): > docs: dt: add an index.rst file for devicetree > docs: dt: convert usage-model.txt to ReST > docs: dt: usage_model.rst: fix link for DT usage > docs: dt: convert booting-without-of.txt to ReST format > docs: dt: convert changesets to ReST > docs: dt: convert dynamic-resolution-notes.txt to ReST > docs: dt: convert of_unittest.txt to ReST > docs: dt: convert overlay-notes.txt to ReST format > docs: dt: minor adjustments at writing-schema.rst > docs: dt: convert ABI.txt to ReST format > docs: dt: convert submitting-patches.txt to ReST format > docs: dt: convert writing-bindings.txt to ReST I've applied all but patch 4 as I'm working on just removing it. Rob
Re: [PATCH v7 6/7] OPP: Update the bandwidth on OPP frequency changes
On 04-05-20, 14:01, Saravana Kannan wrote: > Fair enough. But don't "voltage corner" based devices NEED to use OPP > framework to set their frequencies? No. Anyone can call dev_pm_genpd_set_performance_state(). -- viresh
Re: [PATCH net v1] net: broadcom: fix a mistake about ioremap resource
On 5/4/2020 7:03 PM, Dejin Zheng wrote:
> Commit d7a5502b0bb8b ("net: broadcom: convert to
> devm_platform_ioremap_resource_byname()") will broke this driver.
> idm_base and nicpm_base were optional, after this change, they are
> mandatory. it will probe fails with -22 when the dtb doesn't have them
> defined. so revert part of this commit and make idm_base and nicpm_base
> as optional.
>
> Fixes: d7a5502b0bb8bde ("net: broadcom: convert to
> devm_platform_ioremap_resource_byname()")
> Reported-by: Jonathan Richardson
> Cc: Scott Branden
> Cc: Ray Jui
> Cc: Florian Fainelli
> Cc: David S. Miller
> Signed-off-by: Dejin Zheng
Acked-by: Florian Fainelli
--
Florian
Re: [f2fs-dev] [PATCH] f2fs: remove redundant check in f2fs_force_buffered_io
On 05/05, Chao Yu wrote: > On 2020-5-4 22:35, Jaegeuk Kim wrote: > > From: Daeho Jeong > > > > We already checked whether the file is compressed or not in > > f2fs_post_read_required(). So removed f2fs_compressed_file() > > in f2fs_force_buffered_io(). > > Agreed, since I have sent similar patch before: > > https://lkml.org/lkml/2020/3/24/1819 Heh, as I couldn't find yours, I was actually waiting for you to point out. :) > > Just want to know what's the change of backport concern now. Old ICE support had to decouple f2fs_post_read_required() and f2fs_forced_buffered_io(). Now, I decide to manage this as we need to manage this for one kernel version only. Thanks, > > Thanks, > > > > > Signed-off-by: Daeho Jeong > > Signed-off-by: Jaegeuk Kim > > --- > > fs/f2fs/f2fs.h | 2 -- > > 1 file changed, 2 deletions(-) > > > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > index 6b7b963641696..01a00fc808361 100644 > > --- a/fs/f2fs/f2fs.h > > +++ b/fs/f2fs/f2fs.h > > @@ -4064,8 +4064,6 @@ static inline bool f2fs_force_buffered_io(struct > > inode *inode, > > return true; > > if (f2fs_is_multi_device(sbi)) > > return true; > > - if (f2fs_compressed_file(inode)) > > - return true; > > /* > > * for blkzoned device, fallback direct IO to buffered IO, so > > * all IOs can be serialized by log-structured write. > >
[PATCH v6 0/2] Add a watchdog driver that uses ARM Secure Monitor Calls.
This is currently supported in firmware deployed on oak, hana and elm mt8173 chromebook devices. The kernel driver is written to be a generic SMC watchdog driver. Arm Trusted Firmware upstreaming review: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/3405 Patch to add oak, hana, elm device tree: https://lore.kernel.org/linux-arm-kernel/20200110073730.213789-1-hsi...@chromium.org/ I would like to add the device tree support after the above patch is accepted. Changes in v6: - Don't use dt default - Use default arm,smc-id value if non provided by dt Changes in v5: - Change compatible to arm,smc-wdt - Make timeleft return 0 on error - Use type punning on smc_func_id to save an alloc - Change compatible to arm,smc-wdt Changes in v4: - Add arm,smc-id property - Get smc-id from of property - Return a1 instead of a0 in timeleft Changes in v3: - Change name back to arm - Add optional get_timeleft op - change name to arm_smc_wdt Changes in v2: - Change name arm > mt8173 - use watchdog_stop_on_reboot - use watchdog_stop_on_unregister - use devm_watchdog_register_device - remove smcwd_shutdown, smcwd_remove - change error codes Evan Benn (1): dt-bindings: watchdog: Add ARM smc wdt for mt8173 watchdog Julius Werner (1): watchdog: Add new arm_smc_wdt watchdog driver .../bindings/watchdog/arm-smc-wdt.yaml| 37 MAINTAINERS | 7 + arch/arm64/configs/defconfig | 1 + drivers/watchdog/Kconfig | 13 ++ drivers/watchdog/Makefile | 1 + drivers/watchdog/arm_smc_wdt.c| 188 ++ 6 files changed, 247 insertions(+) create mode 100644 Documentation/devicetree/bindings/watchdog/arm-smc-wdt.yaml create mode 100644 drivers/watchdog/arm_smc_wdt.c -- 2.26.2.526.g744177e7f7-goog
[PATCH v6 2/2] watchdog: Add new arm_smc_wdt watchdog driver
From: Julius Werner
This patch adds a watchdog driver that can be used on ARM systems
with the appropriate watchdog implemented in Secure Monitor firmware.
The driver communicates with firmware via a Secure Monitor Call.
This may be useful for platforms using TrustZone that want
the Secure Monitor firmware to have the final control over the watchdog.
This is implemented on mt8173 chromebook devices oak, elm and hana in
arm trusted firmware file plat/mediatek/mt8173/drivers/wdt/wdt.c.
Signed-off-by: Julius Werner
Signed-off-by: Evan Benn
---
Changes in v6:
- Use default arm,smc-id value if non provided by dt
Changes in v5:
- Make timeleft return 0 on error
- Use type punning on smc_func_id to save an alloc
- Change compatible to arm,smc-wdt
Changes in v4:
- Get smc-id from of property
- Return a1 instead of a0 in timeleft
Changes in v3:
- Add optional get_timeleft op
- change name to arm_smc_wdt
Changes in v2:
- use watchdog_stop_on_reboot
- use watchdog_stop_on_unregister
- use devm_watchdog_register_device
- remove smcwd_shutdown, smcwd_remove
- change error codes
MAINTAINERS| 1 +
arch/arm64/configs/defconfig | 1 +
drivers/watchdog/Kconfig | 13 +++
drivers/watchdog/Makefile | 1 +
drivers/watchdog/arm_smc_wdt.c | 188 +
5 files changed, 204 insertions(+)
create mode 100644 drivers/watchdog/arm_smc_wdt.c
diff --git a/MAINTAINERS b/MAINTAINERS
index 0f2b39767bfa9..2b782bbff200a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1462,6 +1462,7 @@ M:Julius Werner
R: Evan Benn
S: Maintained
F: devicetree/bindings/watchdog/arm-smc-wdt.yaml
+F: drivers/watchdog/arm_smc_wdt.c
ARM SMMU DRIVERS
M: Will Deacon
diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index 24e534d850454..0619df80f7575 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -513,6 +513,7 @@ CONFIG_UNIPHIER_THERMAL=y
CONFIG_WATCHDOG=y
CONFIG_ARM_SP805_WATCHDOG=y
CONFIG_ARM_SBSA_WATCHDOG=y
+CONFIG_ARM_SMC_WATCHDOG=y
CONFIG_S3C2410_WATCHDOG=y
CONFIG_DW_WATCHDOG=y
CONFIG_SUNXI_WATCHDOG=m
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 0663c604bd642..c440b576d23bf 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -867,6 +867,19 @@ config DIGICOLOR_WATCHDOG
To compile this driver as a module, choose M here: the
module will be called digicolor_wdt.
+config ARM_SMC_WATCHDOG
+ tristate "ARM Secure Monitor Call based watchdog support"
+ depends on ARM || ARM64
+ depends on OF
+ depends on HAVE_ARM_SMCCC
+ select WATCHDOG_CORE
+ help
+ Say Y here to include support for a watchdog timer
+ implemented by the EL3 Secure Monitor on ARM platforms.
+ Requires firmware support.
+ To compile this driver as a module, choose M here: the
+ module will be called arm_smc_wdt.
+
config LPC18XX_WATCHDOG
tristate "LPC18xx/43xx Watchdog"
depends on ARCH_LPC18XX || COMPILE_TEST
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 6de2e4ceef190..97bed1d3d97cb 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -94,6 +94,7 @@ obj-$(CONFIG_UNIPHIER_WATCHDOG) += uniphier_wdt.o
obj-$(CONFIG_RTD119X_WATCHDOG) += rtd119x_wdt.o
obj-$(CONFIG_SPRD_WATCHDOG) += sprd_wdt.o
obj-$(CONFIG_PM8916_WATCHDOG) += pm8916_wdt.o
+obj-$(CONFIG_ARM_SMC_WATCHDOG) += arm_smc_wdt.o
# X86 (i386 + ia64 + x86_64) Architecture
obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
diff --git a/drivers/watchdog/arm_smc_wdt.c b/drivers/watchdog/arm_smc_wdt.c
new file mode 100644
index 0..8f3d0c3a005fb
--- /dev/null
+++ b/drivers/watchdog/arm_smc_wdt.c
@@ -0,0 +1,188 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * ARM Secure Monitor Call watchdog driver
+ *
+ * Copyright 2020 Google LLC.
+ * Julius Werner
+ * Based on mtk_wdt.c
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define DRV_NAME "arm_smc_wdt"
+#define DRV_VERSION"1.0"
+
+enum smcwd_call {
+ SMCWD_INIT = 0,
+ SMCWD_SET_TIMEOUT = 1,
+ SMCWD_ENABLE= 2,
+ SMCWD_PET = 3,
+ SMCWD_GET_TIMELEFT = 4,
+};
+
+static bool nowayout = WATCHDOG_NOWAYOUT;
+static unsigned int timeout;
+
+static int smcwd_call(struct watchdog_device *wdd, enum smcwd_call call,
+ unsigned long arg, struct arm_smccc_res *res)
+{
+ struct arm_smccc_res local_res;
+
+ if (!res)
+ res = &local_res;
+
+ arm_smccc_smc((u32)(uintptr_t)watchdog_get_drvdata(wdd), call, arg, 0,
+ 0, 0, 0, 0, res);
+
+ if (res->a0 == PSCI_RET_NOT_SUPPORTED)
+ return -ENODEV;
+ if (res->a0 == PSCI_RET_INVALID_PARAMS)
+ return -EINVAL;
+ if
[PATCH v6 1/2] dt-bindings: watchdog: Add ARM smc wdt for mt8173 watchdog
This watchdog can be used on ARM systems with a Secure
Monitor firmware to forward watchdog operations to
firmware via a Secure Monitor Call.
Signed-off-by: Evan Benn
---
Changes in v6:
- Don't use dt default
Changes in v5:
- Change compatible to arm,smc-wdt
Changes in v4:
- Add arm,smc-id property
Changes in v3:
- Change name back to arm
Changes in v2:
- Change name arm > mt8173
.../bindings/watchdog/arm-smc-wdt.yaml| 37 +++
MAINTAINERS | 6 +++
2 files changed, 43 insertions(+)
create mode 100644 Documentation/devicetree/bindings/watchdog/arm-smc-wdt.yaml
diff --git a/Documentation/devicetree/bindings/watchdog/arm-smc-wdt.yaml
b/Documentation/devicetree/bindings/watchdog/arm-smc-wdt.yaml
new file mode 100644
index 0..bec651541e0c8
--- /dev/null
+++ b/Documentation/devicetree/bindings/watchdog/arm-smc-wdt.yaml
@@ -0,0 +1,37 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/watchdog/arm-smc-wdt.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: ARM Secure Monitor Call based watchdog
+
+allOf:
+ - $ref: "watchdog.yaml#"
+
+maintainers:
+ - Julius Werner
+
+properties:
+ compatible:
+enum:
+ - arm,smc-wdt
+ arm,smc-id:
+allOf:
+ - $ref: /schemas/types.yaml#/definitions/uint32
+description: |
+ The ATF smc function id used by the firmware.
+ Defaults to 0x82003D06 if unset.
+
+required:
+ - compatible
+
+examples:
+ - |
+watchdog {
+ compatible = "arm,smc-wdt";
+ arm,smc-id = <0x82003D06>;
+ timeout-sec = <15>;
+};
+
+...
diff --git a/MAINTAINERS b/MAINTAINERS
index b816a453b10eb..0f2b39767bfa9 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -1457,6 +1457,12 @@ S: Maintained
F: Documentation/devicetree/bindings/interrupt-controller/arm,vic.txt
F: drivers/irqchip/irq-vic.c
+ARM SMC WATCHDOG DRIVER
+M: Julius Werner
+R: Evan Benn
+S: Maintained
+F: devicetree/bindings/watchdog/arm-smc-wdt.yaml
+
ARM SMMU DRIVERS
M: Will Deacon
R: Robin Murphy
--
2.26.2.526.g744177e7f7-goog
Re: [PATCH v1 1/3] printk: honor the max_reason field in kmsg_dumper
On Mon, May 4, 2020 at 10:52 PM Pavel Tatashin
wrote:
>
> > > @@ -3157,12 +3162,9 @@ void kmsg_dump(enum kmsg_dump_reason reason)
> > > struct kmsg_dumper *dumper;
> > > unsigned long flags;
> > >
> > > - if ((reason > KMSG_DUMP_OOPS) && !always_kmsg_dump)
> > > - return;
> > > -
> > > rcu_read_lock();
> > > list_for_each_entry_rcu(dumper, &dump_list, list) {
> > > - if (dumper->max_reason && reason > dumper->max_reason)
> > > + if (reason > dumper->max_reason)
> > > continue;
> >
> > Why always_kmsg_dump check moved from the dumper loop entry point to the
> > dumper registration code? What if the user change always_ksmsg_dump
> > dynamically via sysfs?
>
> Hi Sergey,
>
> I changed it to make code cleaner: for such basic operation there are
> too many conditions if we will keep it inside the kmsg_dump().
> However, if being able to set always_kmsg_dump dynamically during
> runtime is deemed important, I can change it back to be checked in
> kmsg_dump.
If you agree that we do not have to modify this variable dynamically,
I will also change the permission here:
module_param_named(always_kmsg_dump, always_kmsg_dump, bool, S_IRUGO | S_IWUSR);
>
> Thank you,
> Pasha
>
> >
> > -ss
Re: [PATCH v8 4/4] kvm: vmx: virtualize split lock detection
On Wed, Apr 15, 2020 at 02:43:18PM -0700, Sean Christopherson wrote:
> On Wed, Apr 15, 2020 at 11:22:11PM +0200, Thomas Gleixner wrote:
> > Sean Christopherson writes:
> > > I don't see any way to avoid having KVM differentiate between sld_warn and
> > > sld_fatal. Even if KVM is able to virtualize SLD in sld_fatal mode, e.g.
> > > by telling the guest it must not try to disable SLD, KVM would still need
> > > to know the kernel is sld_fatal so that it can forward that information to
> > > the guest.
> >
> > Huch? There is absolutely zero code like that. The only place where
> > sld_state is used is:
> >
> > + static inline void vmx_update_sld(struct kvm_vcpu *vcpu, bool on)
> > + {
> > + if (sld_state == sld_warn && guest_cpu_has_feature_sld(vcpu) &&
> > + on == test_thread_flag(TIF_SLD)) {
> > + sld_update_msr(on);
> > + update_thread_flag(TIF_SLD, !on);
> > + }
> >
> > You might have some faint memories from the previous trainwrecks :)
>
> Yeah, I was thinking SLD was only being exposed if the host is sld_warn.
> I'll work with Xiaoyao to figure out a cleaner interface for this code.
...
> > So we can go with the proposed mode of allowing the write but not
> > propagating it. If the resulting split lock #AC originates from CPL != 3
> > then the guest will be killed with SIGBUS. If it originates from CPL ==
> > 3 and the guest has user #AC disabled then it will be killed as well.
>
> An idea that's been floated around to avoid killing the guest on a CPL==3
> split-lock #AC is to add a STICKY bit to MSR_TEST_CTRL that KVM can
> virtualize to tell the guest that attempting to disable SLD is futile,
> e.g. so that the guest can kill its misbehaving userspace apps instead of
> trying to disable SLD and getting killed by the host.
Circling back to this. KVM needs access to sld_state in one form or another
if we want to add a KVM hint when the host is in fatal mode. Three options
I've come up with:
1. Bite the bullet and export sld_state.
2. Add an is_split_fatal_wrapper(). Ugly since it needs to be non-inline
to avoid triggering (1).
3. Add a synthetic feature flag, e.g. X86_FEATURE_SLD_FATAL, and drop
sld_state altogether.
I like (3) because it requires the least amount of code when all is said
and done, doesn't require more exports, and as a bonus it'd probably be nice
for userspace to see sld_fatal in /proc/cpuinfo.
Thoughts?
Re: [PATCH 2/2] scripts/dtc: compile separate dtc-yaml
On Tue, May 5, 2020 at 4:15 AM Rob Herring wrote: > > On Sun, May 3, 2020 at 9:07 PM Masahiro Yamada wrote: > > > > Marek Behún reported a case where pkg-config fails to detect the > > libyaml-dev package, which is presumably a bug of the distro. > > > > Irrespective of that, I am not a big fan of pkg-config in the Makefile > > parse stage. The cost of pkg-config is quite small, but it is evaluated > > everytime we run make, even when we do 'make mrproper'. This commit > > changes the Makefile to not rely on pkg-config at all. > > I don't really love the solution here... I'm inclined to just make > libyaml always required. Anyone building dtbs should care about > validating them. However, there's some dts files sprinkled in the tree > such as DT unittests and I don't want to break allmodconfig for CI, > 0-day, etc. Though eventually we may have to. > > > The normal build should not require libyaml-dev while we need to compile > > dtc with libyaml for the schema check. > > > > Build two dtc variants: > > > > scripts/dtc/dtc for *.dts -> *.dtb > > scripts/dtc/dtc-yaml for *.dts -> *.dt.yaml > > My longer term plan is to integrate the schema checks into dtc. This > would be some sort of plugin to dtc found or specified at run-time. It > would eliminate the need for 2 passes of dtc and the 2nd case will go > away. OK. > > --- a/scripts/Makefile.lib > > +++ b/scripts/Makefile.lib > > @@ -246,6 +246,7 @@ quiet_cmd_gzip = GZIP$@ > > # DTC > > # > > --- > > DTC ?= $(objtree)/scripts/dtc/dtc > > +DTC_YAML ?= $(objtree)/scripts/dtc/dtc-yaml > > Can we make 'DTC' override both and keep this an internal detail. > No. For parallel building *.dtb and *.dt.yaml, they must be separate instances. > > +HOSTLDLIBS_dtc-yaml := -lyaml > > Does this work for yocto? As we had this before commit 067c650c456e. > Not clear if this changed for any reason or just 'let's use pkg-config > everywhere'. > > Or is there another way to fix yocto issue and we can just check the > header exists. I assume yocto needs some prefix in front of > '/usr/include/yaml.h'? > My bad - I missed 067c650c456e We need pkg-config to deal with yocto. Sorry, I take back this series. -- Best Regards Masahiro Yamada
[PATCH 1/2] ALSA: hda: Use dev_to_hdac_dev macro
Use dev_to_hdac_dev() instead of container_of().
No functional change intended.
Signed-off-by: Kai-Heng Feng
---
sound/hda/ext/hdac_ext_bus.c | 2 +-
sound/hda/hdac_device.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/hda/ext/hdac_ext_bus.c b/sound/hda/ext/hdac_ext_bus.c
index 73bfa71845f6..d0a604c939df 100644
--- a/sound/hda/ext/hdac_ext_bus.c
+++ b/sound/hda/ext/hdac_ext_bus.c
@@ -62,7 +62,7 @@ EXPORT_SYMBOL_GPL(snd_hdac_ext_bus_exit);
static void default_release(struct device *dev)
{
- snd_hdac_ext_bus_device_exit(container_of(dev, struct hdac_device,
dev));
+ snd_hdac_ext_bus_device_exit(dev_to_hdac_dev(dev));
}
/**
diff --git a/sound/hda/hdac_device.c b/sound/hda/hdac_device.c
index e3119f5cb0d5..333220f0f8af 100644
--- a/sound/hda/hdac_device.c
+++ b/sound/hda/hdac_device.c
@@ -20,7 +20,7 @@ static int get_codec_vendor_name(struct hdac_device *codec);
static void default_release(struct device *dev)
{
- snd_hdac_device_exit(container_of(dev, struct hdac_device, dev));
+ snd_hdac_device_exit(dev_to_hdac_dev(dev));
}
/**
--
2.17.1
[PATCH 2/2] ALSA: hda: Use hdac_to_hda_codec macro
Use hdac_to_hda_codec() instead of container_of().
No functional change intended.
Signed-off-by: Kai-Heng Feng
---
include/sound/hda_codec.h | 4
sound/pci/hda/patch_hdmi.c | 12 ++--
sound/soc/codecs/hdac_hda.h | 4
3 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/include/sound/hda_codec.h b/include/sound/hda_codec.h
index 225154a4f2ed..8a32060e3d8d 100644
--- a/include/sound/hda_codec.h
+++ b/include/sound/hda_codec.h
@@ -288,6 +288,10 @@ struct hda_codec {
#define dev_to_hda_codec(_dev) container_of(_dev, struct hda_codec, core.dev)
#define hda_codec_dev(_dev)(&(_dev)->core.dev)
+#define hdac_to_hda_priv(_hdac) \
+ container_of(_hdac, struct hdac_hda_priv, codec.core)
+#define hdac_to_hda_codec(_hdac) container_of(_hdac, struct hda_codec, core)
+
#define list_for_each_codec(c, bus) \
list_for_each_entry(c, &(bus)->core.codec_list, core.list)
#define list_for_each_codec_safe(c, n, bus)\
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c
index 4eff16053bd5..f21b66abb271 100644
--- a/sound/pci/hda/patch_hdmi.c
+++ b/sound/pci/hda/patch_hdmi.c
@@ -2022,7 +2022,7 @@ static const struct hda_pcm_ops generic_ops = {
static int hdmi_get_spk_alloc(struct hdac_device *hdac, int pcm_idx)
{
- struct hda_codec *codec = container_of(hdac, struct hda_codec, core);
+ struct hda_codec *codec = hdac_to_hda_codec(hdac);
struct hdmi_spec *spec = codec->spec;
struct hdmi_spec_per_pin *per_pin = pcm_idx_to_pin(spec, pcm_idx);
@@ -2035,7 +2035,7 @@ static int hdmi_get_spk_alloc(struct hdac_device *hdac,
int pcm_idx)
static void hdmi_get_chmap(struct hdac_device *hdac, int pcm_idx,
unsigned char *chmap)
{
- struct hda_codec *codec = container_of(hdac, struct hda_codec, core);
+ struct hda_codec *codec = hdac_to_hda_codec(hdac);
struct hdmi_spec *spec = codec->spec;
struct hdmi_spec_per_pin *per_pin = pcm_idx_to_pin(spec, pcm_idx);
@@ -2049,7 +2049,7 @@ static void hdmi_get_chmap(struct hdac_device *hdac, int
pcm_idx,
static void hdmi_set_chmap(struct hdac_device *hdac, int pcm_idx,
unsigned char *chmap, int prepared)
{
- struct hda_codec *codec = container_of(hdac, struct hda_codec, core);
+ struct hda_codec *codec = hdac_to_hda_codec(hdac);
struct hdmi_spec *spec = codec->spec;
struct hdmi_spec_per_pin *per_pin = pcm_idx_to_pin(spec, pcm_idx);
@@ -2065,7 +2065,7 @@ static void hdmi_set_chmap(struct hdac_device *hdac, int
pcm_idx,
static bool is_hdmi_pcm_attached(struct hdac_device *hdac, int pcm_idx)
{
- struct hda_codec *codec = container_of(hdac, struct hda_codec, core);
+ struct hda_codec *codec = hdac_to_hda_codec(hdac);
struct hdmi_spec *spec = codec->spec;
struct hdmi_spec_per_pin *per_pin = pcm_idx_to_pin(spec, pcm_idx);
@@ -3783,7 +3783,7 @@ static int atihdmi_paired_chmap_validate(struct
hdac_chmap *chmap,
static int atihdmi_pin_set_slot_channel(struct hdac_device *hdac,
hda_nid_t pin_nid, int hdmi_slot, int stream_channel)
{
- struct hda_codec *codec = container_of(hdac, struct hda_codec, core);
+ struct hda_codec *codec = hdac_to_hda_codec(hdac);
int verb;
int ati_channel_setup = 0;
@@ -3819,7 +3819,7 @@ static int atihdmi_pin_set_slot_channel(struct
hdac_device *hdac,
static int atihdmi_pin_get_slot_channel(struct hdac_device *hdac,
hda_nid_t pin_nid, int asp_slot)
{
- struct hda_codec *codec = container_of(hdac, struct hda_codec, core);
+ struct hda_codec *codec = hdac_to_hda_codec(hdac);
bool was_odd = false;
int ati_asp_slot = asp_slot;
int verb;
diff --git a/sound/soc/codecs/hdac_hda.h b/sound/soc/codecs/hdac_hda.h
index 598b07d9b6fe..d0efc5e254ae 100644
--- a/sound/soc/codecs/hdac_hda.h
+++ b/sound/soc/codecs/hdac_hda.h
@@ -28,10 +28,6 @@ struct hdac_hda_priv {
bool need_display_power;
};
-#define hdac_to_hda_priv(_hdac) \
- container_of(_hdac, struct hdac_hda_priv, codec.core)
-#define hdac_to_hda_codec(_hdac) container_of(_hdac, struct hda_codec, core)
-
struct hdac_ext_bus_ops *snd_soc_hdac_hda_get_ops(void);
#endif /* __HDAC_HDA_H__ */
--
2.17.1
Re: [PATCH 2/3] mfd: Intel Platform Monitoring Technology support
On 5/4/20 7:31 PM, David E. Box wrote: > diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig > index 0a59249198d3..c673031acdf1 100644 > --- a/drivers/mfd/Kconfig > +++ b/drivers/mfd/Kconfig > @@ -632,6 +632,16 @@ config MFD_INTEL_MSIC > Passage) chip. This chip embeds audio, battery, GPIO, etc. > devices used in Intel Medfield platforms. > > +config MFD_INTEL_PMT > + tristate "Intel Platform Monitoring Technology support" > + depends on PCI > + select MFD_CORE > + help > + The Intel Platform Monitoring Technology (PMT) is an interface that > + provides access to hardware monitor registers. This driver supports > + Telemetry, Watcher, and Crashlog PTM capabilities/devices for What is PTM? > + platforms starting from Tiger Lake. > + > config MFD_IPAQ_MICRO > bool "Atmel Micro ASIC (iPAQ h3100/h3600/h3700) Support" > depends on SA1100_H3100 || SA1100_H3600 -- ~Randy
Re: [PATCH 3/3] mm/hugetlb: Introduce HAVE_ARCH_CLEAR_HUGEPAGE_FLAGS
On 04/26/2020 08:31 AM, Andrew Morton wrote:
> On Sun, 26 Apr 2020 08:13:17 +0530 Anshuman Khandual
> wrote:
>
>>
>>
>> On 04/26/2020 06:25 AM, Andrew Morton wrote:
>>> On Tue, 14 Apr 2020 17:14:30 +0530 Anshuman Khandual
>>> wrote:
>>>
>>>> There are multiple similar definitions for arch_clear_hugepage_flags() on
>>>> various platforms. This introduces HAVE_ARCH_CLEAR_HUGEPAGE_FLAGS for those
>>>> platforms that need to define their own arch_clear_hugepage_flags() while
>>>> also providing a generic fallback definition for others to use. This help
>>>> reduce code duplication.
>>>>
>>>> ...
>>>>
>>>> --- a/include/linux/hugetlb.h
>>>> +++ b/include/linux/hugetlb.h
>>>> @@ -544,6 +544,10 @@ static inline int is_hugepage_only_range(struct
>>>> mm_struct *mm,
>>>> }
>>>> #endif
>>>>
>>>> +#ifndef HAVE_ARCH_CLEAR_HUGEPAGE_FLAGS
>>>> +static inline void arch_clear_hugepage_flags(struct page *page) { }
>>>> +#endif
>>>> +
>>>> #ifndef arch_make_huge_pte
>>>> static inline pte_t arch_make_huge_pte(pte_t entry, struct vm_area_struct
>>>> *vma,
>>>> struct page *page, int writable)
>>>
>>> This is the rather old-school way of doing it. The Linus-suggested way is
>>>
>>> #ifndef arch_clear_hugepage_flags
>>> static inline void arch_clear_hugepage_flags(struct page *page)
>>> {
>>> }
>>> #define arch_clear_hugepage_flags arch_clear_hugepage_flags
>>
>> Do we need that above line here ? Is not that implicit.
>
> It depends if other header files want to test whether
> arch_clear_hugepage_flags is already defined. If the header heorarchy
> is well-defined and working properly, they shouldn't need to, because
> we're reliably indluding the relevant arch header before (or early
> within) include/linux/hugetlb.h.
>
> It would be nice if
>
> #define arch_clear_hugepage_flags arch_clear_hugepage_flags
> #define arch_clear_hugepage_flags arch_clear_hugepage_flags
>
> were to generate an compiler error but it doesn't. If it did we could
> detect these incorrect inclusion orders.
>
>>> #endif
>>>
>>> And the various arch headers do
>>>
>>> static inline void arch_clear_hugepage_flags(struct page *page)
>>> {
>>>
>>> }
>>> #define arch_clear_hugepage_flags arch_clear_hugepage_flags
>>>
>>> It's a small difference - mainly to avoid adding two variables to the
>>> overall namespace where one would do.
>>
>> Understood, will change and resend.
>
> That's OK - I've queued up that fix.
>
Hello Andrew,
I might not have searched all the relevant trees or might have just searched
earlier than required. But I dont see these patches (or your proposed fixes)
either in mmotm (2020-04-29-23-04) or in next-20200504. Wondering if you are
waiting on a V2 for this series accommodating the changes you had proposed.
- Anshuman
Re: [PATCH v1 1/3] printk: honor the max_reason field in kmsg_dumper
> > @@ -3157,12 +3162,9 @@ void kmsg_dump(enum kmsg_dump_reason reason)
> > struct kmsg_dumper *dumper;
> > unsigned long flags;
> >
> > - if ((reason > KMSG_DUMP_OOPS) && !always_kmsg_dump)
> > - return;
> > -
> > rcu_read_lock();
> > list_for_each_entry_rcu(dumper, &dump_list, list) {
> > - if (dumper->max_reason && reason > dumper->max_reason)
> > + if (reason > dumper->max_reason)
> > continue;
>
> Why always_kmsg_dump check moved from the dumper loop entry point to the
> dumper registration code? What if the user change always_ksmsg_dump
> dynamically via sysfs?
Hi Sergey,
I changed it to make code cleaner: for such basic operation there are
too many conditions if we will keep it inside the kmsg_dump().
However, if being able to set always_kmsg_dump dynamically during
runtime is deemed important, I can change it back to be checked in
kmsg_dump.
Thank you,
Pasha
>
> -ss
RE: [PATCH net-next] ixgbe: fix signed-integer-overflow warning
> -Original Message- > From: Xie XiuQi > Sent: Monday, May 4, 2020 19:45 > To: Kirsher, Jeffrey T ; da...@davemloft.net > Cc: intel-wired-...@lists.osuosl.org; net...@vger.kernel.org; linux- > ker...@vger.kernel.org > Subject: [PATCH net-next] ixgbe: fix signed-integer-overflow warning > > ubsan report this warning, fix it by adding a unsigned suffix. > > UBSAN: signed-integer-overflow in > drivers/net/ethernet/intel/ixgbe/ixgbe_common.c:2246:26 > 65535 * 65537 cannot be represented in type 'int' > CPU: 21 PID: 7 Comm: kworker/u256:0 Not tainted 5.7.0-rc3-debug+ #39 > Hardware name: Huawei TaiShan 2280 V2/BC82AMDC, BIOS 2280-V2 > 03/27/2020 > Workqueue: ixgbe ixgbe_service_task [ixgbe] Call trace: > dump_backtrace+0x0/0x3f0 > show_stack+0x28/0x38 > dump_stack+0x154/0x1e4 > ubsan_epilogue+0x18/0x60 > handle_overflow+0xf8/0x148 > __ubsan_handle_mul_overflow+0x34/0x48 > ixgbe_fc_enable_generic+0x4d0/0x590 [ixgbe] > ixgbe_service_task+0xc20/0x1f78 [ixgbe] > process_one_work+0x8f0/0xf18 > worker_thread+0x430/0x6d0 > kthread+0x218/0x238 > ret_from_fork+0x10/0x18 > > Reported-by: Hulk Robot > Signed-off-by: Xie XiuQi > --- > drivers/net/ethernet/intel/ixgbe/ixgbe_common.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Dave, I am picking this up.
[PATCH v5 0/8] Bug fixes and improved logging in MHI
A set of patches for bug fixes and improved logging in mhi/core/boot.c. Verified on x86 and arm64 platforms. v5: -Updated the macro MHI_RANDOM_U32_NONZERO to take a bitmask as the input parameter and output a non-zero value between 1 and U32_MAX v4: -Dropped the change: bus: mhi: core: WARN_ON for malformed vector table -Updated bus: mhi: core: Read transfer length from an event properly to include parse rsc events -Use prandom_u32_max() instead of prandom_u32 to avoid if check in bus: mhi: core: Ensure non-zero session or sequence ID values are used v3: -Fixed signed-off-by tags -Add a refactor patch for MHI queue APIs -Commit text fix in bus: mhi: core: Read transfer length from an event properly -Fix channel ID range check for ctrl and data event rings processing v2: -Fix channel ID range check potential infinite loop -Add appropriate signed-off-by tags Bhaumik Bhatt (4): bus: mhi: core: Handle firmware load using state worker bus: mhi: core: Return appropriate error codes for AMSS load failure bus: mhi: core: Improve debug logs for loading firmware bus: mhi: core: Ensure non-zero session or sequence ID values are used Hemant Kumar (4): bus: mhi: core: Refactor mhi queue APIs bus: mhi: core: Cache intmod from mhi event to mhi channel bus: mhi: core: Add range check for channel id received in event ring bus: mhi: core: Read transfer length from an event properly drivers/bus/mhi/core/boot.c | 75 drivers/bus/mhi/core/init.c | 5 +- drivers/bus/mhi/core/internal.h | 6 +- drivers/bus/mhi/core/main.c | 124 drivers/bus/mhi/core/pm.c | 6 +- include/linux/mhi.h | 2 - 6 files changed, 109 insertions(+), 109 deletions(-) -- The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project
[PATCH v5 3/8] bus: mhi: core: Add range check for channel id received in event ring
From: Hemant Kumar
MHI data completion handler function reads channel id from event
ring element. Value is under the control of MHI devices and can be
any value between 0 and 255. In order to prevent out of bound access
add a bound check against the max channel supported by controller
and skip processing of that event ring element.
Signed-off-by: Hemant Kumar
Signed-off-by: Bhaumik Bhatt
Reviewed-by: Jeffrey Hugo
---
drivers/bus/mhi/core/main.c | 8
1 file changed, 8 insertions(+)
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index 605640c..e60ab21 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -776,6 +776,9 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
case MHI_PKT_TYPE_TX_EVENT:
chan = MHI_TRE_GET_EV_CHID(local_rp);
mhi_chan = &mhi_cntrl->mhi_chan[chan];
+ if (WARN_ON(chan >= mhi_cntrl->max_chan))
+ goto next_event;
+
parse_xfer_event(mhi_cntrl, local_rp, mhi_chan);
event_quota--;
break;
@@ -784,6 +787,7 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
break;
}
+next_event:
mhi_recycle_ev_ring_element(mhi_cntrl, ev_ring);
local_rp = ev_ring->rp;
dev_rp = mhi_to_virtual(ev_ring, er_ctxt->rp);
@@ -820,6 +824,9 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
enum mhi_pkt_type type = MHI_TRE_GET_EV_TYPE(local_rp);
chan = MHI_TRE_GET_EV_CHID(local_rp);
+ if (WARN_ON(chan >= mhi_cntrl->max_chan))
+ goto next_event;
+
mhi_chan = &mhi_cntrl->mhi_chan[chan];
if (likely(type == MHI_PKT_TYPE_TX_EVENT)) {
@@ -830,6 +837,7 @@ int mhi_process_data_event_ring(struct mhi_controller
*mhi_cntrl,
event_quota--;
}
+next_event:
mhi_recycle_ev_ring_element(mhi_cntrl, ev_ring);
local_rp = ev_ring->rp;
dev_rp = mhi_to_virtual(ev_ring, er_ctxt->rp);
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v5 8/8] bus: mhi: core: Ensure non-zero session or sequence ID values are used
While writing any sequence or session identifiers, it is possible that
the host could write a zero value, whereas only non-zero values should
be supported writes to those registers. Ensure that the host does not
write a non-zero value for them and also log them in debug messages.
Signed-off-by: Bhaumik Bhatt
---
drivers/bus/mhi/core/boot.c | 15 +++
drivers/bus/mhi/core/internal.h | 2 ++
2 files changed, 9 insertions(+), 8 deletions(-)
diff --git a/drivers/bus/mhi/core/boot.c b/drivers/bus/mhi/core/boot.c
index e5fcde1..7b9b561 100644
--- a/drivers/bus/mhi/core/boot.c
+++ b/drivers/bus/mhi/core/boot.c
@@ -43,10 +43,7 @@ void mhi_rddm_prepare(struct mhi_controller *mhi_cntrl,
lower_32_bits(mhi_buf->dma_addr));
mhi_write_reg(mhi_cntrl, base, BHIE_RXVECSIZE_OFFS, mhi_buf->len);
- sequence_id = prandom_u32() & BHIE_RXVECSTATUS_SEQNUM_BMSK;
-
- if (unlikely(!sequence_id))
- sequence_id = 1;
+ sequence_id = MHI_RANDOM_U32_NONZERO(BHIE_RXVECSTATUS_SEQNUM_BMSK);
mhi_write_reg_field(mhi_cntrl, base, BHIE_RXVECDB_OFFS,
BHIE_RXVECDB_SEQNUM_BMSK, BHIE_RXVECDB_SEQNUM_SHFT,
@@ -189,7 +186,9 @@ static int mhi_fw_load_amss(struct mhi_controller
*mhi_cntrl,
return -EIO;
}
- dev_dbg(dev, "Starting AMSS download via BHIe\n");
+ sequence_id = MHI_RANDOM_U32_NONZERO(BHIE_TXVECSTATUS_SEQNUM_BMSK);
+ dev_dbg(dev, "Starting AMSS download via BHIe. Sequence ID:%u\n",
+ sequence_id);
mhi_write_reg(mhi_cntrl, base, BHIE_TXVECADDR_HIGH_OFFS,
upper_32_bits(mhi_buf->dma_addr));
@@ -198,7 +197,6 @@ static int mhi_fw_load_amss(struct mhi_controller
*mhi_cntrl,
mhi_write_reg(mhi_cntrl, base, BHIE_TXVECSIZE_OFFS, mhi_buf->len);
- sequence_id = prandom_u32() & BHIE_TXVECSTATUS_SEQNUM_BMSK;
mhi_write_reg_field(mhi_cntrl, base, BHIE_TXVECDB_OFFS,
BHIE_TXVECDB_SEQNUM_BMSK, BHIE_TXVECDB_SEQNUM_SHFT,
sequence_id);
@@ -246,14 +244,15 @@ static int mhi_fw_load_sbl(struct mhi_controller
*mhi_cntrl,
goto invalid_pm_state;
}
- dev_dbg(dev, "Starting SBL download via BHI\n");
+ session_id = MHI_RANDOM_U32_NONZERO(BHI_TXDB_SEQNUM_BMSK);
+ dev_dbg(dev, "Starting SBL download via BHI. Session ID:%u\n",
+ session_id);
mhi_write_reg(mhi_cntrl, base, BHI_STATUS, 0);
mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_HIGH,
upper_32_bits(dma_addr));
mhi_write_reg(mhi_cntrl, base, BHI_IMGADDR_LOW,
lower_32_bits(dma_addr));
mhi_write_reg(mhi_cntrl, base, BHI_IMGSIZE, size);
- session_id = prandom_u32() & BHI_TXDB_SEQNUM_BMSK;
mhi_write_reg(mhi_cntrl, base, BHI_IMGTXDB, session_id);
read_unlock_bh(pm_lock);
diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 0965ca3..3205a92 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -452,6 +452,8 @@ enum mhi_pm_state {
#define PRIMARY_CMD_RING 0
#define MHI_DEV_WAKE_DB127
#define MHI_MAX_MTU0x
+#define MHI_RANDOM_U32_NONZERO(bmsk) ((prandom_u32_max(U32_MAX - 1) & \
+(bmsk)) + 1)
enum mhi_er_type {
MHI_ER_TYPE_INVALID = 0x0,
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
[PATCH v5 1/8] bus: mhi: core: Refactor mhi queue APIs
From: Hemant Kumar
Move all the common code to generate TRE from mhi_queue_buf,
mhi_queue_dma and mhi_queue_skb to mhi_gen_tre. This helps
to centralize the TRE generation code which makes any future
bug fixing easier to manage in these APIs.
Suggested-by: Jeffrey Hugo
Signed-off-by: Hemant Kumar
Signed-off-by: Bhaumik Bhatt
Reviewed-by: Jeffrey Hugo
---
drivers/bus/mhi/core/internal.h | 3 +-
drivers/bus/mhi/core/main.c | 107 +---
2 files changed, 47 insertions(+), 63 deletions(-)
diff --git a/drivers/bus/mhi/core/internal.h b/drivers/bus/mhi/core/internal.h
index 095d95b..40c47f9 100644
--- a/drivers/bus/mhi/core/internal.h
+++ b/drivers/bus/mhi/core/internal.h
@@ -670,8 +670,7 @@ int mhi_process_ctrl_ev_ring(struct mhi_controller
*mhi_cntrl,
irqreturn_t mhi_intvec_handler(int irq_number, void *dev);
int mhi_gen_tre(struct mhi_controller *mhi_cntrl, struct mhi_chan *mhi_chan,
- void *buf, void *cb, size_t buf_len, enum mhi_flags flags);
-
+ struct mhi_buf_info *info, enum mhi_flags flags);
int mhi_map_single_no_bb(struct mhi_controller *mhi_cntrl,
struct mhi_buf_info *buf_info);
int mhi_map_single_use_bb(struct mhi_controller *mhi_cntrl,
diff --git a/drivers/bus/mhi/core/main.c b/drivers/bus/mhi/core/main.c
index c26eed0..605640c 100644
--- a/drivers/bus/mhi/core/main.c
+++ b/drivers/bus/mhi/core/main.c
@@ -919,9 +919,7 @@ int mhi_queue_skb(struct mhi_device *mhi_dev, enum
dma_data_direction dir,
struct mhi_chan *mhi_chan = (dir == DMA_TO_DEVICE) ? mhi_dev->ul_chan :
mhi_dev->dl_chan;
struct mhi_ring *tre_ring = &mhi_chan->tre_ring;
- struct mhi_ring *buf_ring = &mhi_chan->buf_ring;
- struct mhi_buf_info *buf_info;
- struct mhi_tre *mhi_tre;
+ struct mhi_buf_info buf_info = { };
int ret;
/* If MHI host pre-allocates buffers then client drivers cannot queue */
@@ -946,27 +944,15 @@ int mhi_queue_skb(struct mhi_device *mhi_dev, enum
dma_data_direction dir,
/* Toggle wake to exit out of M2 */
mhi_cntrl->wake_toggle(mhi_cntrl);
- /* Generate the TRE */
- buf_info = buf_ring->wp;
-
- buf_info->v_addr = skb->data;
- buf_info->cb_buf = skb;
- buf_info->wp = tre_ring->wp;
- buf_info->dir = mhi_chan->dir;
- buf_info->len = len;
- ret = mhi_cntrl->map_single(mhi_cntrl, buf_info);
- if (ret)
- goto map_error;
-
- mhi_tre = tre_ring->wp;
-
- mhi_tre->ptr = MHI_TRE_DATA_PTR(buf_info->p_addr);
- mhi_tre->dword[0] = MHI_TRE_DATA_DWORD0(buf_info->len);
- mhi_tre->dword[1] = MHI_TRE_DATA_DWORD1(1, 1, 0, 0);
+ buf_info.v_addr = skb->data;
+ buf_info.cb_buf = skb;
+ buf_info.len = len;
- /* increment WP */
- mhi_add_ring_element(mhi_cntrl, tre_ring);
- mhi_add_ring_element(mhi_cntrl, buf_ring);
+ ret = mhi_gen_tre(mhi_cntrl, mhi_chan, &buf_info, mflags);
+ if (unlikely(ret)) {
+ read_unlock_bh(&mhi_cntrl->pm_lock);
+ return ret;
+ }
if (mhi_chan->dir == DMA_TO_DEVICE)
atomic_inc(&mhi_cntrl->pending_pkts);
@@ -980,11 +966,6 @@ int mhi_queue_skb(struct mhi_device *mhi_dev, enum
dma_data_direction dir,
read_unlock_bh(&mhi_cntrl->pm_lock);
return 0;
-
-map_error:
- read_unlock_bh(&mhi_cntrl->pm_lock);
-
- return ret;
}
EXPORT_SYMBOL_GPL(mhi_queue_skb);
@@ -996,9 +977,8 @@ int mhi_queue_dma(struct mhi_device *mhi_dev, enum
dma_data_direction dir,
mhi_dev->dl_chan;
struct device *dev = &mhi_cntrl->mhi_dev->dev;
struct mhi_ring *tre_ring = &mhi_chan->tre_ring;
- struct mhi_ring *buf_ring = &mhi_chan->buf_ring;
- struct mhi_buf_info *buf_info;
- struct mhi_tre *mhi_tre;
+ struct mhi_buf_info buf_info = { };
+ int ret;
/* If MHI host pre-allocates buffers then client drivers cannot queue */
if (mhi_chan->pre_alloc)
@@ -1025,25 +1005,16 @@ int mhi_queue_dma(struct mhi_device *mhi_dev, enum
dma_data_direction dir,
/* Toggle wake to exit out of M2 */
mhi_cntrl->wake_toggle(mhi_cntrl);
- /* Generate the TRE */
- buf_info = buf_ring->wp;
- WARN_ON(buf_info->used);
- buf_info->p_addr = mhi_buf->dma_addr;
- buf_info->pre_mapped = true;
- buf_info->cb_buf = mhi_buf;
- buf_info->wp = tre_ring->wp;
- buf_info->dir = mhi_chan->dir;
- buf_info->len = len;
-
- mhi_tre = tre_ring->wp;
-
- mhi_tre->ptr = MHI_TRE_DATA_PTR(buf_info->p_addr);
- mhi_tre->dword[0] = MHI_TRE_DATA_DWORD0(buf_info->len);
- mhi_tre->dword[1] = MHI_TRE_DATA_DWORD1(1, 1, 0, 0);
+ buf_info.p_addr = mhi_buf->dma_addr;
+ buf_info.cb_buf = mhi_buf;
+ buf_info.pre
