[PATCH v2 3/3] ASoC: fsl_micfil: Use SET_SYSTEM_SLEEP_PM_OPS to simplify PM
Use SET_SYSTEM_SLEEP_PM_OPS to simplify suspend and resume function.
fsl_micfil_suspend() and fsl_micfil_resume() can be deleted.
Signed-off-by: Chancel Liu
---
sound/soc/fsl/fsl_micfil.c | 18 ++
1 file changed, 2 insertions(+), 16 deletions(-)
diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index c4ff8ea49390..d6a5527ee7f6 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -1297,26 +1297,12 @@ static int fsl_micfil_runtime_resume(struct device *dev)
return 0;
}
-static int __maybe_unused fsl_micfil_suspend(struct device *dev)
-{
- pm_runtime_force_suspend(dev);
-
- return 0;
-}
-
-static int __maybe_unused fsl_micfil_resume(struct device *dev)
-{
- pm_runtime_force_resume(dev);
-
- return 0;
-}
-
static const struct dev_pm_ops fsl_micfil_pm_ops = {
SET_RUNTIME_PM_OPS(fsl_micfil_runtime_suspend,
fsl_micfil_runtime_resume,
NULL)
- SET_SYSTEM_SLEEP_PM_OPS(fsl_micfil_suspend,
- fsl_micfil_resume)
+ SET_SYSTEM_SLEEP_PM_OPS(pm_runtime_force_suspend,
+ pm_runtime_force_resume)
};
static struct platform_driver fsl_micfil_driver = {
--
2.25.1
[PATCH v2 2/3] ASoC: fsl_micfil: Add fsl_micfil_use_verid function
fsl_micfil_use_verid() can help to parse the version info in VERID and
PARAM registers. Since the two registers are added only on i.MX93
platform, a member flag called use_verid is introduced to soc data
structure which indicates acquiring MICFIL version.
Signed-off-by: Chancel Liu
---
sound/soc/fsl/fsl_micfil.c | 76 +-
sound/soc/fsl/fsl_micfil.h | 36 ++
2 files changed, 110 insertions(+), 2 deletions(-)
diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c
index b15febf19c02..c4ff8ea49390 100644
--- a/sound/soc/fsl/fsl_micfil.c
+++ b/sound/soc/fsl/fsl_micfil.c
@@ -56,6 +56,8 @@ struct fsl_micfil {
int vad_init_mode;
int vad_enabled;
int vad_detected;
+ struct fsl_micfil_verid verid;
+ struct fsl_micfil_param param;
};
struct fsl_micfil_soc_data {
@@ -64,6 +66,7 @@ struct fsl_micfil_soc_data {
unsigned int dataline;
bool imx;
bool use_edma;
+ bool use_verid;
u64 formats;
};
@@ -90,6 +93,7 @@ static struct fsl_micfil_soc_data fsl_micfil_imx93 = {
.dataline = 0xf,
.formats = SNDRV_PCM_FMTBIT_S32_LE,
.use_edma = true,
+ .use_verid = true,
};
static const struct of_device_id fsl_micfil_dt_ids[] = {
@@ -356,6 +360,49 @@ static const struct snd_kcontrol_new
fsl_micfil_snd_controls[] = {
SOC_SINGLE_BOOL_EXT("VAD Detected", 0, hwvad_detected, NULL),
};
+static int fsl_micfil_use_verid(struct device *dev)
+{
+ struct fsl_micfil *micfil = dev_get_drvdata(dev);
+ unsigned int val;
+ int ret;
+
+ if (!micfil->soc->use_verid)
+ return 0;
+
+ ret = regmap_read(micfil->regmap, REG_MICFIL_VERID, );
+ if (ret < 0)
+ return ret;
+
+ dev_dbg(dev, "VERID: 0x%016X\n", val);
+
+ micfil->verid.version = val &
+ (MICFIL_VERID_MAJOR_MASK | MICFIL_VERID_MINOR_MASK);
+ micfil->verid.version >>= MICFIL_VERID_MINOR_SHIFT;
+ micfil->verid.feature = val & MICFIL_VERID_FEATURE_MASK;
+
+ ret = regmap_read(micfil->regmap, REG_MICFIL_PARAM, );
+ if (ret < 0)
+ return ret;
+
+ dev_dbg(dev, "PARAM: 0x%016X\n", val);
+
+ micfil->param.hwvad_num = (val & MICFIL_PARAM_NUM_HWVAD_MASK) >>
+ MICFIL_PARAM_NUM_HWVAD_SHIFT;
+ micfil->param.hwvad_zcd = val & MICFIL_PARAM_HWVAD_ZCD;
+ micfil->param.hwvad_energy_mode = val & MICFIL_PARAM_HWVAD_ENERGY_MODE;
+ micfil->param.hwvad = val & MICFIL_PARAM_HWVAD;
+ micfil->param.dc_out_bypass = val & MICFIL_PARAM_DC_OUT_BYPASS;
+ micfil->param.dc_in_bypass = val & MICFIL_PARAM_DC_IN_BYPASS;
+ micfil->param.low_power = val & MICFIL_PARAM_LOW_POWER;
+ micfil->param.fil_out_width = val & MICFIL_PARAM_FIL_OUT_WIDTH;
+ micfil->param.fifo_ptrwid = (val & MICFIL_PARAM_FIFO_PTRWID_MASK) >>
+ MICFIL_PARAM_FIFO_PTRWID_SHIFT;
+ micfil->param.npair = (val & MICFIL_PARAM_NPAIR_MASK) >>
+ MICFIL_PARAM_NPAIR_SHIFT;
+
+ return 0;
+}
+
/* The SRES is a self-negated bit which provides the CPU with the
* capability to initialize the PDM Interface module through the
* slave-bus interface. This bit always reads as zero, and this
@@ -1037,6 +1084,9 @@ static irqreturn_t hwvad_err_isr(int irq, void *devid)
return IRQ_HANDLED;
}
+static int fsl_micfil_runtime_suspend(struct device *dev);
+static int fsl_micfil_runtime_resume(struct device *dev);
+
static int fsl_micfil_probe(struct platform_device *pdev)
{
struct device_node *np = pdev->dev.of_node;
@@ -1156,6 +1206,25 @@ static int fsl_micfil_probe(struct platform_device *pdev)
platform_set_drvdata(pdev, micfil);
pm_runtime_enable(>dev);
+ if (!pm_runtime_enabled(>dev)) {
+ ret = fsl_micfil_runtime_resume(>dev);
+ if (ret)
+ goto err_pm_disable;
+ }
+
+ ret = pm_runtime_resume_and_get(>dev);
+ if (ret < 0)
+ goto err_pm_get_sync;
+
+ /* Get micfil version */
+ ret = fsl_micfil_use_verid(>dev);
+ if (ret < 0)
+ dev_warn(>dev, "Error reading MICFIL version: %d\n", ret);
+
+ ret = pm_runtime_put_sync(>dev);
+ if (ret < 0 && ret != -ENOSYS)
+ goto err_pm_get_sync;
+
regcache_cache_only(micfil->regmap, true);
/*
@@ -1180,6 +1249,9 @@ static int fsl_micfil_probe(struct platform_device *pdev)
return ret;
+err_pm_get_sync:
+ if (!pm_runtime_status_suspended(>dev))
+ fsl_micfil_runtime_suspend(>dev);
err_pm_disable:
pm_runtime_disable(>dev);
@@ -1191,7 +1263,7 @@ static void fsl_micfil_remove(struct platform_device
*pdev)
pm_runtime_disable(>dev);
}
-static int __maybe_unused fsl_micfil_runtime_suspend(struct device *dev)
+static int fsl_micfil_runtime_suspend(struct device *dev)
{
struct
[PATCH v2 1/3] ASoC: fsl_micfil: Add new registers and new bit definition
MICFIL IP is upgraded on i.MX93 platform. These new registers and new bit definition are added to complete the register list. Signed-off-by: Chancel Liu --- sound/soc/fsl/fsl_micfil.c | 6 ++ sound/soc/fsl/fsl_micfil.h | 28 2 files changed, 34 insertions(+) diff --git a/sound/soc/fsl/fsl_micfil.c b/sound/soc/fsl/fsl_micfil.c index 3f08082a55be..b15febf19c02 100644 --- a/sound/soc/fsl/fsl_micfil.c +++ b/sound/soc/fsl/fsl_micfil.c @@ -825,6 +825,9 @@ static bool fsl_micfil_readable_reg(struct device *dev, unsigned int reg) case REG_MICFIL_DC_CTRL: case REG_MICFIL_OUT_CTRL: case REG_MICFIL_OUT_STAT: + case REG_MICFIL_FSYNC_CTRL: + case REG_MICFIL_VERID: + case REG_MICFIL_PARAM: case REG_MICFIL_VAD0_CTRL1: case REG_MICFIL_VAD0_CTRL2: case REG_MICFIL_VAD0_STAT: @@ -849,6 +852,7 @@ static bool fsl_micfil_writeable_reg(struct device *dev, unsigned int reg) case REG_MICFIL_DC_CTRL: case REG_MICFIL_OUT_CTRL: case REG_MICFIL_OUT_STAT: /* Write 1 to Clear */ + case REG_MICFIL_FSYNC_CTRL: case REG_MICFIL_VAD0_CTRL1: case REG_MICFIL_VAD0_CTRL2: case REG_MICFIL_VAD0_STAT: /* Write 1 to Clear */ @@ -873,6 +877,8 @@ static bool fsl_micfil_volatile_reg(struct device *dev, unsigned int reg) case REG_MICFIL_DATACH5: case REG_MICFIL_DATACH6: case REG_MICFIL_DATACH7: + case REG_MICFIL_VERID: + case REG_MICFIL_PARAM: case REG_MICFIL_VAD0_STAT: case REG_MICFIL_VAD0_NDATA: return true; diff --git a/sound/soc/fsl/fsl_micfil.h b/sound/soc/fsl/fsl_micfil.h index 9237a1c4cb8f..b3c392ef5daf 100644 --- a/sound/soc/fsl/fsl_micfil.h +++ b/sound/soc/fsl/fsl_micfil.h @@ -24,6 +24,9 @@ #define REG_MICFIL_DC_CTRL 0x64 #define REG_MICFIL_OUT_CTRL0x74 #define REG_MICFIL_OUT_STAT0x7C +#define REG_MICFIL_FSYNC_CTRL 0x80 +#define REG_MICFIL_VERID 0x84 +#define REG_MICFIL_PARAM 0x88 #define REG_MICFIL_VAD0_CTRL1 0x90 #define REG_MICFIL_VAD0_CTRL2 0x94 #define REG_MICFIL_VAD0_STAT 0x98 @@ -39,6 +42,8 @@ #define MICFIL_CTRL1_DBG BIT(28) #define MICFIL_CTRL1_SRES BIT(27) #define MICFIL_CTRL1_DBGE BIT(26) +#define MICFIL_CTRL1_DECFILS BIT(20) +#define MICFIL_CTRL1_FSYNCEN BIT(16) #define MICFIL_CTRL1_DISEL_DISABLE 0 #define MICFIL_CTRL1_DISEL_DMA 1 @@ -82,6 +87,29 @@ #define MICFIL_DC_CUTOFF_152Hz 2 #define MICFIL_DC_BYPASS 3 +/* MICFIL VERID Register -- REG_MICFIL_VERID */ +#define MICFIL_VERID_MAJOR_SHIFT24 +#define MICFIL_VERID_MAJOR_MASK GENMASK(31, 24) +#define MICFIL_VERID_MINOR_SHIFT16 +#define MICFIL_VERID_MINOR_MASK GENMASK(23, 16) +#define MICFIL_VERID_FEATURE_SHIFT 0 +#define MICFIL_VERID_FEATURE_MASK GENMASK(15, 0) + +/* MICFIL PARAM Register -- REG_MICFIL_PARAM */ +#define MICFIL_PARAM_NUM_HWVAD_SHIFT24 +#define MICFIL_PARAM_NUM_HWVAD_MASK GENMASK(27, 24) +#define MICFIL_PARAM_HWVAD_ZCD BIT(19) +#define MICFIL_PARAM_HWVAD_ENERGY_MODE BIT(17) +#define MICFIL_PARAM_HWVAD BIT(16) +#define MICFIL_PARAM_DC_OUT_BYPASS BIT(11) +#define MICFIL_PARAM_DC_IN_BYPASS BIT(10) +#define MICFIL_PARAM_LOW_POWER BIT(9) +#define MICFIL_PARAM_FIL_OUT_WIDTH BIT(8) +#define MICFIL_PARAM_FIFO_PTRWID_SHIFT 4 +#define MICFIL_PARAM_FIFO_PTRWID_MASK GENMASK(7, 4) +#define MICFIL_PARAM_NPAIR_SHIFT0 +#define MICFIL_PARAM_NPAIR_MASK GENMASK(3, 0) + /* MICFIL HWVAD0 Control 1 Register -- REG_MICFIL_VAD0_CTRL1*/ #define MICFIL_VAD0_CTRL1_CHSELGENMASK(26, 24) #define MICFIL_VAD0_CTRL1_CICOSR GENMASK(19, 16) -- 2.25.1
[PATCH v2 0/3] Update the register list of MICFIL
MICFIL IP is upgraded on i.MX93 platform. Add new registers and new bit definition. changes in v2: - rename check_version to use_verid to make it more explicit - rename fsl_micfil_check_version to fsl_micfil_use_verid Chancel Liu (3): ASoC: fsl_micfil: Add new registers and new bit definition ASoC: fsl_micfil: Add fsl_micfil_use_verid function ASoC: fsl_micfil: Use SET_SYSTEM_SLEEP_PM_OPS to simplify PM sound/soc/fsl/fsl_micfil.c | 100 ++--- sound/soc/fsl/fsl_micfil.h | 64 2 files changed, 146 insertions(+), 18 deletions(-) -- 2.25.1
Re: [PATCH v7 6/7] mm/memory_hotplug: Embed vmem_altmap details in memory block
On 8/2/23 4:40 AM, Verma, Vishal L wrote:
> On Tue, 2023-08-01 at 10:11 +0530, Aneesh Kumar K.V wrote:
>> With memmap on memory, some architecture needs more details w.r.t altmap
>> such as base_pfn, end_pfn, etc to unmap vmemmap memory. Instead of
>> computing them again when we remove a memory block, embed vmem_altmap
>> details in struct memory_block if we are using memmap on memory block
>> feature.
>>
>> Acked-by: David Hildenbrand
>> Signed-off-by: Aneesh Kumar K.V
>> ---
>> drivers/base/memory.c | 27 +
>> include/linux/memory.h | 8 ++
>> mm/memory_hotplug.c | 55 ++
>> 3 files changed, 53 insertions(+), 37 deletions(-)
>>
>
>
>> @@ -2136,10 +2148,10 @@ EXPORT_SYMBOL(try_offline_node);
>>
>> static int __ref try_remove_memory(u64 start, u64 size)
>> {
>> - struct vmem_altmap mhp_altmap = {};
>> - struct vmem_altmap *altmap = NULL;
>> - unsigned long nr_vmemmap_pages;
>> + int ret;
>
> Minor nit - there is already an 'int rc' below - just use that, or
> rename it to 'ret' if that's better for consistency.
>
I reused the existing rc variable.
>> + struct memory_block *mem;
>> int rc = 0, nid = NUMA_NO_NODE;
>> + struct vmem_altmap *altmap = NULL;
>>
>> BUG_ON(check_hotplug_memory_range(start, size));
>>
>> @@ -2161,25 +2173,20 @@ static int __ref try_remove_memory(u64 start, u64
>> size)
>> * the same granularity it was added - a single memory block.
>> */
>> if (mhp_memmap_on_memory()) {
>> - nr_vmemmap_pages = walk_memory_blocks(start, size, NULL,
>> -
>> get_nr_vmemmap_pages_cb);
>> - if (nr_vmemmap_pages) {
>> + ret = walk_memory_blocks(start, size, ,
>> test_has_altmap_cb);
>> + if (ret) {
>> if (size != memory_block_size_bytes()) {
>> pr_warn("Refuse to remove %#llx - %#llx,"
>> "wrong granularity\n",
>> start, start + size);
>> return -EINVAL;
>> }
>> -
>> + altmap = mem->altmap;
>> /*
>> - * Let remove_pmd_table->free_hugepage_table do the
>> - * right thing if we used vmem_altmap when hot-adding
>> - * the range.
>> + * Mark altmap NULL so that we can add a debug
>> + * check on memblock free.
>> */
>> - mhp_altmap.base_pfn = PHYS_PFN(start);
>> - mhp_altmap.free = nr_vmemmap_pages;
>> - mhp_altmap.alloc = nr_vmemmap_pages;
>> - altmap = _altmap;
>> + mem->altmap = NULL;
>> }
>> }
>>
Thank you for taking the time to review the patch.
-aneesh
Re: [PATCH v7 7/7] mm/memory_hotplug: Enable runtime update of memmap_on_memory parameter
On 8/1/23 4:20 PM, Michal Hocko wrote: > On Tue 01-08-23 14:58:29, Aneesh Kumar K V wrote: >> On 8/1/23 2:28 PM, Michal Hocko wrote: >>> On Tue 01-08-23 10:11:16, Aneesh Kumar K.V wrote: Allow updating memmap_on_memory mode after the kernel boot. Memory hotplug done after the mode update will use the new mmemap_on_memory value. >>> >>> Well, this is a user space kABI extension and as such you should spend >>> more words about the usecase. Why we could live with this static and now >>> need dynamic? >>> >> >> This enables easy testing of memmap_on_memory feature without a kernel >> reboot. > > Testing alone is rather weak argument to be honest. > >> I also expect people wanting to use that when they find dax kmem memory >> online >> failing because of struct page allocation failures[1]. User could reboot >> back with >> memmap_on_memory=y kernel parameter. But being able to enable it via sysfs >> makes >> the feature much more useful. > > Sure it can be useful but that holds for any feature, right. The main > question is whether this is worth maintaing. The current implementation > seems rather trivial which is an argument to have it but are there any > risks long term? Have you evaluated a potential long term maintenance > cost? There is no easy way to go back and disable it later on without > breaking some userspace. > > All that should be in the changelog! I updated it as below. mm/memory_hotplug: Enable runtime update of memmap_on_memory parameter Allow updating memmap_on_memory mode after the kernel boot. Memory hotplug done after the mode update will use the new mmemap_on_memory value. It is now possible to test the memmap_on_memory feature easily without the need for a kernel reboot. Additionally, for those encountering struct page allocation failures while using dax kmem memory online, this feature may prove useful. Instead of rebooting with the memmap_on_memory=y kernel parameter, users can now enable it via sysfs, which greatly enhances its usefulness. Making the necessary changes to support runtime updates is a simple change that should not affect the addition of new features or result in long-term maintenance problems.
Re: [PATCH v7 6/7] mm/memory_hotplug: Embed vmem_altmap details in memory block
On Tue, 2023-08-01 at 10:11 +0530, Aneesh Kumar K.V wrote:
> With memmap on memory, some architecture needs more details w.r.t altmap
> such as base_pfn, end_pfn, etc to unmap vmemmap memory. Instead of
> computing them again when we remove a memory block, embed vmem_altmap
> details in struct memory_block if we are using memmap on memory block
> feature.
>
> Acked-by: David Hildenbrand
> Signed-off-by: Aneesh Kumar K.V
> ---
> drivers/base/memory.c | 27 +
> include/linux/memory.h | 8 ++
> mm/memory_hotplug.c | 55 ++
> 3 files changed, 53 insertions(+), 37 deletions(-)
>
> @@ -2136,10 +2148,10 @@ EXPORT_SYMBOL(try_offline_node);
>
> static int __ref try_remove_memory(u64 start, u64 size)
> {
> - struct vmem_altmap mhp_altmap = {};
> - struct vmem_altmap *altmap = NULL;
> - unsigned long nr_vmemmap_pages;
> + int ret;
Minor nit - there is already an 'int rc' below - just use that, or
rename it to 'ret' if that's better for consistency.
> + struct memory_block *mem;
> int rc = 0, nid = NUMA_NO_NODE;
> + struct vmem_altmap *altmap = NULL;
>
> BUG_ON(check_hotplug_memory_range(start, size));
>
> @@ -2161,25 +2173,20 @@ static int __ref try_remove_memory(u64 start, u64
> size)
> * the same granularity it was added - a single memory block.
> */
> if (mhp_memmap_on_memory()) {
> - nr_vmemmap_pages = walk_memory_blocks(start, size, NULL,
> -
> get_nr_vmemmap_pages_cb);
> - if (nr_vmemmap_pages) {
> + ret = walk_memory_blocks(start, size, ,
> test_has_altmap_cb);
> + if (ret) {
> if (size != memory_block_size_bytes()) {
> pr_warn("Refuse to remove %#llx - %#llx,"
> "wrong granularity\n",
> start, start + size);
> return -EINVAL;
> }
> -
> + altmap = mem->altmap;
> /*
> - * Let remove_pmd_table->free_hugepage_table do the
> - * right thing if we used vmem_altmap when hot-adding
> - * the range.
> + * Mark altmap NULL so that we can add a debug
> + * check on memblock free.
> */
> - mhp_altmap.base_pfn = PHYS_PFN(start);
> - mhp_altmap.free = nr_vmemmap_pages;
> - mhp_altmap.alloc = nr_vmemmap_pages;
> - altmap = _altmap;
> + mem->altmap = NULL;
> }
> }
>
Re: [PATCH v7 2/2] PCI: rpaphp: Error out on busy status from get-sensor-state
On Mon, Jul 24, 2023 at 02:25:19PM +0530, Mahesh Salgaonkar wrote: > When certain PHB HW failure causes pHyp to recover PHB, it marks the PE > state as temporarily unavailable until recovery is complete. This also > triggers an EEH handler in Linux which needs to notify drivers, and perform > recovery. But before notifying the driver about the PCI error it uses > get_adapter_state()->get-sensor-state() operation of the hotplug_slot to > determine if the slot contains a device or not. if the slot is empty, the > recovery is skipped entirely. It's helpful to use the exact function name so it's greppable; I think get_adapter_status() or rpaphp_get_sensor_state()? s/if the slot is empty,/If the slot is empty,/ > However on certain PHB failures, the RTAS call get-sensor-state() returns > extended busy error (9902) until PHB is recovered by pHyp. Once PHB is > recovered, the get-sensor-state() returns success with correct presence > status. The RTAS call interface rtas_get_sensor() loops over the RTAS call > on extended delay return code (9902) until the return value is either > success (0) or error (-1). This causes the EEH handler to get stuck for ~6 > seconds before it could notify that the PCI error has been detected and > stop any active operations. Hence with running I/O traffic, during this 6 > seconds, the network driver continues its operation and hits a timeout > (netdev watchdog). > > > [52732.244731] DEBUG: ibm_read_slot_reset_state2() > [52732.244762] DEBUG: ret = 0, rets[0]=5, rets[1]=1, rets[2]=4000, rets[3]=> > [52732.244798] DEBUG: in eeh_slot_presence_check > [52732.244804] DEBUG: error state check > [52732.244807] DEBUG: Is slot hotpluggable > [52732.244810] DEBUG: hotpluggable ops ? > [52732.244953] DEBUG: Calling ops->get_adapter_status > [52732.244958] DEBUG: calling rpaphp_get_sensor_state > [52736.564262] [ cut here ] > [52736.564299] NETDEV WATCHDOG: enP64p1s0f3 (tg3): transmit queue 0 timed o> > [52736.564324] WARNING: CPU: 1442 PID: 0 at net/sched/sch_generic.c:478 dev> > [...] > [52736.564505] NIP [c0c32368] dev_watchdog+0x438/0x440 > [52736.564513] LR [c0c32364] dev_watchdog+0x434/0x440 > > > On timeouts, network driver starts dumping debug information to console > (e.g bnx2 driver calls bnx2x_panic_dump()), and go into recovery path while > pHyp is still recovering the PHB. As part of recovery, the driver tries to > reset the device and it keeps failing since every PCI read/write returns > ff's. And when EEH recovery kicks-in, the driver is unable to recover the > device. This impacts the ssh connection and leads to the system being > inaccessible. To get the NIC working again it needs a reboot or re-assign > the I/O adapter from HMC. > > [ 9531.168587] EEH: Beginning: 'slot_reset' > [ 9531.168601] PCI 0013:01:00.0#1: EEH: Invoking bnx2x->slot_reset() > [...] > [ 9614.110094] bnx2x: [bnx2x_func_stop:9129(enP19p1s0f0)]FUNC_STOP ramrod > failed. Running a dry transaction > [ 9614.110300] bnx2x: [bnx2x_igu_int_disable:902(enP19p1s0f0)]BUG! Proper val > not read from IGU! > [ 9629.178067] bnx2x: [bnx2x_fw_command:3055(enP19p1s0f0)]FW failed to > respond! > [ 9629.178085] bnx2x 0013:01:00.0 enP19p1s0f0: bc 7.10.4 > [ 9629.178091] bnx2x: [bnx2x_fw_dump_lvl:789(enP19p1s0f0)]Cannot dump MCP > info while in PCI error > [ 9644.241813] bnx2x: [bnx2x_io_slot_reset:14245(enP19p1s0f0)]IO slot reset > --> driver unload > [...] > [ 9644.241819] PCI 0013:01:00.0#1: EEH: bnx2x driver reports: 'disconnect' > [ 9644.241823] PCI 0013:01:00.1#1: EEH: Invoking bnx2x->slot_reset() > [ 9644.241827] bnx2x: [bnx2x_io_slot_reset:14229(enP19p1s0f1)]IO slot reset > initializing... > [ 9644.241916] bnx2x 0013:01:00.1: enabling device (0140 -> 0142) > [ 9644.258604] bnx2x: [bnx2x_io_slot_reset:14245(enP19p1s0f1)]IO slot reset > --> driver unload > [ 9644.258612] PCI 0013:01:00.1#1: EEH: bnx2x driver reports: 'disconnect' > [ 9644.258615] EEH: Finished:'slot_reset' with aggregate recovery > state:'disconnect' > [ 9644.258620] EEH: Unable to recover from failure from PHB#13-PE#1. > [ 9644.261811] EEH: Beginning: 'error_detected(permanent failure)' > [...] > [ 9644.261823] EEH: Finished:'error_detected(permanent failure)' > > Hence, it becomes important to inform driver about the PCI error detection > as early as possible, so that driver is aware of PCI error and waits for > EEH handler's next action for successful recovery. I don't really understand the connection between EEH and get_adapter_status(), but I guess this probably refers to arch/powerpc/kernel/eeh_driver.c, not the PCI core aer.c and err.c? > Current implementation uses rtas_get_sensor() API which blocks the slot > check state until RTAS call returns success. To avoid this, fix the PCI > hotplug driver (rpaphp) to return an error (-EBUSY) if the slot presence > state can not be detected immediately while PE is in EEH recovery state. > Change
[PATCH] powerpc/rtas_flash: allow user copy to flash block cache objects
From: Nathan Lynch
With hardened usercopy enabled (CONFIG_HARDENED_USERCOPY=y), using the
/proc/powerpc/rtas/firmware_update interface to prepare a system
firmware update yields a BUG():
kernel BUG at mm/usercopy.c:102!
Oops: Exception in kernel mode, sig: 5 [#1]
LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
Modules linked in:
CPU: 0 PID: 2232 Comm: dd Not tainted 6.5.0-rc3+ #2
Hardware name: IBM,8408-E8E POWER8E (raw) 0x4b0201 0xf04 of:IBM,FW860.50
(SV860_146) hv:phyp pSeries
NIP: c05991d0 LR: c05991cc CTR:
REGS: c000148c76a0 TRAP: 0700 Not tainted (6.5.0-rc3+)
MSR: 80029033 CR: 24002242 XER: 000c
CFAR: c01fbd34 IRQMASK: 0
[ ... GPRs omitted ... ]
NIP [c05991d0] usercopy_abort+0xa0/0xb0
LR [c05991cc] usercopy_abort+0x9c/0xb0
Call Trace:
[c000148c7940] [c05991cc] usercopy_abort+0x9c/0xb0 (unreliable)
[c000148c79b0] [c0536814] __check_heap_object+0x1b4/0x1d0
[c000148c79f0] [c0599080] __check_object_size+0x2d0/0x380
[c000148c7a30] [c0045ed4] rtas_flash_write+0xe4/0x250
[c000148c7a80] [c068a0fc] proc_reg_write+0xfc/0x160
[c000148c7ab0] [c05a381c] vfs_write+0xfc/0x4e0
[c000148c7b70] [c05a3e10] ksys_write+0x90/0x160
[c000148c7bc0] [c002f2c8] system_call_exception+0x178/0x320
[c000148c7e50] [c000d520] system_call_common+0x160/0x2c4
--- interrupt: c00 at 0x7fff9f17e5e4
The blocks of the firmware image are copied directly from user memory
to objects allocated from flash_block_cache, so flash_block_cache must
be created using kmem_cache_create_usercopy() to mark it safe for user
access.
Fixes: 6d07d1cd300f ("usercopy: Restrict non-usercopy caches to size 0")
Signed-off-by: Nathan Lynch
---
I believe it's much more common to update Power system firmware
without involving a Linux partition, which may explain why this has
gone unreported for so long.
---
arch/powerpc/kernel/rtas_flash.c | 12 +---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/rtas_flash.c b/arch/powerpc/kernel/rtas_flash.c
index 4caf5e3079eb..6a837264a196 100644
--- a/arch/powerpc/kernel/rtas_flash.c
+++ b/arch/powerpc/kernel/rtas_flash.c
@@ -698,6 +698,12 @@ static const struct rtas_flash_file rtas_flash_files[] = {
static int __init rtas_flash_init(void)
{
+ const unsigned int objsize = RTAS_BLK_SIZE;
+ const unsigned int usersize = objsize;
+ const unsigned int align = objsize;
+ void (* const ctor)(void *) = NULL;
+ const unsigned int useroffset = 0;
+ const slab_flags_t flags = 0;
int i;
if (rtas_function_token(RTAS_FN_IBM_UPDATE_FLASH_64_AND_REBOOT) ==
RTAS_UNKNOWN_SERVICE) {
@@ -709,9 +715,9 @@ static int __init rtas_flash_init(void)
if (!rtas_validate_flash_data.buf)
return -ENOMEM;
- flash_block_cache = kmem_cache_create("rtas_flash_cache",
- RTAS_BLK_SIZE, RTAS_BLK_SIZE, 0,
- NULL);
+ flash_block_cache = kmem_cache_create_usercopy("rtas_flash_cache",
+ objsize, align, flags,
+ useroffset, usersize,
ctor);
if (!flash_block_cache) {
printk(KERN_ERR "%s: failed to create block cache\n",
__func__);
---
base-commit: c3cad890877f59aeeaf5a638aa7a7c0612c16fa1
change-id: 20230731-rtas-flash-vs-hardened-usercopy-09a6d236b011
Best regards,
--
Nathan Lynch
Re: Login broken with old userspace (was Re: [PATCH v2] selinux: introduce an initial SID for early boot processes)
On Tue, Aug 1, 2023 at 9:24 AM Ondrej Mosnacek wrote:
> On Fri, Jul 28, 2023 at 5:12 PM Paul Moore wrote:
> >
> > On Fri, Jul 28, 2023 at 9:24 AM Christian Göttsche
> > wrote:
> > >
> > > On Fri, 28 Jul 2023 at 15:14, Ondrej Mosnacek wrote:
> > > >
> > > > On Fri, Jul 28, 2023 at 1:52 PM Stephen Smalley
> > > > wrote:
> > > > >
> > > > > On Fri, Jul 28, 2023 at 7:36 AM Ondrej Mosnacek
> > > > > wrote:
> > > > > >
> > > > > > On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman
> > > > > > wrote:
> > > > > > >
> > > > > > > Ondrej Mosnacek writes:
> > > > > > > > Currently, SELinux doesn't allow distinguishing between kernel
> > > > > > > > threads
> > > > > > > > and userspace processes that are started before the policy is
> > > > > > > > first
> > > > > > > > loaded - both get the label corresponding to the kernel SID.
> > > > > > > > The only
> > > > > > > > way a process that persists from early boot can get a
> > > > > > > > meaningful label
> > > > > > > > is by doing a voluntary dyntransition or re-executing itself.
> > > > > > >
> > > > > > > Hi,
> > > > > > >
> > > > > > > This commit breaks login for me when booting linux-next kernels
> > > > > > > with old
> > > > > > > userspace, specifically Ubuntu 16.04 on ppc64le. 18.04 is OK.
> > > > > > >
> > > > > > > The symptom is that login never accepts the root password, it just
> > > > > > > always says "Login incorrect".
> > > > > > >
> > > > > > > Bisect points to this commit.
> > > > > > >
> > > > > > > Reverting this commit on top of next-20230726, fixes the problem
> > > > > > > (ie. login works again).
> > > > > > >
> > > > > > > Booting with selinux=0 also fixes the problem.
> > > > > > >
> > > > > > > Is this expected? The change log below suggests backward
> > > > > > > compatibility
> > > > > > > was considered, is 16.04 just too old?
> > > > > >
> > > > > > Hi Michael,
> > > > > >
> > > > > > I can reproduce it on Fedora 38 when I boot with SELINUX=disabled in
> > > > > > /etc/selinux/config (+ a kernel including that commit), so it likely
> > > > > > isn't caused by the userspace being old. Can you check what you have
> > > > > > in /etc/selinux/config (or if it exists at all)?
> > > > > >
> > > > > > We have deprecated and removed the "runtime disable" functionality
> > > > > > in
> > > > > > SELinux recently [1], which was used to implement "disabling"
> > > > > > SELinux
> > > > > > via the /etc/selinux/config file, so now the situation (selinux=0 +
> > > > > > SELINUX=disabled in /etc/selinux/config) leads to a state where
> > > > > > SELinux is enabled, but no policy is loaded (and no enforcement is
> > > > > > done). Such a state mostly behaves as if SElinux was truly disabled
> > > > > > (via kernel command line), but there are some subtle differences
> > > > > > and I
> > > > > > believe we don't officially support it (Paul might clarify). With
> > > > > > latest kernels it is recommended to either disable SELinux via the
> > > > > > kernel command line (or Kconfig[2]) or to boot it in Enforcing or
> > > > > > Permissive mode with a valid/usable policy installed.
> > > > > >
> > > > > > So I wonder if Ubuntu ships by default with the bad configuration or
> > > > > > if it's just a result of using the custom-built linux-next kernel
> > > > > > (or
> > > > > > some changes on your part). If Ubuntu's stock kernel is configured
> > > > > > to
> > > > > > boot with SELinux enabled by default, they should also by default
> > > > > > ship
> > > > > > a usable policy and SELINUX=permissive/enforcing in
> > > > > > /etc/selinux/config (or configure the kernel[2] or bootloader to
> > > > > > boot
> > > > > > with SELinux disabled by default). (Although if they ship a pre-[1]
> > > > > > kernel, they may continue to rely on the runtime disable
> > > > > > functionality, but it means people will have to be careful when
> > > > > > booting newer or custom kernels.)
> > > > > >
> > > > > > That said, I'd like to get to the bottom of why the commit causes
> > > > > > the
> > > > > > login to fail and fix it somehow. I presume something in PAM chokes
> > > > > > on
> > > > > > the fact that userspace tasks now have "init" instead of "kernel" as
> > > > > > the pre-policy-load security context, but so far I haven't been able
> > > > > > to pinpoint the problem. I'll keep digging...
> > > > > >
> > > > > > [1]
> > > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f22f9aaf6c3d92ebd5ad9e67acc03afebaaeb289
> > > > > > [2] via CONFIG_LSM (or CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE on
> > > > > > older kernels)
> > > > >
> > > > > Prior to selinux userspace commit
> > > > > 685f4aeeadc0b60f3770404d4f149610d656e3c8 ("libselinux:
> > > > > is_selinux_enabled(): drop no-policy-loaded test.") libselinux was
> > > > > checking the result of reading /proc/self/attr/current to see if it
> > > > > returned the "kernel" string as a means of detecting a system with
> > > > > SELinux enabled but no
Re: [PATCH v2] dmaengine: Explicitly include correct DT includes
On Tue, 18 Jul 2023 08:31:35 -0600, Rob Herring wrote: > The DT of_device.h and of_platform.h date back to the separate > of_platform_bus_type before it as merged into the regular platform bus. > As part of that merge prepping Arm DT support 13 years ago, they > "temporarily" include each other. They also include platform_device.h > and of.h. As a result, there's a pretty much random mix of those include > files used throughout the tree. In order to detangle these headers and > replace the implicit includes with struct declarations, users need to > explicitly include the correct includes. > > [...] Applied, thanks! [1/1] dmaengine: Explicitly include correct DT includes commit: 897500c7ea91702966adb9b412fa39400b4edee6 Best regards, -- ~Vinod
Re: [RFC PATCH v2 5/7] media: imx: fsl_asrc: Add memory to memory driver
On Tue, Jul 25, 2023 at 10:31 AM Shengjiu Wang wrote: > > Implement the ASRC memory to memory function using > the v4l2 framework, user can use this function with > v4l2 ioctl interface. > > User send the output and capture buffer to driver and > driver store the converted data to the capture buffer. > > This feature can be shared by ASRC and EASRC drivers > > Signed-off-by: Shengjiu Wang > --- > drivers/media/platform/nxp/Kconfig| 12 + > drivers/media/platform/nxp/Makefile | 1 + > drivers/media/platform/nxp/fsl_asrc_m2m.c | 962 ++ > include/sound/fsl_asrc_common.h | 9 + > 4 files changed, 984 insertions(+) > create mode 100644 drivers/media/platform/nxp/fsl_asrc_m2m.c > > diff --git a/drivers/media/platform/nxp/Kconfig > b/drivers/media/platform/nxp/Kconfig > index a0ca6b297fb8..359f11fe2a80 100644 > --- a/drivers/media/platform/nxp/Kconfig > +++ b/drivers/media/platform/nxp/Kconfig > @@ -56,3 +56,15 @@ config VIDEO_MX2_EMMAPRP > > source "drivers/media/platform/nxp/dw100/Kconfig" > source "drivers/media/platform/nxp/imx-jpeg/Kconfig" > + > +config VIDEO_FSL_ASRC_M2M > + tristate "MXP i.MX ASRC M2M support" s/MXP/NXP > + depends on V4L_MEM2MEM_DRIVERS > + depends on MEDIA_SUPPORT > + select VIDEOBUF2_DMA_CONTIG > + select V4L2_MEM2MEM_DEV > + help > + Say Y if you want to add ASRC M2M support for NXP CPUs. > + It is a completement for ASRC M2P and ASRC P2M features. Complement for?
Re: [RFC PATCH v2 1/7] ASoC: fsl_asrc: define functions for memory to memory usage
> +static int fsl_asrc_m2m_check_format(u8 dir, u32 rate, u32 channels, u32
> format)
> +{
> + u64 support_format = FSL_ASRC_FORMATS;
> +
> + if (channels < 1 || channels > 10)
> + return -EINVAL;
> +
> + if (rate < 5512 || rate > 192000)
> + return -EINVAL;
> +
I think we can avoid using magic numbers. Instead we could do:
#define FSL_ASRC_MIN_CHANNELS 1
/...
#define FSL_ASRC_MAX_RATE 192000
> + if (dir == IN)
> + support_format |= SNDRV_PCM_FMTBIT_S8;
> +
> + if (!(1 << format & support_format))
> + return -EINVAL;
> +
> + return 0;
> +}
> +
> +/* calculate capture data length according to output data length and sample
> rate */
> +static int fsl_asrc_m2m_calc_out_len(struct fsl_asrc_pair *pair, int
> input_buffer_length)
> +{
> + unsigned int in_width, out_width;
> + unsigned int channels = pair->channels;
> + unsigned int in_samples, out_samples;
> + unsigned int out_length;
> +
> + in_width = snd_pcm_format_physical_width(pair->sample_format[IN]) / 8;
> + out_width = snd_pcm_format_physical_width(pair->sample_format[OUT]) /
> 8;
> +
> + in_samples = input_buffer_length / in_width / channels;
> + out_samples = pair->rate[OUT] * in_samples / pair->rate[IN];
> + out_length = (out_samples - ASRC_OUTPUT_LAST_SAMPLE) * out_width *
> channels;
> +
> + return out_length;
> +}
> +
> +static int fsl_asrc_m2m_get_maxburst(u8 dir, struct fsl_asrc_pair *pair)
> +{
> + struct fsl_asrc *asrc = pair->asrc;
> + struct fsl_asrc_priv *asrc_priv = asrc->private;
> + int wml = (dir == IN) ? ASRC_M2M_INPUTFIFO_WML :
> ASRC_M2M_OUTPUTFIFO_WML;
> +
> + if (!asrc_priv->soc->use_edma)
> + return wml * pair->channels;
> + else
> + return 1;
> +}
> +
> +static int fsl_asrc_m2m_pair_resume(struct fsl_asrc_pair *pair)
> +{
> + struct fsl_asrc *asrc = pair->asrc;
> + int i;
> +
> + for (i = 0; i < pair->channels * 4; i++)
> + regmap_write(asrc->regmap, REG_ASRDI(pair->index), 0);
> +
> + return 0;
> +}
> +
> static int fsl_asrc_runtime_resume(struct device *dev);
> static int fsl_asrc_runtime_suspend(struct device *dev);
There is no implementation for _suspend although you mention it
in the commit message.
> + * @complete: dma task complete
> + * @sample_format: format of m2m
> + * @rate: rate of m2m
> + * @buf_len: buffer length of m2m
> + * @req_pair: flag for request pair
For example @complete field is not used in this patch. Maybe add it in the patch
that uses it?
I think is the same for other fields.
Re: Login broken with old userspace (was Re: [PATCH v2] selinux: introduce an initial SID for early boot processes)
On Fri, Jul 28, 2023 at 5:12 PM Paul Moore wrote:
>
> On Fri, Jul 28, 2023 at 9:24 AM Christian Göttsche
> wrote:
> >
> > On Fri, 28 Jul 2023 at 15:14, Ondrej Mosnacek wrote:
> > >
> > > On Fri, Jul 28, 2023 at 1:52 PM Stephen Smalley
> > > wrote:
> > > >
> > > > On Fri, Jul 28, 2023 at 7:36 AM Ondrej Mosnacek
> > > > wrote:
> > > > >
> > > > > On Fri, Jul 28, 2023 at 4:12 AM Michael Ellerman
> > > > > wrote:
> > > > > >
> > > > > > Ondrej Mosnacek writes:
> > > > > > > Currently, SELinux doesn't allow distinguishing between kernel
> > > > > > > threads
> > > > > > > and userspace processes that are started before the policy is
> > > > > > > first
> > > > > > > loaded - both get the label corresponding to the kernel SID. The
> > > > > > > only
> > > > > > > way a process that persists from early boot can get a meaningful
> > > > > > > label
> > > > > > > is by doing a voluntary dyntransition or re-executing itself.
> > > > > >
> > > > > > Hi,
> > > > > >
> > > > > > This commit breaks login for me when booting linux-next kernels
> > > > > > with old
> > > > > > userspace, specifically Ubuntu 16.04 on ppc64le. 18.04 is OK.
> > > > > >
> > > > > > The symptom is that login never accepts the root password, it just
> > > > > > always says "Login incorrect".
> > > > > >
> > > > > > Bisect points to this commit.
> > > > > >
> > > > > > Reverting this commit on top of next-20230726, fixes the problem
> > > > > > (ie. login works again).
> > > > > >
> > > > > > Booting with selinux=0 also fixes the problem.
> > > > > >
> > > > > > Is this expected? The change log below suggests backward
> > > > > > compatibility
> > > > > > was considered, is 16.04 just too old?
> > > > >
> > > > > Hi Michael,
> > > > >
> > > > > I can reproduce it on Fedora 38 when I boot with SELINUX=disabled in
> > > > > /etc/selinux/config (+ a kernel including that commit), so it likely
> > > > > isn't caused by the userspace being old. Can you check what you have
> > > > > in /etc/selinux/config (or if it exists at all)?
> > > > >
> > > > > We have deprecated and removed the "runtime disable" functionality in
> > > > > SELinux recently [1], which was used to implement "disabling" SELinux
> > > > > via the /etc/selinux/config file, so now the situation (selinux=0 +
> > > > > SELINUX=disabled in /etc/selinux/config) leads to a state where
> > > > > SELinux is enabled, but no policy is loaded (and no enforcement is
> > > > > done). Such a state mostly behaves as if SElinux was truly disabled
> > > > > (via kernel command line), but there are some subtle differences and I
> > > > > believe we don't officially support it (Paul might clarify). With
> > > > > latest kernels it is recommended to either disable SELinux via the
> > > > > kernel command line (or Kconfig[2]) or to boot it in Enforcing or
> > > > > Permissive mode with a valid/usable policy installed.
> > > > >
> > > > > So I wonder if Ubuntu ships by default with the bad configuration or
> > > > > if it's just a result of using the custom-built linux-next kernel (or
> > > > > some changes on your part). If Ubuntu's stock kernel is configured to
> > > > > boot with SELinux enabled by default, they should also by default ship
> > > > > a usable policy and SELINUX=permissive/enforcing in
> > > > > /etc/selinux/config (or configure the kernel[2] or bootloader to boot
> > > > > with SELinux disabled by default). (Although if they ship a pre-[1]
> > > > > kernel, they may continue to rely on the runtime disable
> > > > > functionality, but it means people will have to be careful when
> > > > > booting newer or custom kernels.)
> > > > >
> > > > > That said, I'd like to get to the bottom of why the commit causes the
> > > > > login to fail and fix it somehow. I presume something in PAM chokes on
> > > > > the fact that userspace tasks now have "init" instead of "kernel" as
> > > > > the pre-policy-load security context, but so far I haven't been able
> > > > > to pinpoint the problem. I'll keep digging...
> > > > >
> > > > > [1]
> > > > > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f22f9aaf6c3d92ebd5ad9e67acc03afebaaeb289
> > > > > [2] via CONFIG_LSM (or CONFIG_SECURITY_SELINUX_BOOTPARAM_VALUE on
> > > > > older kernels)
> > > >
> > > > Prior to selinux userspace commit
> > > > 685f4aeeadc0b60f3770404d4f149610d656e3c8 ("libselinux:
> > > > is_selinux_enabled(): drop no-policy-loaded test.") libselinux was
> > > > checking the result of reading /proc/self/attr/current to see if it
> > > > returned the "kernel" string as a means of detecting a system with
> > > > SELinux enabled but no policy loaded, and treated that as if SELinux
> > > > were disabled. Hence, this does break old userspace. Not sure though
> > > > why you'd see the same behavior with modern libselinux.
> > >
> > > Hm... now I tried booting the stock Fedora kernel (without the early
> > > boot initial SID commit) and I got the same failure to login as with
> > > the new kernel.
Re: [PATCH v2 23/28] net: wan: framer: Add support for the Lantiq PEF2256 framer
On Tue, 1 Aug 2023 12:52:29 +0200
Andrew Lunn wrote:
> > > > +static void pef2256_isr_default_handler(struct pef2256 *pef2256, u8
> > > > nbr, u8 isr)
> > > > +{
> > > > + dev_warn(pef2256->dev, "ISR%u: 0x%02x not handled\n", nbr, isr);
> > > > +}
> > >
> > > Should this be rate limited? It is going to be very noise if it gets
> > > called once per frame time.
> >
> > This function should not be called.
> > It is wired on some interrupts and these interrupts should not be triggered.
> > It they fired, something was wrong.
> >
> > I would prefer to keep this dev_warn() to keep the user informed about the
> > problem.
>
> I would definitely keep it, but rate limit it. dev_warn_ratelimited().
>
> Andrew
Ok, will be changed to dev_warn_ratelimited().
Hervé
Re: [PATCH v2 23/28] net: wan: framer: Add support for the Lantiq PEF2256 framer
> > > +static void pef2256_isr_default_handler(struct pef2256 *pef2256, u8 nbr,
> > > u8 isr)
> > > +{
> > > + dev_warn(pef2256->dev, "ISR%u: 0x%02x not handled\n", nbr, isr);
> > > +}
> >
> > Should this be rate limited? It is going to be very noise if it gets
> > called once per frame time.
>
> This function should not be called.
> It is wired on some interrupts and these interrupts should not be triggered.
> It they fired, something was wrong.
>
> I would prefer to keep this dev_warn() to keep the user informed about the
> problem.
I would definitely keep it, but rate limit it. dev_warn_ratelimited().
Andrew
Re: [PATCH v7 7/7] mm/memory_hotplug: Enable runtime update of memmap_on_memory parameter
On Tue 01-08-23 14:58:29, Aneesh Kumar K V wrote: > On 8/1/23 2:28 PM, Michal Hocko wrote: > > On Tue 01-08-23 10:11:16, Aneesh Kumar K.V wrote: > >> Allow updating memmap_on_memory mode after the kernel boot. Memory > >> hotplug done after the mode update will use the new mmemap_on_memory > >> value. > > > > Well, this is a user space kABI extension and as such you should spend > > more words about the usecase. Why we could live with this static and now > > need dynamic? > > > > This enables easy testing of memmap_on_memory feature without a kernel reboot. Testing alone is rather weak argument to be honest. > I also expect people wanting to use that when they find dax kmem memory online > failing because of struct page allocation failures[1]. User could reboot back > with > memmap_on_memory=y kernel parameter. But being able to enable it via sysfs > makes > the feature much more useful. Sure it can be useful but that holds for any feature, right. The main question is whether this is worth maintaing. The current implementation seems rather trivial which is an argument to have it but are there any risks long term? Have you evaluated a potential long term maintenance cost? There is no easy way to go back and disable it later on without breaking some userspace. All that should be in the changelog! -- Michal Hocko SUSE Labs
Re: [PATCH v2 26/28] ASoC: codecs: Add support for the framer codec
On Tue, 1 Aug 2023 12:30:26 +0200
Andrew Lunn wrote:
> On Wed, Jul 26, 2023 at 05:02:22PM +0200, Herve Codina wrote:
> > The framer codec interracts with a framer.
> > It allows to use some of the framer timeslots as audio channels to
> > transport audio data over the framer E1/T1/J1 lines.
> > It also reports line carrier detection events through the ALSA jack
> > detection feature.
> >
> > Signed-off-by: Herve Codina
> > ---
> > sound/soc/codecs/Kconfig| 15 ++
> > sound/soc/codecs/Makefile | 2 +
> > sound/soc/codecs/framer-codec.c | 423
> > 3 files changed, 440 insertions(+)
> > create mode 100644 sound/soc/codecs/framer-codec.c
> >
> > diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> > index f99203ef9b03..a86cdac39b72 100644
> > --- a/sound/soc/codecs/Kconfig
> > +++ b/sound/soc/codecs/Kconfig
> > @@ -110,6 +110,7 @@ config SND_SOC_ALL_CODECS
> > imply SND_SOC_ES8328_I2C
> > imply SND_SOC_ES7134
> > imply SND_SOC_ES7241
> > + imply SND_SOC_FRAMER
> > imply SND_SOC_GTM601
> > imply SND_SOC_HDAC_HDMI
> > imply SND_SOC_HDAC_HDA
> > @@ -1043,6 +1044,20 @@ config SND_SOC_ES8328_SPI
> > depends on SPI_MASTER
> > select SND_SOC_ES8328
> >
> > +config SND_SOC_FRAMER
> > + tristate "Framer codec"
> > + depends on GENERIC_FRAMER
> > + help
> > + Enable support for the framer codec.
> > + The framer codec uses the generic framer infrastructure to transport
> > + some audio data over an analog E1/T1/J1 line.
> > + This codec allows to use some of the time slots available on the TDM
> > + bus on which the framer is connected to transport the audio data.
> > +
> > + To compile this driver as a module, choose M here: the module
> > + will be called snd-soc-framer.
> > +
> > +
> > config SND_SOC_GTM601
> > tristate 'GTM601 UMTS modem audio codec'
> >
> > diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> > index 32dcc6de58bd..54667274a0f6 100644
> > --- a/sound/soc/codecs/Makefile
> > +++ b/sound/soc/codecs/Makefile
> > @@ -116,6 +116,7 @@ snd-soc-es8326-objs := es8326.o
> > snd-soc-es8328-objs := es8328.o
> > snd-soc-es8328-i2c-objs := es8328-i2c.o
> > snd-soc-es8328-spi-objs := es8328-spi.o
> > +snd-soc-framer-objs := framer-codec.o
> > snd-soc-gtm601-objs := gtm601.o
> > snd-soc-hdac-hdmi-objs := hdac_hdmi.o
> > snd-soc-hdac-hda-objs := hdac_hda.o
> > @@ -499,6 +500,7 @@ obj-$(CONFIG_SND_SOC_ES8326)+= snd-soc-es8326.o
> > obj-$(CONFIG_SND_SOC_ES8328) += snd-soc-es8328.o
> > obj-$(CONFIG_SND_SOC_ES8328_I2C)+= snd-soc-es8328-i2c.o
> > obj-$(CONFIG_SND_SOC_ES8328_SPI)+= snd-soc-es8328-spi.o
> > +obj-$(CONFIG_SND_SOC_FRAMER) += snd-soc-framer.o
> > obj-$(CONFIG_SND_SOC_GTM601)+= snd-soc-gtm601.o
> > obj-$(CONFIG_SND_SOC_HDAC_HDMI) += snd-soc-hdac-hdmi.o
> > obj-$(CONFIG_SND_SOC_HDAC_HDA) += snd-soc-hdac-hda.o
> > diff --git a/sound/soc/codecs/framer-codec.c
> > b/sound/soc/codecs/framer-codec.c
> > new file mode 100644
> > index ..52b4546a61ee
> > --- /dev/null
> > +++ b/sound/soc/codecs/framer-codec.c
> > @@ -0,0 +1,423 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +//
> > +// Framer ALSA SoC driver
> > +//
> > +// Copyright 2023 CS GROUP France
> > +//
> > +// Author: Herve Codina
> > +
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +#include
> > +
> > +#define FRAMER_NB_CHANNEL 32
> > +#define FRAMER_JACK_MASK (SND_JACK_LINEIN | SND_JACK_LINEOUT)
> > +
> > +struct framer_codec {
> > + struct framer *framer;
> > + struct device *dev;
> > + struct snd_soc_jack jack;
> > + struct notifier_block nb;
> > + struct work_struct carrier_work;
> > + int max_chan_playback;
> > + int max_chan_capture;
> > +};
> > +
> > +static int framer_dai_set_tdm_slot(struct snd_soc_dai *dai, unsigned int
> > tx_mask,
> > + unsigned int rx_mask, int slots, int width)
> > +{
> > + struct framer_codec *framer =
> > snd_soc_component_get_drvdata(dai->component);
> > +
> > + switch (width) {
> > + case 0:
> > + /* Not set -> default 8 */
> > + case 8:
> > + break;
> > + default:
> > + dev_err(dai->dev, "tdm slot width %d not supported\n", width);
> > + return -EINVAL;
> > + }
> > +
> > + framer->max_chan_playback = hweight32(tx_mask);
> > + if (framer->max_chan_playback > FRAMER_NB_CHANNEL) {
> > + dev_err(dai->dev, "too much tx slots defined (mask = 0x%x)
> > support max %d\n",
>
> "many", not "much".
>
> Also, "supported".
Yes, will be fixed.
Regards,
Hervé
Re: [PATCH v2 23/28] net: wan: framer: Add support for the Lantiq PEF2256 framer
On Tue, 1 Aug 2023 12:22:39 +0200
Andrew Lunn wrote:
> > +static inline u8 pef2256_read8(struct pef2256 *pef2256, int offset)
> > +{
> > + int val;
> > +
> > + regmap_read(pef2256->regmap, offset, );
> > + return val;
> > +}
> > +
> > +static inline void pef2256_write8(struct pef2256 *pef2256, int offset, u8
> > val)
> > +{
> > + regmap_write(pef2256->regmap, offset, val);
> > +}
>
> More cases of inline functions in .C files. Please let the compiler
> decide.
Will be changed.
>
> > +static void pef2256_isr_default_handler(struct pef2256 *pef2256, u8 nbr,
> > u8 isr)
> > +{
> > + dev_warn(pef2256->dev, "ISR%u: 0x%02x not handled\n", nbr, isr);
> > +}
>
> Should this be rate limited? It is going to be very noise if it gets
> called once per frame time.
This function should not be called.
It is wired on some interrupts and these interrupts should not be triggered.
It they fired, something was wrong.
I would prefer to keep this dev_warn() to keep the user informed about the
problem.
Regards,
Hervé
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v2 21/28] dt-bindings: net: Add the Lantiq PEF2256 E1/T1/J1 framer
On Tue, 1 Aug 2023 12:05:07 +0200 Andrew Lunn wrote: > > + clocks: > > +items: > > + - description: Master clock > > + - description: Receive System Clock > > + - description: Transmit System Clock > > + > > + clock-names: > > +items: > > + - const: mclk > > + - const: sclkr > > + - const: sclkx > > Nit pick, but "Receive System Clock", but "sclkr'. Maybe "System Clock > Receive" so you have the same word order? > Will also change the 'Transmit System Clock' to 'System Clock Transmit' Regards, Hervé -- Hervé Codina, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Re: [PATCH v2 00/28] Add support for QMC HDLC, framer infrastruture and PEF2256 framer
> The generic framer has: > - 2 consumers (QMC HDLC drv and codec) > - 1 provider (PEF2256) > > So, the design is the following: > +--+ +-+ > | QMC | <- TDM -> | PEF2256 | <-> E1 > +-+| +-+ | | | > | CPU DAI | <-data--> | QMC channel | | | | > +-+| +-+ | | | > +--+| +-+ | | | > | QMC HDLC drv | <-data--> | QMC channel | | | | > +--+| +-+ | | | > ^ +--+ | | > | ++ +-+| | > +-> | framer | <-> | PEF2256 drv | <- local bus ->| | > || | |+-+ > +-> || | | > | ++ | +---+ | > +---> | codec | | > | +---+ | > +-+ Thanks for adding the framer framework. I did not look into all the details, but the basic design looks good. Andrew
Re: [PATCH v2 20/28] net: wan: Add framer framework support
On Tue, 1 Aug 2023 11:56:12 +0200
Andrew Lunn wrote:
> > +int framer_pm_runtime_get(struct framer *framer)
> > +{
> > + int ret;
> > +
> > + if (!framer)
> > + return 0;
>
> Can framer be a NULL pointer? This sort of test often covers up
> bugs. So either let it dereference the NULL pointer and opps, or
> return -EINVAL.
>
Well, in the current usage, it cannot be a NULL pointer.
I will simply remove the check (every where it is present) and
dereference the pointer instead of returning -EINVAL.
Regards,
Hervé
--
Hervé Codina, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
Re: [PATCH v2 26/28] ASoC: codecs: Add support for the framer codec
On Wed, Jul 26, 2023 at 05:02:22PM +0200, Herve Codina wrote:
> The framer codec interracts with a framer.
> It allows to use some of the framer timeslots as audio channels to
> transport audio data over the framer E1/T1/J1 lines.
> It also reports line carrier detection events through the ALSA jack
> detection feature.
>
> Signed-off-by: Herve Codina
> ---
> sound/soc/codecs/Kconfig| 15 ++
> sound/soc/codecs/Makefile | 2 +
> sound/soc/codecs/framer-codec.c | 423
> 3 files changed, 440 insertions(+)
> create mode 100644 sound/soc/codecs/framer-codec.c
>
> diff --git a/sound/soc/codecs/Kconfig b/sound/soc/codecs/Kconfig
> index f99203ef9b03..a86cdac39b72 100644
> --- a/sound/soc/codecs/Kconfig
> +++ b/sound/soc/codecs/Kconfig
> @@ -110,6 +110,7 @@ config SND_SOC_ALL_CODECS
> imply SND_SOC_ES8328_I2C
> imply SND_SOC_ES7134
> imply SND_SOC_ES7241
> + imply SND_SOC_FRAMER
> imply SND_SOC_GTM601
> imply SND_SOC_HDAC_HDMI
> imply SND_SOC_HDAC_HDA
> @@ -1043,6 +1044,20 @@ config SND_SOC_ES8328_SPI
> depends on SPI_MASTER
> select SND_SOC_ES8328
>
> +config SND_SOC_FRAMER
> + tristate "Framer codec"
> + depends on GENERIC_FRAMER
> + help
> + Enable support for the framer codec.
> + The framer codec uses the generic framer infrastructure to transport
> + some audio data over an analog E1/T1/J1 line.
> + This codec allows to use some of the time slots available on the TDM
> + bus on which the framer is connected to transport the audio data.
> +
> + To compile this driver as a module, choose M here: the module
> + will be called snd-soc-framer.
> +
> +
> config SND_SOC_GTM601
> tristate 'GTM601 UMTS modem audio codec'
>
> diff --git a/sound/soc/codecs/Makefile b/sound/soc/codecs/Makefile
> index 32dcc6de58bd..54667274a0f6 100644
> --- a/sound/soc/codecs/Makefile
> +++ b/sound/soc/codecs/Makefile
> @@ -116,6 +116,7 @@ snd-soc-es8326-objs := es8326.o
> snd-soc-es8328-objs := es8328.o
> snd-soc-es8328-i2c-objs := es8328-i2c.o
> snd-soc-es8328-spi-objs := es8328-spi.o
> +snd-soc-framer-objs := framer-codec.o
> snd-soc-gtm601-objs := gtm601.o
> snd-soc-hdac-hdmi-objs := hdac_hdmi.o
> snd-soc-hdac-hda-objs := hdac_hda.o
> @@ -499,6 +500,7 @@ obj-$(CONFIG_SND_SOC_ES8326)+= snd-soc-es8326.o
> obj-$(CONFIG_SND_SOC_ES8328) += snd-soc-es8328.o
> obj-$(CONFIG_SND_SOC_ES8328_I2C)+= snd-soc-es8328-i2c.o
> obj-$(CONFIG_SND_SOC_ES8328_SPI)+= snd-soc-es8328-spi.o
> +obj-$(CONFIG_SND_SOC_FRAMER) += snd-soc-framer.o
> obj-$(CONFIG_SND_SOC_GTM601)+= snd-soc-gtm601.o
> obj-$(CONFIG_SND_SOC_HDAC_HDMI) += snd-soc-hdac-hdmi.o
> obj-$(CONFIG_SND_SOC_HDAC_HDA) += snd-soc-hdac-hda.o
> diff --git a/sound/soc/codecs/framer-codec.c b/sound/soc/codecs/framer-codec.c
> new file mode 100644
> index ..52b4546a61ee
> --- /dev/null
> +++ b/sound/soc/codecs/framer-codec.c
> @@ -0,0 +1,423 @@
> +// SPDX-License-Identifier: GPL-2.0
> +//
> +// Framer ALSA SoC driver
> +//
> +// Copyright 2023 CS GROUP France
> +//
> +// Author: Herve Codina
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +
> +#define FRAMER_NB_CHANNEL32
> +#define FRAMER_JACK_MASK (SND_JACK_LINEIN | SND_JACK_LINEOUT)
> +
> +struct framer_codec {
> + struct framer *framer;
> + struct device *dev;
> + struct snd_soc_jack jack;
> + struct notifier_block nb;
> + struct work_struct carrier_work;
> + int max_chan_playback;
> + int max_chan_capture;
> +};
> +
> +static int framer_dai_set_tdm_slot(struct snd_soc_dai *dai, unsigned int
> tx_mask,
> +unsigned int rx_mask, int slots, int width)
> +{
> + struct framer_codec *framer =
> snd_soc_component_get_drvdata(dai->component);
> +
> + switch (width) {
> + case 0:
> + /* Not set -> default 8 */
> + case 8:
> + break;
> + default:
> + dev_err(dai->dev, "tdm slot width %d not supported\n", width);
> + return -EINVAL;
> + }
> +
> + framer->max_chan_playback = hweight32(tx_mask);
> + if (framer->max_chan_playback > FRAMER_NB_CHANNEL) {
> + dev_err(dai->dev, "too much tx slots defined (mask = 0x%x)
> support max %d\n",
"many", not "much".
Also, "supported".
Andrew
[PATCH] powerpc/64e: Fix secondary thread bringup for ELFv2 kernels
When booting on e6500 with an ELF v2 ABI kernel, the secondary threads do
not start correctly:
[0.051118] smp: Bringing up secondary CPUs ...
[5.072700] Processor 1 is stuck.
This occurs because the startup code is written to use function
descriptors when loading the entry point for the secondary threads. When
building with ELF v2 ABI there are no function descriptors, and the code
loads junk values for the entry point address.
Fix it by using ppc_function_entry() in C, and DOTSYM() in asm, both of
which work correctly for ELF v2 ABI as well as ELF v1 ABI kernels.
Fixes: 8c5fa3b5c4df ("powerpc/64: Make ELFv2 the default for big-endian builds")
Signed-off-by: Michael Ellerman
---
arch/powerpc/kernel/head_64.S | 3 +--
arch/powerpc/platforms/85xx/smp.c | 2 +-
2 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/arch/powerpc/kernel/head_64.S b/arch/powerpc/kernel/head_64.S
index f132d8704263..6440b1bb332a 100644
--- a/arch/powerpc/kernel/head_64.S
+++ b/arch/powerpc/kernel/head_64.S
@@ -375,8 +375,7 @@ _GLOBAL(generic_secondary_smp_init)
beq 20f
/* start the specified thread */
- LOAD_REG_ADDR(r5, fsl_secondary_thread_init)
- ld r4, 0(r5)
+ LOAD_REG_ADDR(r5, DOTSYM(fsl_secondary_thread_init))
bl book3e_start_thread
/* stop the current thread */
diff --git a/arch/powerpc/platforms/85xx/smp.c
b/arch/powerpc/platforms/85xx/smp.c
index 9c43cf32f4c9..40aa58206888 100644
--- a/arch/powerpc/platforms/85xx/smp.c
+++ b/arch/powerpc/platforms/85xx/smp.c
@@ -180,7 +180,7 @@ static void wake_hw_thread(void *info)
unsigned long inia;
int cpu = *(const int *)info;
- inia = *(unsigned long *)fsl_secondary_thread_init;
+ inia = ppc_function_entry(fsl_secondary_thread_init);
book3e_start_thread(cpu_thread_in_core(cpu), inia);
}
#endif
--
2.41.0
Re: [PATCH v2 10/28] soc: fsl: cpm1: qmc: Introduce qmc_chan_setup_tsa*
On Tue, 1 Aug 2023 11:36:43 +0200
Andrew Lunn wrote:
> > +static inline void qmc_clrsetbits16(void __iomem *addr, u16 clr, u16 set)
> > +{
> > + qmc_write16(addr, (qmc_read16(addr) & ~clr) | set);
> > +}
> > +
>
> Please don't use inline in .c files. Let the compiler decide.
>
>Andrew
Ok, I will remove the inline in the next iteration.
I will also remove the inline of all other similar functions (probably a
separate patch in the series).
Regards,
Hervé
Re: [PATCH v2 23/28] net: wan: framer: Add support for the Lantiq PEF2256 framer
> +static inline u8 pef2256_read8(struct pef2256 *pef2256, int offset)
> +{
> + int val;
> +
> + regmap_read(pef2256->regmap, offset, );
> + return val;
> +}
> +
> +static inline void pef2256_write8(struct pef2256 *pef2256, int offset, u8
> val)
> +{
> + regmap_write(pef2256->regmap, offset, val);
> +}
More cases of inline functions in .C files. Please let the compiler
decide.
> +static void pef2256_isr_default_handler(struct pef2256 *pef2256, u8 nbr, u8
> isr)
> +{
> + dev_warn(pef2256->dev, "ISR%u: 0x%02x not handled\n", nbr, isr);
> +}
Should this be rate limited? It is going to be very noise if it gets
called once per frame time.
Andrew
Re: [PATCH v2 06/28] net: wan: Add support for QMC HDLC
On Tue, 1 Aug 2023 11:31:32 +0200
Andrew Lunn wrote:
> > +static inline struct qmc_hdlc *netdev_to_qmc_hdlc(struct net_device
> > *netdev)
> > +{
> > + return (struct qmc_hdlc *)dev_to_hdlc(netdev)->priv;
>
> priv is a void *, so you don't need the cast.
>
Right, I will change in the next iteration.
Regards,
Hervé
Re: [PATCH v2 08/28] soc: fsl: cpm1: qmc: Introduce available timeslots masks
On Tue, 1 Aug 2023 11:33:39 +0200
Andrew Lunn wrote:
> On Wed, Jul 26, 2023 at 05:02:04PM +0200, Herve Codina wrote:
> > Available timeslots masks define timeslots available for the related
> > channel. These timeslots are defined by the QMC binding.
> >
> > Timeslots used are initialized to available timeslots but can be a
> > subset of available timeslots.
> > This prepares the dynamic timeslots management (ie. changing timeslots
> > at runtime).
> >
> > Signed-off-by: Herve Codina
> > ---
> > drivers/soc/fsl/qe/qmc.c | 8 ++--
> > 1 file changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
> > index 2d2a9d88ba6c..21ad7e79e7bd 100644
> > --- a/drivers/soc/fsl/qe/qmc.c
> > +++ b/drivers/soc/fsl/qe/qmc.c
> > @@ -177,7 +177,9 @@ struct qmc_chan {
> > struct qmc *qmc;
> > void __iomem *s_param;
> > enum qmc_mode mode;
> > + u64 tx_ts_mask_avail;
> > u64 tx_ts_mask;
> > + u64 rx_ts_mask_avail;
> > u64 rx_ts_mask;
>
> Is this for E1? So there is a maximum of 32 slots? A u32 would be
> sufficient i think?
>
The QMC can use up to 64 slots. So masks related to the QMC are on 64bits.
These masks are not specific to the E1 framer but really related to the QMC
capabilities.
Regards,
Hervé
Re: [PATCH v2 21/28] dt-bindings: net: Add the Lantiq PEF2256 E1/T1/J1 framer
> + clocks: > +items: > + - description: Master clock > + - description: Receive System Clock > + - description: Transmit System Clock > + > + clock-names: > +items: > + - const: mclk > + - const: sclkr > + - const: sclkx Nit pick, but "Receive System Clock", but "sclkr'. Maybe "System Clock Receive" so you have the same word order? Andrew
Re: [PATCH v2 20/28] net: wan: Add framer framework support
> +int framer_pm_runtime_get(struct framer *framer)
> +{
> + int ret;
> +
> + if (!framer)
> + return 0;
Can framer be a NULL pointer? This sort of test often covers up
bugs. So either let it dereference the NULL pointer and opps, or
return -EINVAL.
Andrew
Re: [PATCH 0/7] Rework perf and ptrace watchpoint tracking
Le 01/08/2023 à 03:17, Benjamin Gray a écrit : > Syzkaller triggered a null pointer dereference in the > arch_unregister_hw_breakpoint() hook. This is due to accessing > the bp->ctx->task field changing to -1 while we iterate the breakpoints. > > This series refactors the breakpoint tracking logic to remove the > dependency on bp->ctx entirely. It also simplifies handling of ptrace and > perf breakpoints, making insertion less restrictive. Is there any link between this series and the following issue: https://github.com/linuxppc/issues/issues/38 Christophe > > If merged, it allows several arch hooks that PowerPC was the sole user of > to be removed. > > Benjamin Gray (7): >powerpc/watchpoints: Explain thread_change_pc() more >powerpc/watchpoints: Don't track info persistently >powerpc/watchpoints: Track perf single step directly on the breakpoint >powerpc/watchpoints: Simplify watchpoint reinsertion >powerpc/watchpoints: Remove ptrace/perf exclusion tracking >selftests/powerpc/ptrace: Update ptrace-perf watchpoint selftest >perf/hw_breakpoint: Remove arch breakpoint hooks > > arch/powerpc/include/asm/hw_breakpoint.h |1 + > arch/powerpc/include/asm/processor.h |5 - > arch/powerpc/kernel/hw_breakpoint.c | 388 +- > include/linux/hw_breakpoint.h |3 - > kernel/events/hw_breakpoint.c | 28 - > .../testing/selftests/powerpc/ptrace/Makefile |1 + > .../powerpc/ptrace/ptrace-perf-asm.S | 33 + > .../powerpc/ptrace/ptrace-perf-hwbreak.c | 1104 +++-- > 8 files changed, 537 insertions(+), 1026 deletions(-) > create mode 100644 tools/testing/selftests/powerpc/ptrace/ptrace-perf-asm.S > rewrite tools/testing/selftests/powerpc/ptrace/ptrace-perf-hwbreak.c (93%) >
Re: [RFC PATCH v2 4/7] media: v4l2: Add audio capture and output support
On Fri, Jul 28, 2023 at 3:59 PM Tomasz Figa wrote:
>
> Hi Shengjiu,
>
> On Tue, Jul 25, 2023 at 02:12:17PM +0800, Shengjiu Wang wrote:
> > Audio signal processing has the requirement for memory to
> > memory similar as Video.
> >
> > This patch is to add this support in v4l2 framework, defined
> > new buffer type V4L2_BUF_TYPE_AUDIO_CAPTURE and
> > V4L2_BUF_TYPE_AUDIO_OUTPUT, defined new format v4l2_audio_format
> > for audio case usage.
> >
> > The created audio device is named "/dev/audioX".
> >
> > Signed-off-by: Shengjiu Wang
> > ---
> > .../media/common/videobuf2/videobuf2-v4l2.c | 4 ++
> > drivers/media/v4l2-core/v4l2-dev.c| 17 ++
> > drivers/media/v4l2-core/v4l2-ioctl.c | 52 +++
> > include/media/v4l2-dev.h | 2 +
> > include/media/v4l2-ioctl.h| 34
> > include/uapi/linux/videodev2.h| 19 +++
> > 6 files changed, 128 insertions(+)
> >
>
> Thanks for the patch! Please check my comments inline.
Thanks for reviewing.
Sorry for sending again for using the plain text mode.
>
> > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > index c7a54d82a55e..12f2be2773a2 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > @@ -785,6 +785,10 @@ int vb2_create_bufs(struct vb2_queue *q, struct
> > v4l2_create_buffers *create)
> > case V4L2_BUF_TYPE_META_OUTPUT:
> > requested_sizes[0] = f->fmt.meta.buffersize;
> > break;
> > + case V4L2_BUF_TYPE_AUDIO_CAPTURE:
> > + case V4L2_BUF_TYPE_AUDIO_OUTPUT:
> > + requested_sizes[0] = f->fmt.audio.buffersize;
> > + break;
> > default:
> > return -EINVAL;
> > }
> > diff --git a/drivers/media/v4l2-core/v4l2-dev.c
> > b/drivers/media/v4l2-core/v4l2-dev.c
> > index f81279492682..67484f4c6eaf 100644
> > --- a/drivers/media/v4l2-core/v4l2-dev.c
> > +++ b/drivers/media/v4l2-core/v4l2-dev.c
> > @@ -553,6 +553,7 @@ static void determine_valid_ioctls(struct video_device
> > *vdev)
> > bool is_tch = vdev->vfl_type == VFL_TYPE_TOUCH;
> > bool is_meta = vdev->vfl_type == VFL_TYPE_VIDEO &&
> > (vdev->device_caps & meta_caps);
> > + bool is_audio = vdev->vfl_type == VFL_TYPE_AUDIO;
> > bool is_rx = vdev->vfl_dir != VFL_DIR_TX;
> > bool is_tx = vdev->vfl_dir != VFL_DIR_RX;
> > bool is_io_mc = vdev->device_caps & V4L2_CAP_IO_MC;
> > @@ -664,6 +665,19 @@ static void determine_valid_ioctls(struct video_device
> > *vdev)
> > SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_meta_out);
> > SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT, vidioc_try_fmt_meta_out);
> > }
> > + if (is_audio && is_rx) {
> > + /* audio capture specific ioctls */
> > + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT,
> > vidioc_enum_fmt_audio_cap);
> > + SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_audio_cap);
> > + SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_audio_cap);
> > + SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT,
> > vidioc_try_fmt_audio_cap);
> > + } else if (is_audio && is_tx) {
> > + /* audio output specific ioctls */
> > + SET_VALID_IOCTL(ops, VIDIOC_ENUM_FMT,
> > vidioc_enum_fmt_audio_out);
> > + SET_VALID_IOCTL(ops, VIDIOC_G_FMT, vidioc_g_fmt_audio_out);
> > + SET_VALID_IOCTL(ops, VIDIOC_S_FMT, vidioc_s_fmt_audio_out);
> > + SET_VALID_IOCTL(ops, VIDIOC_TRY_FMT,
> > vidioc_try_fmt_audio_out);
> > + }
> > if (is_vbi) {
> > /* vbi specific ioctls */
> > if ((is_rx && (ops->vidioc_g_fmt_vbi_cap ||
> > @@ -927,6 +941,9 @@ int __video_register_device(struct video_device *vdev,
> > case VFL_TYPE_TOUCH:
> > name_base = "v4l-touch";
> > break;
> > + case VFL_TYPE_AUDIO:
> > + name_base = "audio";
>
> I think it was mentioned before that "audio" could be confusing. Wasn't
> there actually some other kind of /dev/audio device long ago?
>
> Seems like for touch, "v4l-touch" was introduced. Maybe it would also
> make sense to call it "v4l-audio" for audio?
Ok, will change to use "v4l-audio".
>
> > + break;
> > default:
> > pr_err("%s called with unknown type: %d\n",
> > __func__, type);
> > diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c
> > b/drivers/media/v4l2-core/v4l2-ioctl.c
> > index 01ba27f2ef87..aa9d872bba8d 100644
> > --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> > +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> > @@ -188,6 +188,8 @@ const char *v4l2_type_names[] = {
> > [V4L2_BUF_TYPE_SDR_OUTPUT] = "sdr-out",
> > [V4L2_BUF_TYPE_META_CAPTURE] = "meta-cap",
> > [V4L2_BUF_TYPE_META_OUTPUT]
Re: [PATCH v2 10/28] soc: fsl: cpm1: qmc: Introduce qmc_chan_setup_tsa*
> +static inline void qmc_clrsetbits16(void __iomem *addr, u16 clr, u16 set)
> +{
> + qmc_write16(addr, (qmc_read16(addr) & ~clr) | set);
> +}
> +
Please don't use inline in .c files. Let the compiler decide.
Andrew
Re: [PATCH v2 08/28] soc: fsl: cpm1: qmc: Introduce available timeslots masks
On Wed, Jul 26, 2023 at 05:02:04PM +0200, Herve Codina wrote:
> Available timeslots masks define timeslots available for the related
> channel. These timeslots are defined by the QMC binding.
>
> Timeslots used are initialized to available timeslots but can be a
> subset of available timeslots.
> This prepares the dynamic timeslots management (ie. changing timeslots
> at runtime).
>
> Signed-off-by: Herve Codina
> ---
> drivers/soc/fsl/qe/qmc.c | 8 ++--
> 1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/soc/fsl/qe/qmc.c b/drivers/soc/fsl/qe/qmc.c
> index 2d2a9d88ba6c..21ad7e79e7bd 100644
> --- a/drivers/soc/fsl/qe/qmc.c
> +++ b/drivers/soc/fsl/qe/qmc.c
> @@ -177,7 +177,9 @@ struct qmc_chan {
> struct qmc *qmc;
> void __iomem *s_param;
> enum qmc_mode mode;
> + u64 tx_ts_mask_avail;
> u64 tx_ts_mask;
> + u64 rx_ts_mask_avail;
> u64 rx_ts_mask;
Is this for E1? So there is a maximum of 32 slots? A u32 would be
sufficient i think?
Andrew
Re: [PATCH v2 06/28] net: wan: Add support for QMC HDLC
> +static inline struct qmc_hdlc *netdev_to_qmc_hdlc(struct net_device *netdev)
> +{
> + return (struct qmc_hdlc *)dev_to_hdlc(netdev)->priv;
priv is a void *, so you don't need the cast.
Andrew
Re: [PATCH v7 7/7] mm/memory_hotplug: Enable runtime update of memmap_on_memory parameter
On 8/1/23 2:28 PM, Michal Hocko wrote: > On Tue 01-08-23 10:11:16, Aneesh Kumar K.V wrote: >> Allow updating memmap_on_memory mode after the kernel boot. Memory >> hotplug done after the mode update will use the new mmemap_on_memory >> value. > > Well, this is a user space kABI extension and as such you should spend > more words about the usecase. Why we could live with this static and now > need dynamic? > This enables easy testing of memmap_on_memory feature without a kernel reboot. I also expect people wanting to use that when they find dax kmem memory online failing because of struct page allocation failures[1]. User could reboot back with memmap_on_memory=y kernel parameter. But being able to enable it via sysfs makes the feature much more useful. -aneesh
Re: [syzbot] [btrfs?] WARNING in btrfs_free_reserved_data_space_noquota
syzbot suspects this issue was fixed by commit: commit 487c20b016dc48230367a7be017f40313e53e3bd Author: Linus Torvalds Date: Thu Mar 30 21:53:51 2023 + iov: improve copy_iovec_from_user() code generation bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=17cfdf19a8 start commit: 4bdec23f971b Merge tag 'hwmon-for-v6.3-rc4' of git://git.k.. git tree: upstream kernel config: https://syzkaller.appspot.com/x/.config?x=acdb62bf488a8fe5 dashboard link: https://syzkaller.appspot.com/bug?extid=adec8406ad17413d4c06 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=11bf8bcec8 C reproducer: https://syzkaller.appspot.com/x/repro.c?x=153d4f75c8 If the result looks correct, please mark the issue as fixed by replying with: #syz fix: iov: improve copy_iovec_from_user() code generation For information about bisection process see: https://goo.gl/tpsmEJ#bisection
Re: [PATCH v7 0/7] Add support for memmap on memory feature on ppc64
I cannot really judge the ppc specific part but other patches seem reasonable. Patch 4 could print a more useful information about the wastage but this is nothing really earth shattering. I am not sure about the last patch which makes the on-memory property dynamic. This needs much more justification and use case description IMHO. That being said for patches 1 - 4 and 6 feel free to add Acked-by: Michal Hocko On Tue 01-08-23 10:11:09, Aneesh Kumar K.V wrote: > This patch series update memmap on memory feature to fall back to > memmap allocation outside the memory block if the alignment rules are > not met. This makes the feature more useful on architectures like > ppc64 where alignment rules are different with 64K page size. > > This patch series is dependent on dax vmemmap optimization series > posted here > https://lore.kernel.org/linux-mm/20230718022934.90447-1-aneesh.ku...@linux.ibm.com/ > > Changes from v6: > * Update comments in the code > * Update commit message for patch 7 > > Changes from v5: > * Update commit message > * Move memory alloc/free to the callers in patch 6 > * Address review feedback w.r.t patch 4 > > Changes from v4: > * Use altmap.free instead of altmap.reserve > * Address review feedback > > Changes from v3: > * Extend the module parameter memmap_on_memory to force allocation even > though we can waste hotplug memory. > > Changes from v2: > * Rebase to latest linus tree > * Redo the series based on review feedback. Multiple changes to the patchset. > > Changes from v1: > * update the memblock to store vmemmap_altmap details. This is required > so that when we remove the memory we can find the altmap details which > is needed on some architectures. > * rebase to latest linus tree > > > > Aneesh Kumar K.V (7): > mm/memory_hotplug: Simplify ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE kconfig > mm/memory_hotplug: Allow memmap on memory hotplug request to fallback > mm/memory_hotplug: Allow architecture to override memmap on memory > support check > mm/memory_hotplug: Support memmap_on_memory when memmap is not aligned > to pageblocks > powerpc/book3s64/memhotplug: Enable memmap on memory for radix > mm/memory_hotplug: Embed vmem_altmap details in memory block > mm/memory_hotplug: Enable runtime update of memmap_on_memory parameter > > .../admin-guide/mm/memory-hotplug.rst | 12 + > arch/arm64/Kconfig| 4 +- > arch/powerpc/Kconfig | 1 + > arch/powerpc/include/asm/pgtable.h| 21 ++ > .../platforms/pseries/hotplug-memory.c| 2 +- > arch/x86/Kconfig | 4 +- > drivers/acpi/acpi_memhotplug.c| 3 +- > drivers/base/memory.c | 27 ++- > include/linux/memory.h| 8 +- > include/linux/memory_hotplug.h| 3 +- > mm/Kconfig| 3 + > mm/memory_hotplug.c | 205 ++ > 12 files changed, 220 insertions(+), 73 deletions(-) > > -- > 2.41.0 -- Michal Hocko SUSE Labs
Re: [PATCH v7 4/7] mm/memory_hotplug: Support memmap_on_memory when memmap is not aligned to pageblocks
On Tue 01-08-23 10:11:13, Aneesh Kumar K.V wrote:
[...]
> + if (mode == MEMMAP_ON_MEMORY_FORCE) {
> + unsigned long memmap_pages =
> memory_block_memmap_on_memory_pages();
unsigned long wastage = memmap_pages -
PFN_UP(memory_block_memmap_size());
if (wastage)
pr_info_once("memmap_on_memory=force will waste %ld
pages in each memory block %ld\n",
wastage, memory_block_size_bytes() >>
PAGE_SHIFT);
would be more useful IMHO.
> +
> + pr_info_once("Memory hotplug will waste %ld pages in each
> memory block\n",
> + memmap_pages - PFN_UP(memory_block_memmap_size()));
--
Michal Hocko
SUSE Labs
Re: [PATCH v7 7/7] mm/memory_hotplug: Enable runtime update of memmap_on_memory parameter
On Tue 01-08-23 10:11:16, Aneesh Kumar K.V wrote: > Allow updating memmap_on_memory mode after the kernel boot. Memory > hotplug done after the mode update will use the new mmemap_on_memory > value. Well, this is a user space kABI extension and as such you should spend more words about the usecase. Why we could live with this static and now need dynamic? -- Michal Hocko SUSE Labs
Re: [PATCH] Documentation: devices.txt: reconcile serial/ucc_uart minor numers
Le 01/08/2023 à 07:30, Randy Dunlap a écrit :
> Hi Christophe,
>
> On 7/31/23 22:21, Christophe Leroy wrote:
>>
>>
>> Le 24/07/2023 à 08:33, Randy Dunlap a écrit :
>>> Reconcile devices.txt with serial/ucc_uart.c regarding device number
>>> assignments. ucc_uart.c supports 4 ports and uses minor devnums
>>> 46-49, so update devices.txt with that info.
>>> Then update ucc_uart.c's reference to the location of the devices.txt
>>> list in the kernel source tree.
>>
>> Devices ttyCPM* belong to cpm_uart driver. As explained in the comment
>> you have modified in UCC uart driver, UCC uart borrows those devices and
>> shall not be considered as the reference. But the lines you modify in
>> device.txt doesn't mention QE UCC, it mentions CPM SCC and CPM SMC.
>>
>> CPM uart driver supports up to 6 ports (4 SCC and 2 SMC).
>>
>
> Thank you for replying.
>
> Does this mean that the patch should be reverted?
Not sure it should be reverted, the best would be to fix it up because
anyway what we had was wrong.
See below.
>
>> On one of my boards which has a powerpc mpc866 CPU, I have:
>>
>> [2.393872] ff000a80.serial: ttyCPM0 at MMIO 0xfff00a80 (irq = 19,
>> base_baud = 825) is a CPM UART
>> [2.411899] ff000a90.serial: ttyCPM1 at MMIO 0xfff00a90 (irq = 20,
>> base_baud = 825) is a CPM UART
>> [2.430352] ff000a00.serial: ttyCPM2 at MMIO 0xfff00a00 (irq = 30,
>> base_baud = 825) is a CPM UART
>> [2.448944] ff000a20.serial: ttyCPM3 at MMIO 0xfff00a20 (irq = 29,
>> base_baud = 825) is a CPM UART
>> [2.467435] ff000a40.serial: ttyCPM4 at MMIO 0xfff00a40 (irq = 28,
>> base_baud = 825) is a CPM UART
>> [2.485924] ff000a60.serial: ttyCPM5 at MMIO 0xfff00a60 (irq = 27,
>> base_baud = 825) is a CPM UART
>>
>> # ll /dev/ttyCPM*
>> crw---1 root root 204, 46 Jan 1 01:01 /dev/ttyCPM0
>> crw---1 root root 204, 47 Jan 1 01:00 /dev/ttyCPM1
>> crw---1 root root 204, 48 Jan 1 01:00 /dev/ttyCPM2
>> crw---1 root root 204, 49 Jan 1 01:00 /dev/ttyCPM3
>> crw---1 root root 204, 50 Jan 1 01:00 /dev/ttyCPM4
>> crw---1 root root 204, 51 Jan 1 01:00 /dev/ttyCPM5
>
> I don't see minors 50-51 allocated in devices.txt for use by this
> device/driver.
> Am I overlooking that allocation somewhere?
I looked into history.
ttyCPM do not appear in devices.txt in v2.6.9.
It was introduced in v2.6.10 as :
46 = /dev/ttyCPM0 PPC CPM (SCC or SMC) - port 0
...
49 = /dev/ttyCPM5 PPC CPM (SCC or SMC) - port 5
Then in v2.6.11 we find:
46 = /dev/ttyCPM0 PPC CPM (SCC or SMC) - port 0
...
47 = /dev/ttyCPM5 PPC CPM (SCC or SMC) - port 5
50 = /dev/ttyIOC40 Altix serial card
...
81 = /dev/ttyIOC431Altix serial card
So it looks like from the begining the intention was to have 6 ports
(from 0 to 5) and it should have been up to 51. For some reason it was
messed up.
If you look into v2.6.10 drivers/serial/cpm_uart/cpm_uart.h you see
there are 6 ports. Even in v2.6.9 that driver exists and has 6 ports.
Anyway, the ttyIOC* were removed by commit a017ef17cfd8 ("tty/serial:
remove the ioc4_serial driver"), so it should be no problem to fix
devices.txt and set ttyCPM from 46 to 51.
Christophe
>
> Thanks for your help.
>
>>
>> # cat /proc/tty/drivers
>> /dev/tty /dev/tty5 0 system:/dev/tty
>> /dev/console /dev/console5 1 system:console
>> /dev/ptmx/dev/ptmx 5 2 system
>> ttyCPM /dev/ttyCPM 204 46-51 serial
>> pty_slave/dev/pts 136 0-1048575 pty:slave
>> pty_master /dev/ptm 128 0-1048575 pty:master
>> pty_slave/dev/ttyp 3 0-4 pty:slave
>> pty_master /dev/pty2 0-4 pty:master
>>
>> Christophe
>>
>>
>>>
>>> Fixes: d7584ed2b994 ("[POWERPC] qe-uart: add support for Freescale
>>> QUICCEngine UART")
>>> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
>>> Signed-off-by: Randy Dunlap
>>> Cc: Timur Tabi
>>> Cc: Kumar Gala
>>> Cc: linuxppc-dev@lists.ozlabs.org
>>> Cc: Greg Kroah-Hartman
>>> Cc: Jiri Slaby
>>> Cc: linux-ser...@vger.kernel.org
>>> Cc: Jonathan Corbet
>>> Cc: linux-...@vger.kernel.org
>>> ---
>>>Documentation/admin-guide/devices.txt |2 +-
>>>drivers/tty/serial/ucc_uart.c |2 +-
>>>2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff -- a/drivers/tty/serial/ucc_uart.c b/drivers/tty/serial/ucc_uart.c
>>> --- a/drivers/tty/serial/ucc_uart.c
>>> +++ b/drivers/tty/serial/ucc_uart.c
>>> @@ -59,7 +59,7 @@ static int firmware_loaded;
>>>/* #define LOOPBACK */
>>>
>>>/* The major and minor device numbers are defined in
>>> - *
