LinuxPPC-Dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [V3 01/10] perf: New conditional branch filter criteria in branch stack sampling
From: Anshuman Khandual @ 2013-12-03 10:21 UTC (permalink / raw)
  To: mpe@ellerman.id.au
  Cc: mikey, michaele, linux-kernel, eranian, linuxppc-dev, sukadev
In-Reply-To: <529474C7.9050005@linux.vnet.ibm.com>

On 11/26/2013 03:45 PM, Anshuman Khandual wrote:
> On 11/26/2013 11:36 AM, mpe@ellerman.id.au wrote:
>> Ideally your commit subject would contain a verb, preferably in the present
>> tense.
>>
>> I think simply "perf: Add PERF_SAMPLE_BRANCH_COND" would be clearer.
> 
> 
> Sure, will change it.
> 
>>
>> On Wed, 2013-16-10 at 06:56:48 UTC, Anshuman Khandual wrote:
>>> POWER8 PMU based BHRB supports filtering for conditional branches.
>>> This patch introduces new branch filter PERF_SAMPLE_BRANCH_COND which
>>> will extend the existing perf ABI. Other architectures can provide
>>> this functionality with either HW filtering support (if present) or
>>> with SW filtering of instructions.
>>>
>>> Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
>>> Reviewed-by: Stephane Eranian <eranian@google.com>
>>> ---
>>>  include/uapi/linux/perf_event.h | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
>>> index 0b1df41..5da52b6 100644
>>> --- a/include/uapi/linux/perf_event.h
>>> +++ b/include/uapi/linux/perf_event.h
>>> @@ -160,8 +160,9 @@ enum perf_branch_sample_type {
>>>  	PERF_SAMPLE_BRANCH_ABORT_TX	= 1U << 7, /* transaction aborts */
>>>  	PERF_SAMPLE_BRANCH_IN_TX	= 1U << 8, /* in transaction */
>>>  	PERF_SAMPLE_BRANCH_NO_TX	= 1U << 9, /* not in transaction */
>>> +	PERF_SAMPLE_BRANCH_COND		= 1U << 10, /* conditional branches */
>>>  
>>> -	PERF_SAMPLE_BRANCH_MAX		= 1U << 10, /* non-ABI */
>>> +	PERF_SAMPLE_BRANCH_MAX		= 1U << 11, /* non-ABI */
>>>  };
>>
>> This no longer applies against Linus' tree, you'll need to rebase it.
> 
> Okay

Hey Michael,

Looks like the patch still applies on top of Linus's tree. The modified patch with
a new commit subject line can be found here.

----------------------------------------------------------------------
>From d368096fc51a8da65f2d80ed5090d43cbc269f62 Mon Sep 17 00:00:00 2001
From: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Date: Mon, 22 Jul 2013 12:22:27 +0530
Subject: [PATCH] perf: Add PERF_SAMPLE_BRANCH_COND

POWER8 PMU based BHRB supports filtering for conditional branches.
This patch introduces new branch filter PERF_SAMPLE_BRANCH_COND which
will extend the existing perf ABI. Other architectures can provide
this functionality with either HW filtering support (if present) or
with SW filtering of instructions.

Signed-off-by: Anshuman Khandual <khandual@linux.vnet.ibm.com>
Reviewed-by: Stephane Eranian <eranian@google.com>
---
 include/uapi/linux/perf_event.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/uapi/linux/perf_event.h b/include/uapi/linux/perf_event.h
index e1802d6..e2d8b8b 100644
--- a/include/uapi/linux/perf_event.h
+++ b/include/uapi/linux/perf_event.h
@@ -163,8 +163,9 @@ enum perf_branch_sample_type {
 	PERF_SAMPLE_BRANCH_ABORT_TX	= 1U << 7, /* transaction aborts */
 	PERF_SAMPLE_BRANCH_IN_TX	= 1U << 8, /* in transaction */
 	PERF_SAMPLE_BRANCH_NO_TX	= 1U << 9, /* not in transaction */
+	PERF_SAMPLE_BRANCH_COND		= 1U << 10, /* conditional branches */
 
-	PERF_SAMPLE_BRANCH_MAX		= 1U << 10, /* non-ABI */
+	PERF_SAMPLE_BRANCH_MAX		= 1U << 11, /* non-ABI */
 };
 
 #define PERF_SAMPLE_BRANCH_PLM_ALL \
-- 
1.7.11.7

^ permalink raw reply related

* [PATCH] powerpc/defconfig: Enable ath9k wireless card support
From: Chunhe Lan @ 2013-12-03  7:32 UTC (permalink / raw)
  To: linuxppc-dev; +Cc: Scott Wood, Chunhe Lan

On the P1023RDB, when board leaves the factory, it has
two Atheros wireless cards on the PCIe slot. So enable
ath9k wireless driver support.

Signed-off-by: Chunhe Lan <Chunhe.Lan@freescale.com>
Cc: Scott Wood <scottwood@freescale.com>
---
 arch/powerpc/configs/85xx/p1023_defconfig |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/arch/powerpc/configs/85xx/p1023_defconfig b/arch/powerpc/configs/85xx/p1023_defconfig
index b06d37d..a3fe941 100644
--- a/arch/powerpc/configs/85xx/p1023_defconfig
+++ b/arch/powerpc/configs/85xx/p1023_defconfig
@@ -62,6 +62,9 @@ CONFIG_INET_ESP=y
 # CONFIG_INET_LRO is not set
 CONFIG_IPV6=y
 CONFIG_IP_SCTP=m
+CONFIG_CFG80211=y
+CONFIG_CFG80211_WEXT=y
+CONFIG_MAC80211=y
 CONFIG_UEVENT_HELPER_PATH="/sbin/hotplug"
 CONFIG_DEVTMPFS=y
 CONFIG_DEVTMPFS_MOUNT=y
@@ -101,6 +104,8 @@ CONFIG_DAVICOM_PHY=y
 CONFIG_CICADA_PHY=y
 CONFIG_VITESSE_PHY=y
 CONFIG_FIXED_PHY=y
+CONFIG_ATH_CARDS=y
+CONFIG_ATH9K=y
 CONFIG_INPUT_FF_MEMLESS=m
 # CONFIG_INPUT_MOUSEDEV is not set
 # CONFIG_INPUT_KEYBOARD is not set
-- 
1.7.6.5

^ permalink raw reply related

* Re: [PATCH 1/3 v2] iommu/fsl: Factor out PCI specific code.
From: Alex Williamson @ 2013-12-02 21:33 UTC (permalink / raw)
  To: Varun Sethi
  Cc: joro, stuart.yoder, linux-kernel, iommu, r65777, scottwood,
	linuxppc-dev
In-Reply-To: <1381922582-28724-2-git-send-email-Varun.Sethi@freescale.com>

On Wed, 2013-10-16 at 16:53 +0530, Varun Sethi wrote:
> Factor out PCI specific code in the PAMU driver.
> 
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> ---
>  drivers/iommu/fsl_pamu_domain.c |   88 +++++++++++++++++++--------------------
>  1 file changed, 43 insertions(+), 45 deletions(-)
> 
> diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
> index c857c30..966ae70 100644
> --- a/drivers/iommu/fsl_pamu_domain.c
> +++ b/drivers/iommu/fsl_pamu_domain.c
> @@ -677,21 +677,15 @@ static int handle_attach_device(struct fsl_dma_domain *dma_domain,
>  	return ret;
>  }
>  
> -static int fsl_pamu_attach_device(struct iommu_domain *domain,
> -				  struct device *dev)
> +static struct device *get_dma_device(struct device *dev)
>  {
> -	struct fsl_dma_domain *dma_domain = domain->priv;
> -	const u32 *liodn;
> -	u32 liodn_cnt;
> -	int len, ret = 0;
> -	struct pci_dev *pdev = NULL;
> -	struct pci_controller *pci_ctl;
> +	struct device *dma_dev = dev;
> +#ifdef CONFIG_PCI
>  
> -	/*
> -	 * Use LIODN of the PCI controller while attaching a
> -	 * PCI device.
> -	 */
>  	if (dev->bus == &pci_bus_type) {
> +		struct pci_controller *pci_ctl;
> +		struct pci_dev *pdev;
> +
>  		pdev = to_pci_dev(dev);
>  		pci_ctl = pci_bus_to_host(pdev->bus);
>  		/*
> @@ -699,17 +693,31 @@ static int fsl_pamu_attach_device(struct iommu_domain *domain,
>  		 * so we can get the LIODN programmed by
>  		 * u-boot.
>  		 */
> -		dev = pci_ctl->parent;
> +		dma_dev = pci_ctl->parent;
>  	}
> +#endif
> +	return dma_dev;
> +}
> +
> +static int fsl_pamu_attach_device(struct iommu_domain *domain,
> +				  struct device *dev)
> +{
> +	struct fsl_dma_domain *dma_domain = domain->priv;
> +	struct device *dma_dev;
> +	const u32 *liodn;
> +	u32 liodn_cnt;
> +	int len, ret = 0;
> +
> +	dma_dev = get_dma_device(dev);
>  
> -	liodn = of_get_property(dev->of_node, "fsl,liodn", &len);
> +	liodn = of_get_property(dma_dev->of_node, "fsl,liodn", &len);
>  	if (liodn) {
>  		liodn_cnt = len / sizeof(u32);
>  		ret = handle_attach_device(dma_domain, dev,
>  					 liodn, liodn_cnt);
>  	} else {
>  		pr_debug("missing fsl,liodn property at %s\n",
> -		          dev->of_node->full_name);
> +		          dma_dev->of_node->full_name);
>  			ret = -EINVAL;
>  	}
>  
> @@ -720,32 +728,18 @@ static void fsl_pamu_detach_device(struct iommu_domain *domain,
>  				      struct device *dev)
>  {
>  	struct fsl_dma_domain *dma_domain = domain->priv;
> +	struct device *dma_dev;
>  	const u32 *prop;
>  	int len;
> -	struct pci_dev *pdev = NULL;
> -	struct pci_controller *pci_ctl;
>  
> -	/*
> -	 * Use LIODN of the PCI controller while detaching a
> -	 * PCI device.
> -	 */
> -	if (dev->bus == &pci_bus_type) {
> -		pdev = to_pci_dev(dev);
> -		pci_ctl = pci_bus_to_host(pdev->bus);
> -		/*
> -		 * make dev point to pci controller device
> -		 * so we can get the LIODN programmed by
> -		 * u-boot.
> -		 */
> -		dev = pci_ctl->parent;
> -	}
> +	dma_dev = get_dma_device(dev);
>  
> -	prop = of_get_property(dev->of_node, "fsl,liodn", &len);
> +	prop = of_get_property(dma_dev->of_node, "fsl,liodn", &len);
>  	if (prop)
>  		detach_device(dev, dma_domain);
>  	else
>  		pr_debug("missing fsl,liodn property at %s\n",
> -		          dev->of_node->full_name);
> +		          dma_dev->of_node->full_name);
>  }
>  
>  static  int configure_domain_geometry(struct iommu_domain *domain, void *data)
> @@ -905,6 +899,7 @@ static struct iommu_group *get_device_iommu_group(struct device *dev)
>  	return group;
>  }
>  
> +#ifdef CONFIG_PCI
>  static  bool check_pci_ctl_endpt_part(struct pci_controller *pci_ctl)
>  {
>  	u32 version;
> @@ -945,13 +940,18 @@ static struct iommu_group *get_shared_pci_device_group(struct pci_dev *pdev)
>  	return NULL;
>  }
>  
> -static struct iommu_group *get_pci_device_group(struct pci_dev *pdev)
> +static struct iommu_group *get_pci_device_group(struct device *dev)
>  {
>  	struct pci_controller *pci_ctl;
>  	bool pci_endpt_partioning;
>  	struct iommu_group *group = NULL;
> -	struct pci_dev *bridge, *dma_pdev = NULL;
> +	struct pci_dev *bridge, *pdev;
> +	struct pci_dev *dma_pdev = NULL;
>  
> +	pdev = to_pci_dev(dev);
> +	/* Don't create device groups for virtual PCI bridges */
> +	if (pdev->subordinate)
> +		return NULL;
>  	pci_ctl = pci_bus_to_host(pdev->bus);
>  	pci_endpt_partioning = check_pci_ctl_endpt_part(pci_ctl);
>  	/* We can partition PCIe devices so assign device group to the device */
> @@ -1044,11 +1044,11 @@ root_bus:
>  
>  	return group;
>  }
> +#endif
>  
>  static int fsl_pamu_add_device(struct device *dev)
>  {
>  	struct iommu_group *group = NULL;
> -	struct pci_dev *pdev;
>  	const u32 *prop;
>  	int ret, len;
>  
> @@ -1056,19 +1056,15 @@ static int fsl_pamu_add_device(struct device *dev)
>  	 * For platform devices we allocate a separate group for
>  	 * each of the devices.
>  	 */
> -	if (dev->bus == &pci_bus_type) {
> -		pdev = to_pci_dev(dev);
> -		/* Don't create device groups for virtual PCI bridges */
> -		if (pdev->subordinate)
> -			return 0;
> -
> -		group = get_pci_device_group(pdev);
> -
> -	} else {
> +	if (dev->bus == &platform_bus_type) {
>  		prop = of_get_property(dev->of_node, "fsl,liodn", &len);
>  		if (prop)
>  			group = get_device_iommu_group(dev);
>  	}
> +#ifdef CONFIG_PCI
> +	else
> +		group = get_pci_device_group(dev);
> +#endif
>  
>  	if (!group || IS_ERR(group))
>  		return PTR_ERR(group);

It feels like there's a bug here that group is initialized to NULL and
there are a few ways for it to remain NULL through the above code, then
we get here and return PTR_ERR(NULL), which is indistinguishable from a
successful return below.  Perhaps !group is ok, but if so I'd suggest a
separate return block for it.  Thanks,

Alex

> @@ -1166,7 +1162,9 @@ int pamu_domain_init()
>  		return ret;
>  
>  	bus_set_iommu(&platform_bus_type, &fsl_pamu_ops);
> +#ifdef CONFIG_PCI
>  	bus_set_iommu(&pci_bus_type, &fsl_pamu_ops);
> +#endif
>  
>  	return ret;
>  }

^ permalink raw reply

* Re: [PATCH 2/3 v2] iommu/fsl: Enable default DMA window for PCIe devices
From: Alex Williamson @ 2013-12-02 21:33 UTC (permalink / raw)
  To: Varun Sethi
  Cc: joro, stuart.yoder, linux-kernel, iommu, r65777, scottwood,
	linuxppc-dev
In-Reply-To: <1381922582-28724-3-git-send-email-Varun.Sethi@freescale.com>

On Wed, 2013-10-16 at 16:53 +0530, Varun Sethi wrote:
> Once the PCIe device assigned to a guest VM (via VFIO) gets detached from the iommu domain
> (when guest terminates), its PAMU table entry is disabled. So, this would prevent the device
> from being used once it's assigned back to the host.
> 
> This patch allows for creation of a default DMA window corresponding to the device
> and subsequently enabling the PAMU table entry. Before we enable the entry, we ensure that
> the device's bus master capability is disabled (device quiesced).
> 
> Signed-off-by: Varun Sethi <Varun.Sethi@freescale.com>
> ---
>  drivers/iommu/fsl_pamu.c        |   43 ++++++++++++++++++++++++++++--------
>  drivers/iommu/fsl_pamu.h        |    1 +
>  drivers/iommu/fsl_pamu_domain.c |   46 ++++++++++++++++++++++++++++++++++++---
>  3 files changed, 78 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/iommu/fsl_pamu.c b/drivers/iommu/fsl_pamu.c
> index cba0498..fb4a031 100644
> --- a/drivers/iommu/fsl_pamu.c
> +++ b/drivers/iommu/fsl_pamu.c
> @@ -225,6 +225,21 @@ static struct paace *pamu_get_spaace(struct paace *paace, u32 wnum)
>  	return spaace;
>  }
>  
> +/*
> + * Defaul PPAACE settings for an LIODN.
> + */
> +static void setup_default_ppaace(struct paace *ppaace)
> +{
> +	pamu_init_ppaace(ppaace);
> +	/* window size is 2^(WSE+1) bytes */
> +	set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35);
> +	ppaace->wbah = 0;
> +	set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0);
> +	set_bf(ppaace->impl_attr, PAACE_IA_ATM,
> +		PAACE_ATM_NO_XLATE);
> +	set_bf(ppaace->addr_bitfields, PAACE_AF_AP,
> +		PAACE_AP_PERMS_ALL);
> +}
>  /**
>   * pamu_get_fspi_and_allocate() - Allocates fspi index and reserves subwindows
>   *                                required for primary PAACE in the secondary
> @@ -253,6 +268,24 @@ static unsigned long pamu_get_fspi_and_allocate(u32 subwin_cnt)
>  	return (spaace_addr - (unsigned long)spaact) / (sizeof(struct paace));
>  }
>  
> +/* Reset the PAACE entry to the default state */
> +void enable_default_dma_window(int liodn)
> +{
> +	struct paace *ppaace;
> +
> +	ppaace = pamu_get_ppaace(liodn);
> +	if (!ppaace) {
> +		pr_debug("Invalid liodn entry\n");
> +		return;
> +	}
> +
> +	memset(ppaace, 0, sizeof(struct paace));
> +
> +	setup_default_ppaace(ppaace);
> +	mb();
> +	pamu_enable_liodn(liodn);
> +}
> +
>  /* Release the subwindows reserved for a particular LIODN */
>  void pamu_free_subwins(int liodn)
>  {
> @@ -752,15 +785,7 @@ static void __init setup_liodns(void)
>  				continue;
>  			}
>  			ppaace = pamu_get_ppaace(liodn);
> -			pamu_init_ppaace(ppaace);
> -			/* window size is 2^(WSE+1) bytes */
> -			set_bf(ppaace->addr_bitfields, PPAACE_AF_WSE, 35);
> -			ppaace->wbah = 0;
> -			set_bf(ppaace->addr_bitfields, PPAACE_AF_WBAL, 0);
> -			set_bf(ppaace->impl_attr, PAACE_IA_ATM,
> -				PAACE_ATM_NO_XLATE);
> -			set_bf(ppaace->addr_bitfields, PAACE_AF_AP,
> -				PAACE_AP_PERMS_ALL);
> +			setup_default_ppaace(ppaace);
>  			if (of_device_is_compatible(node, "fsl,qman-portal"))
>  				setup_qbman_paace(ppaace, QMAN_PORTAL_PAACE);
>  			if (of_device_is_compatible(node, "fsl,qman"))
> diff --git a/drivers/iommu/fsl_pamu.h b/drivers/iommu/fsl_pamu.h
> index 8fc1a12..0edcbbbb 100644
> --- a/drivers/iommu/fsl_pamu.h
> +++ b/drivers/iommu/fsl_pamu.h
> @@ -406,5 +406,6 @@ void get_ome_index(u32 *omi_index, struct device *dev);
>  int  pamu_update_paace_stash(int liodn, u32 subwin, u32 value);
>  int pamu_disable_spaace(int liodn, u32 subwin);
>  u32 pamu_get_max_subwin_cnt(void);
> +void enable_default_dma_window(int liodn);
>  
>  #endif  /* __FSL_PAMU_H */
> diff --git a/drivers/iommu/fsl_pamu_domain.c b/drivers/iommu/fsl_pamu_domain.c
> index 966ae70..dd6cafc 100644
> --- a/drivers/iommu/fsl_pamu_domain.c
> +++ b/drivers/iommu/fsl_pamu_domain.c
> @@ -340,17 +340,57 @@ static inline struct device_domain_info *find_domain(struct device *dev)
>  	return dev->archdata.iommu_domain;
>  }
>  
> +/* Disable device DMA capability and enable default DMA window */
> +static void disable_device_dma(struct device_domain_info *info,
> +				int enable_dma_window)

nit, bool?

> +{
> +#ifdef CONFIG_PCI
> +	if (info->dev->bus == &pci_bus_type) {
> +		struct pci_dev *pdev = NULL;

nit, unnecessary initialization

> +		pdev = to_pci_dev(info->dev);
> +		if (pci_is_enabled(pdev))
> +			pci_disable_device(pdev);

This looks suspicious.  What's the case where you're finding devices
where this is needed?  Logically the driver that called
pci_enable_device() should also call pci_disabled_device() when it's
done.  Maybe there's a case to be made that we can't rely on the driver,
but then maybe there should be a pr_debug/info here to notify that
somebody left the device running.  There might also be the question of
whether you should test the busmaster bit in the command register rather
than trusting these indirect paths if it's really for cleanup.  Thanks,

Alex

> +	}
> +#endif
> +
> +	if (enable_dma_window)
> +		enable_default_dma_window(info->liodn);
> +}
> +
> +static int check_for_shared_liodn(struct device_domain_info *info)
> +{
> +	struct device_domain_info *tmp;
> +
> +	/*
> +	 * Sanity check, to ensure that this is not a
> +	 * shared LIODN. In case of a PCIe controller
> +	 * it's possible that all PCIe devices share
> +	 * the same LIODN.
> +	 */
> +	list_for_each_entry(tmp, &info->domain->devices, link) {
> +		if (info->liodn == tmp->liodn)
> +			return 1;
> +	}
> +
> +	return 0;
> +}
> +
>  static void remove_device_ref(struct device_domain_info *info, u32 win_cnt)
>  {
>  	unsigned long flags;
> +	int enable_dma_window = 0;
>  
>  	list_del(&info->link);
>  	spin_lock_irqsave(&iommu_lock, flags);
> -	if (win_cnt > 1)
> -		pamu_free_subwins(info->liodn);
> -	pamu_disable_liodn(info->liodn);
> +	if (!check_for_shared_liodn(info)) {
> +		if (win_cnt > 1)
> +			pamu_free_subwins(info->liodn);
> +		pamu_disable_liodn(info->liodn);
> +		enable_dma_window = 1;
> +	}
>  	spin_unlock_irqrestore(&iommu_lock, flags);
>  	spin_lock_irqsave(&device_domain_lock, flags);
> +	disable_device_dma(info, enable_dma_window);
>  	info->dev->archdata.iommu_domain = NULL;
>  	kmem_cache_free(iommu_devinfo_cache, info);
>  	spin_unlock_irqrestore(&device_domain_lock, flags);

^ permalink raw reply

* Re: [PATCH V4 7/9] cpuidle/powernv: Add "Fast-Sleep" CPU idle state
From: Thomas Gleixner @ 2013-12-02 18:00 UTC (permalink / raw)
  To: Preeti U Murthy
  Cc: michael, r58472, shangw, arnd, linux-pm, geoff, fweisbec,
	linux-kernel, rostedt, deepthi, rjw, paul.gortmaker, paulus,
	srivatsa.bhat, schwidefsky, john.stultz, paulmck, linuxppc-dev,
	chenhui.zhao
In-Reply-To: <529CA219.20109@linux.vnet.ibm.com>

On Mon, 2 Dec 2013, Preeti U Murthy wrote:
> On 11/29/2013 08:09 PM, Thomas Gleixner wrote:
> > This supports HIGHRES and NO_HZ if done right, without polling at
> > all. So you can even let the last CPU which handles the broadcast
> > hrtimer go for a long sleep, just not in the deepest idle state.
> 
> Thank you for the review. The above points are all valid. I will rework
> the design to:
> 
> 1. Eliminate the concept of a broadcast CPU and integrate its
> functionality in the timekeeping CPU.
> 
> 2. Avoid polling by using IPIs to communicate the next wakeup of the
> CPUs in deep idle state so as to reprogram the broadcast hrtimer.
> 
> 3. Make this feature generic and not arch-specific.

Great. If you need help with the generic bits, please let me know.

Thanks,

	tglx

^ permalink raw reply

* Re: [PATCH v3] ASoC: fsl_ssi: Add monaural audio support for non-ac97 interface
From: Mark Brown @ 2013-12-02 17:37 UTC (permalink / raw)
  To: Nicolin Chen; +Cc: alsa-devel, linuxppc-dev, timur
In-Reply-To: <1385998143-20196-1-git-send-email-b42378@freescale.com>

[-- Attachment #1: Type: text/plain, Size: 358 bytes --]

On Mon, Dec 02, 2013 at 11:29:03PM +0800, Nicolin Chen wrote:
> The normal mode of SSI allows it to send/receive data to/from the first
> slot of each period. So we can use this normal mode to trick I2S signal
> by puting/getting data to/from the first slot only (the left channel)
> so as to support monaural audio playback and recording.

Applied, thanks.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] watchdog: mpc8xxx_wdt convert to watchdog core
From: Guenter Roeck @ 2013-12-02 16:18 UTC (permalink / raw)
  To: leroy christophe
  Cc: scottwood, Wim Van Sebroeck, linuxppc-dev, linux-kernel,
	linux-watchdog
In-Reply-To: <529C6165.3050106@c-s.fr>

On Mon, Dec 02, 2013 at 11:31:01AM +0100, leroy christophe wrote:
> 
> Le 01/12/2013 20:38, Guenter Roeck a écrit :
> >On 11/30/2013 07:33 AM, Christophe Leroy wrote:
> >>Convert mpc8xxx_wdt.c to the new watchdog API.
> >>
> >>Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> >>
> >>diff -ur a/drivers/watchdog/mpc8xxx_wdt.c
> >>b/drivers/watchdog/mpc8xxx_wdt.c
> >>--- a/drivers/watchdog/mpc8xxx_wdt.c    2013-05-11
> >>22:57:46.000000000 +0200
> >>+++ b/drivers/watchdog/mpc8xxx_wdt.c    2013-11-30
> >>16:14:53.803495472 +0100
> >>@@ -72,28 +72,31 @@
> >>   * to 0
> >>   */
> >>  static int prescale = 1;
> >>-static unsigned int timeout_sec;
> >>
> >>-static unsigned long wdt_is_open;
> >>  static DEFINE_SPINLOCK(wdt_spinlock);
> 
_> >>-static void mpc8xxxwdt_keepalive(void)
> >>+static int mpc8xxx_wdt_ping(struct watchdog_device *w)
> >>  {
> >>      /* Ping the WDT */
> >>      spin_lock(&wdt_spinlock);
> >>      out_be16(&wd_base->swsrr, 0x556c);
> >>      out_be16(&wd_base->swsrr, 0xaa39);
> >>      spin_unlock(&wdt_spinlock);
> >>+    return 0;
> >
> >The return code is never checked, so you can make this function void.
> watchdog .h expects an int function.
> Wouldn't it be an error to make it void, if for instance in the
> future the return code is checked by the core ?

This would be correct if the watchdog core would ever call this function,
which it could only do if it was specified in mpc8xxx_wdt_ops, which it isn't.
As written, mpc8xxx_wdt_ping is only called from mpc8xxx_wdt_timer_ping.

If this is not intentional, ie if mpc8xxx_wdt_ping is supposed to be called
from the infrastructure, something else is wrong.

Guenter

> >
> >>  }
> >>
> >>+static struct watchdog_device mpc8xxx_wdt_dev;
> >>  static void mpc8xxx_wdt_timer_ping(unsigned long arg);
> >>-static DEFINE_TIMER(wdt_timer, mpc8xxx_wdt_timer_ping, 0, 0);
> >>+static DEFINE_TIMER(wdt_timer, mpc8xxx_wdt_timer_ping, 0,
> >>+        (unsigned long)&mpc8xxx_wdt_dev);
> >>
> >>  static void mpc8xxx_wdt_timer_ping(unsigned long arg)
> >>  {
> >>-    mpc8xxx_wdt_keepalive();
> >>+    struct watchdog_device *w = (struct watchdog_device *)arg;
> >>+
> >>+    mpc8xxx_wdt_ping(w);
> >>      /* We're pinging it twice faster than needed, just to be sure. */
> >>-    mod_timer(&wdt_timer, jiffies + HZ * timeout_sec / 2);
> >>+    mod_timer(&wdt_timer, jiffies + HZ * w->timeout / 2);
> >>  }
> >>
> >>  static void mpc8xxx_wdt_pr_warn(const char *msg)
> >>@@ -102,19 +105,9 @@
> >>          reset ? "reset" : "machine check exception");
> >>  }
> >>
> >>-static ssize_t mpc8xxx_wdt_write(struct file *file, const char
> >>__user *buf,
> >>-                 size_t count, loff_t *ppos)
> >>-{
> >>-    if (count)
> >>-        mpc8xxx_wdt_keepalive();
> >>-    return count;
> >>-}
> >>-
> >>-static int mpc8xxx_wdt_open(struct inode *inode, struct file *file)
> >>+static int mpc8xxx_wdt_start(struct watchdog_device *w)
> >>  {
> >>      u32 tmp = SWCRR_SWEN;
> >>-    if (test_and_set_bit(0, &wdt_is_open))
> >>-        return -EBUSY;
> >>
> >>      /* Once we start the watchdog we can't stop it */
> >>      if (nowayout)
> >
> >This code and the subsequent module_get can be removed; it is
> >handled by the infrastructure.
> >
> >
> >>@@ -132,59 +125,30 @@
> >>
> >>      del_timer_sync(&wdt_timer);
> >>
> >>-    return nonseekable_open(inode, file);
> >>+    return 0;
> >>  }
> >>
> >>-static int mpc8xxx_wdt_release(struct inode *inode, struct file *file)
> >>+static int mpc8xxx_wdt_stop(struct watchdog_device *w)
> >>  {
> >>-    if (!nowayout)
> >>-        mpc8xxx_wdt_timer_ping(0);
> >>-    else
> >>-        mpc8xxx_wdt_pr_warn("watchdog closed");
> >>-    clear_bit(0, &wdt_is_open);
> >>+    mpc8xxx_wdt_timer_ping((unsigned long)w);
> >
> >I really dislike this typecasting back and forth.
> >
> >I think it would be better to move the mod_timer() call from
> >mpc8xxx_wdt_timer_ping
> >into mpc8xxx_wdt_ping and call mpc8xxx_wdt_ping directly whenever
> >possible.
> >From mpc8xxx_wdt_timer_ping you can then just call
> >mpc8xxx_wdt_ping with the
> >typecasted parameter.
> I'm not sure I understand what you mean.
> mpc8xxx_wdt_ping() is called by the core when the user app writes
> into /dev/watchdog.
> We don't want the set the timer in that case.
> The timer is only used for when no user app has opened /dev/watchdog yet.
> 
> >
> >>      return 0;
> >>  }
> >>
> >>-static long mpc8xxx_wdt_ioctl(struct file *file, unsigned int cmd,
> >>-                            unsigned long arg)
> >>-{
> >>-    void __user *argp = (void __user *)arg;
> >>-    int __user *p = argp;
> >>-    static const struct watchdog_info ident = {
> >>-        .options = WDIOF_KEEPALIVEPING,
> >>-        .firmware_version = 1,
> >>-        .identity = "MPC8xxx",
> >>-    };
> >>-
> >>-    switch (cmd) {
> >>-    case WDIOC_GETSUPPORT:
> >>-        return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
> >>-    case WDIOC_GETSTATUS:
> >>-    case WDIOC_GETBOOTSTATUS:
> >>-        return put_user(0, p);
> >>-    case WDIOC_KEEPALIVE:
> >>-        mpc8xxx_wdt_keepalive();
> >>-        return 0;
> >>-    case WDIOC_GETTIMEOUT:
> >>-        return put_user(timeout_sec, p);
> >>-    default:
> >>-        return -ENOTTY;
> >>-    }
> >>-}
> >>+static struct watchdog_info mpc8xxx_wdt_info = {
> >>+    .options = WDIOF_KEEPALIVEPING,
> >>+    .firmware_version = 1,
> >>+    .identity = "MPC8xxx",
> >>+};
> >>
> >>-static const struct file_operations mpc8xxx_wdt_fops = {
> >>-    .owner        = THIS_MODULE,
> >>-    .llseek        = no_llseek,
> >>-    .write        = mpc8xxx_wdt_write,
> >>-    .unlocked_ioctl    = mpc8xxx_wdt_ioctl,
> >>-    .open        = mpc8xxx_wdt_open,
> >>-    .release    = mpc8xxx_wdt_release,
> >>+static struct watchdog_ops mpc8xxx_wdt_ops = {
> >>+    .owner = THIS_MODULE,
> >>+    .start = mpc8xxx_wdt_start,
> >>+    .stop = mpc8xxx_wdt_stop,
> >>  };
> >>
> >>-static struct miscdevice mpc8xxx_wdt_miscdev = {
> >>-    .minor    = WATCHDOG_MINOR,
> >>-    .name    = "watchdog",
> >>-    .fops    = &mpc8xxx_wdt_fops,
> >>+static struct watchdog_device mpc8xxx_wdt_dev = {
> >>+    .info = &mpc8xxx_wdt_info,
> >>+    .ops = &mpc8xxx_wdt_ops,
> >>  };
> >>
> >>  static const struct of_device_id mpc8xxx_wdt_match[];
> >>@@ -196,6 +160,7 @@
> >>      const struct mpc8xxx_wdt_type *wdt_type;
> >>      u32 freq = fsl_get_sys_freq();
> >>      bool enabled;
> >>+    unsigned int timeout_sec;
> >>
> >>      match = of_match_device(mpc8xxx_wdt_match, &ofdev->dev);
> >>      if (!match)
> >>@@ -222,6 +187,7 @@
> >>      else
> >>          timeout_sec = timeout / freq;
> >>
> >>+    mpc8xxx_wdt_dev.timeout = timeout_sec;
> >>  #ifdef MODULE
> >>      ret = mpc8xxx_wdt_init_late();
> >>      if (ret)
> >>@@ -237,7 +203,7 @@
> >>       * userspace handles it.
> >>       */
> >>      if (enabled)
> >>-        mpc8xxx_wdt_timer_ping(0);
> >>+        mpc8xxx_wdt_timer_ping((unsigned long)&mpc8xxx_wdt_dev);
> >>      return 0;
> >>  err_unmap:
> >>      iounmap(wd_base);
> >>@@ -249,7 +215,7 @@
> >>  {
> >>      mpc8xxx_wdt_pr_warn("watchdog removed");
> >
> >The mpc8xxx_wdt_pr_warn function is now rather unnecessary since
> >it is called only once. Might as well merge it into this function.
> >
> >>      del_timer_sync(&wdt_timer);
> >>-    misc_deregister(&mpc8xxx_wdt_miscdev);
> >>+    watchdog_unregister_device(&mpc8xxx_wdt_dev);
> >>      iounmap(wd_base);
> >>
> >>      return 0;
> >>@@ -301,10 +267,11 @@
> >>      if (!wd_base)
> >>          return -ENODEV;
> >>
> >>-    ret = misc_register(&mpc8xxx_wdt_miscdev);
> >>+    watchdog_set_nowayout(&mpc8xxx_wdt_dev, nowayout);
> >>+
> >>+    ret = watchdog_register_device(&mpc8xxx_wdt_dev);
> >>      if (ret) {
> >>-        pr_err("cannot register miscdev on minor=%d (err=%d)\n",
> >>-               WATCHDOG_MINOR, ret);
> >>+        pr_err("cannot register watchdog device (err=%d)\n", ret);
> >>          return ret;
> >>      }
> >>      return 0;
> >>diff -ur a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> >>--- a/drivers/watchdog/Kconfig
> >>+++ b/drivers/watchdog/Kconfig
> >>@@ -1146,6 +1146,7 @@
> >>  config 8xxx_WDT
> >>      tristate "MPC8xxx Platform Watchdog Timer"
> >>      depends on PPC_8xx || PPC_83xx || PPC_86xx
> >>+    select WATCHDOG_CORE
> >>      help
> >>        This driver is for a SoC level watchdog that exists on some
> >>        Freescale PowerPC processors. So far this driver supports:
> >>
> >>---
> >>Ce courrier électronique ne contient aucun virus ou logiciel
> >>malveillant parce que la protection avast! Antivirus est active.
> >>http://www.avast.com
> >>
> >>-- 
> >>To unsubscribe from this list: send the line "unsubscribe
> >>linux-watchdog" in
> >>the body of a message to majordomo@vger.kernel.org
> >>More majordomo info at http://vger.kernel.org/majordomo-info.html
> >>
> >>
> >
> 
> 

^ permalink raw reply

* Re: [PATCH V4 6/9] cpuidle/ppc: Add basic infrastructure to enable the broadcast framework on ppc
From: Preeti U Murthy @ 2013-12-02 15:27 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: fweisbec, paul.gortmaker, paulus, shangw, rjw, paulmck, arnd,
	linux-pm, rostedt, michael, john.stultz, chenhui.zhao, deepthi,
	r58472, geoff, linux-kernel, srivatsa.bhat, schwidefsky,
	linuxppc-dev
In-Reply-To: <alpine.DEB.2.02.1311291256180.30673@ionos.tec.linutronix.de>

Hi Thomas,

On 11/29/2013 05:28 PM, Thomas Gleixner wrote:
> On Fri, 29 Nov 2013, Preeti U Murthy wrote:
>> diff --git a/arch/powerpc/Kconfig b/arch/powerpc/Kconfig
>> index b44b52c..cafa788 100644
>> --- a/arch/powerpc/Kconfig
>> +++ b/arch/powerpc/Kconfig
>> @@ -129,6 +129,8 @@ config PPC
>>  	select GENERIC_CMOS_UPDATE
>>  	select GENERIC_TIME_VSYSCALL_OLD
>>  	select GENERIC_CLOCKEVENTS
>> +	select GENERIC_CLOCKEVENTS_BROADCAST
>> +	select ARCH_HAS_TICK_BROADCAST if GENERIC_CLOCKEVENTS_BROADCAST
> 
> What's the point of this config switch? It's nowhere used.

When broadcast IPIs are to be sent, either the "broadcast" method
associated with the local timers is used or an arch-specific method
tick_broadcast() is invoked. For the latter be invoked,
ARCH_HAS_TICK_BROADCAST config needs to be set. On PowerPC, the
broadcast method is not associated with the local timer. Hence we invoke
tick_broadcast(). This function has been added in [PATCH 2/9].
> 
>> +static int broadcast_set_next_event(unsigned long evt,
>> +					struct clock_event_device *dev)
>> +{
>> +	return 0;
>> +}
>> +
>> +static void broadcast_set_mode(enum clock_event_mode mode,
>> +				 struct clock_event_device *dev)
>> +{
>> +	if (mode != CLOCK_EVT_MODE_ONESHOT)
>> +		broadcast_set_next_event(DECREMENTER_MAX, dev);
> 
> What's the point of calling an empty function?  

You are right, this should have remained a dummy function like
broadcast_set_next_event() as per the design of this patchset.
> 
>> +}
>> +
>>  static void decrementer_set_mode(enum clock_event_mode mode,
>>  				 struct clock_event_device *dev)
>>  {
>> @@ -840,6 +869,19 @@ static void register_decrementer_clockevent(int cpu)
>>  	clockevents_register_device(dec);
>>  }
>>  
>> +static void register_broadcast_clockevent(int cpu)
>> +{
>> +	struct clock_event_device *bc_evt = &bc_timer;
>> +
>> +	*bc_evt = broadcast_clockevent;
>> +	bc_evt->cpumask = cpu_possible_mask;
>> +
>> +	printk_once(KERN_DEBUG "clockevent: %s mult[%x] shift[%d] cpu[%d]\n",
>> +		    bc_evt->name, bc_evt->mult, bc_evt->shift, cpu);
>> +
>> +	clockevents_register_device(bc_evt);
>> +}
>> +
>>  static void __init init_decrementer_clockevent(void)
>>  {
>>  	int cpu = smp_processor_id();
>> @@ -854,6 +896,19 @@ static void __init init_decrementer_clockevent(void)
>>  	register_decrementer_clockevent(cpu);
>>  }
>>  
>> +static void __init init_broadcast_clockevent(void)
>> +{
>> +	int cpu = smp_processor_id();
>> +
>> +	clockevents_calc_mult_shift(&broadcast_clockevent, ppc_tb_freq, 4);
>> +
>> +	broadcast_clockevent.max_delta_ns =
>> +		clockevent_delta2ns(DECREMENTER_MAX, &broadcast_clockevent);
>> +	broadcast_clockevent.min_delta_ns =
>> +		clockevent_delta2ns(2, &broadcast_clockevent);
> 
> clockevents_config()

Right, I will change this to call clockevents_config(). I see that this
needs to be done during the initialization of the decrementer as well.
Will do the same.

Thank you

Regards
Preeti U Murthy
> 
> Thanks,
> 
> 	tglx
> 

^ permalink raw reply

* Re: [PATCH] watchdog: mpc8xxx_wdt: MPC8xx is HW enabled
From: Guenter Roeck @ 2013-12-02 16:04 UTC (permalink / raw)
  To: Christophe Leroy
  Cc: scottwood, Wim Van Sebroeck, linuxppc-dev, linux-kernel,
	linux-watchdog
In-Reply-To: <20131130154540.E83D743E15@localhost.localdomain>

On Sat, Nov 30, 2013 at 04:45:40PM +0100, Christophe Leroy wrote:
> MPC8xx watchdog is enabled at startup by HW.
> If the bootloader disables it, it cannot be reenabled.
> 
> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
> 
Reviewed-by: Guenter Roeck <linux@roeck-us.net>

> diff -ur a/drivers/watchdog/mpc8xxx_wdt.c b/drivers/watchdog/mpc8xxx_wdt.c
> --- a/drivers/watchdog/mpc8xxx_wdt.c	2013-05-11 22:57:46.000000000 +0200
> +++ b/drivers/watchdog/mpc8xxx_wdt.c	2013-08-08 02:12:15.000000000 +0200
> @@ -273,6 +310,7 @@
>  		.compatible = "fsl,mpc823-wdt",
>  		.data = &(struct mpc8xxx_wdt_type) {
>  			.prescaler = 0x800,
> +			.hw_enabled = true,
>  		},
>  	},
>  	{},
> 
> ---
> Ce courrier électronique ne contient aucun virus ou logiciel malveillant parce que la protection avast! Antivirus est active.
> http://www.avast.com
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-watchdog" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

^ permalink raw reply

* [PATCH v3] ASoC: fsl_ssi: Add monaural audio support for non-ac97 interface
From: Nicolin Chen @ 2013-12-02 15:29 UTC (permalink / raw)
  To: broonie, timur; +Cc: alsa-devel, linuxppc-dev

The normal mode of SSI allows it to send/receive data to/from the first
slot of each period. So we can use this normal mode to trick I2S signal
by puting/getting data to/from the first slot only (the left channel)
so as to support monaural audio playback and recording.

Signed-off-by: Nicolin Chen <b42378@freescale.com>
---
Changelog
v3:
 * Fixed conflict for git apply.
v2:
 * Moved i2s_mode to ssi_private so that we can save and retore it as needed
 * And dropped the horrible static thing.


 sound/soc/fsl/fsl_ssi.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/sound/soc/fsl/fsl_ssi.c b/sound/soc/fsl/fsl_ssi.c
index fb8f52a..83e4ed8 100644
--- a/sound/soc/fsl/fsl_ssi.c
+++ b/sound/soc/fsl/fsl_ssi.c
@@ -143,6 +143,7 @@ struct fsl_ssi_private {
 	bool ssi_on_imx;
 	bool imx_ac97;
 	bool use_dma;
+	u8 i2s_mode;
 	struct clk *clk;
 	struct snd_dmaengine_dai_dma_data dma_params_tx;
 	struct snd_dmaengine_dai_dma_data dma_params_rx;
@@ -354,14 +355,13 @@ static void fsl_ssi_setup_ac97(struct fsl_ssi_private *ssi_private)
 static int fsl_ssi_setup(struct fsl_ssi_private *ssi_private)
 {
 	struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
-	u8 i2s_mode;
 	u8 wm;
 	int synchronous = ssi_private->cpu_dai_drv.symmetric_rates;
 
 	if (ssi_private->imx_ac97)
-		i2s_mode = CCSR_SSI_SCR_I2S_MODE_NORMAL | CCSR_SSI_SCR_NET;
+		ssi_private->i2s_mode = CCSR_SSI_SCR_I2S_MODE_NORMAL | CCSR_SSI_SCR_NET;
 	else
-		i2s_mode = CCSR_SSI_SCR_I2S_MODE_SLAVE;
+		ssi_private->i2s_mode = CCSR_SSI_SCR_I2S_MODE_SLAVE;
 
 	/*
 	 * Section 16.5 of the MPC8610 reference manual says that the SSI needs
@@ -378,7 +378,7 @@ static int fsl_ssi_setup(struct fsl_ssi_private *ssi_private)
 	write_ssi_mask(&ssi->scr,
 		CCSR_SSI_SCR_I2S_MODE_MASK | CCSR_SSI_SCR_SYN,
 		CCSR_SSI_SCR_TFR_CLK_DIS |
-		i2s_mode |
+		ssi_private->i2s_mode |
 		(synchronous ? CCSR_SSI_SCR_SYN : 0));
 
 	write_ssi(CCSR_SSI_STCR_TXBIT0 | CCSR_SSI_STCR_TFEN0 |
@@ -508,6 +508,7 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream,
 {
 	struct fsl_ssi_private *ssi_private = snd_soc_dai_get_drvdata(cpu_dai);
 	struct ccsr_ssi __iomem *ssi = ssi_private->ssi;
+	unsigned int channels = params_channels(hw_params);
 	unsigned int sample_size =
 		snd_pcm_format_width(params_format(hw_params));
 	u32 wl = CCSR_SSI_SxCCR_WL(sample_size);
@@ -537,6 +538,11 @@ static int fsl_ssi_hw_params(struct snd_pcm_substream *substream,
 	else
 		write_ssi_mask(&ssi->srccr, CCSR_SSI_SxCCR_WL_MASK, wl);
 
+	if (!ssi_private->imx_ac97)
+		write_ssi_mask(&ssi->scr,
+				CCSR_SSI_SCR_NET | CCSR_SSI_SCR_I2S_MODE_MASK,
+				channels == 1 ? 0 : ssi_private->i2s_mode);
+
 	return 0;
 }
 
@@ -649,14 +655,13 @@ static const struct snd_soc_dai_ops fsl_ssi_dai_ops = {
 static struct snd_soc_dai_driver fsl_ssi_dai_template = {
 	.probe = fsl_ssi_dai_probe,
 	.playback = {
-		/* The SSI does not support monaural audio. */
-		.channels_min = 2,
+		.channels_min = 1,
 		.channels_max = 2,
 		.rates = FSLSSI_I2S_RATES,
 		.formats = FSLSSI_I2S_FORMATS,
 	},
 	.capture = {
-		.channels_min = 2,
+		.channels_min = 1,
 		.channels_max = 2,
 		.rates = FSLSSI_I2S_RATES,
 		.formats = FSLSSI_I2S_FORMATS,
-- 
1.8.4

^ permalink raw reply related

* Re: [PATCH v2] ASoC: fsl_ssi: Add monaural audio support for non-ac97 interface
From: Nicolin Chen @ 2013-12-02 15:21 UTC (permalink / raw)
  To: Mark Brown
  Cc: alsa-devel, linux, linux-kernel, lgirdwood, timur, linuxppc-dev
In-Reply-To: <20131202115617.GV27568@sirena.org.uk>

On Mon, Dec 02, 2013 at 11:56:17AM +0000, Mark Brown wrote:
> On Sat, Nov 16, 2013 at 12:25:29AM +0800, Nicolin Chen wrote:
> > The normal mode of SSI allows it to send/receive data to/from the first
> > slot of each period. So we can use this normal mode to trick I2S signal
> > by puting/getting data to/from the first slot only (the left channel)
> > so as to support monaural audio playback and recording.
> 
> This looks OK but doesn't apply against current code, can you please
> check and resend?

Ah, I rebased it on the dual-fifo set. Sorry I didn't make it clean.
I will resend it. Thank you.

^ permalink raw reply

* Re: [PATCH v6 08/17] spi: mpc512x: adjust to OF based clock lookup
From: Gerhard Sittig @ 2013-12-02 15:10 UTC (permalink / raw)
  To: Mark Rutland
  Cc: Mike Turquette, Detlev Zundel, linux-spi@vger.kernel.org,
	Mark Brown, Scott Wood, Anatolij Gustschin,
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <20131202095034.GB12952@e106331-lin.cambridge.arm.com>

On Mon, Dec 02, 2013 at 09:50 +0000, Mark Rutland wrote:
> 
> On Sat, Nov 30, 2013 at 10:51:28PM +0000, Gerhard Sittig wrote:
> > after device tree based clock lookup became available, the peripheral
> > driver need no longer construct clock names which include the PSC index,
> > remove the "psc%d_mclk" template and unconditionally use 'mclk'
> > 
> > acquire and release the 'ipg' clock item for register access as well
> 
> For this and the other peripheral updates, it would be nice to have the
> expected clock-names entries added to the respective binding documents.
> Otherwise this looks fine to me.

I will look into adding these 'clock-names' to binding documents
for the peripherals, too (not just SPI but the others as well).
Thank you for the feedback!

It appears that not all of those peripherals have existing
binding docs, so introducing these docs in the first place may
exceed the scope of the CCF support introduction series, and I
may have to create another series for the doc updates.  I assume
that it's OK to address the issue of missing documentation in a
separate action.


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de

^ permalink raw reply

* Re: [PATCH V4 7/9] cpuidle/powernv: Add "Fast-Sleep" CPU idle state
From: Preeti U Murthy @ 2013-12-02 15:07 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: michael, r58472, shangw, arnd, linux-pm, geoff, fweisbec,
	linux-kernel, rostedt, deepthi, rjw, paul.gortmaker, paulus,
	srivatsa.bhat, schwidefsky, john.stultz, paulmck, linuxppc-dev,
	chenhui.zhao
In-Reply-To: <alpine.DEB.2.02.1311291301300.30673@ionos.tec.linutronix.de>

Hi Thomas,

On 11/29/2013 08:09 PM, Thomas Gleixner wrote:
> On Fri, 29 Nov 2013, Preeti U Murthy wrote:
>> +static enum hrtimer_restart handle_broadcast(struct hrtimer *hrtimer)
>> +{
>> +	struct clock_event_device *bc_evt = &bc_timer;
>> +	ktime_t interval, next_bc_tick, now;
>> +
>> +	now = ktime_get();
>> +
>> +	if (!restart_broadcast(bc_evt))
>> +		return HRTIMER_NORESTART;
>> +
>> +	interval = ktime_sub(bc_evt->next_event, now);
>> +	next_bc_tick = get_next_bc_tick();
> 
> So you're seriously using a hrtimer to poll in HZ frequency for
> updates of bc->next_event?
> 
> To be honest, this design sucks.
> 
> First of all, why is this a PPC specific feature? There are probably
> other architectures which could make use of this. So this should be
> implemented in the core code to begin with.
> 
> And a lot of the things you need for this are already available in the
> core in one form or the other.
> 
> For a start you can stick the broadcast hrtimer to the cpu which does
> the timekeeping. The handover in the hotplug case is handled there as
> well as is the handover for the NOHZ case.
> 
> This needs to be extended for this hrtimer broadcast thingy to work,
> but it shouldn't be that hard to do so.
> 
> Now for the polling. That's a complete trainwreck.
> 
> This can be solved via the broadcast IPI as well. When a CPU which
> goes down into deep idle sets the broadcast to expire earlier than the
> active value it can denote that and send the timer broadcast IPI over
> to the CPU which has the honour of dealing with this.
> 
> This supports HIGHRES and NO_HZ if done right, without polling at
> all. So you can even let the last CPU which handles the broadcast
> hrtimer go for a long sleep, just not in the deepest idle state.

Thank you for the review. The above points are all valid. I will rework
the design to:

1. Eliminate the concept of a broadcast CPU and integrate its
functionality in the timekeeping CPU.

2. Avoid polling by using IPIs to communicate the next wakeup of the
CPUs in deep idle state so as to reprogram the broadcast hrtimer.

3. Make this feature generic and not arch-specific.

Regards
Preeti U Murthy
> 
> Thanks,
> 
> 	tglx
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev
> 

^ permalink raw reply

* Re: [PATCH v6 02/17] dts: mpc512x: introduce dt-bindings/clock/ header
From: Gerhard Sittig @ 2013-12-02 14:25 UTC (permalink / raw)
  To: Mark Rutland
  Cc: devicetree@vger.kernel.org, Mike Turquette, Pawel Moll,
	Detlev Zundel, Stephen Warren, rob.herring@calxeda.com,
	Scott Wood, Anatolij Gustschin, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, Ian Campbell
In-Reply-To: <20131202094656.GA12952@e106331-lin.cambridge.arm.com>

On Mon, Dec 02, 2013 at 09:46 +0000, Mark Rutland wrote:
> 
> On Sat, Nov 30, 2013 at 10:51:22PM +0000, Gerhard Sittig wrote:
> > introduce a dt-bindings/ header file for MPC512x clocks,
> > providing symbolic identifiers for those SoC clocks which
> > clients will reference from their device tree nodes
> 
> There should be a binding document update to go with this, pointing out
> that this include file defines the set of clock IDs for the MPC512x
> clocks.

It turns out the Freescale MPC512x did not have a separate clocks
binding document, but "just did the usual" and followed the
common bindings.

I've sent a separate doc update patch which becomes applicable
after the CCF implementation got accepted.

> Otherwise, this looks fine to me.

So I take this as a "reviewed by" of yours for this dt-bindings
header file patch, shall I have to re-submit the series.  Thanks!


virtually yours
Gerhard Sittig
-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr. 5, D-82194 Groebenzell, Germany
Phone: +49-8142-66989-0 Fax: +49-8142-66989-80  Email: office@denx.de

^ permalink raw reply

* Re: [PATCH] powerpc: fix xmon disassembler for little-endian
From: Tom Musta @ 2013-12-02 14:05 UTC (permalink / raw)
  To: linuxppc-dev
In-Reply-To: <1385975412-25869-1-git-send-email-felix@linux.vnet.ibm.com>

On 12/2/2013 3:10 AM, Philippe Bergheaud wrote:
> This patch fixes the disassembler of the powerpc kernel debugger xmon,
> for little-endian.
> 
> Signed-off-by: Philippe Bergheaud <felix@linux.vnet.ibm.com>
> ---
>  arch/powerpc/xmon/xmon.c |    4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
> index af9d346..6c27804 100644
> --- a/arch/powerpc/xmon/xmon.c
> +++ b/arch/powerpc/xmon/xmon.c
> @@ -171,7 +171,11 @@ extern void xmon_leave(void);
>  #define REG		"%.8lx"
>  #endif
>  
> +#ifdef __LITTLE_ENDIAN__
> +#define GETWORD(v)	(((v)[3] << 24) + ((v)[2] << 16) + ((v)[1] << 8) + (v)[0])
> +#else
>  #define GETWORD(v)	(((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + (v)[3])
> +#endif
>  
>  #define isxdigit(c)	(('0' <= (c) && (c) <= '9') \
>  			 || ('a' <= (c) && (c) <= 'f') \
> 

Philippe:  Wouldn't it be better to just do a 32-bit load and let the endianness be worked out
by the hardware?  i.e.

#define GETWORD(v) (*(u32 *)v)


Tom

^ permalink raw reply

* Re: [PATCH v2] ASoC: fsl_ssi: Add monaural audio support for non-ac97 interface
From: Mark Brown @ 2013-12-02 11:56 UTC (permalink / raw)
  To: Nicolin Chen
  Cc: alsa-devel, linux, linux-kernel, lgirdwood, timur, linuxppc-dev
In-Reply-To: <1384532729-6588-1-git-send-email-b42378@freescale.com>

[-- Attachment #1: Type: text/plain, Size: 428 bytes --]

On Sat, Nov 16, 2013 at 12:25:29AM +0800, Nicolin Chen wrote:
> The normal mode of SSI allows it to send/receive data to/from the first
> slot of each period. So we can use this normal mode to trick I2S signal
> by puting/getting data to/from the first slot only (the left channel)
> so as to support monaural audio playback and recording.

This looks OK but doesn't apply against current code, can you please
check and resend?

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH v6 08/17] spi: mpc512x: adjust to OF based clock lookup
From: Mark Brown @ 2013-12-02 11:32 UTC (permalink / raw)
  To: Gerhard Sittig
  Cc: Mike Turquette, Detlev Zundel, linux-spi, Scott Wood,
	Anatolij Gustschin, linuxppc-dev, linux-arm-kernel
In-Reply-To: <1385851897-23475-9-git-send-email-gsi@denx.de>

[-- Attachment #1: Type: text/plain, Size: 402 bytes --]

On Sat, Nov 30, 2013 at 11:51:28PM +0100, Gerhard Sittig wrote:
> after device tree based clock lookup became available, the peripheral
> driver need no longer construct clock names which include the PSC index,
> remove the "psc%d_mclk" template and unconditionally use 'mclk'
> 
> acquire and release the 'ipg' clock item for register access as well

Acked-by: Mark Brown <broonie@linaro.org.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH] watchdog: mpc8xxx_wdt convert to watchdog core
From: leroy christophe @ 2013-12-02 10:31 UTC (permalink / raw)
  To: Guenter Roeck, Wim Van Sebroeck, scottwood
  Cc: linuxppc-dev, linux-kernel, linux-watchdog
In-Reply-To: <529B9019.9090404@roeck-us.net>


Le 01/12/2013 20:38, Guenter Roeck a écrit :
> On 11/30/2013 07:33 AM, Christophe Leroy wrote:
>> Convert mpc8xxx_wdt.c to the new watchdog API.
>>
>> Signed-off-by: Christophe Leroy <christophe.leroy@c-s.fr>
>>
>> diff -ur a/drivers/watchdog/mpc8xxx_wdt.c 
>> b/drivers/watchdog/mpc8xxx_wdt.c
>> --- a/drivers/watchdog/mpc8xxx_wdt.c    2013-05-11 22:57:46.000000000 
>> +0200
>> +++ b/drivers/watchdog/mpc8xxx_wdt.c    2013-11-30 16:14:53.803495472 
>> +0100
>> @@ -72,28 +72,31 @@
>>    * to 0
>>    */
>>   static int prescale = 1;
>> -static unsigned int timeout_sec;
>>
>> -static unsigned long wdt_is_open;
>>   static DEFINE_SPINLOCK(wdt_spinlock);
>>
>> -static void mpc8xxx_wdt_keepalive(void)
>> +static int mpc8xxx_wdt_ping(struct watchdog_device *w)
>>   {
>>       /* Ping the WDT */
>>       spin_lock(&wdt_spinlock);
>>       out_be16(&wd_base->swsrr, 0x556c);
>>       out_be16(&wd_base->swsrr, 0xaa39);
>>       spin_unlock(&wdt_spinlock);
>> +    return 0;
>
> The return code is never checked, so you can make this function void.
watchdog .h expects an int function.
Wouldn't it be an error to make it void, if for instance in the future 
the return code is checked by the core ?
>
>>   }
>>
>> +static struct watchdog_device mpc8xxx_wdt_dev;
>>   static void mpc8xxx_wdt_timer_ping(unsigned long arg);
>> -static DEFINE_TIMER(wdt_timer, mpc8xxx_wdt_timer_ping, 0, 0);
>> +static DEFINE_TIMER(wdt_timer, mpc8xxx_wdt_timer_ping, 0,
>> +        (unsigned long)&mpc8xxx_wdt_dev);
>>
>>   static void mpc8xxx_wdt_timer_ping(unsigned long arg)
>>   {
>> -    mpc8xxx_wdt_keepalive();
>> +    struct watchdog_device *w = (struct watchdog_device *)arg;
>> +
>> +    mpc8xxx_wdt_ping(w);
>>       /* We're pinging it twice faster than needed, just to be sure. */
>> -    mod_timer(&wdt_timer, jiffies + HZ * timeout_sec / 2);
>> +    mod_timer(&wdt_timer, jiffies + HZ * w->timeout / 2);
>>   }
>>
>>   static void mpc8xxx_wdt_pr_warn(const char *msg)
>> @@ -102,19 +105,9 @@
>>           reset ? "reset" : "machine check exception");
>>   }
>>
>> -static ssize_t mpc8xxx_wdt_write(struct file *file, const char 
>> __user *buf,
>> -                 size_t count, loff_t *ppos)
>> -{
>> -    if (count)
>> -        mpc8xxx_wdt_keepalive();
>> -    return count;
>> -}
>> -
>> -static int mpc8xxx_wdt_open(struct inode *inode, struct file *file)
>> +static int mpc8xxx_wdt_start(struct watchdog_device *w)
>>   {
>>       u32 tmp = SWCRR_SWEN;
>> -    if (test_and_set_bit(0, &wdt_is_open))
>> -        return -EBUSY;
>>
>>       /* Once we start the watchdog we can't stop it */
>>       if (nowayout)
>
> This code and the subsequent module_get can be removed; it is handled 
> by the infrastructure.
>
>
>> @@ -132,59 +125,30 @@
>>
>>       del_timer_sync(&wdt_timer);
>>
>> -    return nonseekable_open(inode, file);
>> +    return 0;
>>   }
>>
>> -static int mpc8xxx_wdt_release(struct inode *inode, struct file *file)
>> +static int mpc8xxx_wdt_stop(struct watchdog_device *w)
>>   {
>> -    if (!nowayout)
>> -        mpc8xxx_wdt_timer_ping(0);
>> -    else
>> -        mpc8xxx_wdt_pr_warn("watchdog closed");
>> -    clear_bit(0, &wdt_is_open);
>> +    mpc8xxx_wdt_timer_ping((unsigned long)w);
>
> I really dislike this typecasting back and forth.
>
> I think it would be better to move the mod_timer() call from 
> mpc8xxx_wdt_timer_ping
> into mpc8xxx_wdt_ping and call mpc8xxx_wdt_ping directly whenever 
> possible.
> From mpc8xxx_wdt_timer_ping you can then just call mpc8xxx_wdt_ping 
> with the
> typecasted parameter.
I'm not sure I understand what you mean.
mpc8xxx_wdt_ping() is called by the core when the user app writes into 
/dev/watchdog.
We don't want the set the timer in that case.
The timer is only used for when no user app has opened /dev/watchdog yet.

>
>>       return 0;
>>   }
>>
>> -static long mpc8xxx_wdt_ioctl(struct file *file, unsigned int cmd,
>> -                            unsigned long arg)
>> -{
>> -    void __user *argp = (void __user *)arg;
>> -    int __user *p = argp;
>> -    static const struct watchdog_info ident = {
>> -        .options = WDIOF_KEEPALIVEPING,
>> -        .firmware_version = 1,
>> -        .identity = "MPC8xxx",
>> -    };
>> -
>> -    switch (cmd) {
>> -    case WDIOC_GETSUPPORT:
>> -        return copy_to_user(argp, &ident, sizeof(ident)) ? -EFAULT : 0;
>> -    case WDIOC_GETSTATUS:
>> -    case WDIOC_GETBOOTSTATUS:
>> -        return put_user(0, p);
>> -    case WDIOC_KEEPALIVE:
>> -        mpc8xxx_wdt_keepalive();
>> -        return 0;
>> -    case WDIOC_GETTIMEOUT:
>> -        return put_user(timeout_sec, p);
>> -    default:
>> -        return -ENOTTY;
>> -    }
>> -}
>> +static struct watchdog_info mpc8xxx_wdt_info = {
>> +    .options = WDIOF_KEEPALIVEPING,
>> +    .firmware_version = 1,
>> +    .identity = "MPC8xxx",
>> +};
>>
>> -static const struct file_operations mpc8xxx_wdt_fops = {
>> -    .owner        = THIS_MODULE,
>> -    .llseek        = no_llseek,
>> -    .write        = mpc8xxx_wdt_write,
>> -    .unlocked_ioctl    = mpc8xxx_wdt_ioctl,
>> -    .open        = mpc8xxx_wdt_open,
>> -    .release    = mpc8xxx_wdt_release,
>> +static struct watchdog_ops mpc8xxx_wdt_ops = {
>> +    .owner = THIS_MODULE,
>> +    .start = mpc8xxx_wdt_start,
>> +    .stop = mpc8xxx_wdt_stop,
>>   };
>>
>> -static struct miscdevice mpc8xxx_wdt_miscdev = {
>> -    .minor    = WATCHDOG_MINOR,
>> -    .name    = "watchdog",
>> -    .fops    = &mpc8xxx_wdt_fops,
>> +static struct watchdog_device mpc8xxx_wdt_dev = {
>> +    .info = &mpc8xxx_wdt_info,
>> +    .ops = &mpc8xxx_wdt_ops,
>>   };
>>
>>   static const struct of_device_id mpc8xxx_wdt_match[];
>> @@ -196,6 +160,7 @@
>>       const struct mpc8xxx_wdt_type *wdt_type;
>>       u32 freq = fsl_get_sys_freq();
>>       bool enabled;
>> +    unsigned int timeout_sec;
>>
>>       match = of_match_device(mpc8xxx_wdt_match, &ofdev->dev);
>>       if (!match)
>> @@ -222,6 +187,7 @@
>>       else
>>           timeout_sec = timeout / freq;
>>
>> +    mpc8xxx_wdt_dev.timeout = timeout_sec;
>>   #ifdef MODULE
>>       ret = mpc8xxx_wdt_init_late();
>>       if (ret)
>> @@ -237,7 +203,7 @@
>>        * userspace handles it.
>>        */
>>       if (enabled)
>> -        mpc8xxx_wdt_timer_ping(0);
>> +        mpc8xxx_wdt_timer_ping((unsigned long)&mpc8xxx_wdt_dev);
>>       return 0;
>>   err_unmap:
>>       iounmap(wd_base);
>> @@ -249,7 +215,7 @@
>>   {
>>       mpc8xxx_wdt_pr_warn("watchdog removed");
>
> The mpc8xxx_wdt_pr_warn function is now rather unnecessary since
> it is called only once. Might as well merge it into this function.
>
>>       del_timer_sync(&wdt_timer);
>> -    misc_deregister(&mpc8xxx_wdt_miscdev);
>> +    watchdog_unregister_device(&mpc8xxx_wdt_dev);
>>       iounmap(wd_base);
>>
>>       return 0;
>> @@ -301,10 +267,11 @@
>>       if (!wd_base)
>>           return -ENODEV;
>>
>> -    ret = misc_register(&mpc8xxx_wdt_miscdev);
>> +    watchdog_set_nowayout(&mpc8xxx_wdt_dev, nowayout);
>> +
>> +    ret = watchdog_register_device(&mpc8xxx_wdt_dev);
>>       if (ret) {
>> -        pr_err("cannot register miscdev on minor=%d (err=%d)\n",
>> -               WATCHDOG_MINOR, ret);
>> +        pr_err("cannot register watchdog device (err=%d)\n", ret);
>>           return ret;
>>       }
>>       return 0;
>> diff -ur a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
>> --- a/drivers/watchdog/Kconfig
>> +++ b/drivers/watchdog/Kconfig
>> @@ -1146,6 +1146,7 @@
>>   config 8xxx_WDT
>>       tristate "MPC8xxx Platform Watchdog Timer"
>>       depends on PPC_8xx || PPC_83xx || PPC_86xx
>> +    select WATCHDOG_CORE
>>       help
>>         This driver is for a SoC level watchdog that exists on some
>>         Freescale PowerPC processors. So far this driver supports:
>>
>> ---
>> Ce courrier électronique ne contient aucun virus ou logiciel 
>> malveillant parce que la protection avast! Antivirus est active.
>> http://www.avast.com
>>
>> -- 
>> To unsubscribe from this list: send the line "unsubscribe 
>> linux-watchdog" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>>
>

^ permalink raw reply

* Re: 3.13 Oops on ppc64_cpu --smt=off
From: Preeti U Murthy @ 2013-12-02 11:20 UTC (permalink / raw)
  To: Alexander Graf; +Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <6466B80A-5F19-4D1B-968F-AE19B28EF8DB@suse.de>

Hi,

On 12/02/2013 03:27 PM, Alexander Graf wrote:
> 
> On 02.12.2013, at 05:01, Preeti U Murthy <preeti@linux.vnet.ibm.com> wrote:
> 
>> Hi,
>>
>> On 11/30/2013 11:15 PM, Alexander Graf wrote:
>>> Hi Ben,
>>>
>>> With current linus master (3.13-rc2+) I'm facing an interesting issue with
>>
>> SMT disabling on p7. When I trigger the cpu offlining it works as expected,
>> but after a few seconds the machine goes into an oops as you can see below.
>>>
>>> It looks like a null pointer dereference.
>>
>> tip/sched/urgent has the below fix. Can you please apply the following it and
>> check if the issue gets resolved?  A similar issue was reported earlier as
> 
> I've disabled NO_HZ now on that machine which also "fixed" it for me. Unfortunately I can't reboot that box for at least the next week now to test whether the patch does fix the issue.

The commit 37dc6b50cee9 that has caused this regression is around NO_HZ.
It decides when to kick nohz idle balancing.

Regards
Preeti U Murthy
> 
> 
> Alex
> 

^ permalink raw reply

* Re: 3.13 Oops on ppc64_cpu --smt=off
From: Alexander Graf @ 2013-12-02  9:57 UTC (permalink / raw)
  To: Preeti U Murthy; +Cc: Paul Mackerras, linuxppc-dev
In-Reply-To: <529C0614.6070708@linux.vnet.ibm.com>


On 02.12.2013, at 05:01, Preeti U Murthy <preeti@linux.vnet.ibm.com> =
wrote:

> Hi,
>=20
> On 11/30/2013 11:15 PM, Alexander Graf wrote:
>> Hi Ben,
>>=20
>> With current linus master (3.13-rc2+) I'm facing an interesting issue =
with
>=20
> SMT disabling on p7. When I trigger the cpu offlining it works as =
expected,
> but after a few seconds the machine goes into an oops as you can see =
below.
>>=20
>> It looks like a null pointer dereference.
>=20
> tip/sched/urgent has the below fix. Can you please apply the following =
it and
> check if the issue gets resolved?  A similar issue was reported =
earlier as

I've disabled NO_HZ now on that machine which also "fixed" it for me. =
Unfortunately I can't reboot that box for at least the next week now to =
test whether the patch does fix the issue.


Alex

^ permalink raw reply

* Re: ping: re: [PATCH 1/1] kernel code that do not handle NULL return of kmem_cache_zalloc
From: Alexander Graf @ 2013-12-02  9:54 UTC (permalink / raw)
  To: Zhouyi Zhou
  Cc: kvm@vger.kernel.org mailing list, Zhouyi Zhou, Joerg Roedel,
	linuxppc-dev, kvm-ppc, linux-kernel@vger.kernel.org list,
	Paul Mackerras, David Woodhouse
In-Reply-To: <e9c283.a967.142b14600da.Coremail.yizhouzhou@ict.ac.cn>


On 02.12.2013, at 04:07, Zhouyi Zhou <yizhouzhou@ict.ac.cn> wrote:

> ping
>>  I do a grep for kmem_cache_zalloc and kmem_cache_alloc
>>  in kernel tree, and find some code do not handle NULL
>>  return of kmem_cache_zalloc correctly
>> =20
>> =20
>>  Signed-off-by: Zhouyi Zhou <yizhouzhou@ict.ac.cn>

Thanks a lot for the patch. I'd assume we want something slightly more =
clever even, similar to Paul's d78bca729.

Please beware that it's usually not a good idea to post patches that =
span multiple subtrees. I don't want to apply a patch to my kvm tree =
that touches jffs2 for example, as that's out of my scope. It usually =
makes a maintainer's life easier if you split up a patch like this =
according to tree responsibilities.


Alex

^ permalink raw reply

* Re: [PATCH v6 08/17] spi: mpc512x: adjust to OF based clock lookup
From: Mark Rutland @ 2013-12-02  9:50 UTC (permalink / raw)
  To: Gerhard Sittig
  Cc: Mike Turquette, Detlev Zundel, linux-spi@vger.kernel.org,
	Mark Brown, Scott Wood, Anatolij Gustschin,
	linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org
In-Reply-To: <1385851897-23475-9-git-send-email-gsi@denx.de>

On Sat, Nov 30, 2013 at 10:51:28PM +0000, Gerhard Sittig wrote:
> after device tree based clock lookup became available, the peripheral
> driver need no longer construct clock names which include the PSC index,
> remove the "psc%d_mclk" template and unconditionally use 'mclk'
> 
> acquire and release the 'ipg' clock item for register access as well

For this and the other peripheral updates, it would be nice to have the
expected clock-names entries added to the respective binding documents.
Otherwise this looks fine to me.

Thanks,
Mark.

> 
> Cc: Mark Brown <broonie@kernel.org>
> Cc: linux-spi@vger.kernel.org
> Signed-off-by: Gerhard Sittig <gsi@denx.de>
> ---
>  drivers/spi/spi-mpc512x-psc.c |   26 ++++++++++++++++++--------
>  1 file changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/spi/spi-mpc512x-psc.c b/drivers/spi/spi-mpc512x-psc.c
> index 9602bbd8d7ea..de66c676c248 100644
> --- a/drivers/spi/spi-mpc512x-psc.c
> +++ b/drivers/spi/spi-mpc512x-psc.c
> @@ -40,6 +40,7 @@ struct mpc512x_psc_spi {
>  	unsigned int irq;
>  	u8 bits_per_word;
>  	struct clk *clk_mclk;
> +	struct clk *clk_ipg;
>  	u32 mclk_rate;
>  
>  	struct completion txisrdone;
> @@ -475,8 +476,6 @@ static int mpc512x_psc_spi_do_probe(struct device *dev, u32 regaddr,
>  	struct spi_master *master;
>  	int ret;
>  	void *tempp;
> -	int psc_num;
> -	char clk_name[16];
>  	struct clk *clk;
>  
>  	master = spi_alloc_master(dev, sizeof *mps);
> @@ -520,9 +519,7 @@ static int mpc512x_psc_spi_do_probe(struct device *dev, u32 regaddr,
>  		goto free_master;
>  	init_completion(&mps->txisrdone);
>  
> -	psc_num = master->bus_num;
> -	snprintf(clk_name, sizeof(clk_name), "psc%d_mclk", psc_num);
> -	clk = devm_clk_get(dev, clk_name);
> +	clk = devm_clk_get(dev, "mclk");
>  	if (IS_ERR(clk)) {
>  		ret = PTR_ERR(clk);
>  		goto free_irq;
> @@ -533,17 +530,29 @@ static int mpc512x_psc_spi_do_probe(struct device *dev, u32 regaddr,
>  	mps->clk_mclk = clk;
>  	mps->mclk_rate = clk_get_rate(clk);
>  
> +	clk = devm_clk_get(dev, "ipg");
> +	if (IS_ERR(clk)) {
> +		ret = PTR_ERR(clk);
> +		goto free_mclk_clock;
> +	}
> +	ret = clk_prepare_enable(clk);
> +	if (ret)
> +		goto free_mclk_clock;
> +	mps->clk_ipg = clk;
> +
>  	ret = mpc512x_psc_spi_port_config(master, mps);
>  	if (ret < 0)
> -		goto free_clock;
> +		goto free_ipg_clock;
>  
>  	ret = devm_spi_register_master(dev, master);
>  	if (ret < 0)
> -		goto free_clock;
> +		goto free_ipg_clock;
>  
>  	return ret;
>  
> -free_clock:
> +free_ipg_clock:
> +	clk_disable_unprepare(mps->clk_ipg);
> +free_mclk_clock:
>  	clk_disable_unprepare(mps->clk_mclk);
>  free_irq:
>  	free_irq(mps->irq, mps);
> @@ -561,6 +570,7 @@ static int mpc512x_psc_spi_do_remove(struct device *dev)
>  	struct mpc512x_psc_spi *mps = spi_master_get_devdata(master);
>  
>  	clk_disable_unprepare(mps->clk_mclk);
> +	clk_disable_unprepare(mps->clk_ipg);
>  	free_irq(mps->irq, mps);
>  	if (mps->psc)
>  		iounmap(mps->psc);
> -- 
> 1.7.10.4
> 
> 
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
> 

^ permalink raw reply

* Re: [PATCH v6 02/17] dts: mpc512x: introduce dt-bindings/clock/ header
From: Mark Rutland @ 2013-12-02  9:46 UTC (permalink / raw)
  To: Gerhard Sittig
  Cc: devicetree@vger.kernel.org, Mike Turquette, Pawel Moll,
	Detlev Zundel, Stephen Warren, rob.herring@calxeda.com,
	Scott Wood, Anatolij Gustschin, linuxppc-dev@lists.ozlabs.org,
	linux-arm-kernel@lists.infradead.org, Ian Campbell
In-Reply-To: <1385851897-23475-3-git-send-email-gsi@denx.de>

On Sat, Nov 30, 2013 at 10:51:22PM +0000, Gerhard Sittig wrote:
> introduce a dt-bindings/ header file for MPC512x clocks,
> providing symbolic identifiers for those SoC clocks which
> clients will reference from their device tree nodes

There should be a binding document update to go with this, pointing out
that this include file defines the set of clock IDs for the MPC512x
clocks.

Otherwise, this looks fine to me.

Mark.

> 
> Cc: Rob Herring <rob.herring@calxeda.com>
> Cc: Pawel Moll <pawel.moll@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Stephen Warren <swarren@wwwdotorg.org>
> Cc: Ian Campbell <ian.campbell@citrix.com>
> Cc: devicetree@vger.kernel.org
> Reviewed-by: Mike Turquette <mturquette@linaro.org>	# for v3: w/o bdlc, PSC ipg
> Signed-off-by: Gerhard Sittig <gsi@denx.de>
> ---
>  include/dt-bindings/clock/mpc512x-clock.h |   69 +++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 include/dt-bindings/clock/mpc512x-clock.h
> 
> diff --git a/include/dt-bindings/clock/mpc512x-clock.h b/include/dt-bindings/clock/mpc512x-clock.h
> new file mode 100644
> index 000000000000..9e81b3b99a32
> --- /dev/null
> +++ b/include/dt-bindings/clock/mpc512x-clock.h
> @@ -0,0 +1,69 @@
> +/*
> + * This header provides constants for MPC512x clock specs in DT bindings.
> + */
> +
> +#ifndef _DT_BINDINGS_CLOCK_MPC512x_CLOCK_H
> +#define _DT_BINDINGS_CLOCK_MPC512x_CLOCK_H
> +
> +#define MPC512x_CLK_DUMMY		0
> +#define MPC512x_CLK_REF			1
> +#define MPC512x_CLK_SYS			2
> +#define MPC512x_CLK_DIU			3
> +#define MPC512x_CLK_VIU			4
> +#define MPC512x_CLK_CSB			5
> +#define MPC512x_CLK_E300		6
> +#define MPC512x_CLK_IPS			7
> +#define MPC512x_CLK_FEC			8
> +#define MPC512x_CLK_SATA		9
> +#define MPC512x_CLK_PATA		10
> +#define MPC512x_CLK_NFC			11
> +#define MPC512x_CLK_LPC			12
> +#define MPC512x_CLK_MBX_BUS		13
> +#define MPC512x_CLK_MBX			14
> +#define MPC512x_CLK_MBX_3D		15
> +#define MPC512x_CLK_AXE			16
> +#define MPC512x_CLK_USB1		17
> +#define MPC512x_CLK_USB2		18
> +#define MPC512x_CLK_I2C			19
> +#define MPC512x_CLK_MSCAN0_MCLK		20
> +#define MPC512x_CLK_MSCAN1_MCLK		21
> +#define MPC512x_CLK_MSCAN2_MCLK		22
> +#define MPC512x_CLK_MSCAN3_MCLK		23
> +#define MPC512x_CLK_BDLC		24
> +#define MPC512x_CLK_SDHC		25
> +#define MPC512x_CLK_PCI			26
> +#define MPC512x_CLK_PSC_MCLK_IN		27
> +#define MPC512x_CLK_SPDIF_TX		28
> +#define MPC512x_CLK_SPDIF_RX		29
> +#define MPC512x_CLK_SPDIF_MCLK		30
> +#define MPC512x_CLK_SPDIF		31
> +#define MPC512x_CLK_AC97		32
> +#define MPC512x_CLK_PSC0_MCLK		33
> +#define MPC512x_CLK_PSC1_MCLK		34
> +#define MPC512x_CLK_PSC2_MCLK		35
> +#define MPC512x_CLK_PSC3_MCLK		36
> +#define MPC512x_CLK_PSC4_MCLK		37
> +#define MPC512x_CLK_PSC5_MCLK		38
> +#define MPC512x_CLK_PSC6_MCLK		39
> +#define MPC512x_CLK_PSC7_MCLK		40
> +#define MPC512x_CLK_PSC8_MCLK		41
> +#define MPC512x_CLK_PSC9_MCLK		42
> +#define MPC512x_CLK_PSC10_MCLK		43
> +#define MPC512x_CLK_PSC11_MCLK		44
> +#define MPC512x_CLK_PSC_FIFO		45
> +#define MPC512x_CLK_PSC0		46
> +#define MPC512x_CLK_PSC1		47
> +#define MPC512x_CLK_PSC2		48
> +#define MPC512x_CLK_PSC3		49
> +#define MPC512x_CLK_PSC4		50
> +#define MPC512x_CLK_PSC5		51
> +#define MPC512x_CLK_PSC6		52
> +#define MPC512x_CLK_PSC7		53
> +#define MPC512x_CLK_PSC8		54
> +#define MPC512x_CLK_PSC9		55
> +#define MPC512x_CLK_PSC10		56
> +#define MPC512x_CLK_PSC11		57
> +
> +#define MPC512x_CLK_LAST_PUBLIC		57
> +
> +#endif
> -- 
> 1.7.10.4
> 
> 

^ permalink raw reply

* [PATCH] powerpc: fix xmon disassembler for little-endian
From: Philippe Bergheaud @ 2013-12-02  9:10 UTC (permalink / raw)
  To: Linuxppc-dev; +Cc: Philippe Bergheaud

This patch fixes the disassembler of the powerpc kernel debugger xmon,
for little-endian.

Signed-off-by: Philippe Bergheaud <felix@linux.vnet.ibm.com>
---
 arch/powerpc/xmon/xmon.c |    4 ++++
 1 file changed, 4 insertions(+)

diff --git a/arch/powerpc/xmon/xmon.c b/arch/powerpc/xmon/xmon.c
index af9d346..6c27804 100644
--- a/arch/powerpc/xmon/xmon.c
+++ b/arch/powerpc/xmon/xmon.c
@@ -171,7 +171,11 @@ extern void xmon_leave(void);
 #define REG		"%.8lx"
 #endif
 
+#ifdef __LITTLE_ENDIAN__
+#define GETWORD(v)	(((v)[3] << 24) + ((v)[2] << 16) + ((v)[1] << 8) + (v)[0])
+#else
 #define GETWORD(v)	(((v)[0] << 24) + ((v)[1] << 16) + ((v)[2] << 8) + (v)[3])
+#endif
 
 #define isxdigit(c)	(('0' <= (c) && (c) <= '9') \
 			 || ('a' <= (c) && (c) <= 'f') \
-- 
1.7.10.4

^ permalink raw reply related

* [RFC PATCH powerpc 2/2] adjust the variable type and name in __pte_free_tlb()
From: Li Zhong @ 2013-12-03  8:33 UTC (permalink / raw)
  To: PowerPC email list; +Cc: Paul Mackerras
In-Reply-To: <1386059435.2578.13.camel@ThinkPad-T5421>

The patch adjusts the variable type and name for page in
__pte_free_tlb(), which now seems a little confusing. 

Signed-off-by: Li Zhong <zhong@linux.vnet.ibm.com>
---
 arch/powerpc/include/asm/pgalloc-64.h |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/powerpc/include/asm/pgalloc-64.h b/arch/powerpc/include/asm/pgalloc-64.h
index d7543c2..1580f9e 100644
--- a/arch/powerpc/include/asm/pgalloc-64.h
+++ b/arch/powerpc/include/asm/pgalloc-64.h
@@ -148,11 +148,11 @@ static inline void pgtable_free_tlb(struct mmu_gather *tlb,
 static inline void __pte_free_tlb(struct mmu_gather *tlb, pgtable_t table,
 				  unsigned long address)
 {
-	struct page *page = page_address(table);
+	void *page_addr = page_address(table);
 
 	tlb_flush_pgtable(tlb, address);
 	pgtable_page_dtor(table);
-	pgtable_free_tlb(tlb, page, 0);
+	pgtable_free_tlb(tlb, page_addr, 0);
 }
 
 #else /* if CONFIG_PPC_64K_PAGES */

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox