Re: [PATCH v1 7/7] pseries/setup: Add Initialization of VF Bars
On 14/12/17 02:32, Bryant G. Ly wrote:
> When enabling SR-IOV in pseries platform,
> the VF bar properties for a PF are reported on
> the device node in the device tree.
>
> This patch adds the IOV Bar resources to Linux
> structures from the device tree for later use
> when configuring SR-IOV by PF driver.
>
> Signed-off-by: Bryant G. Ly
> Signed-off-by: Juan J. Alvarez
> ---
> arch/powerpc/include/asm/pci.h | 2 +
> arch/powerpc/kernel/pci_of_scan.c | 2 +-
> arch/powerpc/platforms/pseries/setup.c | 183
> +
> 3 files changed, 186 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/pci.h b/arch/powerpc/include/asm/pci.h
> index 8dc32eacc97c..d82802ff5088 100644
> --- a/arch/powerpc/include/asm/pci.h
> +++ b/arch/powerpc/include/asm/pci.h
> @@ -121,6 +121,8 @@ extern int remove_phb_dynamic(struct pci_controller *phb);
> extern struct pci_dev *of_create_pci_dev(struct device_node *node,
> struct pci_bus *bus, int devfn);
>
> +extern unsigned int pci_parse_of_flags(u32 addr0, int bridge);
> +
> extern void of_scan_pci_bridge(struct pci_dev *dev);
>
> extern void of_scan_bus(struct device_node *node, struct pci_bus *bus);
> diff --git a/arch/powerpc/kernel/pci_of_scan.c
> b/arch/powerpc/kernel/pci_of_scan.c
> index 0d790f8432d2..20ceec4a5f5e 100644
> --- a/arch/powerpc/kernel/pci_of_scan.c
> +++ b/arch/powerpc/kernel/pci_of_scan.c
> @@ -38,7 +38,7 @@ static u32 get_int_prop(struct device_node *np, const char
> *name, u32 def)
> * @addr0: value of 1st cell of a device tree PCI address.
> * @bridge: Set this flag if the address is from a bridge 'ranges' property
> */
> -static unsigned int pci_parse_of_flags(u32 addr0, int bridge)
> +unsigned int pci_parse_of_flags(u32 addr0, int bridge)
> {
> unsigned int flags = 0;
>
> diff --git a/arch/powerpc/platforms/pseries/setup.c
> b/arch/powerpc/platforms/pseries/setup.c
> index 5f1beb8367ac..ce28882cbde8 100644
> --- a/arch/powerpc/platforms/pseries/setup.c
> +++ b/arch/powerpc/platforms/pseries/setup.c
> @@ -459,6 +459,181 @@ static void __init find_and_init_phbs(void)
> of_pci_check_probe_only();
> }
>
> +#ifdef CONFIG_PCI_IOV
> +enum rtas_iov_fw_value_map {
> + NUM_RES_PROPERTY = 0, ///< Number of Resources
> + LOW_INT = 1, ///< Lowest 32 bits of Address
> + START_OF_ENTRIES = 2, ///< Always start of entry
> + APERTURE_PROPERTY = 2, ///< Start of entry+ to Aperture Size
> + WDW_SIZE_PROPERTY = 4, ///< Start of entry+ to Window Size
> + NEXT_ENTRY= 7 ///< Go to next entry on array
> +};
> +
> +enum get_iov_fw_value_index {
> + BAR_ADDRS = 1,///< Get Bar Address
> + APERTURE_SIZE = 2,///< Get Aperture Size
> + WDW_SIZE = 3 ///< Get Window Size
> +};
> +
> +resource_size_t pseries_get_iov_fw_values(struct pci_dev *dev, int resno,
s/pseries_get_iov_fw_values/pseries_get_iov_fw_value/ as it returns a
single value.
> + enum get_iov_fw_value_index value)
> +{
> + struct vf_bar_wdw {
> + __be64 addr;
> + __be64 aperture_size;
> + __be64 wdw_size;
> + };
> +
> + struct vf_bar_wdw window_avail[PCI_SRIOV_NUM_BARS];
> + const int *indexes;
> + struct device_node *dn = pci_device_to_OF_node(dev);
> + int i, r, num_res;
> + resource_size_t return_value;
resource_size_t return_value = 0;
and remove initialization below. Also, way too long, @ret is a more common
name.
> +
> + indexes = of_get_property(dn, "ibm,open-sriov-vf-bar-info", NULL);
> + if (!indexes)
> + return 0;
> +
> + memset(window_avail,
> +0, sizeof(struct vf_bar_wdw) * PCI_SRIOV_NUM_BARS);
This is more common way of doing the same initialization:
struct vf_bar_wdw {
__be64 addr;
__be64 aperture_size;
__be64 wdw_size;
} window_avail[PCI_SRIOV_NUM_BARS] = { 0 };
> + return_value = 0;
> + /*
> + * First element in the array is the number of Bars
> + * returned. Search through the list to find the matching
> + * bar
> + */
> + num_res = of_read_number(&indexes[NUM_RES_PROPERTY], 1);
if (resno >= num_res)
return 0; /* or an errror */
i = START_OF_ENTRIES + NEXT_ENTRY * resno;
switch (value) {
case BAR_ADDRS:
ret = f_read_number(&indexes[i], 2);
break;
case APERTURE_SIZE:
ret = of_read_number(&indexes[i + APERTURE_PROPERTY], 2);
break;
case WDW_SIZE:
ret = of_read_number(&indexes[i + WDW_SIZE_PROPERTY], 2);
break;
}
return ret;
}
and remove the reminder of the function, and window_avail, and vf_bar_wdw?
> + for (i = START_OF_ENTRIES, r = 0; r < num_res && r < PC
Re: [PATCH v1 6/7] pseries/pci: Associate PEs to VFs in configure SR-IOV
On 14/12/17 02:32, Bryant G. Ly wrote:
> After initial validation of SR-IOV resources, firmware will
> associate PEs to the dynamic VFs created within this call. This
> patch adds the association of PEs to the PF array of PE numbers
> indexed by VF.
>
> Signed-off-by: Bryant G. Ly
> Signed-off-by: Juan J. Alvarez
> ---
> arch/powerpc/platforms/pseries/pci.c | 156
> ++-
> 1 file changed, 153 insertions(+), 3 deletions(-)
>
> diff --git a/arch/powerpc/platforms/pseries/pci.c
> b/arch/powerpc/platforms/pseries/pci.c
> index 48d3af026f90..c90e7d1247a8 100644
> --- a/arch/powerpc/platforms/pseries/pci.c
> +++ b/arch/powerpc/platforms/pseries/pci.c
> @@ -57,18 +57,168 @@ void pcibios_name_device(struct pci_dev *dev)
> }
> DECLARE_PCI_FIXUP_HEADER(PCI_ANY_ID, PCI_ANY_ID, pcibios_name_device);
> #endif
> -
> #ifdef CONFIG_PCI_IOV
> +#define MAX_VFS_FOR_MAP_PE 256
> +struct pe_map_bar_entry {
> + __be64 bar; ///< Input: Virtual Function BAR
> + __be16 rid; ///< Input: Virtual Function Router ID
> + __be16 pe_num;///< Output: Virtual Function PE Number
> + __be32 reserved; ///< Reserved Space
'///<' is a very unusual commenting style...
> +};
> +
> +int pseries_send_map_pe(struct pci_dev *pdev,
> + u16 num_vfs,
> + struct pe_map_bar_entry *vf_pe_array)
> +{
> + struct pci_dn *pdn;
> + int rc;
Spaces?
> + unsigned long buid, addr;
> + int ibm_map_pes = rtas_token("ibm,open-sriov-map-pe-number");
> +
> + if (ibm_map_pes == RTAS_UNKNOWN_SERVICE)
> + return -EINVAL;
> +
> + pdn = pci_get_pdn(pdev);
> + addr = rtas_config_addr(pdn->busno, pdn->devfn, 0);
> + buid = pdn->phb->buid;
> + spin_lock(&rtas_data_buf_lock);
> + memcpy(rtas_data_buf, vf_pe_array,
> +RTAS_DATA_BUF_SIZE);
> + rc = rtas_call(ibm_map_pes, 5, 1, NULL, addr,
> +BUID_HI(buid), BUID_LO(buid),
> +rtas_data_buf,
> +num_vfs * sizeof(struct pe_map_bar_entry));
> + memcpy(vf_pe_array, rtas_data_buf,
> +RTAS_DATA_BUF_SIZE);
Can easily be a single line.
> + spin_unlock(&rtas_data_buf_lock);
> +
> + if (rc)
> + dev_err(&pdev->dev,
> + "%s: Failed to associate pes PE#%lx, rc=%x\n",
> + __func__, addr, rc);
> +
> + return rc;
> +}
> +
> +void pseries_set_pe_num(struct pci_dev *pdev,
Spaces again :)
> + u16 vf_index, __be16 pe_num)
> +{
> + struct pci_dn *pdn;
> +
> + pdn = pci_get_pdn(pdev);
> + pdn->pe_num_map[vf_index] = be16_to_cpu(pe_num);
> + dev_dbg(&pdev->dev, "VF %04x:%02x:%02x.%x associated with PE#%x\n",
> + pci_domain_nr(pdev->bus),
> + pdev->bus->number,
> + PCI_SLOT(pci_iov_virtfn_devfn(pdev, vf_index)),
> + PCI_FUNC(pci_iov_virtfn_devfn(pdev, vf_index)),
> + pdn->pe_num_map[vf_index]);
> +}
> +
> +int pseries_associate_pes(struct pci_dev *pdev, u16 num_vfs)
> +{
> + struct pci_dn *pdn;
> + int i, rc, vf_index;
Spaces.
> + struct pe_map_bar_entry *vf_pe_array;
> + struct resource *res;
> + u64 size;
> +
> + vf_pe_array = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
> + if (!vf_pe_array)
> + return -ENOMEM;
> +
> + memset(vf_pe_array, 0, RTAS_DATA_BUF_SIZE);
As mentioned elsewhere, kzalloc() above resets memory.
> + pdn = pci_get_pdn(pdev);
> + /* create firmware structure to associate pes */
> + for (vf_index = 0; vf_index < num_vfs && vf_index < MAX_VFS_FOR_MAP_PE;
It would make the code and your life easier if you check for
num_vfs<=MAX_VFS_FOR_MAP_PE in pseries_pci_sriov_enable() and then you
won't have to check for MAX_VFS_FOR_MAP_PE. As for now, if
num_vfs>=MAX_VFS_FOR_MAP_PE, all VFs above MAX_VFS_FOR_MAP_PE will be ignored.
> + vf_index++) {
> + pdn->pe_num_map[vf_index] = IODA_INVALID_PE;
> + for (i = 0; i < PCI_SRIOV_NUM_BARS; i++) {
> + res = &pdev->resource[i + PCI_IOV_RESOURCES];
> + if (!res->parent)
> + continue;
> + size = pcibios_iov_resource_alignment(pdev, i +
> + PCI_IOV_RESOURCES
> + );
afaik the kernel coding style is tolerant to 2 tabs indents so it is not
necessary to align under the opening bracket and you can do:
> + size = pcibios_iov_resource_alignment(pdev,
i + PCI_IOV_RESOURCES);
> + vf_pe_array[vf_index].bar =
> + be64_to_cpu(res->start + size * vf_index);
cpu_to_be64?
> + vf_pe_array[vf_index].rid =
> + be16_
Re: [PATCH] On ppc64le we HAVE_RELIABLE_STACKTRACE
On Sun, 17 Dec 2017 20:58:54 -0600
Josh Poimboeuf wrote:
> On Fri, Dec 15, 2017 at 07:40:09PM +1000, Nicholas Piggin wrote:
> > On Tue, 12 Dec 2017 08:05:01 -0600
> > Josh Poimboeuf wrote:
> >
> > > On Tue, Dec 12, 2017 at 12:39:12PM +0100, Torsten Duwe wrote:
> > > > Hi all,
> > > >
> > > > The "Power Architecture 64-Bit ELF V2 ABI" says in section 2.3.2.3:
> > > >
> > > > [...] There are several rules that must be adhered to in order to ensure
> > > > reliable and consistent call chain backtracing:
> > > >
> > > > * Before a function calls any other function, it shall establish its
> > > > own stack frame, whose size shall be a multiple of 16 bytes.
> > >
> > > What about leaf functions? If a leaf function doesn't establish a stack
> > > frame, and it has inline asm which contains a blr to another function,
> > > this ABI is broken.
>
> Oops, I meant to say "bl" instead of "blr".
>
> > > Also, even for non-leaf functions, is it possible for GCC to insert the
> > > inline asm before it sets up the stack frame? (This is an occasional
> > > problem on x86.)
> >
> > Inline asm must not have control transfer out of the statement unless
> > it is asm goto.
>
> Can inline asm have calls to other functions?
I don't believe so.
>
> > > Also, what about hand-coded asm?
> >
> > Should follow the same rules if it uses the stack.
>
> How is that enforced?
It's not, AFAIK. Gcc doesn't understand what's inside asm("").
>
> > > > To me this sounds like the equivalent of HAVE_RELIABLE_STACKTRACE.
> > > > This patch may be unneccessarily limited to ppc64le, but OTOH the only
> > > > user of this flag so far is livepatching, which is only implemented on
> > > > PPCs with 64-LE, a.k.a. ELF ABI v2.
> > >
> > > In addition to fixing the above issues, the unwinder also needs to
> > > detect interrupts (i.e., preemption) and page faults on the stack of a
> > > blocked task. If a function were preempted before it created a stack
> > > frame, or if a leaf function blocked on a page fault, the stack trace
> > > will skip the function's caller, so such a trace will need to be
> > > reported to livepatch as unreliable.
> >
> > I don't think there is much problem there for powerpc. Stack frame
> > creation and function call with return pointer are each atomic.
>
> What if the function is interrupted before it creates the stack frame?
>
Then there will be no stack frame, but you still get the caller address
because it's saved in LR register as part of the function call. Then
you get the caller's caller in its stack frame.
Thanks,
Nick
Re: [PATCH] cpufreq: powernv: Add support of frequency domain
On 12/14/2017 10:12 AM, Viresh Kumar wrote:
+ Gautham,
@Gautham: Can you please help reviewing this one ?
On 13-12-17, 13:49, Abhishek Goel wrote:
@@ -693,6 +746,8 @@ static int powernv_cpufreq_target_index(struct
cpufreq_policy *policy,
{
struct powernv_smp_call_data freq_data;
unsigned int cur_msec, gpstate_idx;
+ cpumask_t temp;
+ u32 cpu;
struct global_pstate_info *gpstates = policy->driver_data;
if (unlikely(rebooting) && new_index != get_nominal_index())
@@ -761,24 +816,48 @@ static int powernv_cpufreq_target_index(struct
cpufreq_policy *policy,
spin_unlock(&gpstates->gpstate_lock);
/*
-* Use smp_call_function to send IPI and execute the
-* mtspr on target CPU. We could do that without IPI
-* if current CPU is within policy->cpus (core)
+* Use smp_call_function to send IPI and execute the mtspr on CPU.
+* This needs to be done on every core of the policy
Why on each CPU ?
We need to do it in this way as the current implementation takes the max
of the PMSR of the cores. Thus, when the frequency is required to be
ramped up, it suffices to write to just the local PMSR, but when the
frequency is to be ramped down, if we don't send the IPI it breaks the
compatibility with P8.
*/
- smp_call_function_any(policy->cpus, set_pstate, &freq_data, 1);
+ cpumask_copy(&temp, policy->cpus);
+
+ while (!cpumask_empty(&temp)) {
+ cpu = cpumask_first(&temp);
+ smp_call_function_any(cpu_sibling_mask(cpu),
+ set_pstate, &freq_data, 1);
+ cpumask_andnot(&temp, &temp, cpu_sibling_mask(cpu));
+ }
+
return 0;
}
Re: [PATCH v1 4/7] powerpc/kernel Add EEH operations to notify resume
On 14/12/17 02:32, Bryant G. Ly wrote:
> When pseries SR-IOV is enabled and after a PF driver
> has resumed from EEH, platform has to be notified
> of the event so the child VFs can be allowed to
> resume their normal recovery path.
>
> This patch makes the EEH operation allow unfreeze
> platform dependent code and adds the call to
> pseries EEH code.
>
> Signed-off-by: Bryant G. Ly
> Signed-off-by: Juan J. Alvarez
> ---
> arch/powerpc/include/asm/eeh.h | 1 +
> arch/powerpc/kernel/eeh_driver.c | 4 ++
> arch/powerpc/platforms/powernv/eeh-powernv.c | 3 +-
> arch/powerpc/platforms/pseries/eeh_pseries.c | 100
> ++-
> 4 files changed, 106 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/eeh.h b/arch/powerpc/include/asm/eeh.h
> index 5161c37dd039..12d52a0cd447 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -214,6 +214,7 @@ struct eeh_ops {
> int (*write_config)(struct pci_dn *pdn, int where, int size, u32 val);
> int (*next_error)(struct eeh_pe **pe);
> int (*restore_config)(struct pci_dn *pdn);
> + int (*notify_resume)(struct pci_dn *pdn);
> };
>
> extern int eeh_subsystem_flags;
> diff --git a/arch/powerpc/kernel/eeh_driver.c
> b/arch/powerpc/kernel/eeh_driver.c
> index c61bf770282b..dbda0cda559b 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -361,6 +361,7 @@ static void *eeh_report_resume(void *data, void *userdata)
> bool was_in_error;
> struct pci_driver *driver;
> char *envp[] = {"EVENT=EEH_RESUME", "ONLINE=1", NULL};
> + struct pci_dn *pdn = eeh_dev_to_pdn(edev);
>
> if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
> return NULL;
> @@ -384,6 +385,9 @@ static void *eeh_report_resume(void *data, void *userdata)
> driver->err_handler->resume(dev);
> eeh_pcid_put(dev);
> kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
> +#ifdef CONFIG_PCI_IOV
> + eeh_ops->notify_resume(pdn);
eeh_ops->notify_resume(eeh_dev_to_pdn(edev));
otherwise the compiler will complain at @pdn declaration if
!defined(CONFIG_PCI_IOV). Just try compiling without CONFIG_PCI_IOV.
> +#endif
> return NULL;
> }
>
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c
> b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 961e64115d92..8575b3a29e7c 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -1763,7 +1763,8 @@ static struct eeh_ops pnv_eeh_ops = {
> .read_config= pnv_eeh_read_config,
> .write_config = pnv_eeh_write_config,
> .next_error = pnv_eeh_next_error,
> - .restore_config = pnv_eeh_restore_config
> + .restore_config = pnv_eeh_restore_config,
> + .notify_resume = NULL
.notify_resume is NULL already.
> };
>
> #ifdef CONFIG_PCI_IOV
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c
> b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index 5bdd1678a9ff..2b36fbf4ce74 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -798,6 +798,103 @@ static int pseries_eeh_restore_config(struct pci_dn
> *pdn)
> return 0;
> }
>
> +#ifdef CONFIG_PCI_IOV
> +int pseries_send_allow_unfreeze(struct eeh_pe *pe,
> + u16 *vf_pe_array, int cur_vfs)
> +{
> + int rc, config_addr;
> + int ibm_allow_unfreeze = rtas_token("ibm,open-sriov-allow-unfreeze");
> +
> + config_addr = pe->config_addr;
@config_addr is pointless - it is used just once, even pr_warn few lines
below uses pe->config_addr.
> + spin_lock(&rtas_data_buf_lock);
> + memcpy(rtas_data_buf, vf_pe_array, RTAS_DATA_BUF_SIZE);
> + rc = rtas_call(ibm_allow_unfreeze, 5, 1, NULL,
> +config_addr,
> +BUID_HI(pe->phb->buid),
> +BUID_LO(pe->phb->buid),
> +rtas_data_buf, cur_vfs * sizeof(u16));
> + spin_unlock(&rtas_data_buf_lock);
> + if (rc)
> + pr_warn("%s: Failed to allow unfreeze for PHB#%x-PE#%x,
> rc=%x\n",
> + __func__,
> + pe->phb->global_number,
> + pe->config_addr, rc);
> + return rc;
> +}
> +
> +static int pseries_call_allow_unfreeze(struct eeh_dev *edev)
> +{
> + struct eeh_pe *pe;
> + struct pci_dn *pdn, *tmp, *parent, *physfn_pdn;
> + int cur_vfs, rc, vf_index;
> + u16 *vf_pe_array;
> +
> + vf_pe_array = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
> + if (!vf_pe_array)
> + return -ENOMEM;
> +
> + memset(vf_pe_array, 0, RTAS_DATA_BUF_SIZE);
"z" from kzalloc says it reset the memory.
> + cur_vfs = 0;
> + rc = 0;
> + if (edev->pdev->is_physfn) {
> + pe = eeh_dev_to_pe(e
Re: [PATCH v1 3/7] platforms/pseries: Set eeh_pe of EEH_PE_VF type
On 14/12/17 02:32, Bryant G. Ly wrote:
> To correctly use EEH code one has to make
> sure that the EEH_PE_VF is set for dynamic created
> VFs. Therefore this patch allocates an eeh_pe of
> eeh type EEH_PE_VF and associates PE with parent.
>
> Signed-off-by: Bryant G. Ly
> Signed-off-by: Juan J. Alvarez
> ---
> arch/powerpc/include/asm/pci-bridge.h| 5 -
> arch/powerpc/platforms/pseries/eeh_pseries.c | 9 -
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pci-bridge.h
> b/arch/powerpc/include/asm/pci-bridge.h
> index 9f66ddebb799..c30c7cba4c30 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -211,7 +211,10 @@ struct pci_dn {
> unsigned int *pe_num_map; /* PE# for the first VF PE or array */
> boolm64_single_mode;/* Use M64 BAR in Single Mode */
> #define IODA_INVALID_M64(-1)
> - int (*m64_map)[PCI_SRIOV_NUM_BARS];
> + union {
> + int (*m64_map)[PCI_SRIOV_NUM_BARS];
> + int last_allow_rc;
I'd suggest defining it where is used, easier to follow what this actually
does.
> + };
> #endif /* CONFIG_PCI_IOV */
> int mps;/* Maximum Payload Size */
> struct list_head child_list;
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c
> b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index 1a9a6fa91151..5bdd1678a9ff 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -58,6 +58,8 @@ static int ibm_configure_pe;
> void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
> {
> struct pci_dn *pdn = pci_get_pdn(pdev);
> + struct pci_dn *physfn_pdn;
> + struct eeh_dev *edev;
>
> if (!pdev->is_virtfn)
> return;
> @@ -65,6 +67,10 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
> pdn->device_id = pdev->device;
> pdn->vendor_id = pdev->vendor;
> pdn->class_code = pdev->class;
> + pdn->last_allow_rc = 0;
> + physfn_pdn = pci_get_pdn(pdev->physfn);
> + pdn->pe_number = physfn_pdn->pe_num_map[pdn->vf_index];
> + edev = pdn_to_eeh_dev(pdn);
>
> /*
>* The following operations will fail if VF's sysfs files
> @@ -72,8 +78,9 @@ void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
>*/
> eeh_add_device_early(pdn);
> eeh_add_device_late(pdev);
> + edev->pe_config_addr = (pdn->busno << 16) | (pdn->devfn << 8);
> + eeh_add_to_parent_pe(edev);
powernv does this from eeh_ops::probe, and so does pseries_eeh_probe(), do
you still need this here?
> eeh_sysfs_add_device(pdev);
> -
Unnecessary change.
> }
>
> /*
>
--
Alexey
Re: [PATCH v1 3/7] platforms/pseries: Set eeh_pe of EEH_PE_VF type
On Wed, 2017-12-13 at 09:32 -0600, Bryant G. Ly wrote:
> To correctly use EEH code one has to make
> sure that the EEH_PE_VF is set for dynamic created
> VFs. Therefore this patch allocates an eeh_pe of
> eeh type EEH_PE_VF and associates PE with parent.
>
> Signed-off-by: Bryant G. Ly
> Signed-off-by: Juan J. Alvarez
> ---
> arch/powerpc/include/asm/pci-bridge.h| 5 -
> arch/powerpc/platforms/pseries/eeh_pseries.c | 9 -
> 2 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/pci-bridge.h
> b/arch/powerpc/include/asm/pci-bridge.h
> index 9f66ddebb799..c30c7cba4c30 100644
> --- a/arch/powerpc/include/asm/pci-bridge.h
> +++ b/arch/powerpc/include/asm/pci-bridge.h
> @@ -211,7 +211,10 @@ struct pci_dn {
> unsigned int *pe_num_map; /* PE# for the first VF PE
> or array */
> boolm64_single_mode;/* Use M64 BAR in Single
> Mode */
> #define IODA_INVALID_M64(-1)
> - int (*m64_map)[PCI_SRIOV_NUM_BARS];
> + union {
> + int (*m64_map)[PCI_SRIOV_NUM_BARS];
> + int last_allow_rc;
> + };
A comment would be useful here. Why are these mutually exclusive,
last_allow_rc isn't amazingly self-documenting.
> #endif /* CONFIG_PCI_IOV */
> int mps;/* Maximum Payload
> Size */
> struct list_head child_list;
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c
> b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index 1a9a6fa91151..5bdd1678a9ff 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -58,6 +58,8 @@ static int ibm_configure_pe;
> void pseries_pcibios_bus_add_device(struct pci_dev *pdev)
> {
> struct pci_dn *pdn = pci_get_pdn(pdev);
> + struct pci_dn *physfn_pdn;
> + struct eeh_dev *edev;
>
> if (!pdev->is_virtfn)
> return;
> @@ -65,6 +67,10 @@ void pseries_pcibios_bus_add_device(struct pci_dev
> *pdev)
> pdn->device_id = pdev->device;
> pdn->vendor_id = pdev->vendor;
> pdn->class_code = pdev->class;
> + pdn->last_allow_rc = 0;
> + physfn_pdn = pci_get_pdn(pdev->physfn);
> + pdn->pe_number = physfn_pdn->pe_num_map[pdn->vf_index];
> + edev = pdn_to_eeh_dev(pdn);
>
> /*
>* The following operations will fail if VF's sysfs files
> @@ -72,8 +78,9 @@ void pseries_pcibios_bus_add_device(struct pci_dev
> *pdev)
>*/
> eeh_add_device_early(pdn);
> eeh_add_device_late(pdev);
> + edev->pe_config_addr = (pdn->busno << 16) | (pdn->devfn <<
> 8);
> + eeh_add_to_parent_pe(edev);
> eeh_sysfs_add_device(pdev);
> -
> }
>
> /*
Re: [PATCH v1 4/7] powerpc/kernel Add EEH operations to notify resume
On Wed, 2017-12-13 at 09:32 -0600, Bryant G. Ly wrote:
> When pseries SR-IOV is enabled and after a PF driver
> has resumed from EEH, platform has to be notified
> of the event so the child VFs can be allowed to
> resume their normal recovery path.
>
> This patch makes the EEH operation allow unfreeze
> platform dependent code and adds the call to
> pseries EEH code.
>
> Signed-off-by: Bryant G. Ly
> Signed-off-by: Juan J. Alvarez
Just some nitpicks, there's a lot of weird whitespace in this patch
> ---
> arch/powerpc/include/asm/eeh.h | 1 +
> arch/powerpc/kernel/eeh_driver.c | 4 ++
> arch/powerpc/platforms/powernv/eeh-powernv.c | 3 +-
> arch/powerpc/platforms/pseries/eeh_pseries.c | 100
> ++-
> 4 files changed, 106 insertions(+), 2 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/eeh.h
> b/arch/powerpc/include/asm/eeh.h
> index 5161c37dd039..12d52a0cd447 100644
> --- a/arch/powerpc/include/asm/eeh.h
> +++ b/arch/powerpc/include/asm/eeh.h
> @@ -214,6 +214,7 @@ struct eeh_ops {
> int (*write_config)(struct pci_dn *pdn, int where, int size,
> u32 val);
> int (*next_error)(struct eeh_pe **pe);
> int (*restore_config)(struct pci_dn *pdn);
> + int (*notify_resume)(struct pci_dn *pdn);
> };
>
> extern int eeh_subsystem_flags;
> diff --git a/arch/powerpc/kernel/eeh_driver.c
> b/arch/powerpc/kernel/eeh_driver.c
> index c61bf770282b..dbda0cda559b 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -361,6 +361,7 @@ static void *eeh_report_resume(void *data, void
> *userdata)
> bool was_in_error;
> struct pci_driver *driver;
> char *envp[] = {"EVENT=EEH_RESUME", "ONLINE=1", NULL};
> + struct pci_dn *pdn = eeh_dev_to_pdn(edev);
>
> if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev-
> >pe))
> return NULL;
> @@ -384,6 +385,9 @@ static void *eeh_report_resume(void *data, void
> *userdata)
> driver->err_handler->resume(dev);
> eeh_pcid_put(dev);
> kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
> +#ifdef CONFIG_PCI_IOV
> + eeh_ops->notify_resume(pdn);
> +#endif
> return NULL;
> }
>
> diff --git a/arch/powerpc/platforms/powernv/eeh-powernv.c
> b/arch/powerpc/platforms/powernv/eeh-powernv.c
> index 961e64115d92..8575b3a29e7c 100644
> --- a/arch/powerpc/platforms/powernv/eeh-powernv.c
> +++ b/arch/powerpc/platforms/powernv/eeh-powernv.c
> @@ -1763,7 +1763,8 @@ static struct eeh_ops pnv_eeh_ops = {
> .read_config= pnv_eeh_read_config,
> .write_config = pnv_eeh_write_config,
> .next_error = pnv_eeh_next_error,
> - .restore_config = pnv_eeh_restore_config
> + .restore_config = pnv_eeh_restore_config,
> + .notify_resume = NULL
> };
>
> #ifdef CONFIG_PCI_IOV
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c
> b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index 5bdd1678a9ff..2b36fbf4ce74 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -798,6 +798,103 @@ static int pseries_eeh_restore_config(struct
> pci_dn *pdn)
> return 0;
> }
>
> +#ifdef CONFIG_PCI_IOV
> +int pseries_send_allow_unfreeze(struct eeh_pe *pe,
> + u16 *vf_pe_array, int cur_vfs)
> +{
> + int rc, config_addr;
extra space
> + int ibm_allow_unfreeze = rtas_token("ibm,open-sriov-allow-
> unfreeze");
> +
> + config_addr = pe->config_addr;
> + spin_lock(&rtas_data_buf_lock);
> + memcpy(rtas_data_buf, vf_pe_array, RTAS_DATA_BUF_SIZE);
> + rc = rtas_call(ibm_allow_unfreeze, 5, 1, NULL,
> +config_addr,
> +BUID_HI(pe->phb->buid),
> +BUID_LO(pe->phb->buid),
> +rtas_data_buf, cur_vfs * sizeof(u16));
> + spin_unlock(&rtas_data_buf_lock);
> + if (rc)
> + pr_warn("%s: Failed to allow unfreeze for PHB#%x-
> PE#%x, rc=%x\n",
> + __func__,
> + pe->phb->global_number,
> + pe->config_addr, rc);
> + return rc;
> +}
> +
> +static int pseries_call_allow_unfreeze(struct eeh_dev *edev)
> +{
> + struct eeh_pe *pe;
> + struct pci_dn *pdn, *tmp, *parent, *physfn_pdn;
> + int cur_vfs, rc, vf_index;
> + u16 *vf_pe_array;
> +
> + vf_pe_array = kzalloc(RTAS_DATA_BUF_SIZE, GFP_KERNEL);
> + if (!vf_pe_array)
> + return -ENOMEM;
> +
> + memset(vf_pe_array, 0, RTAS_DATA_BUF_SIZE);
> + cur_vfs = 0;
> + rc = 0;
> + if (edev->pdev->is_physfn) {
> + pe = eeh_dev_to_pe(edev);
> + cur_vfs = pci_num_vf(edev->pdev);
> + pdn = eeh_dev_to_pdn(edev);
> + parent = pdn->parent;
extra space
> + /* For each of its VF
> + * call allow unfreeze
> + */
Weird l
Re: [PATCH v1 2/7] powerpc/kernel: Add uevents in EEH error/resume
On Wed, 2017-12-13 at 09:32 -0600, Bryant G. Ly wrote: > Devices can go offline when EEH is reported. This patch adds > a change to the kernel object and lets udev know of error. > When device resumes a change is also set reporting device as > online. Therefore, EEH events are better propagated to user > space for devices in powerpc arch. > > Signed-off-by: Bryant G. Ly > Signed-off-by: Juan J. Alvarez > It would probably also be useful to communicate when recovery fails and a device is no longer usable, so userspace knows not to keep waiting for recovery to complete.
Re: [PATCH] On ppc64le we HAVE_RELIABLE_STACKTRACE
On Mon, Dec 18, 2017 at 02:39:06PM +1100, Balbir Singh wrote: > On Mon, Dec 18, 2017 at 1:58 PM, Josh Poimboeuf wrote: > > On Fri, Dec 15, 2017 at 07:40:09PM +1000, Nicholas Piggin wrote: > >> On Tue, 12 Dec 2017 08:05:01 -0600 > >> Josh Poimboeuf wrote: > >> > >> > On Tue, Dec 12, 2017 at 12:39:12PM +0100, Torsten Duwe wrote: > >> > > Hi all, > >> > > > >> > > The "Power Architecture 64-Bit ELF V2 ABI" says in section 2.3.2.3: > >> > > > >> > > [...] There are several rules that must be adhered to in order to > >> > > ensure > >> > > reliable and consistent call chain backtracing: > >> > > > >> > > * Before a function calls any other function, it shall establish its > >> > > own stack frame, whose size shall be a multiple of 16 bytes. > >> > > >> > What about leaf functions? If a leaf function doesn't establish a stack > >> > frame, and it has inline asm which contains a blr to another function, > >> > this ABI is broken. > > > > Oops, I meant to say "bl" instead of "blr". > > I was wondering why "blr" mattered, but I guess we should speak of the > consistency > model. By walking a stack trace we expect to find whether a function is in use > or not and can/cannot be live-patched at this point in time. Right? Right. > >> > Also, even for non-leaf functions, is it possible for GCC to insert the > >> > inline asm before it sets up the stack frame? (This is an occasional > >> > problem on x86.) > >> > >> Inline asm must not have control transfer out of the statement unless > >> it is asm goto. > > > > Can inline asm have calls to other functions? > > > >> > Also, what about hand-coded asm? > >> > >> Should follow the same rules if it uses the stack. > > > > How is that enforced? > > > >> > > To me this sounds like the equivalent of HAVE_RELIABLE_STACKTRACE. > >> > > This patch may be unneccessarily limited to ppc64le, but OTOH the only > >> > > user of this flag so far is livepatching, which is only implemented on > >> > > PPCs with 64-LE, a.k.a. ELF ABI v2. > >> > > >> > In addition to fixing the above issues, the unwinder also needs to > >> > detect interrupts (i.e., preemption) and page faults on the stack of a > >> > blocked task. If a function were preempted before it created a stack > >> > frame, or if a leaf function blocked on a page fault, the stack trace > >> > will skip the function's caller, so such a trace will need to be > >> > reported to livepatch as unreliable. > >> > >> I don't think there is much problem there for powerpc. Stack frame > >> creation and function call with return pointer are each atomic. > > > > What if the function is interrupted before it creates the stack frame? > > > > If it is interrupted, the exception handler will establish a new stack frame. > From a consistency viewpoint, I guess the question is -- has the function > been entered or considered to be entered when a stack frame has not > yet been established Actually I think it's the function's *caller* which gets skipped. r1 (stack pointer) will point to the caller's stack frame, and presumably the unwinder would read the caller's caller's stack frame to get the next LR, skipping the caller's return address because it hasn't been saved yet. -- Josh
Re: [PATCH v1 2/7] powerpc/kernel: Add uevents in EEH error/resume
On 14/12/17 02:32, Bryant G. Ly wrote:
> Devices can go offline when EEH is reported. This patch adds
> a change to the kernel object and lets udev know of error.
> When device resumes a change is also set reporting device as
> online. Therefore, EEH events are better propagated to user
> space for devices in powerpc arch.
>
> Signed-off-by: Bryant G. Ly
> Signed-off-by: Juan J. Alvarez
> ---
> arch/powerpc/kernel/eeh_driver.c | 5 -
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/kernel/eeh_driver.c
> b/arch/powerpc/kernel/eeh_driver.c
> index 3c0fa99c5533..c61bf770282b 100644
> --- a/arch/powerpc/kernel/eeh_driver.c
> +++ b/arch/powerpc/kernel/eeh_driver.c
> @@ -204,6 +204,7 @@ static void *eeh_report_error(void *data, void *userdata)
> struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
> enum pci_ers_result rc, *res = userdata;
> struct pci_driver *driver;
> + char *envp[] = {"EVENT=EEH_ERROR", "ONLINE=0", NULL};
scripts/checkpatch.pl:
WARNING: char * array declaration might be better as static const
#27: FILE: arch/powerpc/kernel/eeh_driver.c:207:
+ char *envp[] = {"EVENT=EEH_ERROR", "ONLINE=0", NULL};
>
> if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
> return NULL;
> @@ -228,6 +229,7 @@ static void *eeh_report_error(void *data, void *userdata)
>
> edev->in_error = true;
> eeh_pcid_put(dev);
> + kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
> return NULL;
> }
>
> @@ -358,6 +360,7 @@ static void *eeh_report_resume(void *data, void *userdata)
> struct pci_dev *dev = eeh_dev_to_pci_dev(edev);
> bool was_in_error;
> struct pci_driver *driver;
> + char *envp[] = {"EVENT=EEH_RESUME", "ONLINE=1", NULL};
WARNING: char * array declaration might be better as static const
#43: FILE: arch/powerpc/kernel/eeh_driver.c:363:
+ char *envp[] = {"EVENT=EEH_RESUME", "ONLINE=1", NULL};
>
> if (!dev || eeh_dev_removed(edev) || eeh_pe_passed(edev->pe))
> return NULL;
> @@ -379,8 +382,8 @@ static void *eeh_report_resume(void *data, void *userdata)
> }
>
> driver->err_handler->resume(dev);
> -
Unnecessary change.
> eeh_pcid_put(dev);
> + kobject_uevent_env(&dev->dev.kobj, KOBJ_CHANGE, envp);
> return NULL;
> }
>
>
--
Alexey
Re: [PATCH v1 1/7] platform/pseries: Update VF config space after EEH
On 14/12/17 02:32, Bryant G. Ly wrote:
> Add EEH platform operations for pseries to update VF
> config space. With this change after EEH, the VF
> will have updated config space for pseries platform.
>
> Signed-off-by: Bryant G. Ly
> Signed-off-by: Juan J. Alvarez
> ---
> arch/powerpc/platforms/pseries/eeh_pseries.c | 85
> +++-
> 1 file changed, 84 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/platforms/pseries/eeh_pseries.c
> b/arch/powerpc/platforms/pseries/eeh_pseries.c
> index 2295f117e2d3..1a9a6fa91151 100644
> --- a/arch/powerpc/platforms/pseries/eeh_pseries.c
> +++ b/arch/powerpc/platforms/pseries/eeh_pseries.c
> @@ -708,6 +708,89 @@ static int pseries_eeh_write_config(struct pci_dn *pdn,
> int where, int size, u32
> return rtas_write_config(pdn, where, size, val);
> }
>
> +static int pseries_eeh_restore_vf_config(struct pci_dn *pdn)
This particular function is just a copy of its powernv counterpart -
pnv_eeh_restore_vf_config(), it could go to arch/powerpc/kernel/eeh.c, for
example. Or I am missing something here?
> +{
> + struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
> + u32 devctl, cmd, cap2, aer_capctl;
> + int old_mps;
> +
> + if (edev->pcie_cap) {
> + /* Restore MPS */
> + old_mps = (ffs(pdn->mps) - 8) << 5;
> + eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
> + 2, &devctl);
> + devctl &= ~PCI_EXP_DEVCTL_PAYLOAD;
> + devctl |= old_mps;
> + eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
> + 2, devctl);
> +
> + /* Disable Completion Timeout */
> + eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCAP2,
> + 4, &cap2);
> + if (cap2 & 0x10) {
> + eeh_ops->read_config(pdn,
> + edev->pcie_cap + PCI_EXP_DEVCTL2,
> + 4, &cap2);
> + cap2 |= 0x10;
> + eeh_ops->write_config(pdn,
> + edev->pcie_cap + PCI_EXP_DEVCTL2,
> + 4, cap2);
> + }
> + }
> +
> + /* Enable SERR and parity checking */
> + eeh_ops->read_config(pdn, PCI_COMMAND, 2, &cmd);
> + cmd |= (PCI_COMMAND_PARITY | PCI_COMMAND_SERR);
> + eeh_ops->write_config(pdn, PCI_COMMAND, 2, cmd);
> +
> + /* Enable report various errors */
> + if (edev->pcie_cap) {
> + eeh_ops->read_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
> + 2, &devctl);
> + devctl &= ~PCI_EXP_DEVCTL_CERE;
> + devctl |= (PCI_EXP_DEVCTL_NFERE |
> +PCI_EXP_DEVCTL_FERE |
> +PCI_EXP_DEVCTL_URRE);
> + eeh_ops->write_config(pdn, edev->pcie_cap + PCI_EXP_DEVCTL,
> + 2, devctl);
> + }
> +
> + /* Enable ECRC generation and check */
> + if (edev->pcie_cap && edev->aer_cap) {
> + eeh_ops->read_config(pdn, edev->aer_cap + PCI_ERR_CAP,
> + 4, &aer_capctl);
> + aer_capctl |= (PCI_ERR_CAP_ECRC_GENE | PCI_ERR_CAP_ECRC_CHKE);
> + eeh_ops->write_config(pdn, edev->aer_cap + PCI_ERR_CAP,
> + 4, aer_capctl);
> + }
> +
> + return 0;
> +}
> +
> +static int pseries_eeh_restore_config(struct pci_dn *pdn)
> +{
> + struct eeh_dev *edev = pdn_to_eeh_dev(pdn);
> + s64 ret;
> +
> + if (!edev)
> + return -EEXIST;
> +
> + /*
> + * FIXME: The MPS, error routing rules, timeout setting are worthy
> + * to be exported by firmware in extendible way.
> + */
> + if (edev->physfn)
> + ret = pseries_eeh_restore_vf_config(pdn);
> +
> + if (ret) {
> + pr_warn("%s: Can't reinit PCI dev 0x%x (%lld)\n",
> + __func__, edev->pe_config_addr, ret);
> + return -EIO;
> + }
> +
> + return 0;
> +}
> +
> static struct eeh_ops pseries_eeh_ops = {
> .name = "pseries",
> .init = pseries_eeh_init,
> @@ -723,7 +806,7 @@ static struct eeh_ops pseries_eeh_ops = {
> .read_config= pseries_eeh_read_config,
> .write_config = pseries_eeh_write_config,
> .next_error = NULL,
> - .restore_config = NULL
> + .restore_config = pseries_eeh_restore_config
> };
>
> /**
>
--
Alexey
Re: [PATCH] On ppc64le we HAVE_RELIABLE_STACKTRACE
On Mon, Dec 18, 2017 at 1:58 PM, Josh Poimboeuf wrote: > On Fri, Dec 15, 2017 at 07:40:09PM +1000, Nicholas Piggin wrote: >> On Tue, 12 Dec 2017 08:05:01 -0600 >> Josh Poimboeuf wrote: >> >> > On Tue, Dec 12, 2017 at 12:39:12PM +0100, Torsten Duwe wrote: >> > > Hi all, >> > > >> > > The "Power Architecture 64-Bit ELF V2 ABI" says in section 2.3.2.3: >> > > >> > > [...] There are several rules that must be adhered to in order to ensure >> > > reliable and consistent call chain backtracing: >> > > >> > > * Before a function calls any other function, it shall establish its >> > > own stack frame, whose size shall be a multiple of 16 bytes. >> > >> > What about leaf functions? If a leaf function doesn't establish a stack >> > frame, and it has inline asm which contains a blr to another function, >> > this ABI is broken. > > Oops, I meant to say "bl" instead of "blr". I was wondering why "blr" mattered, but I guess we should speak of the consistency model. By walking a stack trace we expect to find whether a function is in use or not and can/cannot be live-patched at this point in time. Right? > >> > Also, even for non-leaf functions, is it possible for GCC to insert the >> > inline asm before it sets up the stack frame? (This is an occasional >> > problem on x86.) >> >> Inline asm must not have control transfer out of the statement unless >> it is asm goto. > > Can inline asm have calls to other functions? > >> > Also, what about hand-coded asm? >> >> Should follow the same rules if it uses the stack. > > How is that enforced? > >> > > To me this sounds like the equivalent of HAVE_RELIABLE_STACKTRACE. >> > > This patch may be unneccessarily limited to ppc64le, but OTOH the only >> > > user of this flag so far is livepatching, which is only implemented on >> > > PPCs with 64-LE, a.k.a. ELF ABI v2. >> > >> > In addition to fixing the above issues, the unwinder also needs to >> > detect interrupts (i.e., preemption) and page faults on the stack of a >> > blocked task. If a function were preempted before it created a stack >> > frame, or if a leaf function blocked on a page fault, the stack trace >> > will skip the function's caller, so such a trace will need to be >> > reported to livepatch as unreliable. >> >> I don't think there is much problem there for powerpc. Stack frame >> creation and function call with return pointer are each atomic. > > What if the function is interrupted before it creates the stack frame? > If it is interrupted, the exception handler will establish a new stack frame. >From a consistency viewpoint, I guess the question is -- has the function been entered or considered to be entered when a stack frame has not yet been established Balbir Singh.
Re: [PATCH v4 00/11] ASoC: fsl_ssi: Clean up - coding style level
On 12/17/17 8:51 PM, Nicolin Chen wrote: Nicolin Chen (11): ASoC: fsl_ssi: Rename fsl_ssi_private to fsl_ssi ASoC: fsl_ssi: Cache pdev->dev pointer ASoC: fsl_ssi: Refine all comments ASoC: fsl_ssi: Rename registers and fields macros ASoC: fsl_ssi: Refine indentations and wrappings ASoC: fsl_ssi: Refine printk outputs ASoC: fsl_ssi: Rename cpu_dai parameter to dai ASoC: fsl_ssi: Rename scr_val to scr ASoC: fsl_ssi: Replace fsl_ssi_rxtx_reg_val with fsl_ssi_regvals ASoC: fsl_ssi: Rename i2smode to i2s_net ASoC: fsl_ssi: Define ternary macros to simplify code Acked-by: Timur Tabi
Re: [PATCH] On ppc64le we HAVE_RELIABLE_STACKTRACE
On Fri, Dec 15, 2017 at 07:40:09PM +1000, Nicholas Piggin wrote: > On Tue, 12 Dec 2017 08:05:01 -0600 > Josh Poimboeuf wrote: > > > On Tue, Dec 12, 2017 at 12:39:12PM +0100, Torsten Duwe wrote: > > > Hi all, > > > > > > The "Power Architecture 64-Bit ELF V2 ABI" says in section 2.3.2.3: > > > > > > [...] There are several rules that must be adhered to in order to ensure > > > reliable and consistent call chain backtracing: > > > > > > * Before a function calls any other function, it shall establish its > > > own stack frame, whose size shall be a multiple of 16 bytes. > > > > What about leaf functions? If a leaf function doesn't establish a stack > > frame, and it has inline asm which contains a blr to another function, > > this ABI is broken. Oops, I meant to say "bl" instead of "blr". > > Also, even for non-leaf functions, is it possible for GCC to insert the > > inline asm before it sets up the stack frame? (This is an occasional > > problem on x86.) > > Inline asm must not have control transfer out of the statement unless > it is asm goto. Can inline asm have calls to other functions? > > Also, what about hand-coded asm? > > Should follow the same rules if it uses the stack. How is that enforced? > > > To me this sounds like the equivalent of HAVE_RELIABLE_STACKTRACE. > > > This patch may be unneccessarily limited to ppc64le, but OTOH the only > > > user of this flag so far is livepatching, which is only implemented on > > > PPCs with 64-LE, a.k.a. ELF ABI v2. > > > > In addition to fixing the above issues, the unwinder also needs to > > detect interrupts (i.e., preemption) and page faults on the stack of a > > blocked task. If a function were preempted before it created a stack > > frame, or if a leaf function blocked on a page fault, the stack trace > > will skip the function's caller, so such a trace will need to be > > reported to livepatch as unreliable. > > I don't think there is much problem there for powerpc. Stack frame > creation and function call with return pointer are each atomic. What if the function is interrupted before it creates the stack frame? -- Josh
[PATCH v4 11/11] ASoC: fsl_ssi: Define ternary macros to simplify code
Some regmap code looks redudant. So simplify it.
Signed-off-by: Nicolin Chen
Tested-by: Maciej S. Szmigiero
Reviewed-by: Maciej S. Szmigiero
---
sound/soc/fsl/fsl_ssi.c | 27 +++
sound/soc/fsl/fsl_ssi.h | 4
2 files changed, 15 insertions(+), 16 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 2b3915c..aecd00f 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -408,13 +408,10 @@ static void fsl_ssi_rxtx_config(struct fsl_ssi *ssi, bool
enable)
*/
static void fsl_ssi_fifo_clear(struct fsl_ssi *ssi, bool is_rx)
{
- if (is_rx) {
- regmap_update_bits(ssi->regs, REG_SSI_SOR,
- SSI_SOR_RX_CLR, SSI_SOR_RX_CLR);
- } else {
- regmap_update_bits(ssi->regs, REG_SSI_SOR,
- SSI_SOR_TX_CLR, SSI_SOR_TX_CLR);
- }
+ bool tx = !is_rx;
+
+ regmap_update_bits(ssi->regs, REG_SSI_SOR,
+ SSI_SOR_xX_CLR(tx), SSI_SOR_xX_CLR(tx));
}
/**
@@ -681,6 +678,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream
*substream,
struct snd_soc_dai *dai,
struct snd_pcm_hw_params *hw_params)
{
+ bool tx2, tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai);
struct regmap *regs = ssi->regs;
int synchronous = ssi->cpu_dai_drv.symmetric_rates, ret;
@@ -768,10 +766,9 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream
*substream,
(psr ? SSI_SxCCR_PSR : 0);
mask = SSI_SxCCR_PM_MASK | SSI_SxCCR_DIV2 | SSI_SxCCR_PSR;
- if (substream->stream == SNDRV_PCM_STREAM_PLAYBACK || synchronous)
- regmap_update_bits(regs, REG_SSI_STCCR, mask, stccr);
- else
- regmap_update_bits(regs, REG_SSI_SRCCR, mask, stccr);
+ /* STCCR is used for RX in synchronous mode */
+ tx2 = tx || synchronous;
+ regmap_update_bits(regs, REG_SSI_SxCCR(tx2), mask, stccr);
if (!baudclk_is_used) {
ret = clk_set_rate(ssi->baudclk, baudrate);
@@ -799,6 +796,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream
*substream,
struct snd_pcm_hw_params *hw_params,
struct snd_soc_dai *dai)
{
+ bool tx2, tx = substream->stream == SNDRV_PCM_STREAM_PLAYBACK;
struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai);
struct regmap *regs = ssi->regs;
unsigned int channels = params_channels(hw_params);
@@ -849,11 +847,8 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream
*substream,
}
/* In synchronous mode, the SSI uses STCCR for capture */
- if ((substream->stream == SNDRV_PCM_STREAM_PLAYBACK) ||
- ssi->cpu_dai_drv.symmetric_rates)
- regmap_update_bits(regs, REG_SSI_STCCR, SSI_SxCCR_WL_MASK, wl);
- else
- regmap_update_bits(regs, REG_SSI_SRCCR, SSI_SxCCR_WL_MASK, wl);
+ tx2 = tx || ssi->cpu_dai_drv.symmetric_rates;
+ regmap_update_bits(regs, REG_SSI_SxCCR(tx2), SSI_SxCCR_WL_MASK, wl);
return 0;
}
diff --git a/sound/soc/fsl/fsl_ssi.h b/sound/soc/fsl/fsl_ssi.h
index b610087..de2fdc5 100644
--- a/sound/soc/fsl/fsl_ssi.h
+++ b/sound/soc/fsl/fsl_ssi.h
@@ -35,10 +35,12 @@
#define REG_SSI_STCR 0x1c
/* SSI Receive Configuration Register */
#define REG_SSI_SRCR 0x20
+#define REG_SSI_SxCR(tx) ((tx) ? REG_SSI_STCR : REG_SSI_SRCR)
/* SSI Transmit Clock Control Register */
#define REG_SSI_STCCR 0x24
/* SSI Receive Clock Control Register */
#define REG_SSI_SRCCR 0x28
+#define REG_SSI_SxCCR(tx) ((tx) ? REG_SSI_STCCR : REG_SSI_SRCCR)
/* SSI FIFO Control/Status Register */
#define REG_SSI_SFCSR 0x2c
/*
@@ -67,6 +69,7 @@
#define REG_SSI_STMSK 0x48
/* SSI Receive Time Slot Mask Register */
#define REG_SSI_SRMSK 0x4c
+#define REG_SSI_SxMSK(tx) ((tx) ? REG_SSI_STMSK : REG_SSI_SRMSK)
/*
* SSI AC97 Channel Status Register
*
@@ -249,6 +252,7 @@
#define SSI_SOR_CLKOFF 0x0040
#define SSI_SOR_RX_CLR 0x0020
#define SSI_SOR_TX_CLR 0x0010
+#define SSI_SOR_xX_CLR(tx) ((tx) ? SSI_SOR_TX_CLR : SSI_SOR_RX_CLR)
#define SSI_SOR_INIT 0x0008
#define SSI_SOR_WAIT_SHIFT 1
#define SSI_SOR_WAIT_MASK 0x0006
--
2.7.4
[PATCH v4 10/11] ASoC: fsl_ssi: Rename i2smode to i2s_net
Since this i2smode also includes the setting of Network mode, it
should have it in the name. This patch also adds its MASK define.
Signed-off-by: Nicolin Chen
Tested-by: Maciej S. Szmigiero
Reviewed-by: Maciej S. Szmigiero
---
sound/soc/fsl/fsl_ssi.c | 24
sound/soc/fsl/fsl_ssi.h | 1 +
2 files changed, 13 insertions(+), 12 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index aef014c..2b3915c 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -201,7 +201,7 @@ struct fsl_ssi_soc_data {
* @cpu_dai_drv: CPU DAI driver for this device
*
* @dai_fmt: DAI configuration this device is currently used with
- * @i2s_mode: I2S and Network mode configuration of SCR register
+ * @i2s_net: I2S and Network mode configurations of SCR register
* @use_dma: DMA is used or FIQ with stream filter
* @use_dual_fifo: DMA with support for dual FIFO mode
* @has_ipg_clk_name: If "ipg" is in the clock name list of device tree
@@ -245,7 +245,7 @@ struct fsl_ssi {
struct snd_soc_dai_driver cpu_dai_drv;
unsigned int dai_fmt;
- u8 i2s_mode;
+ u8 i2s_net;
bool use_dma;
bool use_dual_fifo;
bool has_ipg_clk_name;
@@ -836,16 +836,16 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream
*substream,
}
if (!fsl_ssi_is_ac97(ssi)) {
- u8 i2smode;
+ u8 i2s_net;
/* Normal + Network mode to send 16-bit data in 32-bit frames */
if (fsl_ssi_is_i2s_cbm_cfs(ssi) && sample_size == 16)
- i2smode = SSI_SCR_I2S_MODE_NORMAL | SSI_SCR_NET;
+ i2s_net = SSI_SCR_I2S_MODE_NORMAL | SSI_SCR_NET;
else
- i2smode = ssi->i2s_mode;
+ i2s_net = ssi->i2s_net;
regmap_update_bits(regs, REG_SSI_SCR,
- SSI_SCR_NET | SSI_SCR_I2S_MODE_MASK,
- channels == 1 ? 0 : i2smode);
+ SSI_SCR_I2S_NET_MASK,
+ channels == 1 ? 0 : i2s_net);
}
/* In synchronous mode, the SSI uses STCCR for capture */
@@ -902,7 +902,7 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
srcr &= ~mask;
/* Use Network mode as default */
- ssi->i2s_mode = SSI_SCR_NET;
+ ssi->i2s_net = SSI_SCR_NET;
switch (fmt & SND_SOC_DAIFMT_FORMAT_MASK) {
case SND_SOC_DAIFMT_I2S:
regmap_update_bits(regs, REG_SSI_STCCR,
@@ -912,10 +912,10 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
switch (fmt & SND_SOC_DAIFMT_MASTER_MASK) {
case SND_SOC_DAIFMT_CBM_CFS:
case SND_SOC_DAIFMT_CBS_CFS:
- ssi->i2s_mode |= SSI_SCR_I2S_MODE_MASTER;
+ ssi->i2s_net |= SSI_SCR_I2S_MODE_MASTER;
break;
case SND_SOC_DAIFMT_CBM_CFM:
- ssi->i2s_mode |= SSI_SCR_I2S_MODE_SLAVE;
+ ssi->i2s_net |= SSI_SCR_I2S_MODE_SLAVE;
break;
default:
return -EINVAL;
@@ -940,12 +940,12 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
break;
case SND_SOC_DAIFMT_AC97:
/* Data on falling edge of bclk, frame high, 1clk before data */
- ssi->i2s_mode |= SSI_SCR_I2S_MODE_NORMAL;
+ ssi->i2s_net |= SSI_SCR_I2S_MODE_NORMAL;
break;
default:
return -EINVAL;
}
- scr |= ssi->i2s_mode;
+ scr |= ssi->i2s_net;
/* DAI clock inversion */
switch (fmt & SND_SOC_DAIFMT_INV_MASK) {
diff --git a/sound/soc/fsl/fsl_ssi.h b/sound/soc/fsl/fsl_ssi.h
index 52b88f1..b610087 100644
--- a/sound/soc/fsl/fsl_ssi.h
+++ b/sound/soc/fsl/fsl_ssi.h
@@ -95,6 +95,7 @@
#define SSI_SCR_I2S_MODE_SLAVE 0x0040
#define SSI_SCR_SYN0x0010
#define SSI_SCR_NET0x0008
+#define SSI_SCR_I2S_NET_MASK (SSI_SCR_NET | SSI_SCR_I2S_MODE_MASK)
#define SSI_SCR_RE 0x0004
#define SSI_SCR_TE 0x0002
#define SSI_SCR_SSIEN 0x0001
--
2.7.4
[PATCH v4 09/11] ASoC: fsl_ssi: Replace fsl_ssi_rxtx_reg_val with fsl_ssi_regvals
The name fsl_ssi_rxtx_reg_val is too long to read comfortably.
So this patch shortens it by using an array (fsl_ssi_regvals,
renamed from fsl_ssi_reg_val). To do that, it also introduces
two macros (TX and RX) to replace the wrapper structure. This
will also help further cleanups.
Meanwhile, it unifies all local variable with the name "vals"
to get rid of the name "reg" -- could be confusing with "regs"
in the private struct for regmap.
Signed-off-by: Nicolin Chen
Tested-by: Maciej S. Szmigiero
Reviewed-by: Maciej S. Szmigiero
---
sound/soc/fsl/fsl_ssi.c | 79 +++--
sound/soc/fsl/fsl_ssi.h | 3 ++
2 files changed, 40 insertions(+), 42 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index af3ba71..aef014c 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -106,18 +106,13 @@ enum fsl_ssi_type {
FSL_SSI_MX51,
};
-struct fsl_ssi_reg_val {
+struct fsl_ssi_regvals {
u32 sier;
u32 srcr;
u32 stcr;
u32 scr;
};
-struct fsl_ssi_rxtx_reg_val {
- struct fsl_ssi_reg_val rx;
- struct fsl_ssi_reg_val tx;
-};
-
static bool fsl_ssi_readable_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
@@ -213,7 +208,7 @@ struct fsl_ssi_soc_data {
* @fifo_depth: Depth of the SSI FIFOs
* @slot_width: Width of each DAI slot
* @slots: Number of slots
- * @rxtx_reg_val: Specific RX/TX register settings
+ * @regvals: Specific RX/TX register settings
*
* @clk: Clock source to access register
* @baudclk: Clock source to generate bit and frame-sync clocks
@@ -257,7 +252,7 @@ struct fsl_ssi {
unsigned int fifo_depth;
unsigned int slot_width;
unsigned int slots;
- struct fsl_ssi_rxtx_reg_val rxtx_reg_val;
+ struct fsl_ssi_regvals regvals[2];
struct clk *clk;
struct clk *baudclk;
@@ -386,25 +381,25 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
static void fsl_ssi_rxtx_config(struct fsl_ssi *ssi, bool enable)
{
struct regmap *regs = ssi->regs;
- struct fsl_ssi_rxtx_reg_val *vals = &ssi->rxtx_reg_val;
+ struct fsl_ssi_regvals *vals = ssi->regvals;
if (enable) {
regmap_update_bits(regs, REG_SSI_SIER,
- vals->rx.sier | vals->tx.sier,
- vals->rx.sier | vals->tx.sier);
+ vals[RX].sier | vals[TX].sier,
+ vals[RX].sier | vals[TX].sier);
regmap_update_bits(regs, REG_SSI_SRCR,
- vals->rx.srcr | vals->tx.srcr,
- vals->rx.srcr | vals->tx.srcr);
+ vals[RX].srcr | vals[TX].srcr,
+ vals[RX].srcr | vals[TX].srcr);
regmap_update_bits(regs, REG_SSI_STCR,
- vals->rx.stcr | vals->tx.stcr,
- vals->rx.stcr | vals->tx.stcr);
+ vals[RX].stcr | vals[TX].stcr,
+ vals[RX].stcr | vals[TX].stcr);
} else {
regmap_update_bits(regs, REG_SSI_SRCR,
- vals->rx.srcr | vals->tx.srcr, 0);
+ vals[RX].srcr | vals[TX].srcr, 0);
regmap_update_bits(regs, REG_SSI_STCR,
- vals->rx.stcr | vals->tx.stcr, 0);
+ vals[RX].stcr | vals[TX].stcr, 0);
regmap_update_bits(regs, REG_SSI_SIER,
- vals->rx.sier | vals->tx.sier, 0);
+ vals[RX].sier | vals[TX].sier, 0);
}
}
@@ -446,10 +441,10 @@ static void fsl_ssi_fifo_clear(struct fsl_ssi *ssi, bool
is_rx)
* Enable or disable SSI configuration.
*/
static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
- struct fsl_ssi_reg_val *vals)
+ struct fsl_ssi_regvals *vals)
{
struct regmap *regs = ssi->regs;
- struct fsl_ssi_reg_val *avals;
+ struct fsl_ssi_regvals *avals;
int nr_active_streams;
u32 scr;
int keep_active;
@@ -464,10 +459,10 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool
enable,
keep_active = 0;
/* Get the opposite direction to keep its values untouched */
- if (&ssi->rxtx_reg_val.rx == vals)
- avals = &ssi->rxtx_reg_val.tx;
+ if (&ssi->regvals[RX] == vals)
+ avals = &ssi->regvals[TX];
else
- avals = &ssi->rxtx_reg_val.rx;
+ avals = &ssi->regvals[RX];
if (!enable) {
/*
@@ -558,7 +553,7 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
static void fsl_ssi_rx_config(struct fsl_ssi *ssi, bool enable)
{
- fsl_ssi_con
[PATCH v4 08/11] ASoC: fsl_ssi: Rename scr_val to scr
Simplify the variable name. This reduces one over-80-character line. Signed-off-by: Nicolin Chen Tested-by: Maciej S. Szmigiero Reviewed-by: Maciej S. Szmigiero --- sound/soc/fsl/fsl_ssi.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c index 237302f..af3ba71 100644 --- a/sound/soc/fsl/fsl_ssi.c +++ b/sound/soc/fsl/fsl_ssi.c @@ -451,12 +451,12 @@ static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable, struct regmap *regs = ssi->regs; struct fsl_ssi_reg_val *avals; int nr_active_streams; - u32 scr_val; + u32 scr; int keep_active; - regmap_read(regs, REG_SSI_SCR, &scr_val); + regmap_read(regs, REG_SSI_SCR, &scr); - nr_active_streams = !!(scr_val & SSI_SCR_TE) + !!(scr_val & SSI_SCR_RE); + nr_active_streams = !!(scr & SSI_SCR_TE) + !!(scr & SSI_SCR_RE); if (nr_active_streams - 1 > 0) keep_active = 1; @@ -810,11 +810,11 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream, unsigned int sample_size = params_width(hw_params); u32 wl = SSI_SxCCR_WL(sample_size); int ret; - u32 scr_val; + u32 scr; int enabled; - regmap_read(regs, REG_SSI_SCR, &scr_val); - enabled = scr_val & SSI_SCR_SSIEN; + regmap_read(regs, REG_SSI_SCR, &scr); + enabled = scr & SSI_SCR_SSIEN; /* * SSI is properly configured if it is enabled and running in -- 2.7.4
[PATCH v4 07/11] ASoC: fsl_ssi: Rename cpu_dai parameter to dai
Shortens the variable name to save space, useful for dev_err outputs.
Signed-off-by: Nicolin Chen
Tested-by: Maciej S. Szmigiero
Reviewed-by: Maciej S. Szmigiero
---
sound/soc/fsl/fsl_ssi.c | 32
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index eb9ac84..237302f 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -683,10 +683,10 @@ static void fsl_ssi_shutdown(struct snd_pcm_substream
*substream,
* (In 2-channel I2S Master mode, slot_width is fixed 32)
*/
static int fsl_ssi_set_bclk(struct snd_pcm_substream *substream,
- struct snd_soc_dai *cpu_dai,
+ struct snd_soc_dai *dai,
struct snd_pcm_hw_params *hw_params)
{
- struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(cpu_dai);
+ struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai);
struct regmap *regs = ssi->regs;
int synchronous = ssi->cpu_dai_drv.symmetric_rates, ret;
u32 pm = 999, div2, psr, stccr, mask, afreq, factor, i;
@@ -716,7 +716,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream
*substream,
* never greater than 1/5 IPG clock rate
*/
if (freq * 5 > clk_get_rate(ssi->clk)) {
- dev_err(cpu_dai->dev, "bitclk > ipgclk / 5\n");
+ dev_err(dai->dev, "bitclk > ipgclk / 5\n");
return -EINVAL;
}
@@ -765,7 +765,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream
*substream,
/* No proper pm found if it is still remaining the initial value */
if (pm == 999) {
- dev_err(cpu_dai->dev, "failed to handle the required sysclk\n");
+ dev_err(dai->dev, "failed to handle the required sysclk\n");
return -EINVAL;
}
@@ -781,7 +781,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream
*substream,
if (!baudclk_is_used) {
ret = clk_set_rate(ssi->baudclk, baudrate);
if (ret) {
- dev_err(cpu_dai->dev, "failed to set baudclk rate\n");
+ dev_err(dai->dev, "failed to set baudclk rate\n");
return -EINVAL;
}
}
@@ -802,9 +802,9 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream
*substream,
*/
static int fsl_ssi_hw_params(struct snd_pcm_substream *substream,
struct snd_pcm_hw_params *hw_params,
-struct snd_soc_dai *cpu_dai)
+struct snd_soc_dai *dai)
{
- struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(cpu_dai);
+ struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai);
struct regmap *regs = ssi->regs;
unsigned int channels = params_channels(hw_params);
unsigned int sample_size = params_width(hw_params);
@@ -826,7 +826,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream
*substream,
return 0;
if (fsl_ssi_is_i2s_master(ssi)) {
- ret = fsl_ssi_set_bclk(substream, cpu_dai, hw_params);
+ ret = fsl_ssi_set_bclk(substream, dai, hw_params);
if (ret)
return ret;
@@ -864,7 +864,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream
*substream,
}
static int fsl_ssi_hw_free(struct snd_pcm_substream *substream,
- struct snd_soc_dai *cpu_dai)
+ struct snd_soc_dai *dai)
{
struct snd_soc_pcm_runtime *rtd = substream->private_data;
struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(rtd->cpu_dai);
@@ -1033,30 +1033,30 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
/**
* Configure Digital Audio Interface (DAI) Format
*/
-static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *cpu_dai, unsigned int fmt)
+static int fsl_ssi_set_dai_fmt(struct snd_soc_dai *dai, unsigned int fmt)
{
- struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(cpu_dai);
+ struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai);
/* AC97 configured DAIFMT earlier in the probe() */
if (fsl_ssi_is_ac97(ssi))
return 0;
- return _fsl_ssi_set_dai_fmt(cpu_dai->dev, ssi, fmt);
+ return _fsl_ssi_set_dai_fmt(dai->dev, ssi, fmt);
}
/**
* Set TDM slot number and slot width
*/
-static int fsl_ssi_set_dai_tdm_slot(struct snd_soc_dai *cpu_dai, u32 tx_mask,
+static int fsl_ssi_set_dai_tdm_slot(struct snd_soc_dai *dai, u32 tx_mask,
u32 rx_mask, int slots, int slot_width)
{
- struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(cpu_dai);
+ struct fsl_ssi *ssi = snd_soc_dai_get_drvdata(dai);
struct regmap *regs = ssi->regs;
u32 val;
/* The word length should be 8, 10, 12, 16, 18, 20, 22 or 24 */
if (slot_width & 1 || slot_width < 8 || slot_width > 24) {
- dev_err(cp
[PATCH v4 06/11] ASoC: fsl_ssi: Refine printk outputs
This patches unifies the error message in the "failed to " format.
It also reduces the length of one line and adds spaces to an operator.
Signed-off-by: Nicolin Chen
Tested-by: Maciej S. Szmigiero
Reviewed-by: Maciej S. Szmigiero
---
sound/soc/fsl/fsl_ssi.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index ed9ac75..eb9ac84 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -716,7 +716,7 @@ static int fsl_ssi_set_bclk(struct snd_pcm_substream
*substream,
* never greater than 1/5 IPG clock rate
*/
if (freq * 5 > clk_get_rate(ssi->clk)) {
- dev_err(cpu_dai->dev, "bitclk > ipgclk/5\n");
+ dev_err(cpu_dai->dev, "bitclk > ipgclk / 5\n");
return -EINVAL;
}
@@ -888,7 +888,7 @@ static int _fsl_ssi_set_dai_fmt(struct device *dev,
ssi->dai_fmt = fmt;
if (fsl_ssi_is_i2s_master(ssi) && IS_ERR(ssi->baudclk)) {
- dev_err(dev, "baudclk is missing which is necessary for master
mode\n");
+ dev_err(dev, "missing baudclk for master mode\n");
return -EINVAL;
}
@@ -1307,7 +1307,7 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
ssi->clk = devm_clk_get(dev, NULL);
if (IS_ERR(ssi->clk)) {
ret = PTR_ERR(ssi->clk);
- dev_err(dev, "could not get clock: %d\n", ret);
+ dev_err(dev, "failed to get clock: %d\n", ret);
return ret;
}
@@ -1323,7 +1323,7 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
/* Do not error out for slave cases that live without a baud clock */
ssi->baudclk = devm_clk_get(dev, "baud");
if (IS_ERR(ssi->baudclk))
- dev_dbg(dev, "could not get baud clock: %ld\n",
+ dev_dbg(dev, "failed to get baud clock: %ld\n",
PTR_ERR(ssi->baudclk));
ssi->dma_params_tx.maxburst = ssi->dma_maxburst;
@@ -1447,7 +1447,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
®config);
}
if (IS_ERR(ssi->regs)) {
- dev_err(dev, "Failed to init register map\n");
+ dev_err(dev, "failed to init register map\n");
return PTR_ERR(ssi->regs);
}
@@ -1513,7 +1513,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
mutex_init(&ssi->ac97_reg_lock);
ret = snd_soc_set_ac97_ops_of_reset(&fsl_ssi_ac97_ops, pdev);
if (ret) {
- dev_err(dev, "could not set AC'97 ops\n");
+ dev_err(dev, "failed to set AC'97 ops\n");
goto error_ac97_ops;
}
}
@@ -1529,7 +1529,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
ret = devm_request_irq(dev, ssi->irq, fsl_ssi_isr, 0,
dev_name(dev), ssi);
if (ret < 0) {
- dev_err(dev, "could not claim irq %u\n", ssi->irq);
+ dev_err(dev, "failed to claim irq %u\n", ssi->irq);
goto error_asoc_register;
}
}
@@ -1571,7 +1571,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
ret = of_property_read_u32(np, "cell-index", &ssi_idx);
if (ret) {
- dev_err(dev, "cannot get SSI index property\n");
+ dev_err(dev, "failed to get SSI index property\n");
goto error_sound_card;
}
--
2.7.4
[PATCH v4 05/11] ASoC: fsl_ssi: Refine indentations and wrappings
This patch just simply unifies the coding style.
Signed-off-by: Nicolin Chen
Tested-by: Maciej S. Szmigiero
Reviewed-by: Maciej S. Szmigiero
---
sound/soc/fsl/fsl_ssi.c | 239 +---
sound/soc/fsl/fsl_ssi.h | 2 +-
sound/soc/fsl/fsl_ssi_dbg.c | 3 +-
3 files changed, 118 insertions(+), 126 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 24d9695..ed9ac75 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -69,21 +69,35 @@
* samples will be written to STX properly.
*/
#ifdef __BIG_ENDIAN
-#define FSLSSI_I2S_FORMATS (SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_BE | \
-SNDRV_PCM_FMTBIT_S18_3BE | SNDRV_PCM_FMTBIT_S20_3BE | \
-SNDRV_PCM_FMTBIT_S24_3BE | SNDRV_PCM_FMTBIT_S24_BE)
+#define FSLSSI_I2S_FORMATS \
+ (SNDRV_PCM_FMTBIT_S8 | \
+SNDRV_PCM_FMTBIT_S16_BE | \
+SNDRV_PCM_FMTBIT_S18_3BE | \
+SNDRV_PCM_FMTBIT_S20_3BE | \
+SNDRV_PCM_FMTBIT_S24_3BE | \
+SNDRV_PCM_FMTBIT_S24_BE)
#else
-#define FSLSSI_I2S_FORMATS (SNDRV_PCM_FMTBIT_S8 | SNDRV_PCM_FMTBIT_S16_LE | \
-SNDRV_PCM_FMTBIT_S18_3LE | SNDRV_PCM_FMTBIT_S20_3LE | \
-SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_LE)
+#define FSLSSI_I2S_FORMATS \
+ (SNDRV_PCM_FMTBIT_S8 | \
+SNDRV_PCM_FMTBIT_S16_LE | \
+SNDRV_PCM_FMTBIT_S18_3LE | \
+SNDRV_PCM_FMTBIT_S20_3LE | \
+SNDRV_PCM_FMTBIT_S24_3LE | \
+SNDRV_PCM_FMTBIT_S24_LE)
#endif
-#define FSLSSI_SIER_DBG_RX_FLAGS (SSI_SIER_RFF0_EN | \
- SSI_SIER_RLS_EN | SSI_SIER_RFS_EN | \
- SSI_SIER_ROE0_EN | SSI_SIER_RFRC_EN)
-#define FSLSSI_SIER_DBG_TX_FLAGS (SSI_SIER_TFE0_EN | \
- SSI_SIER_TLS_EN | SSI_SIER_TFS_EN | \
- SSI_SIER_TUE0_EN | SSI_SIER_TFRC_EN)
+#define FSLSSI_SIER_DBG_RX_FLAGS \
+ (SSI_SIER_RFF0_EN | \
+SSI_SIER_RLS_EN | \
+SSI_SIER_RFS_EN | \
+SSI_SIER_ROE0_EN | \
+SSI_SIER_RFRC_EN)
+#define FSLSSI_SIER_DBG_TX_FLAGS \
+ (SSI_SIER_TFE0_EN | \
+SSI_SIER_TLS_EN | \
+SSI_SIER_TFS_EN | \
+SSI_SIER_TUE0_EN | \
+SSI_SIER_TFRC_EN)
enum fsl_ssi_type {
FSL_SSI_MCP8610,
@@ -291,8 +305,8 @@ static struct fsl_ssi_soc_data fsl_ssi_mpc8610 = {
.imx = false,
.offline_config = true,
.sisr_write_mask = SSI_SISR_RFRC | SSI_SISR_TFRC |
- SSI_SISR_ROE0 | SSI_SISR_ROE1 |
- SSI_SISR_TUE0 | SSI_SISR_TUE1,
+ SSI_SISR_ROE0 | SSI_SISR_ROE1 |
+ SSI_SISR_TUE0 | SSI_SISR_TUE1,
};
static struct fsl_ssi_soc_data fsl_ssi_imx21 = {
@@ -306,15 +320,15 @@ static struct fsl_ssi_soc_data fsl_ssi_imx35 = {
.imx = true,
.offline_config = true,
.sisr_write_mask = SSI_SISR_RFRC | SSI_SISR_TFRC |
- SSI_SISR_ROE0 | SSI_SISR_ROE1 |
- SSI_SISR_TUE0 | SSI_SISR_TUE1,
+ SSI_SISR_ROE0 | SSI_SISR_ROE1 |
+ SSI_SISR_TUE0 | SSI_SISR_TUE1,
};
static struct fsl_ssi_soc_data fsl_ssi_imx51 = {
.imx = true,
.offline_config = false,
.sisr_write_mask = SSI_SISR_ROE0 | SSI_SISR_ROE1 |
- SSI_SISR_TUE0 | SSI_SISR_TUE1,
+ SSI_SISR_TUE0 | SSI_SISR_TUE1,
};
static const struct of_device_id fsl_ssi_ids[] = {
@@ -376,21 +390,21 @@ static void fsl_ssi_rxtx_config(struct fsl_ssi *ssi, bool
enable)
if (enable) {
regmap_update_bits(regs, REG_SSI_SIER,
- vals->rx.sier | vals->tx.sier,
- vals->rx.sier | vals->tx.sier);
+ vals->rx.sier | vals->tx.sier,
+ vals->rx.sier | vals->tx.sier);
regmap_update_bits(regs, REG_SSI_SRCR,
- vals->rx.srcr | vals->tx.srcr,
- vals->rx.srcr | vals->tx.srcr);
+ vals->rx.srcr | vals->tx.srcr,
+ vals->rx.srcr | vals->tx.srcr);
regmap_update_bits(regs, REG_SSI_STCR,
- vals->rx.stcr | vals->tx.stcr,
- vals->rx.stcr | vals->tx.stcr);
+ vals->rx.stcr | vals->tx.stcr,
+ vals->rx.stcr | vals->tx.stcr);
} else {
regmap_update_bits(regs, REG_SSI_SRCR,
- vals->rx.srcr | vals->tx.srcr, 0);
+ vals->rx.srcr | vals->tx.srcr, 0);
regmap_update_bits(regs, REG_SSI_STCR,
- vals->rx.stcr | vals->tx.stcr, 0);
+ vals->rx.stcr | vals->tx.stcr, 0);
regmap_update_bits(regs, REG_SSI_SIER,
-
[PATCH v4 04/11] ASoC: fsl_ssi: Rename registers and fields macros
This patch renames CCSR_SSI_xxx to REG_SSI_xxx and SSI_xxx_yyy style.
It also slightly reduces the length of them to save some space.
Signed-off-by: Nicolin Chen
Tested-by: Maciej S. Szmigiero
Reviewed-by: Maciej S. Szmigiero
---
sound/soc/fsl/fsl_ssi.c | 374 +--
sound/soc/fsl/fsl_ssi.h | 376 ++--
sound/soc/fsl/fsl_ssi_dbg.c | 44 +++---
3 files changed, 397 insertions(+), 397 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index ff1827a..24d9695 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -78,12 +78,12 @@
SNDRV_PCM_FMTBIT_S24_3LE | SNDRV_PCM_FMTBIT_S24_LE)
#endif
-#define FSLSSI_SIER_DBG_RX_FLAGS (CCSR_SSI_SIER_RFF0_EN | \
- CCSR_SSI_SIER_RLS_EN | CCSR_SSI_SIER_RFS_EN | \
- CCSR_SSI_SIER_ROE0_EN | CCSR_SSI_SIER_RFRC_EN)
-#define FSLSSI_SIER_DBG_TX_FLAGS (CCSR_SSI_SIER_TFE0_EN | \
- CCSR_SSI_SIER_TLS_EN | CCSR_SSI_SIER_TFS_EN | \
- CCSR_SSI_SIER_TUE0_EN | CCSR_SSI_SIER_TFRC_EN)
+#define FSLSSI_SIER_DBG_RX_FLAGS (SSI_SIER_RFF0_EN | \
+ SSI_SIER_RLS_EN | SSI_SIER_RFS_EN | \
+ SSI_SIER_ROE0_EN | SSI_SIER_RFRC_EN)
+#define FSLSSI_SIER_DBG_TX_FLAGS (SSI_SIER_TFE0_EN | \
+ SSI_SIER_TLS_EN | SSI_SIER_TFS_EN | \
+ SSI_SIER_TUE0_EN | SSI_SIER_TFRC_EN)
enum fsl_ssi_type {
FSL_SSI_MCP8610,
@@ -107,8 +107,8 @@ struct fsl_ssi_rxtx_reg_val {
static bool fsl_ssi_readable_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
- case CCSR_SSI_SACCEN:
- case CCSR_SSI_SACCDIS:
+ case REG_SSI_SACCEN:
+ case REG_SSI_SACCDIS:
return false;
default:
return true;
@@ -118,18 +118,18 @@ static bool fsl_ssi_readable_reg(struct device *dev,
unsigned int reg)
static bool fsl_ssi_volatile_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
- case CCSR_SSI_STX0:
- case CCSR_SSI_STX1:
- case CCSR_SSI_SRX0:
- case CCSR_SSI_SRX1:
- case CCSR_SSI_SISR:
- case CCSR_SSI_SFCSR:
- case CCSR_SSI_SACNT:
- case CCSR_SSI_SACADD:
- case CCSR_SSI_SACDAT:
- case CCSR_SSI_SATAG:
- case CCSR_SSI_SACCST:
- case CCSR_SSI_SOR:
+ case REG_SSI_STX0:
+ case REG_SSI_STX1:
+ case REG_SSI_SRX0:
+ case REG_SSI_SRX1:
+ case REG_SSI_SISR:
+ case REG_SSI_SFCSR:
+ case REG_SSI_SACNT:
+ case REG_SSI_SACADD:
+ case REG_SSI_SACDAT:
+ case REG_SSI_SATAG:
+ case REG_SSI_SACCST:
+ case REG_SSI_SOR:
return true;
default:
return false;
@@ -139,12 +139,12 @@ static bool fsl_ssi_volatile_reg(struct device *dev,
unsigned int reg)
static bool fsl_ssi_precious_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
- case CCSR_SSI_SRX0:
- case CCSR_SSI_SRX1:
- case CCSR_SSI_SISR:
- case CCSR_SSI_SACADD:
- case CCSR_SSI_SACDAT:
- case CCSR_SSI_SATAG:
+ case REG_SSI_SRX0:
+ case REG_SSI_SRX1:
+ case REG_SSI_SISR:
+ case REG_SSI_SACADD:
+ case REG_SSI_SACDAT:
+ case REG_SSI_SATAG:
return true;
default:
return false;
@@ -154,9 +154,9 @@ static bool fsl_ssi_precious_reg(struct device *dev,
unsigned int reg)
static bool fsl_ssi_writeable_reg(struct device *dev, unsigned int reg)
{
switch (reg) {
- case CCSR_SSI_SRX0:
- case CCSR_SSI_SRX1:
- case CCSR_SSI_SACCST:
+ case REG_SSI_SRX0:
+ case REG_SSI_SRX1:
+ case REG_SSI_SACCST:
return false;
default:
return true;
@@ -164,12 +164,12 @@ static bool fsl_ssi_writeable_reg(struct device *dev,
unsigned int reg)
}
static const struct regmap_config fsl_ssi_regconfig = {
- .max_register = CCSR_SSI_SACCDIS,
+ .max_register = REG_SSI_SACCDIS,
.reg_bits = 32,
.val_bits = 32,
.reg_stride = 4,
.val_format_endian = REGMAP_ENDIAN_NATIVE,
- .num_reg_defaults_raw = CCSR_SSI_SACCDIS / sizeof(uint32_t) + 1,
+ .num_reg_defaults_raw = REG_SSI_SACCDIS / sizeof(uint32_t) + 1,
.readable_reg = fsl_ssi_readable_reg,
.volatile_reg = fsl_ssi_volatile_reg,
.precious_reg = fsl_ssi_precious_reg,
@@ -290,9 +290,9 @@ struct fsl_ssi {
static struct fsl_ssi_soc_data fsl_ssi_mpc8610 = {
.imx = false,
.offline_config = true,
- .sisr_write_mask = CCSR_SSI_SISR_RFRC | CCSR_SSI_SISR_TFRC |
- CCSR_SSI_SISR_ROE0 | CCSR_SSI_SISR_ROE1 |
- CCSR_SSI_SISR_TUE0 | CCSR_SSI_SISR_TUE1,
+ .sisr_write_mask = SSI_SISR_RFRC | SSI_SISR_TFRC |
+ SSI_SISR_ROE0 | SSI_SISR_ROE1 |
+ SSI_SISR_TUE0 | SSI_SISR_TUE1,
};
static struct fs
[PATCH v4 03/11] ASoC: fsl_ssi: Refine all comments
This patch refines the comments by:
1) Removing all out-of-date comments
2) Removing all not-so-useful comments
3) Unifying the styles of all comments
4) Shortening comments to be more conise
5) Adding comments to improve code readablity
6) Moving all register related comments to fsl_ssi.h
7) Adding comments to all register and field defines
Signed-off-by: Nicolin Chen
Tested-by: Maciej S. Szmigiero
Reviewed-by: Maciej S. Szmigiero
---
Changelog
v3->v4
* Fixed two typos in the comments
* Added comments for interrupt handler
* Added comments for fsl_ssi_trigger()
* Refined comments in fsl_ssi_config()
* Refined comments of watermarks in fsl_ssi_probe()
* Refined comments of imx-fiq-pcm in fsl_ssi_imx_probe()
v2->v3
* Revised a comment in hw_params() by taking Maciej's advice
v1->v2
* Added some new comments at "SoC specific data" to be more precise
* Revised one comment in fsl_ssi_config()
* Revised the comment of fsl_ssi_setup_reg_vals()
* Added one comment for AC97 in fsl_ssi_setup_reg_vals()
* Revised the comment of fsl_ssi_hw_params() to be more precise
* Added some comments in _fsl_ssi_set_dai_fmt() to help understand
the formats
* Added one comment in fsl_ssi_set_dai_fmt() to explain why AC97
needs to bypass it
* Revised comments in fsl_ssi_set_dai_tdm_slot() to be more precise
* Revised comments around dual FIFO code in fsl_ssi_imx_probe() to
be more precise
sound/soc/fsl/fsl_ssi.c | 383
sound/soc/fsl/fsl_ssi.h | 67 +++-
sound/soc/fsl/fsl_ssi_dbg.c | 12 +-
3 files changed, 208 insertions(+), 254 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index e903c92..ff1827a 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -187,42 +187,48 @@ struct fsl_ssi_soc_data {
/**
* fsl_ssi: per-SSI private data
*
- * @reg: Pointer to the regmap registers
+ * @regs: Pointer to the regmap registers
* @irq: IRQ of this SSI
* @cpu_dai_drv: CPU DAI driver for this device
*
* @dai_fmt: DAI configuration this device is currently used with
- * @i2s_mode: i2s and network mode configuration of the device. Is used to
- * switch between normal and i2s/network mode
- * mode depending on the number of channels
+ * @i2s_mode: I2S and Network mode configuration of SCR register
* @use_dma: DMA is used or FIQ with stream filter
- * @use_dual_fifo: DMA with support for both FIFOs used
- * @fifo_deph: Depth of the SSI FIFOs
- * @slot_width: width of each DAI slot
- * @slots: number of slots
- * @rxtx_reg_val: Specific register settings for receive/transmit configuration
+ * @use_dual_fifo: DMA with support for dual FIFO mode
+ * @has_ipg_clk_name: If "ipg" is in the clock name list of device tree
+ * @fifo_depth: Depth of the SSI FIFOs
+ * @slot_width: Width of each DAI slot
+ * @slots: Number of slots
+ * @rxtx_reg_val: Specific RX/TX register settings
*
- * @clk: SSI clock
- * @baudclk: SSI baud clock for master mode
+ * @clk: Clock source to access register
+ * @baudclk: Clock source to generate bit and frame-sync clocks
* @baudclk_streams: Active streams that are using baudclk
*
+ * @regcache_sfcsr: Cache sfcsr register value during suspend and resume
+ * @regcache_sacnt: Cache sacnt register value during suspend and resume
+ *
* @dma_params_tx: DMA transmit parameters
* @dma_params_rx: DMA receive parameters
* @ssi_phys: physical address of the SSI registers
*
* @fiq_params: FIQ stream filtering parameters
*
- * @pdev: Pointer to pdev used for deprecated fsl-ssi sound card
+ * @pdev: Pointer to pdev when using fsl-ssi as sound card (ppc only)
+ *TODO: Should be replaced with simple-sound-card
*
* @dbg_stats: Debugging statistics
*
* @soc: SoC specific data
+ * @dev: Pointer to &pdev->dev
+ *
+ * @fifo_watermark: The FIFO watermark setting. Notifies DMA when there are
+ * @fifo_watermark or fewer words in TX fifo or
+ * @fifo_watermark or more empty words in RX fifo.
+ * @dma_maxburst: Max number of words to transfer in one go. So far,
+ *this is always the same as fifo_watermark.
*
- * @fifo_watermark: the FIFO watermark setting. Notifies DMA when
- * there are @fifo_watermark or fewer words in TX fifo or
- * @fifo_watermark or more empty words in RX fifo.
- * @dma_maxburst: max number of words to transfer in one go. So far,
- * this is always the same as fifo_watermark.
+ * @ac97_reg_lock: Mutex lock to serialize AC97 register access operations
*/
struct fsl_ssi {
struct regmap *regs;
@@ -243,20 +249,15 @@ struct fsl_ssi {
struct clk *baudclk;
unsigned int baudclk_streams;
- /* regcache for volatile regs */
u32 regcache_sfcsr;
u32 regcache_sacnt;
- /* DMA params */
struct snd_dmaengine_dai_dma_data dma_params_tx;
struct snd_dmaengine_dai_dma_data dma_params_rx;
dma_addr_t s
[PATCH v4 02/11] ASoC: fsl_ssi: Cache pdev->dev pointer
There should be no trouble to understand dev = pdev->dev.
This can save some space to have more print info or save
some wrapped lines.
Signed-off-by: Nicolin Chen
Tested-by: Maciej S. Szmigiero
Reviewed-by: Maciej S. Szmigiero
---
sound/soc/fsl/fsl_ssi.c | 64 -
1 file changed, 31 insertions(+), 33 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index 84d2f7e..e903c92 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -1379,23 +1379,24 @@ static int fsl_ssi_imx_probe(struct platform_device
*pdev,
struct fsl_ssi *ssi, void __iomem *iomem)
{
struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
u32 dmas[4];
int ret;
if (ssi->has_ipg_clk_name)
- ssi->clk = devm_clk_get(&pdev->dev, "ipg");
+ ssi->clk = devm_clk_get(dev, "ipg");
else
- ssi->clk = devm_clk_get(&pdev->dev, NULL);
+ ssi->clk = devm_clk_get(dev, NULL);
if (IS_ERR(ssi->clk)) {
ret = PTR_ERR(ssi->clk);
- dev_err(&pdev->dev, "could not get clock: %d\n", ret);
+ dev_err(dev, "could not get clock: %d\n", ret);
return ret;
}
if (!ssi->has_ipg_clk_name) {
ret = clk_prepare_enable(ssi->clk);
if (ret) {
- dev_err(&pdev->dev, "clk_prepare_enable failed: %d\n",
ret);
+ dev_err(dev, "clk_prepare_enable failed: %d\n", ret);
return ret;
}
}
@@ -1403,9 +1404,9 @@ static int fsl_ssi_imx_probe(struct platform_device *pdev,
/* For those SLAVE implementations, we ignore non-baudclk cases
* and, instead, abandon MASTER mode that needs baud clock.
*/
- ssi->baudclk = devm_clk_get(&pdev->dev, "baud");
+ ssi->baudclk = devm_clk_get(dev, "baud");
if (IS_ERR(ssi->baudclk))
- dev_dbg(&pdev->dev, "could not get baud clock: %ld\n",
+ dev_dbg(dev, "could not get baud clock: %ld\n",
PTR_ERR(ssi->baudclk));
ssi->dma_params_tx.maxburst = ssi->dma_maxburst;
@@ -1469,6 +1470,7 @@ static int fsl_ssi_probe(struct platform_device *pdev)
struct fsl_ssi *ssi;
int ret = 0;
struct device_node *np = pdev->dev.of_node;
+ struct device *dev = &pdev->dev;
const struct of_device_id *of_id;
const char *p, *sprop;
const uint32_t *iprop;
@@ -1477,17 +1479,16 @@ static int fsl_ssi_probe(struct platform_device *pdev)
char name[64];
struct regmap_config regconfig = fsl_ssi_regconfig;
- of_id = of_match_device(fsl_ssi_ids, &pdev->dev);
+ of_id = of_match_device(fsl_ssi_ids, dev);
if (!of_id || !of_id->data)
return -EINVAL;
- ssi = devm_kzalloc(&pdev->dev, sizeof(*ssi),
- GFP_KERNEL);
+ ssi = devm_kzalloc(dev, sizeof(*ssi), GFP_KERNEL);
if (!ssi)
return -ENOMEM;
ssi->soc = of_id->data;
- ssi->dev = &pdev->dev;
+ ssi->dev = dev;
sprop = of_get_property(np, "fsl,mode", NULL);
if (sprop) {
@@ -1507,10 +1508,10 @@ static int fsl_ssi_probe(struct platform_device *pdev)
memcpy(&ssi->cpu_dai_drv, &fsl_ssi_dai_template,
sizeof(fsl_ssi_dai_template));
}
- ssi->cpu_dai_drv.name = dev_name(&pdev->dev);
+ ssi->cpu_dai_drv.name = dev_name(dev);
res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- iomem = devm_ioremap_resource(&pdev->dev, res);
+ iomem = devm_ioremap_resource(dev, res);
if (IS_ERR(iomem))
return PTR_ERR(iomem);
ssi->ssi_phys = res->start;
@@ -1528,21 +1529,20 @@ static int fsl_ssi_probe(struct platform_device *pdev)
ret = of_property_match_string(np, "clock-names", "ipg");
if (ret < 0) {
ssi->has_ipg_clk_name = false;
- ssi->regs = devm_regmap_init_mmio(&pdev->dev, iomem,
- ®config);
+ ssi->regs = devm_regmap_init_mmio(dev, iomem, ®config);
} else {
ssi->has_ipg_clk_name = true;
- ssi->regs = devm_regmap_init_mmio_clk(&pdev->dev,
- "ipg", iomem, ®config);
+ ssi->regs = devm_regmap_init_mmio_clk(dev, "ipg", iomem,
+ ®config);
}
if (IS_ERR(ssi->regs)) {
- dev_err(&pdev->dev, "Failed to init register map\n");
+ dev_err(dev, "Failed to init register map\n");
return PTR_ERR(ssi->regs);
}
ssi->irq = platform_get_irq(pdev, 0);
if (ssi->irq < 0) {
- dev_err(&pdev->dev, "no irq for node %s\n", pdev->name);
+ dev_err(
[PATCH v4 01/11] ASoC: fsl_ssi: Rename fsl_ssi_private to fsl_ssi
Shorten the private data structure to save some wrapped lines.
Signed-off-by: Nicolin Chen
Tested-by: Maciej S. Szmigiero
Reviewed-by: Maciej S. Szmigiero
---
sound/soc/fsl/fsl_ssi.c | 456 +++-
1 file changed, 220 insertions(+), 236 deletions(-)
diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index c350117..84d2f7e 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -185,7 +185,7 @@ struct fsl_ssi_soc_data {
};
/**
- * fsl_ssi_private: per-SSI private data
+ * fsl_ssi: per-SSI private data
*
* @reg: Pointer to the regmap registers
* @irq: IRQ of this SSI
@@ -224,7 +224,7 @@ struct fsl_ssi_soc_data {
* @dma_maxburst: max number of words to transfer in one go. So far,
* this is always the same as fifo_watermark.
*/
-struct fsl_ssi_private {
+struct fsl_ssi {
struct regmap *regs;
int irq;
struct snd_soc_dai_driver cpu_dai_drv;
@@ -325,21 +325,21 @@ static const struct of_device_id fsl_ssi_ids[] = {
};
MODULE_DEVICE_TABLE(of, fsl_ssi_ids);
-static bool fsl_ssi_is_ac97(struct fsl_ssi_private *ssi_private)
+static bool fsl_ssi_is_ac97(struct fsl_ssi *ssi)
{
- return (ssi_private->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK) ==
+ return (ssi->dai_fmt & SND_SOC_DAIFMT_FORMAT_MASK) ==
SND_SOC_DAIFMT_AC97;
}
-static bool fsl_ssi_is_i2s_master(struct fsl_ssi_private *ssi_private)
+static bool fsl_ssi_is_i2s_master(struct fsl_ssi *ssi)
{
- return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) ==
+ return (ssi->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) ==
SND_SOC_DAIFMT_CBS_CFS;
}
-static bool fsl_ssi_is_i2s_cbm_cfs(struct fsl_ssi_private *ssi_private)
+static bool fsl_ssi_is_i2s_cbm_cfs(struct fsl_ssi *ssi)
{
- return (ssi_private->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) ==
+ return (ssi->dai_fmt & SND_SOC_DAIFMT_MASTER_MASK) ==
SND_SOC_DAIFMT_CBM_CFS;
}
/**
@@ -352,12 +352,12 @@ static bool fsl_ssi_is_i2s_cbm_cfs(struct fsl_ssi_private
*ssi_private)
* This interrupt handler is used only to gather statistics.
*
* @irq: IRQ of the SSI device
- * @dev_id: pointer to the ssi_private structure for this SSI device
+ * @dev_id: pointer to the fsl_ssi structure for this SSI device
*/
static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
{
- struct fsl_ssi_private *ssi_private = dev_id;
- struct regmap *regs = ssi_private->regs;
+ struct fsl_ssi *ssi = dev_id;
+ struct regmap *regs = ssi->regs;
__be32 sisr;
__be32 sisr2;
@@ -367,12 +367,12 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
*/
regmap_read(regs, CCSR_SSI_SISR, &sisr);
- sisr2 = sisr & ssi_private->soc->sisr_write_mask;
+ sisr2 = sisr & ssi->soc->sisr_write_mask;
/* Clear the bits that we set */
if (sisr2)
regmap_write(regs, CCSR_SSI_SISR, sisr2);
- fsl_ssi_dbg_isr(&ssi_private->dbg_stats, sisr);
+ fsl_ssi_dbg_isr(&ssi->dbg_stats, sisr);
return IRQ_HANDLED;
}
@@ -380,11 +380,10 @@ static irqreturn_t fsl_ssi_isr(int irq, void *dev_id)
/*
* Enable/Disable all rx/tx config flags at once.
*/
-static void fsl_ssi_rxtx_config(struct fsl_ssi_private *ssi_private,
- bool enable)
+static void fsl_ssi_rxtx_config(struct fsl_ssi *ssi, bool enable)
{
- struct regmap *regs = ssi_private->regs;
- struct fsl_ssi_rxtx_reg_val *vals = &ssi_private->rxtx_reg_val;
+ struct regmap *regs = ssi->regs;
+ struct fsl_ssi_rxtx_reg_val *vals = &ssi->rxtx_reg_val;
if (enable) {
regmap_update_bits(regs, CCSR_SSI_SIER,
@@ -414,14 +413,13 @@ static void fsl_ssi_rxtx_config(struct fsl_ssi_private
*ssi_private,
* Note: The SOR is not documented in recent IMX datasheet, but
* is described in IMX51 reference manual at section 56.3.3.15.
*/
-static void fsl_ssi_fifo_clear(struct fsl_ssi_private *ssi_private,
- bool is_rx)
+static void fsl_ssi_fifo_clear(struct fsl_ssi *ssi, bool is_rx)
{
if (is_rx) {
- regmap_update_bits(ssi_private->regs, CCSR_SSI_SOR,
+ regmap_update_bits(ssi->regs, CCSR_SSI_SOR,
CCSR_SSI_SOR_RX_CLR, CCSR_SSI_SOR_RX_CLR);
} else {
- regmap_update_bits(ssi_private->regs, CCSR_SSI_SOR,
+ regmap_update_bits(ssi->regs, CCSR_SSI_SOR,
CCSR_SSI_SOR_TX_CLR, CCSR_SSI_SOR_TX_CLR);
}
}
@@ -448,12 +446,12 @@ static void fsl_ssi_fifo_clear(struct fsl_ssi_private
*ssi_private,
/*
* Enable/Disable a ssi configuration. You have to pass either
- * ssi_private->rxtx_reg_val.rx or tx as vals parameter.
+ * ssi->rxtx_reg_val.rx or tx as vals parameter.
*/
-static void fsl_ssi_config(struct fsl_ssi_private *ssi_private, bool enable,
+static void fsl_ssi_config(struct fsl_ssi *ssi, bool enable,
[PATCH v4 00/11] ASoC: fsl_ssi: Clean up - coding style level
==Changelog== v3->v4 * Revised PATCH-03 "Refine all comments" by adding Timur's inputs * Rebased all other patches v2->v3 * Added Tested-by and Reviewed-by from Maciej * Revised PATCH-03 "Refine all comments" by adding Maciej's advice * Revised PATCH-05 "Refine indentations and wrappings" * Rebased all other patches v1->v2 * Dropped one patch to remove "struct device" * Revised PATCH-03 "Refine all comments" * Revised PATCH-05 "Refine indentations and wrappings" * Rebased all other patches * Added PATCH-10 "Rename i2smode to i2s_net" * Added PATCH-11 "Define ternary macros to simplify code" # Detialed changes are described in each updated patch. Nicolin Chen (11): ASoC: fsl_ssi: Rename fsl_ssi_private to fsl_ssi ASoC: fsl_ssi: Cache pdev->dev pointer ASoC: fsl_ssi: Refine all comments ASoC: fsl_ssi: Rename registers and fields macros ASoC: fsl_ssi: Refine indentations and wrappings ASoC: fsl_ssi: Refine printk outputs ASoC: fsl_ssi: Rename cpu_dai parameter to dai ASoC: fsl_ssi: Rename scr_val to scr ASoC: fsl_ssi: Replace fsl_ssi_rxtx_reg_val with fsl_ssi_regvals ASoC: fsl_ssi: Rename i2smode to i2s_net ASoC: fsl_ssi: Define ternary macros to simplify code sound/soc/fsl/fsl_ssi.c | 1380 +++ sound/soc/fsl/fsl_ssi.h | 427 +++-- sound/soc/fsl/fsl_ssi_dbg.c | 59 +- 3 files changed, 896 insertions(+), 970 deletions(-) -- 2.7.4
[trivial PATCH] treewide: Align function definition open/close braces
Some functions definitions have either the initial open brace and/or
the closing brace outside of column 1.
Move those braces to column 1.
This allows various function analyzers like gnu complexity to work
properly for these modified functions.
Miscellanea:
o Remove extra trailing ; and blank line from xfs_agf_verify
Signed-off-by: Joe Perches
---
git diff -w shows no difference other than the above 'Miscellanea'
(this is against -next, but it applies against Linus' tree
with a couple offsets)
arch/x86/include/asm/atomic64_32.h | 2 +-
drivers/acpi/custom_method.c | 2 +-
drivers/acpi/fan.c | 2 +-
drivers/gpu/drm/amd/display/dc/core/dc.c | 2 +-
drivers/media/i2c/msp3400-kthreads.c | 2 +-
drivers/message/fusion/mptsas.c | 2 +-
drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c | 2 +-
drivers/net/wireless/ath/ath9k/xmit.c| 2 +-
drivers/platform/x86/eeepc-laptop.c | 2 +-
drivers/rtc/rtc-ab-b5ze-s3.c | 2 +-
drivers/scsi/dpt_i2o.c | 2 +-
drivers/scsi/sym53c8xx_2/sym_glue.c | 2 +-
fs/locks.c | 2 +-
fs/ocfs2/stack_user.c| 2 +-
fs/xfs/libxfs/xfs_alloc.c| 5 ++---
fs/xfs/xfs_export.c | 2 +-
kernel/audit.c | 6 +++---
kernel/trace/trace_printk.c | 4 ++--
lib/raid6/sse2.c | 14 +++---
sound/soc/fsl/fsl_dma.c | 2 +-
20 files changed, 30 insertions(+), 31 deletions(-)
diff --git a/arch/x86/include/asm/atomic64_32.h
b/arch/x86/include/asm/atomic64_32.h
index 97c46b8169b7..d4d4883080fa 100644
--- a/arch/x86/include/asm/atomic64_32.h
+++ b/arch/x86/include/asm/atomic64_32.h
@@ -122,7 +122,7 @@ static inline long long atomic64_read(const atomic64_t *v)
long long r;
alternative_atomic64(read, "=&A" (r), "c" (v) : "memory");
return r;
- }
+}
/**
* atomic64_add_return - add and return
diff --git a/drivers/acpi/custom_method.c b/drivers/acpi/custom_method.c
index c68e72414a67..e967c1173ba3 100644
--- a/drivers/acpi/custom_method.c
+++ b/drivers/acpi/custom_method.c
@@ -94,7 +94,7 @@ static void __exit acpi_custom_method_exit(void)
{
if (cm_dentry)
debugfs_remove(cm_dentry);
- }
+}
module_init(acpi_custom_method_init);
module_exit(acpi_custom_method_exit);
diff --git a/drivers/acpi/fan.c b/drivers/acpi/fan.c
index 6cf4988206f2..3563103590c6 100644
--- a/drivers/acpi/fan.c
+++ b/drivers/acpi/fan.c
@@ -219,7 +219,7 @@ fan_set_cur_state(struct thermal_cooling_device *cdev,
unsigned long state)
return fan_set_state_acpi4(device, state);
else
return fan_set_state(device, state);
- }
+}
static const struct thermal_cooling_device_ops fan_cooling_ops = {
.get_max_state = fan_get_max_state,
diff --git a/drivers/gpu/drm/amd/display/dc/core/dc.c
b/drivers/gpu/drm/amd/display/dc/core/dc.c
index d1488d5ee028..1e0d1e7c5324 100644
--- a/drivers/gpu/drm/amd/display/dc/core/dc.c
+++ b/drivers/gpu/drm/amd/display/dc/core/dc.c
@@ -461,7 +461,7 @@ static void disable_dangling_plane(struct dc *dc, struct
dc_state *context)
**/
struct dc *dc_create(const struct dc_init_data *init_params)
- {
+{
struct dc *dc = kzalloc(sizeof(*dc), GFP_KERNEL);
unsigned int full_pipe_count;
diff --git a/drivers/media/i2c/msp3400-kthreads.c
b/drivers/media/i2c/msp3400-kthreads.c
index 4dd01e9f553b..dc6cb8d475b3 100644
--- a/drivers/media/i2c/msp3400-kthreads.c
+++ b/drivers/media/i2c/msp3400-kthreads.c
@@ -885,7 +885,7 @@ static int msp34xxg_modus(struct i2c_client *client)
}
static void msp34xxg_set_source(struct i2c_client *client, u16 reg, int in)
- {
+{
struct msp_state *state = to_state(i2c_get_clientdata(client));
int source, matrix;
diff --git a/drivers/message/fusion/mptsas.c b/drivers/message/fusion/mptsas.c
index 345f6035599e..69a62d23514b 100644
--- a/drivers/message/fusion/mptsas.c
+++ b/drivers/message/fusion/mptsas.c
@@ -2968,7 +2968,7 @@ mptsas_exp_repmanufacture_info(MPT_ADAPTER *ioc,
mutex_unlock(&ioc->sas_mgmt.mutex);
out:
return ret;
- }
+}
static void
mptsas_parse_device_info(struct sas_identify *identify,
diff --git a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
index 3dd973475125..0ea141ece19e 100644
--- a/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
+++ b/drivers/net/ethernet/qlogic/netxen/netxen_nic_init.c
@@ -603,7 +603,7 @@ static struct uni_table_desc *nx_get_table_desc(const u8
*u
Re: [PATCH 17/17] memremap: merge find_dev_pagemap into get_dev_pagemap
On Fri, Dec 15, 2017 at 6:09 AM, Christoph Hellwig wrote: > There is only one caller of the trivial function find_dev_pagemap left, > so just merge it into the caller. > > Signed-off-by: Christoph Hellwig Looks good, Reviewed-by: Dan Williams ...and all of these pass the nvdimm unit tests, so I think we're good to go. I'll rebase the filesystem-DAX vs DMA collision series on top of this.
Re: [PATCH 16/17] memremap: change devm_memremap_pages interface to use struct dev_pagemap
On Fri, Dec 15, 2017 at 6:09 AM, Christoph Hellwig wrote: > From: Logan Gunthorpe > > This new interface is similar to how struct device (and many others) > work. The caller initializes a 'struct dev_pagemap' as required > and calls 'devm_memremap_pages'. This allows the pagemap structure to > be embedded in another structure and thus container_of can be used. In > this way application specific members can be stored in a containing > struct. > > This will be used by the P2P infrastructure and HMM could probably > be cleaned up to use it as well (instead of having it's own, similar > 'hmm_devmem_pages_create' function). > > Signed-off-by: Logan Gunthorpe > Signed-off-by: Christoph Hellwig Looks good, I notice that this does not initialize pgmap->type to MEMORY_DEVICE_HOST, but since that value is zero and likely won't change we're ok. Reviewed-by: Dan Williams
Re: [PATCH 15/17] memremap: drop private struct page_map
On Fri, Dec 15, 2017 at 6:09 AM, Christoph Hellwig wrote: > From: Logan Gunthorpe > > 'struct page_map' is a private structure of 'struct dev_pagemap' but the > latter replicates all the same fields as the former so there isn't much > value in it. Thus drop it in favour of a completely public struct. > > This is a clean up in preperation for a more generally useful > 'devm_memeremap_pages' interface. > > Signed-off-by: Logan Gunthorpe > Signed-off-by: Christoph Hellwig Looks good, Reviewed-by: Dan Williams
Re: [PATCH 14/17] memremap: simplify duplicate region handling in devm_memremap_pages
On Fri, Dec 15, 2017 at 6:09 AM, Christoph Hellwig wrote:
> __radix_tree_insert already checks for duplicates and returns -EEXIST in
> that case, so remove the duplicate (and racy) duplicates check.
>
> Signed-off-by: Christoph Hellwig
> Reviewed-by: Logan Gunthorpe
> ---
> kernel/memremap.c | 11 ---
> 1 file changed, 11 deletions(-)
>
> diff --git a/kernel/memremap.c b/kernel/memremap.c
> index 891491ddccdb..901404094df1 100644
> --- a/kernel/memremap.c
> +++ b/kernel/memremap.c
> @@ -395,17 +395,6 @@ void *devm_memremap_pages(struct device *dev, struct
> resource *res,
> align_end = align_start + align_size - 1;
>
> foreach_order_pgoff(res, order, pgoff) {
> - struct dev_pagemap *dup;
> -
> - rcu_read_lock();
> - dup = find_dev_pagemap(res->start + PFN_PHYS(pgoff));
> - rcu_read_unlock();
> - if (dup) {
> - dev_err(dev, "%s: %pr collides with mapping for %s\n",
> - __func__, res, dev_name(dup->dev));
> - error = -EBUSY;
> - break;
> - }
> error = __radix_tree_insert(&pgmap_radix,
> PHYS_PFN(res->start) + pgoff, order,
> page_map);
> if (error) {
This is not racy, we'll catch the error on insert, and I think the
extra debug information is useful for debugging a broken memory map or
alignment math.
Re: [PATCH 13/17] memremap: remove to_vmem_altmap
On Fri, Dec 15, 2017 at 6:09 AM, Christoph Hellwig wrote: > All callers are gone now. > > Signed-off-by: Christoph Hellwig > --- Nice, Reviewed-by: Dan Williams
Re: [PATCH 12/17] mm: optimize dev_pagemap reference counting around get_dev_pagemap
On Fri, Dec 15, 2017 at 6:09 AM, Christoph Hellwig wrote: > Change the calling convention so that get_dev_pagemap always consumes the > previous reference instead of doing this using an explicit earlier call to > put_dev_pagemap in the callers. > > The callers will still need to put the final reference after finishing the > loop over the pages. > > Signed-off-by: Christoph Hellwig > Reviewed-by: Logan Gunthorpe > --- > kernel/memremap.c | 17 + > mm/gup.c | 7 +-- > 2 files changed, 14 insertions(+), 10 deletions(-) > > diff --git a/kernel/memremap.c b/kernel/memremap.c > index 43d94db97ff4..26764085785d 100644 > --- a/kernel/memremap.c > +++ b/kernel/memremap.c > @@ -506,22 +506,23 @@ struct vmem_altmap *to_vmem_altmap(unsigned long > memmap_start) > * @pfn: page frame number to lookup page_map > * @pgmap: optional known pgmap that already has a reference > * > - * @pgmap allows the overhead of a lookup to be bypassed when @pfn lands in > the > - * same mapping. > + * If @pgmap is non-NULL and covers @pfn it will be returned as-is. If > @pgmap > + * is non-NULL but does not cover @pfn the reference to it while be released. s/while/will/ Other than that you can add: Reviewed-by: Dan Williams
Re: [PATCH 11/17] mm: move get_dev_pagemap out of line
On Fri, Dec 15, 2017 at 6:09 AM, Christoph Hellwig wrote:
> This is a pretty big function, which should be out of line in general,
> and a no-op stub if CONFIG_ZONE_DEVICЕ is not set.
>
> Signed-off-by: Christoph Hellwig
> Reviewed-by: Logan Gunthorpe
[..]
> +/**
> + * get_dev_pagemap() - take a new live reference on the dev_pagemap for @pfn
> + * @pfn: page frame number to lookup page_map
> + * @pgmap: optional known pgmap that already has a reference
> + *
> + * @pgmap allows the overhead of a lookup to be bypassed when @pfn lands in
> the
> + * same mapping.
> + */
> +struct dev_pagemap *get_dev_pagemap(unsigned long pfn,
> + struct dev_pagemap *pgmap)
> +{
> + const struct resource *res = pgmap ? pgmap->res : NULL;
> + resource_size_t phys = PFN_PHYS(pfn);
> +
> + /*
> +* In the cached case we're already holding a live reference so
> +* we can simply do a blind increment
> +*/
> + if (res && phys >= res->start && phys <= res->end) {
> + percpu_ref_get(pgmap->ref);
> + return pgmap;
> + }
I was going to say keep the cached case in the static inline, but with
the optimization to the calling convention in the following patch I
think that makes this moot.
So,
Reviewed-by: Dan Williams
Re: [PATCH 04/17] mm: pass the vmem_altmap to arch_add_memory and __add_pages
On Fri, Dec 15, 2017 at 5:48 PM, Dan Williams wrote:
> On Fri, Dec 15, 2017 at 6:09 AM, Christoph Hellwig wrote:
>> We can just pass this on instead of having to do a radix tree lookup
>> without proper locking 2 levels into the callchain.
>>
>> Signed-off-by: Christoph Hellwig
>
> Yeah, the lookup of vmem_altmap is too magical and surprising this is better.
>
> Reviewed-by: Dan Williams
I'll also note that the locking is not necessary in the memory map
init path because we can't possibly be racing mutations of the radix
as everyone who might touch the radix is serialized by the
mem_hotplug_begin() lock. It's only accesses outside of the
arch_{add,remove}_memory() that need the rcu lock. However, that is
another subtle/magic assumption of this code and its better to pass
the altmap down through the call chain. I just don't want people
thinking that -stable needs to pick any of this up, because afaics the
locking is fine as is, and we can drop that mention from the
changelog.
Re: Mac Mini G4 defconfig ?
On Fri, Dec 15, 2017 at 10:01 PM, Mathieu Malaterre wrote: > On Fri, Dec 15, 2017 at 9:52 PM, Mathieu Malaterre wrote: >> On Fri, Dec 15, 2017 at 8:50 PM, Mathieu Malaterre wrote: >>> Hi there, >>> >>> Does anyone has working defconfig for a Mac Mini G4 ? >>> >>> Here is what I tried: >>> >>> $ cat ./arch/powerpc/configs/g4_defconfig >>> CONFIG_PPC_FPU=y >>> CONFIG_ALTIVEC=y >>> $ make ARCH=powerpc g4_defconfig >>> $ make -j8 ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- V=1 >>> set -e; : ' CHK include/config/kernel.release'; mkdir -p >> >> That is odd. Doing a quick git bisect: >> >> $ git checkout 3298b690b21cdbe6b2ae8076d9147027f396f2b1 >> $ make -n ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -f ./Makefile >> silentoldconfig V=1 >> + make -n ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- -f ./Makefile >> silentoldconfig V=1 >> /bin/sh: line 0: [: -ge: unary operator expected >> make -f ./scripts/Makefile.build obj=scripts/basic >> >> I cannot make sense of this shell error, maybe this is unrelated but >> things start breaking around this commit. > > Even if I discard this shell error, the next error comes with: > > 433dc2ebe7d17dd21cba7ad5c362d37323592236 is the first bad commit > > With: > > $ make ARCH=powerpc CROSS_COMPILE=powerpc-linux-gnu- V=1 > [...] > powerpc-linux-gnu-gcc -Wp,-MD,kernel/.bounds.s.d -nostdinc -isystem > -I./arch/powerpc/include -I./arch/powerpc/include/generated > -I./include -I./arch/powerpc/include/uapi > -I./arch/powerpc/include/generated/uapi -I./include/uapi > -I./include/generated/uapi -include ./include/linux/kconfig.h > -D__KERNEL__ -Iarch/powerpc -Wall -Wundef -Wstrict-prototypes > -Wno-trigraphs -fno-strict-aliasing -fno-common -fshort-wchar > -Werror-implicit-function-declaration -Wno-format-security -std=gnu89 > -pipe -Iarch/powerpc -ffixed-r2 -mmultiple -mcpu=powerpc -Wa,-maltivec > -mbig-endian -fno-delete-null-pointer-checks -Wno-frame-address -O2 > -Wno-maybe-uninitialized --param=allow-store-data-races=0 > -DCC_HAVE_ASM_GOTO -Wframe-larger-than=1024 -fno-stack-protector > -Wno-unused-but-set-variable -Wno-unused-const-variable > -fomit-frame-pointer -fno-var-tracking-assignments > -Wdeclaration-after-statement -Wno-pointer-sign -fno-strict-overflow > -fconserve-stack -Werror=implicit-int -Werror=strict-prototypes > -Werror=date-time -Werror=incompatible-pointer-types > -Werror=designated-init-DKBUILD_BASENAME='"bounds"' > -DKBUILD_MODNAME='"bounds"' -fverbose-asm -S -o kernel/bounds.s > kernel/bounds.c > In file included from ./include/linux/page-flags.h:9:0, > from kernel/bounds.c:9: > ./include/linux/bug.h:4:21: fatal error: asm/bug.h: No such file or directory > #include > ^ > compilation terminated. > Kbuild:20: recipe for target 'kernel/bounds.s' failed > make[1]: *** [kernel/bounds.s] Error 1 > Makefile:1051: recipe for target 'prepare0' failed > make: *** [prepare0] Error 2 > > > Comments ? Solution is: $ rm .cache.mk I guess the cache was not cleared in between my different kernel compilations. Sorry for the noise
