Re: [PATCH][RFC] VM: x86: Return ealier if clocksource has not changed
Hi Marcelo, On Mon, Dec 26, 2016 at 05:44:25PM -0200, Marcelo Tosatti wrote: > On Fri, Dec 23, 2016 at 04:41:53PM +0800, Chen Yu wrote: > > Currently the notifier of pvclock_gtod_notify() get invoked > > frequently due to the periodic update_wall_time(). This might > > slow down the system a little bit as there might be redundant > > execution code path and unnecessary lock contention > > in update_pvclock_gtod(), which was found when I was doing > > suspend/resume speed testings. As pvclock_gtod_notify() > > should be invoked only when clocksource has changed, according to > > Commit 16e8d74d2da9 ("KVM: x86: notifier for clocksource changes") > > , either we can add a new notifier for clocksource switch, > > or we can simply bypass the following code in pvclock_gtod_notify() > > earlier if there is no clocksource switch. > > > > Cc: Paolo Bonzini> > Cc: "Radim Krcmar" > > Cc: Marcelo Tosatti > > Cc: Thomas Gleixner > > Cc: Ingo Molnar > > Cc: "H. Peter Anvin" > > Cc: Wanpeng Li > > Signed-off-by: Chen Yu > > --- > > arch/x86/kvm/x86.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 445c51b..54aa32d 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -5961,13 +5961,14 @@ static int pvclock_gtod_notify(struct > > notifier_block *nb, unsigned long unused, > > struct pvclock_gtod_data *gtod = _gtod_data; > > struct timekeeper *tk = priv; > > > > + if (likely(gtod->clock.vclock_mode == VCLOCK_TSC)) > > + return 0; > > I think this is only safe if any of the values in "struct > pvclock_gtod_data" are unchanged. Otherwise the local (KVM) copy is > kept incorrect. I missread the code previously and I thought only under the condition the clocksource has been switched to another one will the KVM copy be touched. Apparently it is not the case because the copy should be updated on-time during normal tick, right? thanks for your reply, Yu > > > update_pvclock_gtod(tk); > > > > /* disable master clock if host does not trust, or does not > > * use, TSC clocksource > > */ > > - if (gtod->clock.vclock_mode != VCLOCK_TSC && > > - atomic_read(_guest_has_master_clock) != 0) > > + if (atomic_read(_guest_has_master_clock) != 0) > > queue_work(system_long_wq, _gtod_work); > > > > return 0; > > -- > > 2.7.4
Re: [PATCH][RFC] VM: x86: Return ealier if clocksource has not changed
Hi Marcelo, On Mon, Dec 26, 2016 at 05:44:25PM -0200, Marcelo Tosatti wrote: > On Fri, Dec 23, 2016 at 04:41:53PM +0800, Chen Yu wrote: > > Currently the notifier of pvclock_gtod_notify() get invoked > > frequently due to the periodic update_wall_time(). This might > > slow down the system a little bit as there might be redundant > > execution code path and unnecessary lock contention > > in update_pvclock_gtod(), which was found when I was doing > > suspend/resume speed testings. As pvclock_gtod_notify() > > should be invoked only when clocksource has changed, according to > > Commit 16e8d74d2da9 ("KVM: x86: notifier for clocksource changes") > > , either we can add a new notifier for clocksource switch, > > or we can simply bypass the following code in pvclock_gtod_notify() > > earlier if there is no clocksource switch. > > > > Cc: Paolo Bonzini > > Cc: "Radim Krcmar" > > Cc: Marcelo Tosatti > > Cc: Thomas Gleixner > > Cc: Ingo Molnar > > Cc: "H. Peter Anvin" > > Cc: Wanpeng Li > > Signed-off-by: Chen Yu > > --- > > arch/x86/kvm/x86.c | 5 +++-- > > 1 file changed, 3 insertions(+), 2 deletions(-) > > > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 445c51b..54aa32d 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -5961,13 +5961,14 @@ static int pvclock_gtod_notify(struct > > notifier_block *nb, unsigned long unused, > > struct pvclock_gtod_data *gtod = _gtod_data; > > struct timekeeper *tk = priv; > > > > + if (likely(gtod->clock.vclock_mode == VCLOCK_TSC)) > > + return 0; > > I think this is only safe if any of the values in "struct > pvclock_gtod_data" are unchanged. Otherwise the local (KVM) copy is > kept incorrect. I missread the code previously and I thought only under the condition the clocksource has been switched to another one will the KVM copy be touched. Apparently it is not the case because the copy should be updated on-time during normal tick, right? thanks for your reply, Yu > > > update_pvclock_gtod(tk); > > > > /* disable master clock if host does not trust, or does not > > * use, TSC clocksource > > */ > > - if (gtod->clock.vclock_mode != VCLOCK_TSC && > > - atomic_read(_guest_has_master_clock) != 0) > > + if (atomic_read(_guest_has_master_clock) != 0) > > queue_work(system_long_wq, _gtod_work); > > > > return 0; > > -- > > 2.7.4
[PATCH] net: fix incorrect original ingress device index in PKTINFO
When we send a packet for our own local address on a non-loopback interface (e.g. eth0), due to the change had been introduced from commit 0b922b7a829c ("net: original ingress device index in PKTINFO"), the original ingress device index would be set as the loopback interface. However, the packet should be considered as if it is being arrived via the sending interface (eth0), otherwise it would break the expectation of the userspace application (e.g. the DHCPRELEASE message from dhcp_release binary would be ignored by the dnsmasq daemon) Signed-off-by: Wei Zhang--- net/ipv4/ip_sockglue.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c index b8a2d63..b6a6d35 100644 --- a/net/ipv4/ip_sockglue.c +++ b/net/ipv4/ip_sockglue.c @@ -1202,8 +1202,13 @@ void ipv4_pktinfo_prepare(const struct sock *sk, struct sk_buff *skb) * which has interface index (iif) as the first member of the * underlying inet{6}_skb_parm struct. This code then overlays * PKTINFO_SKB_CB and in_pktinfo also has iif as the first -* element so the iif is picked up from the prior IPCB +* element so the iif is picked up from the prior IPCB except +* iif is loopback interface which the packet should be +* considered as if it is being arrived via the sending interface */ + if (pktinfo->ipi_ifindex == LOOPBACK_IFINDEX) { + pktinfo->ipi_ifindex = inet_iif(skb); + } pktinfo->ipi_spec_dst.s_addr = fib_compute_spec_dst(skb); } else { pktinfo->ipi_ifindex = 0; -- 1.8.3.1
[PATCH] net: fix incorrect original ingress device index in PKTINFO
When we send a packet for our own local address on a non-loopback interface (e.g. eth0), due to the change had been introduced from commit 0b922b7a829c ("net: original ingress device index in PKTINFO"), the original ingress device index would be set as the loopback interface. However, the packet should be considered as if it is being arrived via the sending interface (eth0), otherwise it would break the expectation of the userspace application (e.g. the DHCPRELEASE message from dhcp_release binary would be ignored by the dnsmasq daemon) Signed-off-by: Wei Zhang --- net/ipv4/ip_sockglue.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c index b8a2d63..b6a6d35 100644 --- a/net/ipv4/ip_sockglue.c +++ b/net/ipv4/ip_sockglue.c @@ -1202,8 +1202,13 @@ void ipv4_pktinfo_prepare(const struct sock *sk, struct sk_buff *skb) * which has interface index (iif) as the first member of the * underlying inet{6}_skb_parm struct. This code then overlays * PKTINFO_SKB_CB and in_pktinfo also has iif as the first -* element so the iif is picked up from the prior IPCB +* element so the iif is picked up from the prior IPCB except +* iif is loopback interface which the packet should be +* considered as if it is being arrived via the sending interface */ + if (pktinfo->ipi_ifindex == LOOPBACK_IFINDEX) { + pktinfo->ipi_ifindex = inet_iif(skb); + } pktinfo->ipi_spec_dst.s_addr = fib_compute_spec_dst(skb); } else { pktinfo->ipi_ifindex = 0; -- 1.8.3.1
Re: Question regarding power button of Dell XPS13
Lo! On 23.12.2016 13:36, Paul Menzel wrote: > > I heard that you both have a Dell XPS13. I got the “revision” 9360, and > installed Debian Stretch/testing on it with Linux 4.8.15 and Linux 4.9-rc8. > > When pressing the power button the GNOME dialog, asking what to do > (restart, power off, …) doesn’t appear. > > Neither `xev` nor `acpi_listen` show something, so I submitted ticket > #190871 [1], and Lv already looked at it. > > Just to make sure, that it is really a Linux problem, does the power > button work for you? TWIMC: A power button press works fine for me on Fedora 25 Workstation Edition (running 4.9 currently, but I think it worked with the stock Fedora kernel, too) on my 9360. From a quick look in the bug report you mentioned it looks a bit like you might need to enable INTEL_VBTN in your kernel config. From the Kconfig: This driver provides support for the Intel Virtual Button interface. Some laptops require this driver for power button support. HTH, Ciao, Thorsten
Re: Question regarding power button of Dell XPS13
Lo! On 23.12.2016 13:36, Paul Menzel wrote: > > I heard that you both have a Dell XPS13. I got the “revision” 9360, and > installed Debian Stretch/testing on it with Linux 4.8.15 and Linux 4.9-rc8. > > When pressing the power button the GNOME dialog, asking what to do > (restart, power off, …) doesn’t appear. > > Neither `xev` nor `acpi_listen` show something, so I submitted ticket > #190871 [1], and Lv already looked at it. > > Just to make sure, that it is really a Linux problem, does the power > button work for you? TWIMC: A power button press works fine for me on Fedora 25 Workstation Edition (running 4.9 currently, but I think it worked with the stock Fedora kernel, too) on my 9360. From a quick look in the bug report you mentioned it looks a bit like you might need to enable INTEL_VBTN in your kernel config. From the Kconfig: This driver provides support for the Intel Virtual Button interface. Some laptops require this driver for power button support. HTH, Ciao, Thorsten
Re: [PATCH v4 0/9] mm/swap: Regular page swap optimizations
Hi, On Fri, Dec 09, 2016 at 01:09:13PM -0800, Tim Chen wrote: > Change Log: > v4: > 1. Fix a bug in unlock cluster in add_swap_count_continuation(). We > should use unlock_cluster() instead of unlock_cluser_or_swap_info(). > 2. During swap off, handle race when swap slot is marked unused but allocated, > and not yet placed in swap cache. Wait for swap slot to be placed in swap > cache > and not abort swap off. > 3. Initialize n_ret in get_swap_pages(). > > v3: > 1. Fix bug that didn't check for page already in swap cache before skipping > read ahead and return null page. > 2. Fix bug that didn't try to allocate from global pool if allocation > from swap slot cache did not succeed. > 3. Fix memory allocation bug for spaces to store split up 64MB radix tree > 4. Fix problems caused by races between get_swap_page, cpu online/offline and > swap_on/off > > v2: > 1. Fix bug in the index limit used in scan_swap_map_try_ssd_cluster > when searching for empty slots in cluster. > 2. Fix bug in swap off that incorrectly determines if we still have > swap devices left. > 3. Port patches to mmotm-2016-10-11-15-46 branch > > Andrew, > > We're updating this patch series with some minor fixes. > Please consider this patch series for inclusion to 4.10. > > Times have changed. Coming generation of Solid state Block device > latencies are getting down to sub 100 usec, which is within an order of > magnitude of DRAM, and their performance is orders of magnitude higher > than the single- spindle rotational media we've swapped to historically. > > This could benefit many usage scenearios. For example cloud providers who > overcommit their memory (as VM don't use all the memory provisioned). > Having a fast swap will allow them to be more aggressive in memory > overcommit and fit more VMs to a platform. > > In our testing [see footnote], the median latency that the > kernel adds to a page fault is 15 usec, which comes quite close > to the amount that will be contributed by the underlying I/O > devices. > > The software latency comes mostly from contentions on the locks > protecting the radix tree of the swap cache and also the locks protecting > the individual swap devices. The lock contentions already consumed > 35% of cpu cycles in our test. In the very near future, > software latency will become the bottleneck to swap performnace as > block device I/O latency gets within the shouting distance of DRAM speed. > > This patch set, plus a previous patch Ying already posted > (commit: f6498b3f) reduced the median page fault latency > from 15 usec to 4 usec (375% reduction) for DRAM based pmem > block device. The patchset has used several techniqueus to reduce lock contention, for example, batching alloc/free, fine-grained lock and cluster distribution to avoid cache false-sharing. Each items has different complexity and benefits so could you show the number for each step of pathchset? It would be better to include the nubmer in each description. It helps how the patch is important when we consider complexitiy of the patch. > > Patch 1 is a clean up patch. Could it be separated patch? > Patch 2 creates a lock per cluster, this gives us a more fine graind lock > that can be used for accessing swap_map, and not lock the whole > swap device I hope you make three steps to review easier. You can create some functions like swap_map_lock and cluster_lock which are wrapper functions just hold swap_lock. It doesn't change anything performance pov but it clearly shows what kinds of lock we should use in specific context. Then, you can introduce more fine-graind lock in next patch and apply it into those wrapper functions. And last patch, you can adjust cluster distribution to avoid false-sharing. And the description should include how it's bad in testing so it's worth. Frankly speaking, although I'm huge user of bit_spin_lock(zram/zsmalloc have used it heavily), I don't like swap subsystem uses it. During zram development, it really hurts debugging due to losing lockdep. The reason zram have used it is by size concern of embedded world but server would be not critical so please consider trade-off of spinlock vs. bit_spin_lock. > Patch 3 splits the swap cache radix tree into 64MB chunks, reducing > the rate that we have to contende for the radix tree. To me, it's rather hacky. I think it might be common problem for page cache so can we think another generalized way like range_lock? Ccing Jan. > Patch 4 eliminates unnecessary page allocation for read ahead. Could it be separated patch? > Patch 5-9 create a per cpu cache of the swap slots, so we don't have > to contend on the swap device to get a swap slot or to release > a swap slot. And we allocate and release the swap slots > in batches for better efficiency. To me, idea is good although I feel the amount of code is rather huge and messy so it should include the number about the benefit, at least. And it
Re: [PATCH v4 0/9] mm/swap: Regular page swap optimizations
Hi, On Fri, Dec 09, 2016 at 01:09:13PM -0800, Tim Chen wrote: > Change Log: > v4: > 1. Fix a bug in unlock cluster in add_swap_count_continuation(). We > should use unlock_cluster() instead of unlock_cluser_or_swap_info(). > 2. During swap off, handle race when swap slot is marked unused but allocated, > and not yet placed in swap cache. Wait for swap slot to be placed in swap > cache > and not abort swap off. > 3. Initialize n_ret in get_swap_pages(). > > v3: > 1. Fix bug that didn't check for page already in swap cache before skipping > read ahead and return null page. > 2. Fix bug that didn't try to allocate from global pool if allocation > from swap slot cache did not succeed. > 3. Fix memory allocation bug for spaces to store split up 64MB radix tree > 4. Fix problems caused by races between get_swap_page, cpu online/offline and > swap_on/off > > v2: > 1. Fix bug in the index limit used in scan_swap_map_try_ssd_cluster > when searching for empty slots in cluster. > 2. Fix bug in swap off that incorrectly determines if we still have > swap devices left. > 3. Port patches to mmotm-2016-10-11-15-46 branch > > Andrew, > > We're updating this patch series with some minor fixes. > Please consider this patch series for inclusion to 4.10. > > Times have changed. Coming generation of Solid state Block device > latencies are getting down to sub 100 usec, which is within an order of > magnitude of DRAM, and their performance is orders of magnitude higher > than the single- spindle rotational media we've swapped to historically. > > This could benefit many usage scenearios. For example cloud providers who > overcommit their memory (as VM don't use all the memory provisioned). > Having a fast swap will allow them to be more aggressive in memory > overcommit and fit more VMs to a platform. > > In our testing [see footnote], the median latency that the > kernel adds to a page fault is 15 usec, which comes quite close > to the amount that will be contributed by the underlying I/O > devices. > > The software latency comes mostly from contentions on the locks > protecting the radix tree of the swap cache and also the locks protecting > the individual swap devices. The lock contentions already consumed > 35% of cpu cycles in our test. In the very near future, > software latency will become the bottleneck to swap performnace as > block device I/O latency gets within the shouting distance of DRAM speed. > > This patch set, plus a previous patch Ying already posted > (commit: f6498b3f) reduced the median page fault latency > from 15 usec to 4 usec (375% reduction) for DRAM based pmem > block device. The patchset has used several techniqueus to reduce lock contention, for example, batching alloc/free, fine-grained lock and cluster distribution to avoid cache false-sharing. Each items has different complexity and benefits so could you show the number for each step of pathchset? It would be better to include the nubmer in each description. It helps how the patch is important when we consider complexitiy of the patch. > > Patch 1 is a clean up patch. Could it be separated patch? > Patch 2 creates a lock per cluster, this gives us a more fine graind lock > that can be used for accessing swap_map, and not lock the whole > swap device I hope you make three steps to review easier. You can create some functions like swap_map_lock and cluster_lock which are wrapper functions just hold swap_lock. It doesn't change anything performance pov but it clearly shows what kinds of lock we should use in specific context. Then, you can introduce more fine-graind lock in next patch and apply it into those wrapper functions. And last patch, you can adjust cluster distribution to avoid false-sharing. And the description should include how it's bad in testing so it's worth. Frankly speaking, although I'm huge user of bit_spin_lock(zram/zsmalloc have used it heavily), I don't like swap subsystem uses it. During zram development, it really hurts debugging due to losing lockdep. The reason zram have used it is by size concern of embedded world but server would be not critical so please consider trade-off of spinlock vs. bit_spin_lock. > Patch 3 splits the swap cache radix tree into 64MB chunks, reducing > the rate that we have to contende for the radix tree. To me, it's rather hacky. I think it might be common problem for page cache so can we think another generalized way like range_lock? Ccing Jan. > Patch 4 eliminates unnecessary page allocation for read ahead. Could it be separated patch? > Patch 5-9 create a per cpu cache of the swap slots, so we don't have > to contend on the swap device to get a swap slot or to release > a swap slot. And we allocate and release the swap slots > in batches for better efficiency. To me, idea is good although I feel the amount of code is rather huge and messy so it should include the number about the benefit, at least. And it
Re: [PATCH v3 RESEND 02/11] pwm: imx: remove ipg clock
On 2016-12-26 23:55, Lukasz Majewski wrote: > From: Sascha Hauer> > The use of the ipg clock was introduced with commit 7b27c160c681 > ("pwm: i.MX: fix clock lookup"). > In the commit message it was claimed that the ipg clock is enabled for > register accesses. This is true for the ->config() callback, but not > for the ->set_enable() callback. Given that the ipg clock is not > consistently enabled for all register accesses we can assume that either > it is not required at all or that the current code does not work. > Remove the ipg clock code for now so that it's no longer in the way of > refactoring the driver. Hi Lukasz, Has my concern addressed in any way with this resend? https://lkml.org/lkml/2016/11/22/729 Breaking hardware is usually not an option :-) I checked the i.MX 7 reference manual again, and in this case the peripheral access clock is a clock line named "ipg_clk_s" (Table 12-20), with a clock root "PWM1_CLK_ROOT" (Table 5-12). In i.MX 7 all clocks are behind a single gate, so in fact it does not matter which clock we take. Given that others have peripheral access behind the "pwm" gate, I guess we should take the "pwm" gate... -- Stefan > > Signed-off-by: Sascha Hauer > Cc: Philipp Zabel > --- > [commit message text refactored by Lukasz Majewski ] > --- > Changes for v3: > - New patch > --- > drivers/pwm/pwm-imx.c | 19 +-- > 1 file changed, 1 insertion(+), 18 deletions(-) > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c > index d600fd5..70609ef2 100644 > --- a/drivers/pwm/pwm-imx.c > +++ b/drivers/pwm/pwm-imx.c > @@ -49,7 +49,6 @@ > > struct imx_chip { > struct clk *clk_per; > - struct clk *clk_ipg; > > void __iomem*mmio_base; > > @@ -204,17 +203,8 @@ static int imx_pwm_config(struct pwm_chip *chip, > struct pwm_device *pwm, int duty_ns, int period_ns) > { > struct imx_chip *imx = to_imx_chip(chip); > - int ret; > - > - ret = clk_prepare_enable(imx->clk_ipg); > - if (ret) > - return ret; > > - ret = imx->config(chip, pwm, duty_ns, period_ns); > - > - clk_disable_unprepare(imx->clk_ipg); > - > - return ret; > + return imx->config(chip, pwm, duty_ns, period_ns); > } > > static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) > @@ -293,13 +283,6 @@ static int imx_pwm_probe(struct platform_device *pdev) > return PTR_ERR(imx->clk_per); > } > > - imx->clk_ipg = devm_clk_get(>dev, "ipg"); > - if (IS_ERR(imx->clk_ipg)) { > - dev_err(>dev, "getting ipg clock failed with %ld\n", > - PTR_ERR(imx->clk_ipg)); > - return PTR_ERR(imx->clk_ipg); > - } > - > imx->chip.ops = _pwm_ops; > imx->chip.dev = >dev; > imx->chip.base = -1;
Re: [PATCH v3 RESEND 02/11] pwm: imx: remove ipg clock
On 2016-12-26 23:55, Lukasz Majewski wrote: > From: Sascha Hauer > > The use of the ipg clock was introduced with commit 7b27c160c681 > ("pwm: i.MX: fix clock lookup"). > In the commit message it was claimed that the ipg clock is enabled for > register accesses. This is true for the ->config() callback, but not > for the ->set_enable() callback. Given that the ipg clock is not > consistently enabled for all register accesses we can assume that either > it is not required at all or that the current code does not work. > Remove the ipg clock code for now so that it's no longer in the way of > refactoring the driver. Hi Lukasz, Has my concern addressed in any way with this resend? https://lkml.org/lkml/2016/11/22/729 Breaking hardware is usually not an option :-) I checked the i.MX 7 reference manual again, and in this case the peripheral access clock is a clock line named "ipg_clk_s" (Table 12-20), with a clock root "PWM1_CLK_ROOT" (Table 5-12). In i.MX 7 all clocks are behind a single gate, so in fact it does not matter which clock we take. Given that others have peripheral access behind the "pwm" gate, I guess we should take the "pwm" gate... -- Stefan > > Signed-off-by: Sascha Hauer > Cc: Philipp Zabel > --- > [commit message text refactored by Lukasz Majewski ] > --- > Changes for v3: > - New patch > --- > drivers/pwm/pwm-imx.c | 19 +-- > 1 file changed, 1 insertion(+), 18 deletions(-) > > diff --git a/drivers/pwm/pwm-imx.c b/drivers/pwm/pwm-imx.c > index d600fd5..70609ef2 100644 > --- a/drivers/pwm/pwm-imx.c > +++ b/drivers/pwm/pwm-imx.c > @@ -49,7 +49,6 @@ > > struct imx_chip { > struct clk *clk_per; > - struct clk *clk_ipg; > > void __iomem*mmio_base; > > @@ -204,17 +203,8 @@ static int imx_pwm_config(struct pwm_chip *chip, > struct pwm_device *pwm, int duty_ns, int period_ns) > { > struct imx_chip *imx = to_imx_chip(chip); > - int ret; > - > - ret = clk_prepare_enable(imx->clk_ipg); > - if (ret) > - return ret; > > - ret = imx->config(chip, pwm, duty_ns, period_ns); > - > - clk_disable_unprepare(imx->clk_ipg); > - > - return ret; > + return imx->config(chip, pwm, duty_ns, period_ns); > } > > static int imx_pwm_enable(struct pwm_chip *chip, struct pwm_device *pwm) > @@ -293,13 +283,6 @@ static int imx_pwm_probe(struct platform_device *pdev) > return PTR_ERR(imx->clk_per); > } > > - imx->clk_ipg = devm_clk_get(>dev, "ipg"); > - if (IS_ERR(imx->clk_ipg)) { > - dev_err(>dev, "getting ipg clock failed with %ld\n", > - PTR_ERR(imx->clk_ipg)); > - return PTR_ERR(imx->clk_ipg); > - } > - > imx->chip.ops = _pwm_ops; > imx->chip.dev = >dev; > imx->chip.base = -1;
Re: [PATCH 01/14] x86/cqm: Intel Resource Monitoring Documentation
> +LAZY and NOLAZY Monitoring > +-- > +LAZY: > +By default when monitoring is enabled, the RMIDs are not allocated > +immediately and allocated lazily only at the first sched_in. > +There are 2-4 RMIDs per logical processor on each package. So if a > dual > +package has 48 logical processors, there would be upto 192 RMIDs on > each > +package = total of 192x2 RMIDs. > +There is a possibility that RMIDs can runout and in that case the read > +reports an error since there was no RMID available to monitor for an > +event. > + > +NOLAZY: > +When user wants guaranteed monitoring, he can enable the 'monitoring > +mask' which is basically used to specify the packages he wants to > +monitor. The RMIDs are statically allocated at open and failure is > +indicated if RMIDs are not available. > + > +To specify monitoring on package 0 and package 1: > +#echo 0-1 > /sys/fs/cgroup/perf_event/p1/perf_event.cqm_mon_mask > + > +An error is thrown if packages not online are specified. I very much dislike both those for adding files to the perf cgroup. Drivers should really not do that. >>> >>> >>> Is the continuous monitoring the issue or the interface (adding a file in >>> perf_cgroup) ? I have not mentioned in the documentaion but this >>> continuous >>> monitoring/ monitoring mask applies only to cgroup in this patch and >>> hence >>> we thought a good place for that is in the cgroup itself because its per >>> cgroup. >>> >>> For task events , this wont apply and we are thinking of providing a >>> prctl >>> based interface for user to toggle the continous monitoring .. >> >> >> More fail.. >> I absolutely hate the second because events already have affinity. >>> The per-package NOLAZY flags are distinct than affinity. They modify the behavior of something already running on that package. Besides that, this is intended to work when there are no perf_events and perf_events cpu field is already used in cgroup events. >>> >>> This applies to continuous monitoring as well when there are no events >>> associated. Meaning if the monitoring mask is chosen and user tries to >>> enable continuous monitoring using the cgrp->cont_mon - all RMIDs are >>> allocated immediately. the mon_mask provides a way for the user to have >>> guarenteed RMIDs for both that have events and for continoous >>> monitoring(no >>> perf event associated) (assuming user uses it when user knows he would >>> definitely use it.. or else there is LAZY mode) >>> >>> Again this is cgroup specific and wont apply to task events and is needed >>> when there are no events associated. >> >> >> So no, the problem is that a driver introduces special ABI and behaviour >> that radically departs from the regular behaviour. > > > Ok , looks like the interface is the problem. Will try to fix this. We are > just trying to have a light weight monitoring > option so that its reasonable to monitor for a > very long time (like lifetime of process etc). Mainly to not have all the > perf scheduling overhead. > May be a perf event attr option is a more reasonable approach for the user > to choose the option ? (rather than some new interface like prctl / cgroup > file..) I don't see how a perf event attr option would work, since the goal of continuous monitoring is to start CQM/CMT without a perf event. An alternative is to add a single file to the cqm pmu directory. The file contains which cgroups must be continuously monitored (optionally with per-package flags): $ cat /sys/devices/intel_cmt/cgroup_cont_monitoring cgroup per-pkg flags /0;1;0;0 g1 0;0;0;0 g1/g1_10:0;0;0 g2 0:1;0;0;0 to start continuous monitoring in a cgroup (flags optional, default to all 0's): $ echo "g2/g2_1 0;1;0;0" > /sys/devices/intel_cmt/cgroup_cont_monitoring to stop it: $ echo "-g2/g2_1" Note that the cgroup name is what perf_event_attr takes now, so it's not that different from creating a perf event. Another option is to create a directory per cgroup to monitor, so: $ mkdir /sys/devices/intel_cmt/cgroup_cont_monitoring/g1 starts continuous monitoring in g1. This approach is problematic, though, because the cont_monitoring property is not hierarchical, i.e. a cgroup g1/g1_1 may need cont_monitoring while g1 doesn't. Supporting this would require to either do something funny with the cgroup name or add extra files to each folder and expose all cgroups. None of these options seem good to me. So, my money is on a single file "/sys/devices/intel_cmt/cgroup_cont_monitoring". Thoughts? Thanks, David
Re: [PATCH 01/14] x86/cqm: Intel Resource Monitoring Documentation
> +LAZY and NOLAZY Monitoring > +-- > +LAZY: > +By default when monitoring is enabled, the RMIDs are not allocated > +immediately and allocated lazily only at the first sched_in. > +There are 2-4 RMIDs per logical processor on each package. So if a > dual > +package has 48 logical processors, there would be upto 192 RMIDs on > each > +package = total of 192x2 RMIDs. > +There is a possibility that RMIDs can runout and in that case the read > +reports an error since there was no RMID available to monitor for an > +event. > + > +NOLAZY: > +When user wants guaranteed monitoring, he can enable the 'monitoring > +mask' which is basically used to specify the packages he wants to > +monitor. The RMIDs are statically allocated at open and failure is > +indicated if RMIDs are not available. > + > +To specify monitoring on package 0 and package 1: > +#echo 0-1 > /sys/fs/cgroup/perf_event/p1/perf_event.cqm_mon_mask > + > +An error is thrown if packages not online are specified. I very much dislike both those for adding files to the perf cgroup. Drivers should really not do that. >>> >>> >>> Is the continuous monitoring the issue or the interface (adding a file in >>> perf_cgroup) ? I have not mentioned in the documentaion but this >>> continuous >>> monitoring/ monitoring mask applies only to cgroup in this patch and >>> hence >>> we thought a good place for that is in the cgroup itself because its per >>> cgroup. >>> >>> For task events , this wont apply and we are thinking of providing a >>> prctl >>> based interface for user to toggle the continous monitoring .. >> >> >> More fail.. >> I absolutely hate the second because events already have affinity. >>> The per-package NOLAZY flags are distinct than affinity. They modify the behavior of something already running on that package. Besides that, this is intended to work when there are no perf_events and perf_events cpu field is already used in cgroup events. >>> >>> This applies to continuous monitoring as well when there are no events >>> associated. Meaning if the monitoring mask is chosen and user tries to >>> enable continuous monitoring using the cgrp->cont_mon - all RMIDs are >>> allocated immediately. the mon_mask provides a way for the user to have >>> guarenteed RMIDs for both that have events and for continoous >>> monitoring(no >>> perf event associated) (assuming user uses it when user knows he would >>> definitely use it.. or else there is LAZY mode) >>> >>> Again this is cgroup specific and wont apply to task events and is needed >>> when there are no events associated. >> >> >> So no, the problem is that a driver introduces special ABI and behaviour >> that radically departs from the regular behaviour. > > > Ok , looks like the interface is the problem. Will try to fix this. We are > just trying to have a light weight monitoring > option so that its reasonable to monitor for a > very long time (like lifetime of process etc). Mainly to not have all the > perf scheduling overhead. > May be a perf event attr option is a more reasonable approach for the user > to choose the option ? (rather than some new interface like prctl / cgroup > file..) I don't see how a perf event attr option would work, since the goal of continuous monitoring is to start CQM/CMT without a perf event. An alternative is to add a single file to the cqm pmu directory. The file contains which cgroups must be continuously monitored (optionally with per-package flags): $ cat /sys/devices/intel_cmt/cgroup_cont_monitoring cgroup per-pkg flags /0;1;0;0 g1 0;0;0;0 g1/g1_10:0;0;0 g2 0:1;0;0;0 to start continuous monitoring in a cgroup (flags optional, default to all 0's): $ echo "g2/g2_1 0;1;0;0" > /sys/devices/intel_cmt/cgroup_cont_monitoring to stop it: $ echo "-g2/g2_1" Note that the cgroup name is what perf_event_attr takes now, so it's not that different from creating a perf event. Another option is to create a directory per cgroup to monitor, so: $ mkdir /sys/devices/intel_cmt/cgroup_cont_monitoring/g1 starts continuous monitoring in g1. This approach is problematic, though, because the cont_monitoring property is not hierarchical, i.e. a cgroup g1/g1_1 may need cont_monitoring while g1 doesn't. Supporting this would require to either do something funny with the cgroup name or add extra files to each folder and expose all cgroups. None of these options seem good to me. So, my money is on a single file "/sys/devices/intel_cmt/cgroup_cont_monitoring". Thoughts? Thanks, David
Re: [patch 2/2] x86/mce/AMD: Make the init code more robust
On Mon, Dec 26, 2016 at 10:58:20PM +0100, Thomas Gleixner wrote: > If mce_device_init() fails then the mce device pointer is NULL and the AMD > mce code happily dereferences it. > > Add a sanity check. > > Reported-by: Markus Trippelsdorf> Reported-by: Boris Ostrovsky > Signed-off-by: Thomas Gleixner > --- > arch/x86/kernel/cpu/mcheck/mce_amd.c |3 +++ > 1 file changed, 3 insertions(+) > > --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c > +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c > @@ -1182,6 +1182,9 @@ static int threshold_create_bank(unsigne > const char *name = get_name(bank, NULL); > int err = 0; > > + if (!dev) > + return -ENODEV; > + > if (is_shared_bank(bank)) { > nb = node_to_amd_nb(amd_get_nb_id(cpu)); Acked-by: Borislav Petkov >From looking at that code, though, it could use some more involved straightening later. /me puts it on TODO list. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --
Re: [patch 2/2] x86/mce/AMD: Make the init code more robust
On Mon, Dec 26, 2016 at 10:58:20PM +0100, Thomas Gleixner wrote: > If mce_device_init() fails then the mce device pointer is NULL and the AMD > mce code happily dereferences it. > > Add a sanity check. > > Reported-by: Markus Trippelsdorf > Reported-by: Boris Ostrovsky > Signed-off-by: Thomas Gleixner > --- > arch/x86/kernel/cpu/mcheck/mce_amd.c |3 +++ > 1 file changed, 3 insertions(+) > > --- a/arch/x86/kernel/cpu/mcheck/mce_amd.c > +++ b/arch/x86/kernel/cpu/mcheck/mce_amd.c > @@ -1182,6 +1182,9 @@ static int threshold_create_bank(unsigne > const char *name = get_name(bank, NULL); > int err = 0; > > + if (!dev) > + return -ENODEV; > + > if (is_shared_bank(bank)) { > nb = node_to_amd_nb(amd_get_nb_id(cpu)); Acked-by: Borislav Petkov >From looking at that code, though, it could use some more involved straightening later. /me puts it on TODO list. -- Regards/Gruss, Boris. ECO tip #101: Trim your mails when you reply. --
surface3_button.c:undefined reference to `i2c_register_driver'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 0dad3a3014a0b9e72521ff44f17e0054f43dcdea commit: 1a64b719d3ae0e4fb939d9a9e31abb60b4ce4eb1 platform/x86: Introduce button support for the Surface 3 date: 10 days ago config: x86_64-randconfig-ne0-12271055 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: git checkout 1a64b719d3ae0e4fb939d9a9e31abb60b4ce4eb1 # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/built-in.o: In function `surface3_driver_init': >> surface3_button.c:(.init.text+0x75cb0): undefined reference to >> `i2c_register_driver' drivers/built-in.o: In function `surface3_driver_exit': >> surface3_button.c:(.exit.text+0x31a8): undefined reference to >> `i2c_del_driver' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
surface3_button.c:undefined reference to `i2c_register_driver'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 0dad3a3014a0b9e72521ff44f17e0054f43dcdea commit: 1a64b719d3ae0e4fb939d9a9e31abb60b4ce4eb1 platform/x86: Introduce button support for the Surface 3 date: 10 days ago config: x86_64-randconfig-ne0-12271055 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: git checkout 1a64b719d3ae0e4fb939d9a9e31abb60b4ce4eb1 # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): drivers/built-in.o: In function `surface3_driver_init': >> surface3_button.c:(.init.text+0x75cb0): undefined reference to >> `i2c_register_driver' drivers/built-in.o: In function `surface3_driver_exit': >> surface3_button.c:(.exit.text+0x31a8): undefined reference to >> `i2c_del_driver' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
Hi Arnd, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 0dad3a3014a0b9e72521ff44f17e0054f43dcdea commit: a76bcf557ef408b368cf26f52a60865bfc27b632 Kbuild: enable -Wmaybe-uninitialized warning for "make W=1" date: 7 weeks ago config: x86_64-randconfig-s0-12271426 (attached as .config) compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7 reproduce: git checkout a76bcf557ef408b368cf26f52a60865bfc27b632 # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): cc1: warnings being treated as errors drivers/gpu/drm/i915/i915_gem_gtt.c: In function 'gen8_ggtt_insert_entries': drivers/gpu/drm/i915/i915_gem_gtt.c:2341: error: 'gtt_entry' may be used uninitialized in this function drivers/gpu/drm/i915/i915_gem_gtt.c: In function 'gen6_ggtt_insert_entries': drivers/gpu/drm/i915/i915_gem_gtt.c:2438: error: 'gtt_entry' may be used uninitialized in this function At top level: >> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized" -- cc1: warnings being treated as errors drivers/gpu/drm/i915/i915_gem.c: In function 'i915_gem_object_map': drivers/gpu/drm/i915/i915_gem.c:2382: error: 'pgprot.pgprot' may be used uninitialized in this function At top level: >> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized" -- cc1: warnings being treated as errors drivers/gpu/drm/i915/intel_ringbuffer.c: In function 'intel_ring_setup_status_page': drivers/gpu/drm/i915/intel_ringbuffer.c:438: error: 'mmio.reg' may be used uninitialized in this function At top level: >> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized" --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
cc1: error: unrecognized command line option "-Wno-maybe-uninitialized"
Hi Arnd, FYI, the error/warning still remains. tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master head: 0dad3a3014a0b9e72521ff44f17e0054f43dcdea commit: a76bcf557ef408b368cf26f52a60865bfc27b632 Kbuild: enable -Wmaybe-uninitialized warning for "make W=1" date: 7 weeks ago config: x86_64-randconfig-s0-12271426 (attached as .config) compiler: gcc-4.4 (Debian 4.4.7-8) 4.4.7 reproduce: git checkout a76bcf557ef408b368cf26f52a60865bfc27b632 # save the attached .config to linux build tree make ARCH=x86_64 All errors (new ones prefixed by >>): cc1: warnings being treated as errors drivers/gpu/drm/i915/i915_gem_gtt.c: In function 'gen8_ggtt_insert_entries': drivers/gpu/drm/i915/i915_gem_gtt.c:2341: error: 'gtt_entry' may be used uninitialized in this function drivers/gpu/drm/i915/i915_gem_gtt.c: In function 'gen6_ggtt_insert_entries': drivers/gpu/drm/i915/i915_gem_gtt.c:2438: error: 'gtt_entry' may be used uninitialized in this function At top level: >> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized" -- cc1: warnings being treated as errors drivers/gpu/drm/i915/i915_gem.c: In function 'i915_gem_object_map': drivers/gpu/drm/i915/i915_gem.c:2382: error: 'pgprot.pgprot' may be used uninitialized in this function At top level: >> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized" -- cc1: warnings being treated as errors drivers/gpu/drm/i915/intel_ringbuffer.c: In function 'intel_ring_setup_status_page': drivers/gpu/drm/i915/intel_ringbuffer.c:438: error: 'mmio.reg' may be used uninitialized in this function At top level: >> cc1: error: unrecognized command line option "-Wno-maybe-uninitialized" --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[PATCH] virtio-crypto: support crypto engine framework
crypto engine was introduced since 'commit 735d37b5424b ("crypto: engine - Introduce the block request crypto engine framework")' which uses work queue to realize the asynchronous processing for ablk_cipher and ahash. For virtio-crypto device, I register an engine for each data virtqueue so that we can use the capability of multiple data queues in future. Cc: Baolin WangCc: Herbert Xu Cc: Michael S. Tsirkin Signed-off-by: Gonglei --- drivers/crypto/virtio/Kconfig| 1 + drivers/crypto/virtio/virtio_crypto_algs.c | 52 --- drivers/crypto/virtio/virtio_crypto_common.h | 16 ++ drivers/crypto/virtio/virtio_crypto_core.c | 74 ++-- 4 files changed, 121 insertions(+), 22 deletions(-) diff --git a/drivers/crypto/virtio/Kconfig b/drivers/crypto/virtio/Kconfig index d80f733..5db0749 100644 --- a/drivers/crypto/virtio/Kconfig +++ b/drivers/crypto/virtio/Kconfig @@ -4,6 +4,7 @@ config CRYPTO_DEV_VIRTIO select CRYPTO_AEAD select CRYPTO_AUTHENC select CRYPTO_BLKCIPHER + select CRYPTO_ENGINE default m help This driver provides support for virtio crypto device. If you diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c index c2374df..970d0ca 100644 --- a/drivers/crypto/virtio/virtio_crypto_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_algs.c @@ -288,8 +288,7 @@ static int virtio_crypto_ablkcipher_setkey(struct crypto_ablkcipher *tfm, static int __virtio_crypto_ablkcipher_do_req(struct virtio_crypto_request *vc_req, struct ablkcipher_request *req, - struct data_queue *data_vq, - __u8 op) + struct data_queue *data_vq) { struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req); unsigned int ivsize = crypto_ablkcipher_ivsize(tfm); @@ -329,7 +328,7 @@ static int virtio_crypto_ablkcipher_setkey(struct crypto_ablkcipher *tfm, vc_req->req_data = req_data; vc_req->type = VIRTIO_CRYPTO_SYM_OP_CIPHER; /* Head of operation */ - if (op) { + if (vc_req->encrypt) { req_data->header.session_id = cpu_to_le64(ctx->enc_sess_info.session_id); req_data->header.opcode = @@ -424,19 +423,15 @@ static int virtio_crypto_ablkcipher_encrypt(struct ablkcipher_request *req) struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(atfm); struct virtio_crypto_request *vc_req = ablkcipher_request_ctx(req); struct virtio_crypto *vcrypto = ctx->vcrypto; - int ret; /* Use the first data virtqueue as default */ struct data_queue *data_vq = >data_vq[0]; vc_req->ablkcipher_ctx = ctx; vc_req->ablkcipher_req = req; - ret = __virtio_crypto_ablkcipher_do_req(vc_req, req, data_vq, 1); - if (ret < 0) { - pr_err("virtio_crypto: Encryption failed!\n"); - return ret; - } + vc_req->encrypt = true; + vc_req->dataq = data_vq; - return -EINPROGRESS; + return crypto_transfer_cipher_request_to_engine(data_vq->engine, req); } static int virtio_crypto_ablkcipher_decrypt(struct ablkcipher_request *req) @@ -445,20 +440,16 @@ static int virtio_crypto_ablkcipher_decrypt(struct ablkcipher_request *req) struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(atfm); struct virtio_crypto_request *vc_req = ablkcipher_request_ctx(req); struct virtio_crypto *vcrypto = ctx->vcrypto; - int ret; /* Use the first data virtqueue as default */ struct data_queue *data_vq = >data_vq[0]; vc_req->ablkcipher_ctx = ctx; vc_req->ablkcipher_req = req; - ret = __virtio_crypto_ablkcipher_do_req(vc_req, req, data_vq, 0); - if (ret < 0) { - pr_err("virtio_crypto: Decryption failed!\n"); - return ret; - } + vc_req->encrypt = false; + vc_req->dataq = data_vq; - return -EINPROGRESS; + return crypto_transfer_cipher_request_to_engine(data_vq->engine, req); } static int virtio_crypto_ablkcipher_init(struct crypto_tfm *tfm) @@ -484,6 +475,33 @@ static void virtio_crypto_ablkcipher_exit(struct crypto_tfm *tfm) ctx->vcrypto = NULL; } +int virtio_crypto_ablkcipher_crypt_req( + struct crypto_engine *engine, + struct ablkcipher_request *req) +{ + struct virtio_crypto_request *vc_req = ablkcipher_request_ctx(req); + struct data_queue *data_vq = vc_req->dataq; + int ret; + + ret = __virtio_crypto_ablkcipher_do_req(vc_req, req, data_vq); + if (ret < 0) + return ret; + + virtqueue_kick(data_vq->vq); + + return 0; +} + +void virtio_crypto_ablkcipher_finalize_req( + struct virtio_crypto_request
[PATCH] virtio-crypto: support crypto engine framework
crypto engine was introduced since 'commit 735d37b5424b ("crypto: engine - Introduce the block request crypto engine framework")' which uses work queue to realize the asynchronous processing for ablk_cipher and ahash. For virtio-crypto device, I register an engine for each data virtqueue so that we can use the capability of multiple data queues in future. Cc: Baolin Wang Cc: Herbert Xu Cc: Michael S. Tsirkin Signed-off-by: Gonglei --- drivers/crypto/virtio/Kconfig| 1 + drivers/crypto/virtio/virtio_crypto_algs.c | 52 --- drivers/crypto/virtio/virtio_crypto_common.h | 16 ++ drivers/crypto/virtio/virtio_crypto_core.c | 74 ++-- 4 files changed, 121 insertions(+), 22 deletions(-) diff --git a/drivers/crypto/virtio/Kconfig b/drivers/crypto/virtio/Kconfig index d80f733..5db0749 100644 --- a/drivers/crypto/virtio/Kconfig +++ b/drivers/crypto/virtio/Kconfig @@ -4,6 +4,7 @@ config CRYPTO_DEV_VIRTIO select CRYPTO_AEAD select CRYPTO_AUTHENC select CRYPTO_BLKCIPHER + select CRYPTO_ENGINE default m help This driver provides support for virtio crypto device. If you diff --git a/drivers/crypto/virtio/virtio_crypto_algs.c b/drivers/crypto/virtio/virtio_crypto_algs.c index c2374df..970d0ca 100644 --- a/drivers/crypto/virtio/virtio_crypto_algs.c +++ b/drivers/crypto/virtio/virtio_crypto_algs.c @@ -288,8 +288,7 @@ static int virtio_crypto_ablkcipher_setkey(struct crypto_ablkcipher *tfm, static int __virtio_crypto_ablkcipher_do_req(struct virtio_crypto_request *vc_req, struct ablkcipher_request *req, - struct data_queue *data_vq, - __u8 op) + struct data_queue *data_vq) { struct crypto_ablkcipher *tfm = crypto_ablkcipher_reqtfm(req); unsigned int ivsize = crypto_ablkcipher_ivsize(tfm); @@ -329,7 +328,7 @@ static int virtio_crypto_ablkcipher_setkey(struct crypto_ablkcipher *tfm, vc_req->req_data = req_data; vc_req->type = VIRTIO_CRYPTO_SYM_OP_CIPHER; /* Head of operation */ - if (op) { + if (vc_req->encrypt) { req_data->header.session_id = cpu_to_le64(ctx->enc_sess_info.session_id); req_data->header.opcode = @@ -424,19 +423,15 @@ static int virtio_crypto_ablkcipher_encrypt(struct ablkcipher_request *req) struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(atfm); struct virtio_crypto_request *vc_req = ablkcipher_request_ctx(req); struct virtio_crypto *vcrypto = ctx->vcrypto; - int ret; /* Use the first data virtqueue as default */ struct data_queue *data_vq = >data_vq[0]; vc_req->ablkcipher_ctx = ctx; vc_req->ablkcipher_req = req; - ret = __virtio_crypto_ablkcipher_do_req(vc_req, req, data_vq, 1); - if (ret < 0) { - pr_err("virtio_crypto: Encryption failed!\n"); - return ret; - } + vc_req->encrypt = true; + vc_req->dataq = data_vq; - return -EINPROGRESS; + return crypto_transfer_cipher_request_to_engine(data_vq->engine, req); } static int virtio_crypto_ablkcipher_decrypt(struct ablkcipher_request *req) @@ -445,20 +440,16 @@ static int virtio_crypto_ablkcipher_decrypt(struct ablkcipher_request *req) struct virtio_crypto_ablkcipher_ctx *ctx = crypto_ablkcipher_ctx(atfm); struct virtio_crypto_request *vc_req = ablkcipher_request_ctx(req); struct virtio_crypto *vcrypto = ctx->vcrypto; - int ret; /* Use the first data virtqueue as default */ struct data_queue *data_vq = >data_vq[0]; vc_req->ablkcipher_ctx = ctx; vc_req->ablkcipher_req = req; - ret = __virtio_crypto_ablkcipher_do_req(vc_req, req, data_vq, 0); - if (ret < 0) { - pr_err("virtio_crypto: Decryption failed!\n"); - return ret; - } + vc_req->encrypt = false; + vc_req->dataq = data_vq; - return -EINPROGRESS; + return crypto_transfer_cipher_request_to_engine(data_vq->engine, req); } static int virtio_crypto_ablkcipher_init(struct crypto_tfm *tfm) @@ -484,6 +475,33 @@ static void virtio_crypto_ablkcipher_exit(struct crypto_tfm *tfm) ctx->vcrypto = NULL; } +int virtio_crypto_ablkcipher_crypt_req( + struct crypto_engine *engine, + struct ablkcipher_request *req) +{ + struct virtio_crypto_request *vc_req = ablkcipher_request_ctx(req); + struct data_queue *data_vq = vc_req->dataq; + int ret; + + ret = __virtio_crypto_ablkcipher_do_req(vc_req, req, data_vq); + if (ret < 0) + return ret; + + virtqueue_kick(data_vq->vq); + + return 0; +} + +void virtio_crypto_ablkcipher_finalize_req( + struct virtio_crypto_request *vc_req, + struct ablkcipher_request *req, + int err) +{ +
[PATCH v4 3/4] clk: rockchip: add new pll-type for rk3328
The rk3328's pll and clock are similar with rk3036's, it different with pll_mode_mask, the rk3328 soc pll mode only one bit(rk3036 soc have two bits) so these should be independent and separate from the series of rk3328s. Changes in v4: adjust the pacth 3 and 4 order. move pll_rk3328 to patch 3. Changes in v3: fix up the pll type pll_rk3328 description and use Signed-off-by: Elaine Zhang--- drivers/clk/rockchip/clk-pll.c | 16 +--- drivers/clk/rockchip/clk.h | 1 + 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c index 6ed605776abd..eec51893a7e6 100644 --- a/drivers/clk/rockchip/clk-pll.c +++ b/drivers/clk/rockchip/clk-pll.c @@ -29,6 +29,7 @@ #define PLL_MODE_SLOW 0x0 #define PLL_MODE_NORM 0x1 #define PLL_MODE_DEEP 0x2 +#define PLL_RK3328_MODE_MASK 0x1 struct rockchip_clk_pll { struct clk_hw hw; @@ -848,7 +849,8 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, struct clk *pll_clk, *mux_clk; char pll_name[20]; - if (num_parents != 2) { + if ((pll_type != pll_rk3328 && num_parents != 2) || + (pll_type == pll_rk3328 && num_parents != 1)) { pr_err("%s: needs two parent clocks\n", __func__); return ERR_PTR(-EINVAL); } @@ -865,13 +867,17 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, pll_mux = >pll_mux; pll_mux->reg = ctx->reg_base + mode_offset; pll_mux->shift = mode_shift; - pll_mux->mask = PLL_MODE_MASK; + if (pll_type == pll_rk3328) + pll_mux->mask = PLL_RK3328_MODE_MASK; + else + pll_mux->mask = PLL_MODE_MASK; pll_mux->flags = 0; pll_mux->lock = >lock; pll_mux->hw.init = if (pll_type == pll_rk3036 || pll_type == pll_rk3066 || + pll_type == pll_rk3328 || pll_type == pll_rk3399) pll_mux->flags |= CLK_MUX_HIWORD_MASK; @@ -884,7 +890,10 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, init.flags = CLK_SET_RATE_PARENT; init.ops = pll->pll_mux_ops; init.parent_names = pll_parents; - init.num_parents = ARRAY_SIZE(pll_parents); + if (pll_type == pll_rk3328) + init.num_parents = 2; + else + init.num_parents = ARRAY_SIZE(pll_parents); mux_clk = clk_register(NULL, _mux->hw); if (IS_ERR(mux_clk)) @@ -918,6 +927,7 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, switch (pll_type) { case pll_rk3036: + case pll_rk3328: if (!pll->rate_table || IS_ERR(ctx->grf)) init.ops = _rk3036_pll_clk_norate_ops; else diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h index d67eecc4ade9..06acb7e0911f 100644 --- a/drivers/clk/rockchip/clk.h +++ b/drivers/clk/rockchip/clk.h @@ -130,6 +130,7 @@ enum rockchip_pll_type { pll_rk3036, pll_rk3066, + pll_rk3328, pll_rk3399, }; -- 1.9.1
[PATCH v4 3/4] clk: rockchip: add new pll-type for rk3328
The rk3328's pll and clock are similar with rk3036's, it different with pll_mode_mask, the rk3328 soc pll mode only one bit(rk3036 soc have two bits) so these should be independent and separate from the series of rk3328s. Changes in v4: adjust the pacth 3 and 4 order. move pll_rk3328 to patch 3. Changes in v3: fix up the pll type pll_rk3328 description and use Signed-off-by: Elaine Zhang --- drivers/clk/rockchip/clk-pll.c | 16 +--- drivers/clk/rockchip/clk.h | 1 + 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/drivers/clk/rockchip/clk-pll.c b/drivers/clk/rockchip/clk-pll.c index 6ed605776abd..eec51893a7e6 100644 --- a/drivers/clk/rockchip/clk-pll.c +++ b/drivers/clk/rockchip/clk-pll.c @@ -29,6 +29,7 @@ #define PLL_MODE_SLOW 0x0 #define PLL_MODE_NORM 0x1 #define PLL_MODE_DEEP 0x2 +#define PLL_RK3328_MODE_MASK 0x1 struct rockchip_clk_pll { struct clk_hw hw; @@ -848,7 +849,8 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, struct clk *pll_clk, *mux_clk; char pll_name[20]; - if (num_parents != 2) { + if ((pll_type != pll_rk3328 && num_parents != 2) || + (pll_type == pll_rk3328 && num_parents != 1)) { pr_err("%s: needs two parent clocks\n", __func__); return ERR_PTR(-EINVAL); } @@ -865,13 +867,17 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, pll_mux = >pll_mux; pll_mux->reg = ctx->reg_base + mode_offset; pll_mux->shift = mode_shift; - pll_mux->mask = PLL_MODE_MASK; + if (pll_type == pll_rk3328) + pll_mux->mask = PLL_RK3328_MODE_MASK; + else + pll_mux->mask = PLL_MODE_MASK; pll_mux->flags = 0; pll_mux->lock = >lock; pll_mux->hw.init = if (pll_type == pll_rk3036 || pll_type == pll_rk3066 || + pll_type == pll_rk3328 || pll_type == pll_rk3399) pll_mux->flags |= CLK_MUX_HIWORD_MASK; @@ -884,7 +890,10 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, init.flags = CLK_SET_RATE_PARENT; init.ops = pll->pll_mux_ops; init.parent_names = pll_parents; - init.num_parents = ARRAY_SIZE(pll_parents); + if (pll_type == pll_rk3328) + init.num_parents = 2; + else + init.num_parents = ARRAY_SIZE(pll_parents); mux_clk = clk_register(NULL, _mux->hw); if (IS_ERR(mux_clk)) @@ -918,6 +927,7 @@ struct clk *rockchip_clk_register_pll(struct rockchip_clk_provider *ctx, switch (pll_type) { case pll_rk3036: + case pll_rk3328: if (!pll->rate_table || IS_ERR(ctx->grf)) init.ops = _rk3036_pll_clk_norate_ops; else diff --git a/drivers/clk/rockchip/clk.h b/drivers/clk/rockchip/clk.h index d67eecc4ade9..06acb7e0911f 100644 --- a/drivers/clk/rockchip/clk.h +++ b/drivers/clk/rockchip/clk.h @@ -130,6 +130,7 @@ enum rockchip_pll_type { pll_rk3036, pll_rk3066, + pll_rk3328, pll_rk3399, }; -- 1.9.1
[PATCH v4 2/4] dt-bindings: add bindings for rk3328 clock controller
Add devicetree bindings for Rockchip cru which found on Rockchip SoCs. Changes in v4: dropping the "rockchip,cru" and "syscon" properties for bindings of rk3328 Signed-off-by: Elaine Zhang--- .../bindings/clock/rockchip,rk3328-cru.txt | 57 ++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3328-cru.txt diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3328-cru.txt b/Documentation/devicetree/bindings/clock/rockchip,rk3328-cru.txt new file mode 100644 index ..e71c675ba5da --- /dev/null +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3328-cru.txt @@ -0,0 +1,57 @@ +* Rockchip RK3328 Clock and Reset Unit + +The RK3328 clock controller generates and supplies clock to various +controllers within the SoC and also implements a reset controller for SoC +peripherals. + +Required Properties: + +- compatible: should be "rockchip,rk3328-cru" +- reg: physical base address of the controller and length of memory mapped + region. +- #clock-cells: should be 1. +- #reset-cells: should be 1. + +Optional Properties: + +- rockchip,grf: phandle to the syscon managing the "general register files" + If missing pll rates are not changeable, due to the missing pll lock status. + +Each clock is assigned an identifier and client nodes can use this identifier +to specify the clock which they consume. All available clocks are defined as +preprocessor macros in the dt-bindings/clock/rk3328-cru.h headers and can be +used in device tree sources. Similar macros exist for the reset sources in +these files. + +External clocks: + +There are several clocks that are generated outside the SoC. It is expected +that they are defined using standard clock bindings with following +clock-output-names: + - "xin24m" - crystal input - required, + - "clkin_i2s" - external I2S clock - optional, + - "gmac_clkin" - external GMAC clock - optional + - "phy_50m_out" - output clock of the pll in the mac phy + +Example: Clock controller node: + + cru: clock-controller@ff44 { + compatible = "rockchip,rk3328-cru"; + reg = <0x0 0xff44 0x0 0x1000>; + rockchip,grf = <>; + + #clock-cells = <1>; + #reset-cells = <1>; + }; + +Example: UART controller node that consumes the clock generated by the clock + controller: + + uart0: serial@ff12 { + compatible = "snps,dw-apb-uart"; + reg = <0xff12 0x100>; + interrupts = ; + reg-shift = <2>; + reg-io-width = <4>; + clocks = < SCLK_UART0>; + }; -- 1.9.1
[PATCH v4 2/4] dt-bindings: add bindings for rk3328 clock controller
Add devicetree bindings for Rockchip cru which found on Rockchip SoCs. Changes in v4: dropping the "rockchip,cru" and "syscon" properties for bindings of rk3328 Signed-off-by: Elaine Zhang --- .../bindings/clock/rockchip,rk3328-cru.txt | 57 ++ 1 file changed, 57 insertions(+) create mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3328-cru.txt diff --git a/Documentation/devicetree/bindings/clock/rockchip,rk3328-cru.txt b/Documentation/devicetree/bindings/clock/rockchip,rk3328-cru.txt new file mode 100644 index ..e71c675ba5da --- /dev/null +++ b/Documentation/devicetree/bindings/clock/rockchip,rk3328-cru.txt @@ -0,0 +1,57 @@ +* Rockchip RK3328 Clock and Reset Unit + +The RK3328 clock controller generates and supplies clock to various +controllers within the SoC and also implements a reset controller for SoC +peripherals. + +Required Properties: + +- compatible: should be "rockchip,rk3328-cru" +- reg: physical base address of the controller and length of memory mapped + region. +- #clock-cells: should be 1. +- #reset-cells: should be 1. + +Optional Properties: + +- rockchip,grf: phandle to the syscon managing the "general register files" + If missing pll rates are not changeable, due to the missing pll lock status. + +Each clock is assigned an identifier and client nodes can use this identifier +to specify the clock which they consume. All available clocks are defined as +preprocessor macros in the dt-bindings/clock/rk3328-cru.h headers and can be +used in device tree sources. Similar macros exist for the reset sources in +these files. + +External clocks: + +There are several clocks that are generated outside the SoC. It is expected +that they are defined using standard clock bindings with following +clock-output-names: + - "xin24m" - crystal input - required, + - "clkin_i2s" - external I2S clock - optional, + - "gmac_clkin" - external GMAC clock - optional + - "phy_50m_out" - output clock of the pll in the mac phy + +Example: Clock controller node: + + cru: clock-controller@ff44 { + compatible = "rockchip,rk3328-cru"; + reg = <0x0 0xff44 0x0 0x1000>; + rockchip,grf = <>; + + #clock-cells = <1>; + #reset-cells = <1>; + }; + +Example: UART controller node that consumes the clock generated by the clock + controller: + + uart0: serial@ff12 { + compatible = "snps,dw-apb-uart"; + reg = <0xff12 0x100>; + interrupts = ; + reg-shift = <2>; + reg-io-width = <4>; + clocks = < SCLK_UART0>; + }; -- 1.9.1
[PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and stmmac_dvr_probe
From: "Kweh, Hock Leong"If kernel module stmmac driver being loaded after OS booted, there is a race condition between stmmac_open() and stmmac_mdio_register(), which is invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as PHY not found and stmmac_open() failed: [ 473.919358] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): stmmac_dvr_probe: warning: cannot get CSR clock [ 473.919382] stmmaceth :01:00.0: no reset control found [ 473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42 [ 473.919429] stmmaceth :01:00.0: DMA HW capability register supported [ 473.919436] stmmaceth :01:00.0: RX Checksum Offload Engine supported [ 473.919443] stmmaceth :01:00.0: TX Checksum insertion supported [ 473.919451] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): Enable RX Mitigation via HW Watchdog Timer [ 473.921395] libphy: PHY stmmac-1:00 not found [ 473.921417] stmmaceth :01:00.0 eth0: Could not attach to PHY [ 473.921427] stmmaceth :01:00.0 eth0: stmmac_open: Cannot attach to PHY (error: -19) [ 473.959710] libphy: stmmac: probed [ 473.959724] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL (stmmac-1:00) active [ 473.959728] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL (stmmac-1:01) [ 473.959731] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL (stmmac-1:02) [ 473.959734] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL (stmmac-1:03) The resolution moved the register_netdev() function call to the end of stmmac_dvr_probe() after stmmac_mdio_register(). Suggested-by: Florian Fainelli Signed-off-by: Kweh, Hock Leong --- changelog v2: * Re-implemented the fix base on David & Florian suggestion and tested. drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index bb40382..263ac53 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3339,13 +3339,6 @@ int stmmac_dvr_probe(struct device *device, spin_lock_init(>lock); - ret = register_netdev(ndev); - if (ret) { - netdev_err(priv->dev, "%s: ERROR %i registering the device\n", - __func__, ret); - goto error_netdev_register; - } - /* If a specific clk_csr value is passed from the platform * this means that the CSR Clock Range selection cannot be * changed at run-time and it is fixed. Viceversa the driver'll try to @@ -3372,11 +3365,18 @@ int stmmac_dvr_probe(struct device *device, } } + ret = register_netdev(ndev); + if (ret) { + netdev_err(priv->dev, "%s: ERROR %i registering the device\n", + __func__, ret); + goto error_netdev_register; + } + return 0; -error_mdio_register: - unregister_netdev(ndev); error_netdev_register: + stmmac_mdio_unregister(ndev); +error_mdio_register: netif_napi_del(>napi); error_hw_init: clk_disable_unprepare(priv->pclk); -- 1.7.9.5
[PATCH v2] net: stmmac: bug fix to synchronize stmmac_open and stmmac_dvr_probe
From: "Kweh, Hock Leong" If kernel module stmmac driver being loaded after OS booted, there is a race condition between stmmac_open() and stmmac_mdio_register(), which is invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as PHY not found and stmmac_open() failed: [ 473.919358] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): stmmac_dvr_probe: warning: cannot get CSR clock [ 473.919382] stmmaceth :01:00.0: no reset control found [ 473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42 [ 473.919429] stmmaceth :01:00.0: DMA HW capability register supported [ 473.919436] stmmaceth :01:00.0: RX Checksum Offload Engine supported [ 473.919443] stmmaceth :01:00.0: TX Checksum insertion supported [ 473.919451] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): Enable RX Mitigation via HW Watchdog Timer [ 473.921395] libphy: PHY stmmac-1:00 not found [ 473.921417] stmmaceth :01:00.0 eth0: Could not attach to PHY [ 473.921427] stmmaceth :01:00.0 eth0: stmmac_open: Cannot attach to PHY (error: -19) [ 473.959710] libphy: stmmac: probed [ 473.959724] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL (stmmac-1:00) active [ 473.959728] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL (stmmac-1:01) [ 473.959731] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL (stmmac-1:02) [ 473.959734] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL (stmmac-1:03) The resolution moved the register_netdev() function call to the end of stmmac_dvr_probe() after stmmac_mdio_register(). Suggested-by: Florian Fainelli Signed-off-by: Kweh, Hock Leong --- changelog v2: * Re-implemented the fix base on David & Florian suggestion and tested. drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index bb40382..263ac53 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3339,13 +3339,6 @@ int stmmac_dvr_probe(struct device *device, spin_lock_init(>lock); - ret = register_netdev(ndev); - if (ret) { - netdev_err(priv->dev, "%s: ERROR %i registering the device\n", - __func__, ret); - goto error_netdev_register; - } - /* If a specific clk_csr value is passed from the platform * this means that the CSR Clock Range selection cannot be * changed at run-time and it is fixed. Viceversa the driver'll try to @@ -3372,11 +3365,18 @@ int stmmac_dvr_probe(struct device *device, } } + ret = register_netdev(ndev); + if (ret) { + netdev_err(priv->dev, "%s: ERROR %i registering the device\n", + __func__, ret); + goto error_netdev_register; + } + return 0; -error_mdio_register: - unregister_netdev(ndev); error_netdev_register: + stmmac_mdio_unregister(ndev); +error_mdio_register: netif_napi_del(>napi); error_hw_init: clk_disable_unprepare(priv->pclk); -- 1.7.9.5
Re: [PATCH] PCI: exynos: refactor exynos pcie driver
Hi Jaehoon, On 12/27/2016 08:00 AM, Jaehoon Chung wrote: On 12/27/2016 11:12 AM, Alim Akhtar wrote: On 12/27/2016 06:39 AM, Jaehoon Chung wrote: Dear Alim, On 12/26/2016 06:46 PM, Alim Akhtar wrote: Hello Jaehoon On 12/26/2016 02:32 PM, Jaehoon Chung wrote: Hi Pankaj, On 12/23/2016 07:56 PM, Pankaj Dubey wrote: From: Niyas Ahmed S TCurrently Exynos PCIe driver is only supported for Exynos5440 SoC. This patch does refactoring of Exynos PCIe driver to extend support for other Exynos SoC. Following are the main changes done via this patch: 1) It adds separate structs for memory, clock resources. 2) It add exynos_pcie_ops struct which will allow us to support the differences in resources in different Exynos SoC. It's nice to me for reusing this file. but after considering too many times, i decided not to use this file. I'm not sure what block base is..actually this pci-exynos.c is really black-box. (No one maintains this file, even Samsung didn't care.) Who is using this? If Someone can share the information about exynos5440, i can refactor everything. Otherwise, there are two solution.. One is "adding the new pci-exynos.c" likes pci-exynos5433.c Other is "refactor this file" under assuming the each register's usage. Its alway good to reuse code as far as possible. I am yet to see the details of 5440, but since people are now going to support more Exynos variants, in my opinion, instead of adding pci-exynos5433.c, you can rename current pci-exynos.c to something like pci-exynos5440.c (only in case its too much changes needed to support 5433/exynos7) and lets have a common pci-exynos.c where we can add exynos7/5433 and others SoCs, AFAIK at least exynos7 and 5433 has similar pci controller. Yes, It's always good to reuse. but as you know, current pci-exynos.c file is really specific for only exynos5440. I will try to check about combining. I want to use the PHY generic Framework for EXYNOS PCIe. I don't think you have an option here, you should use generic PHY APIs, otherwise phy drive should go to drivers/misc. Other thoughts are welcome. Why go to drivers/misc? There is driver/phy/ for PHY generic Framework. If i will touch this file, then i will put our phy-exynos-pcie file under driver/phy/ . I already sent the patch for this. Could you check them? My point was, if you are not going to use generic PHY APIs, then phy driver should go into drivers/misc. Anyway as you said you have already posted patches with generic phy framework, I will take a look. Ah. Right..And i'm doing the refactoring to reuse the current pci-exynos.c. There is a nice refactoring patch posted by Pankaj recently @ https://lkml.org/lkml/2016/12/23/73 I would suggest you to rebase your work on this top. Maybe..Today or Tomorrow..I will send the patches..At that time, could you also check them? Any comments might be helpful to me! :) Will wait for you patches :-) Best Regards, Jaehoon Chung http://patchwork.ozlabs.org/patch/708738/ http://patchwork.ozlabs.org/patch/708742/ Thanks. Best Regards, Jaehoon Chung If you or other guys really want to use the pci-exynos.c for other exynos, I will rework with PHY generic framework. Then i will resend the my patches as V2. One more thing..Does anyone know what the usage of block base is? Can i use that register as "syscon"? Best Regards, Jaehoon Chung No functional change intended. Signed-off-by: Niyas Ahmed S T Signed-off-by: Pankaj Dubey --- This patch set is prepared on top of Krzysztof's for-next and PCIe driver cleanup patch [1] by Jaehoon Chung. [1]: https://lkml.org/lkml/2016/12/19/44 drivers/pci/host/pci-exynos.c | 346 ++ 1 file changed, 217 insertions(+), 129 deletions(-) diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c index 33562cf..2dc54f7 100644 --- a/drivers/pci/host/pci-exynos.c +++ b/drivers/pci/host/pci-exynos.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -28,16 +29,6 @@ #define to_exynos_pcie(x)container_of(x, struct exynos_pcie, pp) -struct exynos_pcie { -struct pcie_portpp; -void __iomem*elbi_base;/* DT 0th resource */ -void __iomem*phy_base;/* DT 1st resource */ -void __iomem*block_base;/* DT 2nd resource */ -intreset_gpio; -struct clk*clk; -struct clk*bus_clk; -}; - /* PCIe ELBI registers */ #define PCIE_IRQ_PULSE0x000 #define IRQ_INTA_ASSERTBIT(0) @@ -102,6 +93,122 @@ struct exynos_pcie { #define PCIE_PHY_TRSV3_PD_TSVBIT(7) #define PCIE_PHY_TRSV3_LVCC0x31c +struct exynos_pcie_mem_res { +void __iomem *elbi_base; /* DT 0th resource: PCIE CTRL */ +void __iomem *phy_base; /* DT 1st resource: PHY CTRL */ +void
Re: [PATCH] PCI: exynos: refactor exynos pcie driver
Hi Jaehoon, On 12/27/2016 08:00 AM, Jaehoon Chung wrote: On 12/27/2016 11:12 AM, Alim Akhtar wrote: On 12/27/2016 06:39 AM, Jaehoon Chung wrote: Dear Alim, On 12/26/2016 06:46 PM, Alim Akhtar wrote: Hello Jaehoon On 12/26/2016 02:32 PM, Jaehoon Chung wrote: Hi Pankaj, On 12/23/2016 07:56 PM, Pankaj Dubey wrote: From: Niyas Ahmed S T Currently Exynos PCIe driver is only supported for Exynos5440 SoC. This patch does refactoring of Exynos PCIe driver to extend support for other Exynos SoC. Following are the main changes done via this patch: 1) It adds separate structs for memory, clock resources. 2) It add exynos_pcie_ops struct which will allow us to support the differences in resources in different Exynos SoC. It's nice to me for reusing this file. but after considering too many times, i decided not to use this file. I'm not sure what block base is..actually this pci-exynos.c is really black-box. (No one maintains this file, even Samsung didn't care.) Who is using this? If Someone can share the information about exynos5440, i can refactor everything. Otherwise, there are two solution.. One is "adding the new pci-exynos.c" likes pci-exynos5433.c Other is "refactor this file" under assuming the each register's usage. Its alway good to reuse code as far as possible. I am yet to see the details of 5440, but since people are now going to support more Exynos variants, in my opinion, instead of adding pci-exynos5433.c, you can rename current pci-exynos.c to something like pci-exynos5440.c (only in case its too much changes needed to support 5433/exynos7) and lets have a common pci-exynos.c where we can add exynos7/5433 and others SoCs, AFAIK at least exynos7 and 5433 has similar pci controller. Yes, It's always good to reuse. but as you know, current pci-exynos.c file is really specific for only exynos5440. I will try to check about combining. I want to use the PHY generic Framework for EXYNOS PCIe. I don't think you have an option here, you should use generic PHY APIs, otherwise phy drive should go to drivers/misc. Other thoughts are welcome. Why go to drivers/misc? There is driver/phy/ for PHY generic Framework. If i will touch this file, then i will put our phy-exynos-pcie file under driver/phy/ . I already sent the patch for this. Could you check them? My point was, if you are not going to use generic PHY APIs, then phy driver should go into drivers/misc. Anyway as you said you have already posted patches with generic phy framework, I will take a look. Ah. Right..And i'm doing the refactoring to reuse the current pci-exynos.c. There is a nice refactoring patch posted by Pankaj recently @ https://lkml.org/lkml/2016/12/23/73 I would suggest you to rebase your work on this top. Maybe..Today or Tomorrow..I will send the patches..At that time, could you also check them? Any comments might be helpful to me! :) Will wait for you patches :-) Best Regards, Jaehoon Chung http://patchwork.ozlabs.org/patch/708738/ http://patchwork.ozlabs.org/patch/708742/ Thanks. Best Regards, Jaehoon Chung If you or other guys really want to use the pci-exynos.c for other exynos, I will rework with PHY generic framework. Then i will resend the my patches as V2. One more thing..Does anyone know what the usage of block base is? Can i use that register as "syscon"? Best Regards, Jaehoon Chung No functional change intended. Signed-off-by: Niyas Ahmed S T Signed-off-by: Pankaj Dubey --- This patch set is prepared on top of Krzysztof's for-next and PCIe driver cleanup patch [1] by Jaehoon Chung. [1]: https://lkml.org/lkml/2016/12/19/44 drivers/pci/host/pci-exynos.c | 346 ++ 1 file changed, 217 insertions(+), 129 deletions(-) diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c index 33562cf..2dc54f7 100644 --- a/drivers/pci/host/pci-exynos.c +++ b/drivers/pci/host/pci-exynos.c @@ -17,6 +17,7 @@ #include #include #include +#include #include #include #include @@ -28,16 +29,6 @@ #define to_exynos_pcie(x)container_of(x, struct exynos_pcie, pp) -struct exynos_pcie { -struct pcie_portpp; -void __iomem*elbi_base;/* DT 0th resource */ -void __iomem*phy_base;/* DT 1st resource */ -void __iomem*block_base;/* DT 2nd resource */ -intreset_gpio; -struct clk*clk; -struct clk*bus_clk; -}; - /* PCIe ELBI registers */ #define PCIE_IRQ_PULSE0x000 #define IRQ_INTA_ASSERTBIT(0) @@ -102,6 +93,122 @@ struct exynos_pcie { #define PCIE_PHY_TRSV3_PD_TSVBIT(7) #define PCIE_PHY_TRSV3_LVCC0x31c +struct exynos_pcie_mem_res { +void __iomem *elbi_base; /* DT 0th resource: PCIE CTRL */ +void __iomem *phy_base; /* DT 1st resource: PHY CTRL */ +void __iomem *block_base; /* DT 2nd resource: PHY ADDITIONAL CTRL */ +}; + +struct
[PATCH v4 1/4] clk: rockchip: add dt-binding header for rk3328
Add the dt-bindings header for the rk3328, that gets shared between the clock controller and the clock references in the dts. Add softreset ID for rk3328. Signed-off-by: Elaine Zhang--- include/dt-bindings/clock/rk3328-cru.h | 403 + 1 file changed, 403 insertions(+) create mode 100644 include/dt-bindings/clock/rk3328-cru.h diff --git a/include/dt-bindings/clock/rk3328-cru.h b/include/dt-bindings/clock/rk3328-cru.h new file mode 100644 index ..545ed7541316 --- /dev/null +++ b/include/dt-bindings/clock/rk3328-cru.h @@ -0,0 +1,403 @@ +/* + * Copyright (c) 2016 Rockchip Electronics Co. Ltd. + * Author: Elaine + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _DT_BINDINGS_CLK_ROCKCHIP_RK3328_H +#define _DT_BINDINGS_CLK_ROCKCHIP_RK3328_H + +/* core clocks */ +#define PLL_APLL 1 +#define PLL_DPLL 2 +#define PLL_CPLL 3 +#define PLL_GPLL 4 +#define PLL_NPLL 5 +#define ARMCLK 6 + +/* sclk gates (special clocks) */ +#define SCLK_RTC32K30 +#define SCLK_SDMMC_EXT 31 +#define SCLK_SPI 32 +#define SCLK_SDMMC 33 +#define SCLK_SDIO 34 +#define SCLK_EMMC 35 +#define SCLK_TSADC 36 +#define SCLK_SARADC37 +#define SCLK_UART0 38 +#define SCLK_UART1 39 +#define SCLK_UART2 40 +#define SCLK_I2S0 41 +#define SCLK_I2S1 42 +#define SCLK_I2S2 43 +#define SCLK_I2S1_OUT 44 +#define SCLK_I2S2_OUT 45 +#define SCLK_SPDIF 46 +#define SCLK_TIMER047 +#define SCLK_TIMER148 +#define SCLK_TIMER249 +#define SCLK_TIMER350 +#define SCLK_TIMER451 +#define SCLK_TIMER552 +#define SCLK_WIFI 53 +#define SCLK_CIF_OUT 54 +#define SCLK_I2C0 55 +#define SCLK_I2C1 56 +#define SCLK_I2C2 57 +#define SCLK_I2C3 58 +#define SCLK_CRYPTO59 +#define SCLK_PWM 60 +#define SCLK_PDM 61 +#define SCLK_EFUSE 62 +#define SCLK_OTP 63 +#define SCLK_DDRCLK64 +#define SCLK_VDEC_CABAC65 +#define SCLK_VDEC_CORE 66 +#define SCLK_VENC_DSP 67 +#define SCLK_VENC_CORE 68 +#define SCLK_RGA 69 +#define SCLK_HDMI_SFC 70 +#define SCLK_HDMI_CEC 71 +#define SCLK_USB3_REF 72 +#define SCLK_USB3_SUSPEND 73 +#define SCLK_SDMMC_DRV 74 +#define SCLK_SDIO_DRV 75 +#define SCLK_EMMC_DRV 76 +#define SCLK_SDMMC_EXT_DRV 77 +#define SCLK_SDMMC_SAMPLE 78 +#define SCLK_SDIO_SAMPLE 79 +#define SCLK_EMMC_SAMPLE 80 +#define SCLK_SDMMC_EXT_SAMPLE 81 +#define SCLK_VOP 82 +#define SCLK_MAC2PHY_RXTX 83 +#define SCLK_MAC2PHY_SRC 84 +#define SCLK_MAC2PHY_REF 85 +#define SCLK_MAC2PHY_OUT 86 +#define SCLK_MAC2IO_RX 87 +#define SCLK_MAC2IO_TX 88 +#define SCLK_MAC2IO_REFOUT 89 +#define SCLK_MAC2IO_REF90 +#define SCLK_MAC2IO_OUT91 +#define SCLK_TSP 92 +#define SCLK_HSADC_TSP 93 +#define SCLK_USB3PHY_REF 94 +#define SCLK_REF_USB3OTG 95 +#define SCLK_USB3OTG_REF 96 +#define SCLK_USB3OTG_SUSPEND 97 +#define SCLK_REF_USB3OTG_SRC 98 +#define SCLK_MAC2IO_SRC99 + +/* dclk gates */ +#define DCLK_LCDC 180 +#define DCLK_HDMIPHY 181 +#define HDMIPHY182 +#define USB480M183 +#define DCLK_LCDC_SRC 184 + +/* aclk gates */ +#define ACLK_AXISRAM 190 +#define ACLK_VOP_PRE 191 +#define ACLK_USB3OTG 192 +#define ACLK_RGA_PRE 193 +#define ACLK_DMAC 194 +#define ACLK_GPU 195 +#define ACLK_BUS_PRE 196 +#define ACLK_PERI_PRE 197 +#define ACLK_RKVDEC_PRE198 +#define ACLK_RKVDEC199 +#define ACLK_RKVENC200 +#define ACLK_VPU_PRE 201 +#define ACLK_VIO_PRE 202 +#define ACLK_VPU 203 +#define ACLK_VIO 204 +#define ACLK_VOP 205 +#define ACLK_GMAC 206 +#define ACLK_H265 207 +#define ACLK_H264 208
[PATCH v4 0/4] clk: rockchip: support clk controller for rk3328 SoC
Changes in v4: dropping the "rockchip,cru" and "syscon" properties for bindings of rk3328 adjust the pacth 3 and 4 order. move pll_rk3328 to patch 3. Changes in v3: fix up the pll type pll_rk3328 description and use. Changes in v2: add bindings for rk3328 clock controller Elaine Zhang (4): clk: rockchip: add dt-binding header for rk3328 dt-bindings: add bindings for rk3328 clock controller clk: rockchip: add new pll-type for rk3328 clk: rockchip: add clock controller for rk3328 .../bindings/clock/rockchip,rk3328-cru.txt | 57 ++ drivers/clk/rockchip/Makefile |1 + drivers/clk/rockchip/clk-pll.c | 16 +- drivers/clk/rockchip/clk-rk3328.c | 1068 drivers/clk/rockchip/clk.h | 23 + include/dt-bindings/clock/rk3328-cru.h | 403 6 files changed, 1565 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3328-cru.txt create mode 100644 drivers/clk/rockchip/clk-rk3328.c create mode 100644 include/dt-bindings/clock/rk3328-cru.h -- 1.9.1
[PATCH v4 4/4] clk: rockchip: add clock controller for rk3328
Add the clock tree definition for the new rk3328 SoC. Changes in v4: adjust the pacth 3 and 4 order. Changes in v3: fix up the pll parent only xin24m. Changes in v2: fix up these *_sample error description. Signed-off-by: Elaine Zhang--- drivers/clk/rockchip/Makefile |1 + drivers/clk/rockchip/clk-rk3328.c | 1068 + drivers/clk/rockchip/clk.h| 22 + 3 files changed, 1091 insertions(+) create mode 100644 drivers/clk/rockchip/clk-rk3328.c diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile index 16e098c36f90..68b04bfca282 100644 --- a/drivers/clk/rockchip/Makefile +++ b/drivers/clk/rockchip/Makefile @@ -16,5 +16,6 @@ obj-y += clk-rk3036.o obj-y += clk-rk3188.o obj-y += clk-rk3228.o obj-y += clk-rk3288.o +obj-y += clk-rk3328.o obj-y += clk-rk3368.o obj-y += clk-rk3399.o diff --git a/drivers/clk/rockchip/clk-rk3328.c b/drivers/clk/rockchip/clk-rk3328.c new file mode 100644 index ..9958ce7d0dcd --- /dev/null +++ b/drivers/clk/rockchip/clk-rk3328.c @@ -0,0 +1,1068 @@ +/* + * Copyright (c) 2016 Rockchip Electronics Co. Ltd. + * Author: Elaine + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include "clk.h" + +#define RK3328_GRF_SOC_STATUS0 0x480 +#define RK3328_GRF_MAC_CON10x904 +#define RK3328_GRF_MAC_CON20x908 + +enum rk3328_plls { + apll, dpll, cpll, gpll, npll, +}; + +static struct rockchip_pll_rate_table rk3328_pll_rates[] = { + /* _mhz, _refdiv, _fbdiv, _postdiv1, _postdiv2, _dsmpd, _frac */ + RK3036_PLL_RATE(160800, 1, 67, 1, 1, 1, 0), + RK3036_PLL_RATE(158400, 1, 66, 1, 1, 1, 0), + RK3036_PLL_RATE(156000, 1, 65, 1, 1, 1, 0), + RK3036_PLL_RATE(153600, 1, 64, 1, 1, 1, 0), + RK3036_PLL_RATE(151200, 1, 63, 1, 1, 1, 0), + RK3036_PLL_RATE(148800, 1, 62, 1, 1, 1, 0), + RK3036_PLL_RATE(146400, 1, 61, 1, 1, 1, 0), + RK3036_PLL_RATE(144000, 1, 60, 1, 1, 1, 0), + RK3036_PLL_RATE(141600, 1, 59, 1, 1, 1, 0), + RK3036_PLL_RATE(139200, 1, 58, 1, 1, 1, 0), + RK3036_PLL_RATE(136800, 1, 57, 1, 1, 1, 0), + RK3036_PLL_RATE(134400, 1, 56, 1, 1, 1, 0), + RK3036_PLL_RATE(132000, 1, 55, 1, 1, 1, 0), + RK3036_PLL_RATE(129600, 1, 54, 1, 1, 1, 0), + RK3036_PLL_RATE(127200, 1, 53, 1, 1, 1, 0), + RK3036_PLL_RATE(124800, 1, 52, 1, 1, 1, 0), + RK3036_PLL_RATE(12, 1, 50, 1, 1, 1, 0), + RK3036_PLL_RATE(118800, 2, 99, 1, 1, 1, 0), + RK3036_PLL_RATE(110400, 1, 46, 1, 1, 1, 0), + RK3036_PLL_RATE(11, 12, 550, 1, 1, 1, 0), + RK3036_PLL_RATE(100800, 1, 84, 2, 1, 1, 0), + RK3036_PLL_RATE(10, 6, 500, 2, 1, 1, 0), + RK3036_PLL_RATE(98400, 1, 82, 2, 1, 1, 0), + RK3036_PLL_RATE(96000, 1, 80, 2, 1, 1, 0), + RK3036_PLL_RATE(93600, 1, 78, 2, 1, 1, 0), + RK3036_PLL_RATE(91200, 1, 76, 2, 1, 1, 0), + RK3036_PLL_RATE(9, 4, 300, 2, 1, 1, 0), + RK3036_PLL_RATE(88800, 1, 74, 2, 1, 1, 0), + RK3036_PLL_RATE(86400, 1, 72, 2, 1, 1, 0), + RK3036_PLL_RATE(84000, 1, 70, 2, 1, 1, 0), + RK3036_PLL_RATE(81600, 1, 68, 2, 1, 1, 0), + RK3036_PLL_RATE(8, 6, 400, 2, 1, 1, 0), + RK3036_PLL_RATE(7, 6, 350, 2, 1, 1, 0), + RK3036_PLL_RATE(69600, 1, 58, 2, 1, 1, 0), + RK3036_PLL_RATE(6, 1, 75, 3, 1, 1, 0), + RK3036_PLL_RATE(59400, 2, 99, 2, 1, 1, 0), + RK3036_PLL_RATE(50400, 1, 63, 3, 1, 1, 0), + RK3036_PLL_RATE(5, 6, 250, 2, 1, 1, 0), + RK3036_PLL_RATE(40800, 1, 68, 2, 2, 1, 0), + RK3036_PLL_RATE(31200, 1, 52, 2, 2, 1, 0), + RK3036_PLL_RATE(21600, 1, 72, 4, 2, 1, 0), + RK3036_PLL_RATE(9600, 1, 64, 4, 4, 1, 0), + { /* sentinel */ }, +}; + +static struct rockchip_pll_rate_table rk3328_pll_frac_rates[] = { + /* _mhz, _refdiv, _fbdiv, _postdiv1, _postdiv2, _dsmpd, _frac */ + RK3036_PLL_RATE(1016064000, 3, 127, 1, 1, 0, 134217), + /* vco = 1016064000 */ + RK3036_PLL_RATE(98304, 24, 983, 1, 1, 0, 671088), + /* vco = 98304 */ + RK3036_PLL_RATE(49152, 24, 983, 2, 1, 0, 671088), + /* vco = 98304 */ + RK3036_PLL_RATE(6144, 6, 215, 7, 2, 0,
[PATCH v4 1/4] clk: rockchip: add dt-binding header for rk3328
Add the dt-bindings header for the rk3328, that gets shared between the clock controller and the clock references in the dts. Add softreset ID for rk3328. Signed-off-by: Elaine Zhang --- include/dt-bindings/clock/rk3328-cru.h | 403 + 1 file changed, 403 insertions(+) create mode 100644 include/dt-bindings/clock/rk3328-cru.h diff --git a/include/dt-bindings/clock/rk3328-cru.h b/include/dt-bindings/clock/rk3328-cru.h new file mode 100644 index ..545ed7541316 --- /dev/null +++ b/include/dt-bindings/clock/rk3328-cru.h @@ -0,0 +1,403 @@ +/* + * Copyright (c) 2016 Rockchip Electronics Co. Ltd. + * Author: Elaine + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#ifndef _DT_BINDINGS_CLK_ROCKCHIP_RK3328_H +#define _DT_BINDINGS_CLK_ROCKCHIP_RK3328_H + +/* core clocks */ +#define PLL_APLL 1 +#define PLL_DPLL 2 +#define PLL_CPLL 3 +#define PLL_GPLL 4 +#define PLL_NPLL 5 +#define ARMCLK 6 + +/* sclk gates (special clocks) */ +#define SCLK_RTC32K30 +#define SCLK_SDMMC_EXT 31 +#define SCLK_SPI 32 +#define SCLK_SDMMC 33 +#define SCLK_SDIO 34 +#define SCLK_EMMC 35 +#define SCLK_TSADC 36 +#define SCLK_SARADC37 +#define SCLK_UART0 38 +#define SCLK_UART1 39 +#define SCLK_UART2 40 +#define SCLK_I2S0 41 +#define SCLK_I2S1 42 +#define SCLK_I2S2 43 +#define SCLK_I2S1_OUT 44 +#define SCLK_I2S2_OUT 45 +#define SCLK_SPDIF 46 +#define SCLK_TIMER047 +#define SCLK_TIMER148 +#define SCLK_TIMER249 +#define SCLK_TIMER350 +#define SCLK_TIMER451 +#define SCLK_TIMER552 +#define SCLK_WIFI 53 +#define SCLK_CIF_OUT 54 +#define SCLK_I2C0 55 +#define SCLK_I2C1 56 +#define SCLK_I2C2 57 +#define SCLK_I2C3 58 +#define SCLK_CRYPTO59 +#define SCLK_PWM 60 +#define SCLK_PDM 61 +#define SCLK_EFUSE 62 +#define SCLK_OTP 63 +#define SCLK_DDRCLK64 +#define SCLK_VDEC_CABAC65 +#define SCLK_VDEC_CORE 66 +#define SCLK_VENC_DSP 67 +#define SCLK_VENC_CORE 68 +#define SCLK_RGA 69 +#define SCLK_HDMI_SFC 70 +#define SCLK_HDMI_CEC 71 +#define SCLK_USB3_REF 72 +#define SCLK_USB3_SUSPEND 73 +#define SCLK_SDMMC_DRV 74 +#define SCLK_SDIO_DRV 75 +#define SCLK_EMMC_DRV 76 +#define SCLK_SDMMC_EXT_DRV 77 +#define SCLK_SDMMC_SAMPLE 78 +#define SCLK_SDIO_SAMPLE 79 +#define SCLK_EMMC_SAMPLE 80 +#define SCLK_SDMMC_EXT_SAMPLE 81 +#define SCLK_VOP 82 +#define SCLK_MAC2PHY_RXTX 83 +#define SCLK_MAC2PHY_SRC 84 +#define SCLK_MAC2PHY_REF 85 +#define SCLK_MAC2PHY_OUT 86 +#define SCLK_MAC2IO_RX 87 +#define SCLK_MAC2IO_TX 88 +#define SCLK_MAC2IO_REFOUT 89 +#define SCLK_MAC2IO_REF90 +#define SCLK_MAC2IO_OUT91 +#define SCLK_TSP 92 +#define SCLK_HSADC_TSP 93 +#define SCLK_USB3PHY_REF 94 +#define SCLK_REF_USB3OTG 95 +#define SCLK_USB3OTG_REF 96 +#define SCLK_USB3OTG_SUSPEND 97 +#define SCLK_REF_USB3OTG_SRC 98 +#define SCLK_MAC2IO_SRC99 + +/* dclk gates */ +#define DCLK_LCDC 180 +#define DCLK_HDMIPHY 181 +#define HDMIPHY182 +#define USB480M183 +#define DCLK_LCDC_SRC 184 + +/* aclk gates */ +#define ACLK_AXISRAM 190 +#define ACLK_VOP_PRE 191 +#define ACLK_USB3OTG 192 +#define ACLK_RGA_PRE 193 +#define ACLK_DMAC 194 +#define ACLK_GPU 195 +#define ACLK_BUS_PRE 196 +#define ACLK_PERI_PRE 197 +#define ACLK_RKVDEC_PRE198 +#define ACLK_RKVDEC199 +#define ACLK_RKVENC200 +#define ACLK_VPU_PRE 201 +#define ACLK_VIO_PRE 202 +#define ACLK_VPU 203 +#define ACLK_VIO 204 +#define ACLK_VOP 205 +#define ACLK_GMAC 206 +#define ACLK_H265 207 +#define ACLK_H264 208 +#define ACLK_MAC2PHY 209 +#define
[PATCH v4 0/4] clk: rockchip: support clk controller for rk3328 SoC
Changes in v4: dropping the "rockchip,cru" and "syscon" properties for bindings of rk3328 adjust the pacth 3 and 4 order. move pll_rk3328 to patch 3. Changes in v3: fix up the pll type pll_rk3328 description and use. Changes in v2: add bindings for rk3328 clock controller Elaine Zhang (4): clk: rockchip: add dt-binding header for rk3328 dt-bindings: add bindings for rk3328 clock controller clk: rockchip: add new pll-type for rk3328 clk: rockchip: add clock controller for rk3328 .../bindings/clock/rockchip,rk3328-cru.txt | 57 ++ drivers/clk/rockchip/Makefile |1 + drivers/clk/rockchip/clk-pll.c | 16 +- drivers/clk/rockchip/clk-rk3328.c | 1068 drivers/clk/rockchip/clk.h | 23 + include/dt-bindings/clock/rk3328-cru.h | 403 6 files changed, 1565 insertions(+), 3 deletions(-) create mode 100644 Documentation/devicetree/bindings/clock/rockchip,rk3328-cru.txt create mode 100644 drivers/clk/rockchip/clk-rk3328.c create mode 100644 include/dt-bindings/clock/rk3328-cru.h -- 1.9.1
[PATCH v4 4/4] clk: rockchip: add clock controller for rk3328
Add the clock tree definition for the new rk3328 SoC. Changes in v4: adjust the pacth 3 and 4 order. Changes in v3: fix up the pll parent only xin24m. Changes in v2: fix up these *_sample error description. Signed-off-by: Elaine Zhang --- drivers/clk/rockchip/Makefile |1 + drivers/clk/rockchip/clk-rk3328.c | 1068 + drivers/clk/rockchip/clk.h| 22 + 3 files changed, 1091 insertions(+) create mode 100644 drivers/clk/rockchip/clk-rk3328.c diff --git a/drivers/clk/rockchip/Makefile b/drivers/clk/rockchip/Makefile index 16e098c36f90..68b04bfca282 100644 --- a/drivers/clk/rockchip/Makefile +++ b/drivers/clk/rockchip/Makefile @@ -16,5 +16,6 @@ obj-y += clk-rk3036.o obj-y += clk-rk3188.o obj-y += clk-rk3228.o obj-y += clk-rk3288.o +obj-y += clk-rk3328.o obj-y += clk-rk3368.o obj-y += clk-rk3399.o diff --git a/drivers/clk/rockchip/clk-rk3328.c b/drivers/clk/rockchip/clk-rk3328.c new file mode 100644 index ..9958ce7d0dcd --- /dev/null +++ b/drivers/clk/rockchip/clk-rk3328.c @@ -0,0 +1,1068 @@ +/* + * Copyright (c) 2016 Rockchip Electronics Co. Ltd. + * Author: Elaine + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include "clk.h" + +#define RK3328_GRF_SOC_STATUS0 0x480 +#define RK3328_GRF_MAC_CON10x904 +#define RK3328_GRF_MAC_CON20x908 + +enum rk3328_plls { + apll, dpll, cpll, gpll, npll, +}; + +static struct rockchip_pll_rate_table rk3328_pll_rates[] = { + /* _mhz, _refdiv, _fbdiv, _postdiv1, _postdiv2, _dsmpd, _frac */ + RK3036_PLL_RATE(160800, 1, 67, 1, 1, 1, 0), + RK3036_PLL_RATE(158400, 1, 66, 1, 1, 1, 0), + RK3036_PLL_RATE(156000, 1, 65, 1, 1, 1, 0), + RK3036_PLL_RATE(153600, 1, 64, 1, 1, 1, 0), + RK3036_PLL_RATE(151200, 1, 63, 1, 1, 1, 0), + RK3036_PLL_RATE(148800, 1, 62, 1, 1, 1, 0), + RK3036_PLL_RATE(146400, 1, 61, 1, 1, 1, 0), + RK3036_PLL_RATE(144000, 1, 60, 1, 1, 1, 0), + RK3036_PLL_RATE(141600, 1, 59, 1, 1, 1, 0), + RK3036_PLL_RATE(139200, 1, 58, 1, 1, 1, 0), + RK3036_PLL_RATE(136800, 1, 57, 1, 1, 1, 0), + RK3036_PLL_RATE(134400, 1, 56, 1, 1, 1, 0), + RK3036_PLL_RATE(132000, 1, 55, 1, 1, 1, 0), + RK3036_PLL_RATE(129600, 1, 54, 1, 1, 1, 0), + RK3036_PLL_RATE(127200, 1, 53, 1, 1, 1, 0), + RK3036_PLL_RATE(124800, 1, 52, 1, 1, 1, 0), + RK3036_PLL_RATE(12, 1, 50, 1, 1, 1, 0), + RK3036_PLL_RATE(118800, 2, 99, 1, 1, 1, 0), + RK3036_PLL_RATE(110400, 1, 46, 1, 1, 1, 0), + RK3036_PLL_RATE(11, 12, 550, 1, 1, 1, 0), + RK3036_PLL_RATE(100800, 1, 84, 2, 1, 1, 0), + RK3036_PLL_RATE(10, 6, 500, 2, 1, 1, 0), + RK3036_PLL_RATE(98400, 1, 82, 2, 1, 1, 0), + RK3036_PLL_RATE(96000, 1, 80, 2, 1, 1, 0), + RK3036_PLL_RATE(93600, 1, 78, 2, 1, 1, 0), + RK3036_PLL_RATE(91200, 1, 76, 2, 1, 1, 0), + RK3036_PLL_RATE(9, 4, 300, 2, 1, 1, 0), + RK3036_PLL_RATE(88800, 1, 74, 2, 1, 1, 0), + RK3036_PLL_RATE(86400, 1, 72, 2, 1, 1, 0), + RK3036_PLL_RATE(84000, 1, 70, 2, 1, 1, 0), + RK3036_PLL_RATE(81600, 1, 68, 2, 1, 1, 0), + RK3036_PLL_RATE(8, 6, 400, 2, 1, 1, 0), + RK3036_PLL_RATE(7, 6, 350, 2, 1, 1, 0), + RK3036_PLL_RATE(69600, 1, 58, 2, 1, 1, 0), + RK3036_PLL_RATE(6, 1, 75, 3, 1, 1, 0), + RK3036_PLL_RATE(59400, 2, 99, 2, 1, 1, 0), + RK3036_PLL_RATE(50400, 1, 63, 3, 1, 1, 0), + RK3036_PLL_RATE(5, 6, 250, 2, 1, 1, 0), + RK3036_PLL_RATE(40800, 1, 68, 2, 2, 1, 0), + RK3036_PLL_RATE(31200, 1, 52, 2, 2, 1, 0), + RK3036_PLL_RATE(21600, 1, 72, 4, 2, 1, 0), + RK3036_PLL_RATE(9600, 1, 64, 4, 4, 1, 0), + { /* sentinel */ }, +}; + +static struct rockchip_pll_rate_table rk3328_pll_frac_rates[] = { + /* _mhz, _refdiv, _fbdiv, _postdiv1, _postdiv2, _dsmpd, _frac */ + RK3036_PLL_RATE(1016064000, 3, 127, 1, 1, 0, 134217), + /* vco = 1016064000 */ + RK3036_PLL_RATE(98304, 24, 983, 1, 1, 0, 671088), + /* vco = 98304 */ + RK3036_PLL_RATE(49152, 24, 983, 2, 1, 0, 671088), + /* vco = 98304 */ + RK3036_PLL_RATE(6144, 6, 215, 7, 2, 0, 671088), + /* vco = 860156000 */ +
Re: [PATCH tip/master v2] kprobes: extable: Identify kprobes' insn-slots as kernel text area
Oops, I found I missed an rcu_read_unlock on the fast path... I'll send v3. Thanks, On Tue, 27 Dec 2016 00:34:20 +0900 Masami Hiramatsuwrote: > Make __kernel_text_address()/kernel_text_address() returns > true if the given address is on a kprobe's instruction slot, > which is generated by kprobes as a trampoline code. > This can help stacktraces to determine the address is on a > text area or not. > > To implement this without any sleep in is_kprobe_*_slot(), > this also modify insn_cache page list as a rcu list. It may > increase processing deley (not processing time) for garbage > slot collection, because it requires to wait an additional > rcu grance period when freeing a page from the list. > However, since it is not a hot path, we may not take care of it. > > Signed-off-by: Masami Hiramatsu > > --- > V2: Fix build error when CONFIG_KPROBES=n > > Hi Josh, could check this patch fixes your issue? It will > enable unwinder code to validate return address by using > __kernel_text_address() again. > --- > include/linux/kprobes.h | 22 ++- > kernel/extable.c|9 +- > kernel/kprobes.c| 70 > --- > 3 files changed, 82 insertions(+), 19 deletions(-) > > diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h > index 8f68490..f0496b0 100644 > --- a/include/linux/kprobes.h > +++ b/include/linux/kprobes.h > @@ -281,6 +281,9 @@ struct kprobe_insn_cache { > extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c); > extern void __free_insn_slot(struct kprobe_insn_cache *c, >kprobe_opcode_t *slot, int dirty); > +/* sleep-less address checking routine */ > +extern bool __is_insn_slot_addr(struct kprobe_insn_cache *c, > + unsigned long addr); > > #define DEFINE_INSN_CACHE_OPS(__name) > \ > extern struct kprobe_insn_cache kprobe_##__name##_slots; \ > @@ -294,6 +297,11 @@ static inline void free_##__name##_slot(kprobe_opcode_t > *slot, int dirty)\ > {\ > __free_insn_slot(_##__name##_slots, slot, dirty);\ > }\ > + \ > +static inline bool is_kprobe_##__name##_slot(unsigned long addr) \ > +{\ > + return __is_insn_slot_addr(_##__name##_slots, addr); \ > +} > > DEFINE_INSN_CACHE_OPS(insn); > > @@ -330,7 +338,6 @@ extern int proc_kprobes_optimization_handler(struct > ctl_table *table, >int write, void __user *buffer, >size_t *length, loff_t *ppos); > #endif > - > #endif /* CONFIG_OPTPROBES */ > #ifdef CONFIG_KPROBES_ON_FTRACE > extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, > @@ -481,6 +488,19 @@ static inline int enable_jprobe(struct jprobe *jp) > return enable_kprobe(>kp); > } > > +#ifndef CONFIG_KPROBES > +static inline bool is_kprobe_insn_slot(unsigned long addr) > +{ > + return false; > +} > +#endif > +#ifndef CONFIG_OPTPROBES > +static inline bool is_kprobe_optinsn_slot(unsigned long addr) > +{ > + return false; > +} > +#endif > + > #ifdef CONFIG_KPROBES > /* > * Blacklist ganerating macro. Specify functions which is not probed > diff --git a/kernel/extable.c b/kernel/extable.c > index e820cce..81c9633 100644 > --- a/kernel/extable.c > +++ b/kernel/extable.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -104,6 +105,8 @@ int __kernel_text_address(unsigned long addr) > return 1; > if (is_ftrace_trampoline(addr)) > return 1; > + if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr)) > + return 1; > /* >* There might be init symbols in saved stacktraces. >* Give those symbols a chance to be printed in > @@ -123,7 +126,11 @@ int kernel_text_address(unsigned long addr) > return 1; > if (is_module_text_address(addr)) > return 1; > - return is_ftrace_trampoline(addr); > + if (is_ftrace_trampoline(addr)) > + return 1; > + if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr)) > + return 1; > + return 0; > } > > /* > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index d630954..1bd1c17 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -149,9 +149,11 @@ kprobe_opcode_t *__get_insn_slot(struct > kprobe_insn_cache *c) > struct kprobe_insn_page *kip; > kprobe_opcode_t *slot = NULL; > > + /* Since the slot array is not protected by rcu, we need a mutex */ >
Re: [PATCH tip/master v2] kprobes: extable: Identify kprobes' insn-slots as kernel text area
Oops, I found I missed an rcu_read_unlock on the fast path... I'll send v3. Thanks, On Tue, 27 Dec 2016 00:34:20 +0900 Masami Hiramatsu wrote: > Make __kernel_text_address()/kernel_text_address() returns > true if the given address is on a kprobe's instruction slot, > which is generated by kprobes as a trampoline code. > This can help stacktraces to determine the address is on a > text area or not. > > To implement this without any sleep in is_kprobe_*_slot(), > this also modify insn_cache page list as a rcu list. It may > increase processing deley (not processing time) for garbage > slot collection, because it requires to wait an additional > rcu grance period when freeing a page from the list. > However, since it is not a hot path, we may not take care of it. > > Signed-off-by: Masami Hiramatsu > > --- > V2: Fix build error when CONFIG_KPROBES=n > > Hi Josh, could check this patch fixes your issue? It will > enable unwinder code to validate return address by using > __kernel_text_address() again. > --- > include/linux/kprobes.h | 22 ++- > kernel/extable.c|9 +- > kernel/kprobes.c| 70 > --- > 3 files changed, 82 insertions(+), 19 deletions(-) > > diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h > index 8f68490..f0496b0 100644 > --- a/include/linux/kprobes.h > +++ b/include/linux/kprobes.h > @@ -281,6 +281,9 @@ struct kprobe_insn_cache { > extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c); > extern void __free_insn_slot(struct kprobe_insn_cache *c, >kprobe_opcode_t *slot, int dirty); > +/* sleep-less address checking routine */ > +extern bool __is_insn_slot_addr(struct kprobe_insn_cache *c, > + unsigned long addr); > > #define DEFINE_INSN_CACHE_OPS(__name) > \ > extern struct kprobe_insn_cache kprobe_##__name##_slots; \ > @@ -294,6 +297,11 @@ static inline void free_##__name##_slot(kprobe_opcode_t > *slot, int dirty)\ > {\ > __free_insn_slot(_##__name##_slots, slot, dirty);\ > }\ > + \ > +static inline bool is_kprobe_##__name##_slot(unsigned long addr) \ > +{\ > + return __is_insn_slot_addr(_##__name##_slots, addr); \ > +} > > DEFINE_INSN_CACHE_OPS(insn); > > @@ -330,7 +338,6 @@ extern int proc_kprobes_optimization_handler(struct > ctl_table *table, >int write, void __user *buffer, >size_t *length, loff_t *ppos); > #endif > - > #endif /* CONFIG_OPTPROBES */ > #ifdef CONFIG_KPROBES_ON_FTRACE > extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, > @@ -481,6 +488,19 @@ static inline int enable_jprobe(struct jprobe *jp) > return enable_kprobe(>kp); > } > > +#ifndef CONFIG_KPROBES > +static inline bool is_kprobe_insn_slot(unsigned long addr) > +{ > + return false; > +} > +#endif > +#ifndef CONFIG_OPTPROBES > +static inline bool is_kprobe_optinsn_slot(unsigned long addr) > +{ > + return false; > +} > +#endif > + > #ifdef CONFIG_KPROBES > /* > * Blacklist ganerating macro. Specify functions which is not probed > diff --git a/kernel/extable.c b/kernel/extable.c > index e820cce..81c9633 100644 > --- a/kernel/extable.c > +++ b/kernel/extable.c > @@ -20,6 +20,7 @@ > #include > #include > #include > +#include > > #include > #include > @@ -104,6 +105,8 @@ int __kernel_text_address(unsigned long addr) > return 1; > if (is_ftrace_trampoline(addr)) > return 1; > + if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr)) > + return 1; > /* >* There might be init symbols in saved stacktraces. >* Give those symbols a chance to be printed in > @@ -123,7 +126,11 @@ int kernel_text_address(unsigned long addr) > return 1; > if (is_module_text_address(addr)) > return 1; > - return is_ftrace_trampoline(addr); > + if (is_ftrace_trampoline(addr)) > + return 1; > + if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr)) > + return 1; > + return 0; > } > > /* > diff --git a/kernel/kprobes.c b/kernel/kprobes.c > index d630954..1bd1c17 100644 > --- a/kernel/kprobes.c > +++ b/kernel/kprobes.c > @@ -149,9 +149,11 @@ kprobe_opcode_t *__get_insn_slot(struct > kprobe_insn_cache *c) > struct kprobe_insn_page *kip; > kprobe_opcode_t *slot = NULL; > > + /* Since the slot array is not protected by rcu, we need a mutex */ > mutex_lock(>mutex); > retry: > -
[PATCH tip/master v3] kprobes: extable: Identify kprobes' insn-slots as kernel text area
Make __kernel_text_address()/kernel_text_address() returns true if the given address is on a kprobe's instruction slot, which is generated by kprobes as a trampoline code. This can help stacktraces to determine the address is on a text area or not. To implement this without any sleep in is_kprobe_*_slot(), this also modify insn_cache page list as a rcu list. It may increase processing deley (not processing time) for garbage slot collection, because it requires to wait an additional rcu grance period when freeing a page from the list. However, since it is not a hot path, we may not take care of it. Signed-off-by: Masami Hiramatsu--- V3: - Fix build error on archs which don't need insn_slot (e.g. sh, sparc). - Fix a missed rcu_read_unlock() in fast path of __get_insn_slot. --- include/linux/kprobes.h | 30 kernel/extable.c|9 +- kernel/kprobes.c| 69 +++ 3 files changed, 89 insertions(+), 19 deletions(-) diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h index 8f68490..16ddfb8 100644 --- a/include/linux/kprobes.h +++ b/include/linux/kprobes.h @@ -278,9 +278,13 @@ struct kprobe_insn_cache { int nr_garbage; }; +#ifdef __ARCH_WANT_KPROBES_INSN_SLOT extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c); extern void __free_insn_slot(struct kprobe_insn_cache *c, kprobe_opcode_t *slot, int dirty); +/* sleep-less address checking routine */ +extern bool __is_insn_slot_addr(struct kprobe_insn_cache *c, + unsigned long addr); #define DEFINE_INSN_CACHE_OPS(__name) \ extern struct kprobe_insn_cache kprobe_##__name##_slots; \ @@ -294,6 +298,18 @@ static inline void free_##__name##_slot(kprobe_opcode_t *slot, int dirty)\ { \ __free_insn_slot(_##__name##_slots, slot, dirty);\ } \ + \ +static inline bool is_kprobe_##__name##_slot(unsigned long addr) \ +{ \ + return __is_insn_slot_addr(_##__name##_slots, addr); \ +} +#else /* __ARCH_WANT_KPROBES_INSN_SLOT */ +#define DEFINE_INSN_CACHE_OPS(__name) \ +static inline bool is_kprobe_##__name##_slot(unsigned long addr) \ +{ \ + return 0; \ +} +#endif DEFINE_INSN_CACHE_OPS(insn); @@ -330,7 +346,6 @@ extern int proc_kprobes_optimization_handler(struct ctl_table *table, int write, void __user *buffer, size_t *length, loff_t *ppos); #endif - #endif /* CONFIG_OPTPROBES */ #ifdef CONFIG_KPROBES_ON_FTRACE extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, @@ -481,6 +496,19 @@ static inline int enable_jprobe(struct jprobe *jp) return enable_kprobe(>kp); } +#ifndef CONFIG_KPROBES +static inline bool is_kprobe_insn_slot(unsigned long addr) +{ + return false; +} +#endif +#ifndef CONFIG_OPTPROBES +static inline bool is_kprobe_optinsn_slot(unsigned long addr) +{ + return false; +} +#endif + #ifdef CONFIG_KPROBES /* * Blacklist ganerating macro. Specify functions which is not probed diff --git a/kernel/extable.c b/kernel/extable.c index e820cce..81c9633 100644 --- a/kernel/extable.c +++ b/kernel/extable.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -104,6 +105,8 @@ int __kernel_text_address(unsigned long addr) return 1; if (is_ftrace_trampoline(addr)) return 1; + if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr)) + return 1; /* * There might be init symbols in saved stacktraces. * Give those symbols a chance to be printed in @@ -123,7 +126,11 @@ int kernel_text_address(unsigned long addr) return 1; if (is_module_text_address(addr)) return 1; - return is_ftrace_trampoline(addr); + if (is_ftrace_trampoline(addr)) + return 1; + if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr)) + return 1; + return 0; } /* diff --git a/kernel/kprobes.c b/kernel/kprobes.c index d630954..be41f6d 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -149,9 +149,11 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c) struct kprobe_insn_page *kip; kprobe_opcode_t *slot = NULL; + /* Since the slot array is not protected by rcu, we need a
[PATCH tip/master v3] kprobes: extable: Identify kprobes' insn-slots as kernel text area
Make __kernel_text_address()/kernel_text_address() returns true if the given address is on a kprobe's instruction slot, which is generated by kprobes as a trampoline code. This can help stacktraces to determine the address is on a text area or not. To implement this without any sleep in is_kprobe_*_slot(), this also modify insn_cache page list as a rcu list. It may increase processing deley (not processing time) for garbage slot collection, because it requires to wait an additional rcu grance period when freeing a page from the list. However, since it is not a hot path, we may not take care of it. Signed-off-by: Masami Hiramatsu --- V3: - Fix build error on archs which don't need insn_slot (e.g. sh, sparc). - Fix a missed rcu_read_unlock() in fast path of __get_insn_slot. --- include/linux/kprobes.h | 30 kernel/extable.c|9 +- kernel/kprobes.c| 69 +++ 3 files changed, 89 insertions(+), 19 deletions(-) diff --git a/include/linux/kprobes.h b/include/linux/kprobes.h index 8f68490..16ddfb8 100644 --- a/include/linux/kprobes.h +++ b/include/linux/kprobes.h @@ -278,9 +278,13 @@ struct kprobe_insn_cache { int nr_garbage; }; +#ifdef __ARCH_WANT_KPROBES_INSN_SLOT extern kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c); extern void __free_insn_slot(struct kprobe_insn_cache *c, kprobe_opcode_t *slot, int dirty); +/* sleep-less address checking routine */ +extern bool __is_insn_slot_addr(struct kprobe_insn_cache *c, + unsigned long addr); #define DEFINE_INSN_CACHE_OPS(__name) \ extern struct kprobe_insn_cache kprobe_##__name##_slots; \ @@ -294,6 +298,18 @@ static inline void free_##__name##_slot(kprobe_opcode_t *slot, int dirty)\ { \ __free_insn_slot(_##__name##_slots, slot, dirty);\ } \ + \ +static inline bool is_kprobe_##__name##_slot(unsigned long addr) \ +{ \ + return __is_insn_slot_addr(_##__name##_slots, addr); \ +} +#else /* __ARCH_WANT_KPROBES_INSN_SLOT */ +#define DEFINE_INSN_CACHE_OPS(__name) \ +static inline bool is_kprobe_##__name##_slot(unsigned long addr) \ +{ \ + return 0; \ +} +#endif DEFINE_INSN_CACHE_OPS(insn); @@ -330,7 +346,6 @@ extern int proc_kprobes_optimization_handler(struct ctl_table *table, int write, void __user *buffer, size_t *length, loff_t *ppos); #endif - #endif /* CONFIG_OPTPROBES */ #ifdef CONFIG_KPROBES_ON_FTRACE extern void kprobe_ftrace_handler(unsigned long ip, unsigned long parent_ip, @@ -481,6 +496,19 @@ static inline int enable_jprobe(struct jprobe *jp) return enable_kprobe(>kp); } +#ifndef CONFIG_KPROBES +static inline bool is_kprobe_insn_slot(unsigned long addr) +{ + return false; +} +#endif +#ifndef CONFIG_OPTPROBES +static inline bool is_kprobe_optinsn_slot(unsigned long addr) +{ + return false; +} +#endif + #ifdef CONFIG_KPROBES /* * Blacklist ganerating macro. Specify functions which is not probed diff --git a/kernel/extable.c b/kernel/extable.c index e820cce..81c9633 100644 --- a/kernel/extable.c +++ b/kernel/extable.c @@ -20,6 +20,7 @@ #include #include #include +#include #include #include @@ -104,6 +105,8 @@ int __kernel_text_address(unsigned long addr) return 1; if (is_ftrace_trampoline(addr)) return 1; + if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr)) + return 1; /* * There might be init symbols in saved stacktraces. * Give those symbols a chance to be printed in @@ -123,7 +126,11 @@ int kernel_text_address(unsigned long addr) return 1; if (is_module_text_address(addr)) return 1; - return is_ftrace_trampoline(addr); + if (is_ftrace_trampoline(addr)) + return 1; + if (is_kprobe_optinsn_slot(addr) || is_kprobe_insn_slot(addr)) + return 1; + return 0; } /* diff --git a/kernel/kprobes.c b/kernel/kprobes.c index d630954..be41f6d 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -149,9 +149,11 @@ kprobe_opcode_t *__get_insn_slot(struct kprobe_insn_cache *c) struct kprobe_insn_page *kip; kprobe_opcode_t *slot = NULL; + /* Since the slot array is not protected by rcu, we need a mutex */
Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy
Hi Jaehoon, On Mon, Dec 26, 2016 at 10:50 AM, Jaehoon Chungwrote: > This patch supports to use Generic Phy framework for Exynos PCIe phy. > When Exynos that supported the pcie want to use the PCIe, > it needs to control the phy resgister. > But it should be more complex to control in their own PCIe device drivers. > > Signed-off-by: Jaehoon Chung > --- > drivers/phy/Kconfig | 9 ++ > drivers/phy/Makefile | 1 + > drivers/phy/phy-exynos-pcie.c | 227 > ++ > 3 files changed, 237 insertions(+) > create mode 100644 drivers/phy/phy-exynos-pcie.c > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index fe00f91..94b0433 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -341,6 +341,15 @@ config PHY_EXYNOS5_USBDRD > This driver provides PHY interface for USB 3.0 DRD controller > present on Exynos5 SoC series. > > +config PHY_EXYNOS_PCIE > + bool "Exynos PCIe PHY driver" Is there a reason for this not being 'tristate' ? > + depends on ARCH_EXYNOS && OF > + depends on PCI_EXYNOS5433 > + select GENERIC_PHY > + help > + Enable PCIe PHY support for Exynos SoC series. If this driver is for Exynos5433, then same should come in this help text as well. > + This driver provides PHY interface for Exynos PCIe controller. > + > config PHY_PISTACHIO_USB > tristate "IMG Pistachio USB2.0 PHY driver" > depends on MACH_PISTACHIO > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > index a534cf5..586344d 100644 > --- a/drivers/phy/Makefile > +++ b/drivers/phy/Makefile > @@ -38,6 +38,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2) += > phy-exynos4x12-usb2.o > phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o > phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o > obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o > +obj-$(CONFIG_PHY_EXYNOS_PCIE) += phy-exynos-pcie.o > obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)+= phy-qcom-apq8064-sata.o > obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o > obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2) += phy-rockchip-inno-usb2.o > diff --git a/drivers/phy/phy-exynos-pcie.c b/drivers/phy/phy-exynos-pcie.c > new file mode 100644 > index 000..0f5eefd > --- /dev/null > +++ b/drivers/phy/phy-exynos-pcie.c > @@ -0,0 +1,227 @@ > +/* > + * Samsung EXYNOS SoC series PCIe PHY driver > + * > + * Phy provider for PCIe controller on Exynos SoC series > + * > + * Copyright (C) 2016 Samsung Electronics Co., Ltd. > + * Jaehoon Chung > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include nit: It's good to have these includes in alphabetical order. > + > +#define PCIE_EXYNOS5433_PMU_PHY_OFFSET 0x730 > +#define PCIE_PHY_OFFSET(x) ((x) * 0x4) > + > +/* Sysreg Fsys register offset and bit for Exynos5433 */ > +#define PCIE_PHY_MAC_RESET 0x208 > +#define PCIE_MAC_RESET_MASK0xFF > +#define PCIE_MAC_RESET BIT(4) > +#define PCIE_L1SUB_CM_CON 0x1010 > +#define PCIE_REFCLK_GATING_EN BIT(0) > +#define PCIE_PHY_COMMON_RESET 0x1020 > +#define PCIE_PHY_RESET BIT(0) > +#define PCIE_PHY_GLOBAL_RESET 0x1040 > +#define PCIE_GLOBAL_RESET BIT(0) > +#define PCIE_REFCLKBIT(1) > +#define PCIE_REFCLK_MASK 0x16 > +#define PCIE_APP_REQ_EXIT_L1_MODE BIT(5) > + > +enum exynos_pcie_phy_data_type { > + PCIE_PHY_TYPE_EXYNOS5433, > +}; > + > +struct exynos_pcie_phy_data { > + enum exynos_pcie_phy_data_type ctrl_type; Why do we need this controller type ? If there are changes in the IP between different version, then you can as well use different compatibles. > + u32 pmureg_offset; /* PMU_REG offset */ Please use top comments. > + struct phy_ops *ops; > +}; > + > +/* for Exynos pcie phy */ > +struct exynos_pcie_phy { > + const struct exynos_pcie_phy_data *drv_data; > + struct regmap *pmureg; > + struct regmap *fsysreg; > + void __iomem *phy_base; just 'base' ? > +}; > + > +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset) > +{ > + writel(val, base + offset); > +} > + > +static int exynos_pcie_phy_init(struct phy *phy) > +{ > + struct exynos_pcie_phy *ep = phy_get_drvdata(phy); > + > + if (ep->fsysreg) { > + regmap_update_bits(ep->fsysreg, PCIE_PHY_COMMON_RESET, > + PCIE_PHY_RESET, 1); > + regmap_update_bits(ep->fsysreg,
Re: [RFC PATCH 1/6] phy: exynos-pcie: Add support for Exynos PCIe phy
Hi Jaehoon, On Mon, Dec 26, 2016 at 10:50 AM, Jaehoon Chung wrote: > This patch supports to use Generic Phy framework for Exynos PCIe phy. > When Exynos that supported the pcie want to use the PCIe, > it needs to control the phy resgister. > But it should be more complex to control in their own PCIe device drivers. > > Signed-off-by: Jaehoon Chung > --- > drivers/phy/Kconfig | 9 ++ > drivers/phy/Makefile | 1 + > drivers/phy/phy-exynos-pcie.c | 227 > ++ > 3 files changed, 237 insertions(+) > create mode 100644 drivers/phy/phy-exynos-pcie.c > > diff --git a/drivers/phy/Kconfig b/drivers/phy/Kconfig > index fe00f91..94b0433 100644 > --- a/drivers/phy/Kconfig > +++ b/drivers/phy/Kconfig > @@ -341,6 +341,15 @@ config PHY_EXYNOS5_USBDRD > This driver provides PHY interface for USB 3.0 DRD controller > present on Exynos5 SoC series. > > +config PHY_EXYNOS_PCIE > + bool "Exynos PCIe PHY driver" Is there a reason for this not being 'tristate' ? > + depends on ARCH_EXYNOS && OF > + depends on PCI_EXYNOS5433 > + select GENERIC_PHY > + help > + Enable PCIe PHY support for Exynos SoC series. If this driver is for Exynos5433, then same should come in this help text as well. > + This driver provides PHY interface for Exynos PCIe controller. > + > config PHY_PISTACHIO_USB > tristate "IMG Pistachio USB2.0 PHY driver" > depends on MACH_PISTACHIO > diff --git a/drivers/phy/Makefile b/drivers/phy/Makefile > index a534cf5..586344d 100644 > --- a/drivers/phy/Makefile > +++ b/drivers/phy/Makefile > @@ -38,6 +38,7 @@ phy-exynos-usb2-$(CONFIG_PHY_EXYNOS4X12_USB2) += > phy-exynos4x12-usb2.o > phy-exynos-usb2-$(CONFIG_PHY_EXYNOS5250_USB2) += phy-exynos5250-usb2.o > phy-exynos-usb2-$(CONFIG_PHY_S5PV210_USB2) += phy-s5pv210-usb2.o > obj-$(CONFIG_PHY_EXYNOS5_USBDRD) += phy-exynos5-usbdrd.o > +obj-$(CONFIG_PHY_EXYNOS_PCIE) += phy-exynos-pcie.o > obj-$(CONFIG_PHY_QCOM_APQ8064_SATA)+= phy-qcom-apq8064-sata.o > obj-$(CONFIG_PHY_ROCKCHIP_USB) += phy-rockchip-usb.o > obj-$(CONFIG_PHY_ROCKCHIP_INNO_USB2) += phy-rockchip-inno-usb2.o > diff --git a/drivers/phy/phy-exynos-pcie.c b/drivers/phy/phy-exynos-pcie.c > new file mode 100644 > index 000..0f5eefd > --- /dev/null > +++ b/drivers/phy/phy-exynos-pcie.c > @@ -0,0 +1,227 @@ > +/* > + * Samsung EXYNOS SoC series PCIe PHY driver > + * > + * Phy provider for PCIe controller on Exynos SoC series > + * > + * Copyright (C) 2016 Samsung Electronics Co., Ltd. > + * Jaehoon Chung > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License version 2 as > + * published by the Free Software Foundation. > + */ > + > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include > +#include nit: It's good to have these includes in alphabetical order. > + > +#define PCIE_EXYNOS5433_PMU_PHY_OFFSET 0x730 > +#define PCIE_PHY_OFFSET(x) ((x) * 0x4) > + > +/* Sysreg Fsys register offset and bit for Exynos5433 */ > +#define PCIE_PHY_MAC_RESET 0x208 > +#define PCIE_MAC_RESET_MASK0xFF > +#define PCIE_MAC_RESET BIT(4) > +#define PCIE_L1SUB_CM_CON 0x1010 > +#define PCIE_REFCLK_GATING_EN BIT(0) > +#define PCIE_PHY_COMMON_RESET 0x1020 > +#define PCIE_PHY_RESET BIT(0) > +#define PCIE_PHY_GLOBAL_RESET 0x1040 > +#define PCIE_GLOBAL_RESET BIT(0) > +#define PCIE_REFCLKBIT(1) > +#define PCIE_REFCLK_MASK 0x16 > +#define PCIE_APP_REQ_EXIT_L1_MODE BIT(5) > + > +enum exynos_pcie_phy_data_type { > + PCIE_PHY_TYPE_EXYNOS5433, > +}; > + > +struct exynos_pcie_phy_data { > + enum exynos_pcie_phy_data_type ctrl_type; Why do we need this controller type ? If there are changes in the IP between different version, then you can as well use different compatibles. > + u32 pmureg_offset; /* PMU_REG offset */ Please use top comments. > + struct phy_ops *ops; > +}; > + > +/* for Exynos pcie phy */ > +struct exynos_pcie_phy { > + const struct exynos_pcie_phy_data *drv_data; > + struct regmap *pmureg; > + struct regmap *fsysreg; > + void __iomem *phy_base; just 'base' ? > +}; > + > +static void exynos_pcie_phy_writel(void __iomem *base, u32 val, u32 offset) > +{ > + writel(val, base + offset); > +} > + > +static int exynos_pcie_phy_init(struct phy *phy) > +{ > + struct exynos_pcie_phy *ep = phy_get_drvdata(phy); > + > + if (ep->fsysreg) { > + regmap_update_bits(ep->fsysreg, PCIE_PHY_COMMON_RESET, > + PCIE_PHY_RESET, 1); > + regmap_update_bits(ep->fsysreg, PCIE_PHY_MAC_RESET, > + PCIE_MAC_RESET, 0); >
RE: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Tuesday, December 27, 2016 12:55 PM > To: Kweh, Hock Leong> Cc: joao.pi...@synopsys.com; peppe.cavall...@st.com; > seraphin.bonna...@st.com; alexandre.tor...@gmail.com; > manab...@gmail.com; niklas.cas...@axis.com; jo...@kernel.org; > pa...@ucw.cz; Ong, Boon Leong ; > net...@vger.kernel.org; linux-kernel@vger.kernel.org; Voon, Weifeng > ; lars.pers...@axis.com > Subject: Re: [PATCH] net: stmmac: synchronize stmmac_open and > stmmac_dvr_probe > > From: "Kweh, Hock Leong" > Date: Tue, 27 Dec 2016 19:44:59 +0800 > > > From: "Kweh, Hock Leong" > > > > If kernel module stmmac driver being loaded after OS booted, there is a > > race condition between stmmac_open() and stmmac_mdio_register(), which is > > invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as > > PHY not found and stmmac_open() failed: > ... > > The resolution used wait_for_completion_interruptible() to synchronize > > stmmac_open() and stmmac_dvr_probe() to prevent the race condition > > happening. > > > > Signed-off-by: Kweh, Hock Leong > > The proper thing to do is to make sure register_netdevice() is not > invoked until it is %100 safe to call stmmac_open(). Noted & thanks. Will look into it. Regards, Wilson
RE: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
> -Original Message- > From: David Miller [mailto:da...@davemloft.net] > Sent: Tuesday, December 27, 2016 12:55 PM > To: Kweh, Hock Leong > Cc: joao.pi...@synopsys.com; peppe.cavall...@st.com; > seraphin.bonna...@st.com; alexandre.tor...@gmail.com; > manab...@gmail.com; niklas.cas...@axis.com; jo...@kernel.org; > pa...@ucw.cz; Ong, Boon Leong ; > net...@vger.kernel.org; linux-kernel@vger.kernel.org; Voon, Weifeng > ; lars.pers...@axis.com > Subject: Re: [PATCH] net: stmmac: synchronize stmmac_open and > stmmac_dvr_probe > > From: "Kweh, Hock Leong" > Date: Tue, 27 Dec 2016 19:44:59 +0800 > > > From: "Kweh, Hock Leong" > > > > If kernel module stmmac driver being loaded after OS booted, there is a > > race condition between stmmac_open() and stmmac_mdio_register(), which is > > invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as > > PHY not found and stmmac_open() failed: > ... > > The resolution used wait_for_completion_interruptible() to synchronize > > stmmac_open() and stmmac_dvr_probe() to prevent the race condition > > happening. > > > > Signed-off-by: Kweh, Hock Leong > > The proper thing to do is to make sure register_netdevice() is not > invoked until it is %100 safe to call stmmac_open(). Noted & thanks. Will look into it. Regards, Wilson
RE: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
> -Original Message- > From: Florian Fainelli [mailto:f.faine...@gmail.com] > Sent: Tuesday, December 27, 2016 1:14 PM > To: Kweh, Hock Leong; David S. Miller > ; Joao Pinto ; Giuseppe > CAVALLARO ; seraphin.bonna...@st.com > Cc: Alexandre TORGUE ; Joachim Eastwood > ; Niklas Cassel ; Johan Hovold > ; pa...@ucw.cz; Ong, Boon Leong > ; netdev ; LKML ker...@vger.kernel.org>; Voon, Weifeng ; Lars > Persson > Subject: Re: [PATCH] net: stmmac: synchronize stmmac_open and > stmmac_dvr_probe > > > > On 12/26/2016 09:10 PM, Florian Fainelli wrote: > > > > > > On 12/27/2016 03:44 AM, Kweh, Hock Leong wrote: > >> From: "Kweh, Hock Leong" > >> > >> If kernel module stmmac driver being loaded after OS booted, there is a > >> race condition between stmmac_open() and stmmac_mdio_register(), which > is > >> invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as > >> PHY not found and stmmac_open() failed: > >> [ 473.919358] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): > >>stmmac_dvr_probe: warning: cannot get CSR clock > >> [ 473.919382] stmmaceth :01:00.0: no reset control found > >> [ 473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42 > >> [ 473.919429] stmmaceth :01:00.0: DMA HW capability register > supported > >> [ 473.919436] stmmaceth :01:00.0: RX Checksum Offload Engine > supported > >> [ 473.919443] stmmaceth :01:00.0: TX Checksum insertion supported > >> [ 473.919451] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): > >>Enable RX Mitigation via HW Watchdog Timer > >> [ 473.921395] libphy: PHY stmmac-1:00 not found > >> [ 473.921417] stmmaceth :01:00.0 eth0: Could not attach to PHY > >> [ 473.921427] stmmaceth :01:00.0 eth0: stmmac_open: Cannot attach > to > >>PHY (error: -19) > >> [ 473.959710] libphy: stmmac: probed > >> [ 473.959724] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL > >>(stmmac-1:00) active > >> [ 473.959728] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL > >>(stmmac-1:01) > >> [ 473.959731] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL > >>(stmmac-1:02) > >> [ 473.959734] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL > >>(stmmac-1:03) > >> > >> The resolution used wait_for_completion_interruptible() to synchronize > >> stmmac_open() and stmmac_dvr_probe() to prevent the race condition > >> happening. > > > > The proper fix for this would be to have register_netdev() be the last > > thing done in stmmac_drv_probe(), whereas right now, the last thing done > > is stmmac_mdio_register(), leading the window you are seeing here, where > > the network interface can be open prior to all resources being set up, > > including, but not limited to MDIO devices. > > Something like the following untested patch should plug this race: > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index bb40382e205d..5910ea51f8f6 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -3339,13 +3339,6 @@ int stmmac_dvr_probe(struct device *device, > > spin_lock_init(>lock); > > - ret = register_netdev(ndev); > - if (ret) { > - netdev_err(priv->dev, "%s: ERROR %i registering the > device\n", > - __func__, ret); > - goto error_netdev_register; > - } > - > /* If a specific clk_csr value is passed from the platform > * this means that the CSR Clock Range selection cannot be > * changed at run-time and it is fixed. Viceversa the driver'll > try to > @@ -3372,11 +3365,14 @@ int stmmac_dvr_probe(struct device *device, > } > } > > - return 0; > + ret = register_netdev(ndev); > + if (ret) > + netdev_err(priv->dev, "%s: ERROR %i registering the > device\n", > + __func__, ret); > + > + return ret; > > error_mdio_register: > - unregister_netdev(ndev); > -error_netdev_register: > netif_napi_del(>napi); > error_hw_init: > clk_disable_unprepare(priv->pclk); > > -- > Florian Thanks. Will try out to confirm. Regards, Wilson
RE: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
> -Original Message- > From: Florian Fainelli [mailto:f.faine...@gmail.com] > Sent: Tuesday, December 27, 2016 1:14 PM > To: Kweh, Hock Leong ; David S. Miller > ; Joao Pinto ; Giuseppe > CAVALLARO ; seraphin.bonna...@st.com > Cc: Alexandre TORGUE ; Joachim Eastwood > ; Niklas Cassel ; Johan Hovold > ; pa...@ucw.cz; Ong, Boon Leong > ; netdev ; LKML ker...@vger.kernel.org>; Voon, Weifeng ; Lars > Persson > Subject: Re: [PATCH] net: stmmac: synchronize stmmac_open and > stmmac_dvr_probe > > > > On 12/26/2016 09:10 PM, Florian Fainelli wrote: > > > > > > On 12/27/2016 03:44 AM, Kweh, Hock Leong wrote: > >> From: "Kweh, Hock Leong" > >> > >> If kernel module stmmac driver being loaded after OS booted, there is a > >> race condition between stmmac_open() and stmmac_mdio_register(), which > is > >> invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as > >> PHY not found and stmmac_open() failed: > >> [ 473.919358] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): > >>stmmac_dvr_probe: warning: cannot get CSR clock > >> [ 473.919382] stmmaceth :01:00.0: no reset control found > >> [ 473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42 > >> [ 473.919429] stmmaceth :01:00.0: DMA HW capability register > supported > >> [ 473.919436] stmmaceth :01:00.0: RX Checksum Offload Engine > supported > >> [ 473.919443] stmmaceth :01:00.0: TX Checksum insertion supported > >> [ 473.919451] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): > >>Enable RX Mitigation via HW Watchdog Timer > >> [ 473.921395] libphy: PHY stmmac-1:00 not found > >> [ 473.921417] stmmaceth :01:00.0 eth0: Could not attach to PHY > >> [ 473.921427] stmmaceth :01:00.0 eth0: stmmac_open: Cannot attach > to > >>PHY (error: -19) > >> [ 473.959710] libphy: stmmac: probed > >> [ 473.959724] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL > >>(stmmac-1:00) active > >> [ 473.959728] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL > >>(stmmac-1:01) > >> [ 473.959731] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL > >>(stmmac-1:02) > >> [ 473.959734] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL > >>(stmmac-1:03) > >> > >> The resolution used wait_for_completion_interruptible() to synchronize > >> stmmac_open() and stmmac_dvr_probe() to prevent the race condition > >> happening. > > > > The proper fix for this would be to have register_netdev() be the last > > thing done in stmmac_drv_probe(), whereas right now, the last thing done > > is stmmac_mdio_register(), leading the window you are seeing here, where > > the network interface can be open prior to all resources being set up, > > including, but not limited to MDIO devices. > > Something like the following untested patch should plug this race: > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > index bb40382e205d..5910ea51f8f6 100644 > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > @@ -3339,13 +3339,6 @@ int stmmac_dvr_probe(struct device *device, > > spin_lock_init(>lock); > > - ret = register_netdev(ndev); > - if (ret) { > - netdev_err(priv->dev, "%s: ERROR %i registering the > device\n", > - __func__, ret); > - goto error_netdev_register; > - } > - > /* If a specific clk_csr value is passed from the platform > * this means that the CSR Clock Range selection cannot be > * changed at run-time and it is fixed. Viceversa the driver'll > try to > @@ -3372,11 +3365,14 @@ int stmmac_dvr_probe(struct device *device, > } > } > > - return 0; > + ret = register_netdev(ndev); > + if (ret) > + netdev_err(priv->dev, "%s: ERROR %i registering the > device\n", > + __func__, ret); > + > + return ret; > > error_mdio_register: > - unregister_netdev(ndev); > -error_netdev_register: > netif_napi_del(>napi); > error_hw_init: > clk_disable_unprepare(priv->pclk); > > -- > Florian Thanks. Will try out to confirm. Regards, Wilson
Re: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
On 12/26/2016 09:10 PM, Florian Fainelli wrote: > > > On 12/27/2016 03:44 AM, Kweh, Hock Leong wrote: >> From: "Kweh, Hock Leong">> >> If kernel module stmmac driver being loaded after OS booted, there is a >> race condition between stmmac_open() and stmmac_mdio_register(), which is >> invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as >> PHY not found and stmmac_open() failed: >> [ 473.919358] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): >> stmmac_dvr_probe: warning: cannot get CSR clock >> [ 473.919382] stmmaceth :01:00.0: no reset control found >> [ 473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42 >> [ 473.919429] stmmaceth :01:00.0: DMA HW capability register supported >> [ 473.919436] stmmaceth :01:00.0: RX Checksum Offload Engine supported >> [ 473.919443] stmmaceth :01:00.0: TX Checksum insertion supported >> [ 473.919451] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): >> Enable RX Mitigation via HW Watchdog Timer >> [ 473.921395] libphy: PHY stmmac-1:00 not found >> [ 473.921417] stmmaceth :01:00.0 eth0: Could not attach to PHY >> [ 473.921427] stmmaceth :01:00.0 eth0: stmmac_open: Cannot attach to >> PHY (error: -19) >> [ 473.959710] libphy: stmmac: probed >> [ 473.959724] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL >> (stmmac-1:00) active >> [ 473.959728] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL >> (stmmac-1:01) >> [ 473.959731] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL >> (stmmac-1:02) >> [ 473.959734] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL >> (stmmac-1:03) >> >> The resolution used wait_for_completion_interruptible() to synchronize >> stmmac_open() and stmmac_dvr_probe() to prevent the race condition >> happening. > > The proper fix for this would be to have register_netdev() be the last > thing done in stmmac_drv_probe(), whereas right now, the last thing done > is stmmac_mdio_register(), leading the window you are seeing here, where > the network interface can be open prior to all resources being set up, > including, but not limited to MDIO devices. Something like the following untested patch should plug this race: diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index bb40382e205d..5910ea51f8f6 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3339,13 +3339,6 @@ int stmmac_dvr_probe(struct device *device, spin_lock_init(>lock); - ret = register_netdev(ndev); - if (ret) { - netdev_err(priv->dev, "%s: ERROR %i registering the device\n", - __func__, ret); - goto error_netdev_register; - } - /* If a specific clk_csr value is passed from the platform * this means that the CSR Clock Range selection cannot be * changed at run-time and it is fixed. Viceversa the driver'll try to @@ -3372,11 +3365,14 @@ int stmmac_dvr_probe(struct device *device, } } - return 0; + ret = register_netdev(ndev); + if (ret) + netdev_err(priv->dev, "%s: ERROR %i registering the device\n", + __func__, ret); + + return ret; error_mdio_register: - unregister_netdev(ndev); -error_netdev_register: netif_napi_del(>napi); error_hw_init: clk_disable_unprepare(priv->pclk); -- Florian
Re: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
On 12/26/2016 09:10 PM, Florian Fainelli wrote: > > > On 12/27/2016 03:44 AM, Kweh, Hock Leong wrote: >> From: "Kweh, Hock Leong" >> >> If kernel module stmmac driver being loaded after OS booted, there is a >> race condition between stmmac_open() and stmmac_mdio_register(), which is >> invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as >> PHY not found and stmmac_open() failed: >> [ 473.919358] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): >> stmmac_dvr_probe: warning: cannot get CSR clock >> [ 473.919382] stmmaceth :01:00.0: no reset control found >> [ 473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42 >> [ 473.919429] stmmaceth :01:00.0: DMA HW capability register supported >> [ 473.919436] stmmaceth :01:00.0: RX Checksum Offload Engine supported >> [ 473.919443] stmmaceth :01:00.0: TX Checksum insertion supported >> [ 473.919451] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): >> Enable RX Mitigation via HW Watchdog Timer >> [ 473.921395] libphy: PHY stmmac-1:00 not found >> [ 473.921417] stmmaceth :01:00.0 eth0: Could not attach to PHY >> [ 473.921427] stmmaceth :01:00.0 eth0: stmmac_open: Cannot attach to >> PHY (error: -19) >> [ 473.959710] libphy: stmmac: probed >> [ 473.959724] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL >> (stmmac-1:00) active >> [ 473.959728] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL >> (stmmac-1:01) >> [ 473.959731] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL >> (stmmac-1:02) >> [ 473.959734] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL >> (stmmac-1:03) >> >> The resolution used wait_for_completion_interruptible() to synchronize >> stmmac_open() and stmmac_dvr_probe() to prevent the race condition >> happening. > > The proper fix for this would be to have register_netdev() be the last > thing done in stmmac_drv_probe(), whereas right now, the last thing done > is stmmac_mdio_register(), leading the window you are seeing here, where > the network interface can be open prior to all resources being set up, > including, but not limited to MDIO devices. Something like the following untested patch should plug this race: diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index bb40382e205d..5910ea51f8f6 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -3339,13 +3339,6 @@ int stmmac_dvr_probe(struct device *device, spin_lock_init(>lock); - ret = register_netdev(ndev); - if (ret) { - netdev_err(priv->dev, "%s: ERROR %i registering the device\n", - __func__, ret); - goto error_netdev_register; - } - /* If a specific clk_csr value is passed from the platform * this means that the CSR Clock Range selection cannot be * changed at run-time and it is fixed. Viceversa the driver'll try to @@ -3372,11 +3365,14 @@ int stmmac_dvr_probe(struct device *device, } } - return 0; + ret = register_netdev(ndev); + if (ret) + netdev_err(priv->dev, "%s: ERROR %i registering the device\n", + __func__, ret); + + return ret; error_mdio_register: - unregister_netdev(ndev); -error_netdev_register: netif_napi_del(>napi); error_hw_init: clk_disable_unprepare(priv->pclk); -- Florian
Re: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
On 12/27/2016 03:44 AM, Kweh, Hock Leong wrote: > From: "Kweh, Hock Leong"> > If kernel module stmmac driver being loaded after OS booted, there is a > race condition between stmmac_open() and stmmac_mdio_register(), which is > invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as > PHY not found and stmmac_open() failed: > [ 473.919358] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): > stmmac_dvr_probe: warning: cannot get CSR clock > [ 473.919382] stmmaceth :01:00.0: no reset control found > [ 473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42 > [ 473.919429] stmmaceth :01:00.0: DMA HW capability register supported > [ 473.919436] stmmaceth :01:00.0: RX Checksum Offload Engine supported > [ 473.919443] stmmaceth :01:00.0: TX Checksum insertion supported > [ 473.919451] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): > Enable RX Mitigation via HW Watchdog Timer > [ 473.921395] libphy: PHY stmmac-1:00 not found > [ 473.921417] stmmaceth :01:00.0 eth0: Could not attach to PHY > [ 473.921427] stmmaceth :01:00.0 eth0: stmmac_open: Cannot attach to > PHY (error: -19) > [ 473.959710] libphy: stmmac: probed > [ 473.959724] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL > (stmmac-1:00) active > [ 473.959728] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL > (stmmac-1:01) > [ 473.959731] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL > (stmmac-1:02) > [ 473.959734] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL > (stmmac-1:03) > > The resolution used wait_for_completion_interruptible() to synchronize > stmmac_open() and stmmac_dvr_probe() to prevent the race condition > happening. The proper fix for this would be to have register_netdev() be the last thing done in stmmac_drv_probe(), whereas right now, the last thing done is stmmac_mdio_register(), leading the window you are seeing here, where the network interface can be open prior to all resources being set up, including, but not limited to MDIO devices. -- Florian
Re: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
On 12/27/2016 03:44 AM, Kweh, Hock Leong wrote: > From: "Kweh, Hock Leong" > > If kernel module stmmac driver being loaded after OS booted, there is a > race condition between stmmac_open() and stmmac_mdio_register(), which is > invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as > PHY not found and stmmac_open() failed: > [ 473.919358] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): > stmmac_dvr_probe: warning: cannot get CSR clock > [ 473.919382] stmmaceth :01:00.0: no reset control found > [ 473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42 > [ 473.919429] stmmaceth :01:00.0: DMA HW capability register supported > [ 473.919436] stmmaceth :01:00.0: RX Checksum Offload Engine supported > [ 473.919443] stmmaceth :01:00.0: TX Checksum insertion supported > [ 473.919451] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): > Enable RX Mitigation via HW Watchdog Timer > [ 473.921395] libphy: PHY stmmac-1:00 not found > [ 473.921417] stmmaceth :01:00.0 eth0: Could not attach to PHY > [ 473.921427] stmmaceth :01:00.0 eth0: stmmac_open: Cannot attach to > PHY (error: -19) > [ 473.959710] libphy: stmmac: probed > [ 473.959724] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL > (stmmac-1:00) active > [ 473.959728] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL > (stmmac-1:01) > [ 473.959731] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL > (stmmac-1:02) > [ 473.959734] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL > (stmmac-1:03) > > The resolution used wait_for_completion_interruptible() to synchronize > stmmac_open() and stmmac_dvr_probe() to prevent the race condition > happening. The proper fix for this would be to have register_netdev() be the last thing done in stmmac_drv_probe(), whereas right now, the last thing done is stmmac_mdio_register(), leading the window you are seeing here, where the network interface can be open prior to all resources being set up, including, but not limited to MDIO devices. -- Florian
Re: [PATCH v2] watchdog: constify watchdog_info structures
Hi Bhumika Goyal, On Mon, Dec 26, 2016 at 10:35:11PM +0530, Bhumika Goyal wrote: > Declare watchdog_info structures as const as they are only stored in the > info field of watchdog_device structures. This field is of type const > struct watchdog_info *, so watchdog_info structures having this property > can be declared const too. > Done using Coccinelle: > > @r1 disable optional_qualifier@ > identifier i; > position p; > @@ > static struct watchdog_info i@p={...}; > > @ok@ > identifier r1.i; > position p; > struct watchdog_device obj; > @@ > obj.info=@p; > > @bad@ > position p!={r1.p,ok.p}; > identifier r1.i; > @@ > i@p > > @depends on !bad disable optional_qualifier@ > identifier r1.i; > @@ > +const > struct watchdog_info i; > > Signed-off-by: Bhumika GoyalFor digicolor_wdt.c: Acked-by: Baruch Siach Thanks, baruch -- http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}ooO--U--Ooo{= - bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
Re: [PATCH v2] watchdog: constify watchdog_info structures
Hi Bhumika Goyal, On Mon, Dec 26, 2016 at 10:35:11PM +0530, Bhumika Goyal wrote: > Declare watchdog_info structures as const as they are only stored in the > info field of watchdog_device structures. This field is of type const > struct watchdog_info *, so watchdog_info structures having this property > can be declared const too. > Done using Coccinelle: > > @r1 disable optional_qualifier@ > identifier i; > position p; > @@ > static struct watchdog_info i@p={...}; > > @ok@ > identifier r1.i; > position p; > struct watchdog_device obj; > @@ > obj.info=@p; > > @bad@ > position p!={r1.p,ok.p}; > identifier r1.i; > @@ > i@p > > @depends on !bad disable optional_qualifier@ > identifier r1.i; > @@ > +const > struct watchdog_info i; > > Signed-off-by: Bhumika Goyal For digicolor_wdt.c: Acked-by: Baruch Siach Thanks, baruch -- http://baruch.siach.name/blog/ ~. .~ Tk Open Systems =}ooO--U--Ooo{= - bar...@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
Re: Unregistering extcon providers while they are in use leads to kernel crashes
Hi Hans, Thanks for your report. I'll check this problem and try to resolve it. On 2016년 12월 22일 00:54, Hans de Goede wrote: > Hi, > > With the recent extcon work I've been doing I noticed that > if I want to rmmod and then insmod say extcon_axp288 I can > do so without problems even if axp288_charger is holding > a reference to the extcon device returned by extcon_get_extcon_dev. > > The problem is that extcon_get_extcon_dev simply looks up > the extcon-device in the list of current registered extcon-s > and then returns a pointer to it, without any reference > counting. > > The rmmod scenario can be fixed by doing a module_get from > extcon_get_extcon_dev, but that still leaves the same problem > when root manually unbinds the driver through sysfs. > > A possible way fix this would be: > > 1) Make all extcon providers use devm_extcon_dev_allocate and document > using this to allocate an extcon_dev mandatory > > 2) Add a refcount to struct extcon_dev and introduce extcon_dev_get > and extcon_dev_put helpers which modify the refcount and only free > the memory on the final put (and make the evm_extcon_dev_allocate > cleanup function call extcon_dev_put) > > 3) On extcon_dev_unregister set a flag in the extcon_dev that it > has been free-ed, make all extcon consumer functions which take > an extcon_dev (extcon_get_state, extcon_register_notifier, etc.) > check this flag and return -ENODEV when the extcon has been unregistered > > 4) Make extcon_get_extcon_dev call extcon_dev_get on the returned edev > before returning it > > From here on we've fixed the crash, but we now leak the extcon_dev > when the consumer gets unbound. > > 5) Add a devm_extcon_get_extcon_dev which calls extcon_dev_put as the devm > cleanup function > > 6) Convert all extcon consumers to use devm_extcon_get_extcon_dev > > Regards, > > Hans > > > -- Regards, Chanwoo Choi
Re: Unregistering extcon providers while they are in use leads to kernel crashes
Hi Hans, Thanks for your report. I'll check this problem and try to resolve it. On 2016년 12월 22일 00:54, Hans de Goede wrote: > Hi, > > With the recent extcon work I've been doing I noticed that > if I want to rmmod and then insmod say extcon_axp288 I can > do so without problems even if axp288_charger is holding > a reference to the extcon device returned by extcon_get_extcon_dev. > > The problem is that extcon_get_extcon_dev simply looks up > the extcon-device in the list of current registered extcon-s > and then returns a pointer to it, without any reference > counting. > > The rmmod scenario can be fixed by doing a module_get from > extcon_get_extcon_dev, but that still leaves the same problem > when root manually unbinds the driver through sysfs. > > A possible way fix this would be: > > 1) Make all extcon providers use devm_extcon_dev_allocate and document > using this to allocate an extcon_dev mandatory > > 2) Add a refcount to struct extcon_dev and introduce extcon_dev_get > and extcon_dev_put helpers which modify the refcount and only free > the memory on the final put (and make the evm_extcon_dev_allocate > cleanup function call extcon_dev_put) > > 3) On extcon_dev_unregister set a flag in the extcon_dev that it > has been free-ed, make all extcon consumer functions which take > an extcon_dev (extcon_get_state, extcon_register_notifier, etc.) > check this flag and return -ENODEV when the extcon has been unregistered > > 4) Make extcon_get_extcon_dev call extcon_dev_get on the returned edev > before returning it > > From here on we've fixed the crash, but we now leak the extcon_dev > when the consumer gets unbound. > > 5) Add a devm_extcon_get_extcon_dev which calls extcon_dev_put as the devm > cleanup function > > 6) Convert all extcon consumers to use devm_extcon_get_extcon_dev > > Regards, > > Hans > > > -- Regards, Chanwoo Choi
Re: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
From: "Kweh, Hock Leong"Date: Tue, 27 Dec 2016 19:44:59 +0800 > From: "Kweh, Hock Leong" > > If kernel module stmmac driver being loaded after OS booted, there is a > race condition between stmmac_open() and stmmac_mdio_register(), which is > invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as > PHY not found and stmmac_open() failed: ... > The resolution used wait_for_completion_interruptible() to synchronize > stmmac_open() and stmmac_dvr_probe() to prevent the race condition > happening. > > Signed-off-by: Kweh, Hock Leong The proper thing to do is to make sure register_netdevice() is not invoked until it is %100 safe to call stmmac_open().
Re: [PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
From: "Kweh, Hock Leong" Date: Tue, 27 Dec 2016 19:44:59 +0800 > From: "Kweh, Hock Leong" > > If kernel module stmmac driver being loaded after OS booted, there is a > race condition between stmmac_open() and stmmac_mdio_register(), which is > invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as > PHY not found and stmmac_open() failed: ... > The resolution used wait_for_completion_interruptible() to synchronize > stmmac_open() and stmmac_dvr_probe() to prevent the race condition > happening. > > Signed-off-by: Kweh, Hock Leong The proper thing to do is to make sure register_netdevice() is not invoked until it is %100 safe to call stmmac_open().
Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler
Hi, On 12/27/2016 10:58 AM, Baolin Wang wrote: > Hi, > > On 27 December 2016 at 10:39, Lu Baoluwrote: >> Hi, >> >> On 12/26/2016 04:01 PM, Baolin Wang wrote: >>> On some platfroms(like x86 platform), when one core is running the USB >>> gadget >>> irq thread handler by dwc3_thread_interrupt(), meanwhile another core also >>> can >>> respond other interrupts from dwc3 controller and modify the event buffer by >>> dwc3_interrupt() function, that will cause getting the wrong event count in >>> irq thread handler to make the USB function abnormal. >>> >>> We should add spin_lock/unlock() in dwc3_check_event_buf() to avoid this >>> race. >> Why not spin_lock_irq ones? This lock seems to be used in both >> normal and interrupt threads. Or, I missed anything? > I assumed there are no nested interrupts, when one core is running at > interrupt context, then it can not respond any other interrupts, which > means we don't need to disable local IRQ now, right? > Fair enough. Thanks. Best regards, Lu Baolu
Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler
Hi, On 12/27/2016 10:58 AM, Baolin Wang wrote: > Hi, > > On 27 December 2016 at 10:39, Lu Baolu wrote: >> Hi, >> >> On 12/26/2016 04:01 PM, Baolin Wang wrote: >>> On some platfroms(like x86 platform), when one core is running the USB >>> gadget >>> irq thread handler by dwc3_thread_interrupt(), meanwhile another core also >>> can >>> respond other interrupts from dwc3 controller and modify the event buffer by >>> dwc3_interrupt() function, that will cause getting the wrong event count in >>> irq thread handler to make the USB function abnormal. >>> >>> We should add spin_lock/unlock() in dwc3_check_event_buf() to avoid this >>> race. >> Why not spin_lock_irq ones? This lock seems to be used in both >> normal and interrupt threads. Or, I missed anything? > I assumed there are no nested interrupts, when one core is running at > interrupt context, then it can not respond any other interrupts, which > means we don't need to disable local IRQ now, right? > Fair enough. Thanks. Best regards, Lu Baolu
Re: [PATCH] PCI: exynos: refactor exynos pcie driver
Hi Jaehoon, On 26 December 2016 at 14:32, Jaehoon Chungwrote: > Hi Pankaj, > > On 12/23/2016 07:56 PM, Pankaj Dubey wrote: >> From: Niyas Ahmed S T >> >> Currently Exynos PCIe driver is only supported for Exynos5440 SoC. >> This patch does refactoring of Exynos PCIe driver to extend support >> for other Exynos SoC. >> >> Following are the main changes done via this patch: >> 1) It adds separate structs for memory, clock resources. >> 2) It add exynos_pcie_ops struct which will allow us to support the >> differences in resources in different Exynos SoC. > > It's nice to me for reusing this file. > but after considering too many times, i decided not to use this file. > It would be better if we redesign/modify existing driver to support new single driver for all Exynos SoC. For this we can have internal discussion and design it so that it becomes suitable for other Exynos SoC PCIe controllers. > I'm not sure what block base is..actually this pci-exynos.c is really > black-box. I saw Jingoo already explained about block base, so it may or may not be used in other exynos. If it is used they can reuse this variable and related code, if not the implementation of exynos_pcie_ops hooks should be written in such a way that ignores (do not initializes/uses) block_base. > (No one maintains this file, even Samsung didn't care.) > Who is using this? I feel now we we (you and me) will care for it, at least we have use case for this now :) > If Someone can share the information about exynos5440, i can refactor > everything. > Otherwise, there are two solution.. > We also do not have access to exynos5440 hardware, but we do have access to exynos5440 UM, so we can see some details of these SFRs. Based on our understanding we refactored this code for making it more suitable for other SoC. Regarding phy_base and block_base as they are phy related it need to be replaced with phy driver, so for this we can reuse your RFC patch of pcie-phy. Even phy driver design should be such that it can support multiple Exynos SoC PCIe-Phy. > One is "adding the new pci-exynos.c" likes pci-exynos5433.c I know exynos5433 and exynos7 SoC have similar PCIe controller, although they may share comparatively less similarity with exynos5440. If start adding new file for each SoC we may end of lot of code duplication. > Other is "refactor this file" under assuming the each register's usage. > IMHO, this would be good design decision. We have separate various resources of exynos_pcie struct such as iomem and clks for the timebeing, and these resources can be managed via exynos_pcie_ops which can have different implementation for different SoCs, we have freedom to handle these resources per SoC. If some SoCs share similar PCIe controller h/w design, code will be reused. > I want to use the PHY generic Framework for EXYNOS PCIe. > > If you or other guys really want to use the pci-exynos.c for other exynos, > I will rework with PHY generic framework. Then i will resend the my patches > as V2. > > One more thing..Does anyone know what the usage of block base is? > Can i use that register as "syscon"? > I think this is not exactly syscon, as it only be used in pcie-phy, so we can use this as child device of phy-device. > Best Regards, > Jaehoon Chung > Thanks, Pankaj Dubey
Re: [PATCH] PCI: exynos: refactor exynos pcie driver
Hi Jaehoon, On 26 December 2016 at 14:32, Jaehoon Chung wrote: > Hi Pankaj, > > On 12/23/2016 07:56 PM, Pankaj Dubey wrote: >> From: Niyas Ahmed S T >> >> Currently Exynos PCIe driver is only supported for Exynos5440 SoC. >> This patch does refactoring of Exynos PCIe driver to extend support >> for other Exynos SoC. >> >> Following are the main changes done via this patch: >> 1) It adds separate structs for memory, clock resources. >> 2) It add exynos_pcie_ops struct which will allow us to support the >> differences in resources in different Exynos SoC. > > It's nice to me for reusing this file. > but after considering too many times, i decided not to use this file. > It would be better if we redesign/modify existing driver to support new single driver for all Exynos SoC. For this we can have internal discussion and design it so that it becomes suitable for other Exynos SoC PCIe controllers. > I'm not sure what block base is..actually this pci-exynos.c is really > black-box. I saw Jingoo already explained about block base, so it may or may not be used in other exynos. If it is used they can reuse this variable and related code, if not the implementation of exynos_pcie_ops hooks should be written in such a way that ignores (do not initializes/uses) block_base. > (No one maintains this file, even Samsung didn't care.) > Who is using this? I feel now we we (you and me) will care for it, at least we have use case for this now :) > If Someone can share the information about exynos5440, i can refactor > everything. > Otherwise, there are two solution.. > We also do not have access to exynos5440 hardware, but we do have access to exynos5440 UM, so we can see some details of these SFRs. Based on our understanding we refactored this code for making it more suitable for other SoC. Regarding phy_base and block_base as they are phy related it need to be replaced with phy driver, so for this we can reuse your RFC patch of pcie-phy. Even phy driver design should be such that it can support multiple Exynos SoC PCIe-Phy. > One is "adding the new pci-exynos.c" likes pci-exynos5433.c I know exynos5433 and exynos7 SoC have similar PCIe controller, although they may share comparatively less similarity with exynos5440. If start adding new file for each SoC we may end of lot of code duplication. > Other is "refactor this file" under assuming the each register's usage. > IMHO, this would be good design decision. We have separate various resources of exynos_pcie struct such as iomem and clks for the timebeing, and these resources can be managed via exynos_pcie_ops which can have different implementation for different SoCs, we have freedom to handle these resources per SoC. If some SoCs share similar PCIe controller h/w design, code will be reused. > I want to use the PHY generic Framework for EXYNOS PCIe. > > If you or other guys really want to use the pci-exynos.c for other exynos, > I will rework with PHY generic framework. Then i will resend the my patches > as V2. > > One more thing..Does anyone know what the usage of block base is? > Can i use that register as "syscon"? > I think this is not exactly syscon, as it only be used in pcie-phy, so we can use this as child device of phy-device. > Best Regards, > Jaehoon Chung > Thanks, Pankaj Dubey
Re: [RFC PATCH 1/7] PF driver modified to enable HW filter support, changes works in backward compatibility mode Enable required things in Makefile Enable LZ4 dependecy inside config file
>> #define NIC_MAX_RSS_HASH_BITS 8 >> #define NIC_MAX_RSS_IDR_TBL_SIZE (1 << NIC_MAX_RSS_HASH_BITS) >> +#define NIC_TNS_RSS_IDR_TBL_SIZE 5 > > So you want to use only 5 queues per VF when TNS is enabled, is it ?? > There are 4096 RSS indices in total, for each VF you can use max 32. > I guess you wanted to set no of hash bits to 5 instead of table size. > > SATHA>>> We enabled 8 queues for VF. Yes Macro name misleads it has to be > hash bits, will change this in next version No, I am not referring to any discrepancy in naming the macro. If you check your code - hw->rss_ind_tbl_size = NIC_MAX_RSS_IDR_TBL_SIZE; + hw->rss_ind_tbl_size = veb_enabled ? NIC_TNS_RSS_IDR_TBL_SIZE : + NIC_MAX_RSS_IDR_TBL_SIZE; You are setting RSS table size to 5, i.e RSS hash bits will be set to 2. Hence only 4 queues (not even 5 as i mentioned earlier). Please check 'nicvf_rss_init' you will understand what i am saying. Have you tested and observed pkts in all 8 queues ?
Re: [RFC PATCH 1/7] PF driver modified to enable HW filter support, changes works in backward compatibility mode Enable required things in Makefile Enable LZ4 dependecy inside config file
>> #define NIC_MAX_RSS_HASH_BITS 8 >> #define NIC_MAX_RSS_IDR_TBL_SIZE (1 << NIC_MAX_RSS_HASH_BITS) >> +#define NIC_TNS_RSS_IDR_TBL_SIZE 5 > > So you want to use only 5 queues per VF when TNS is enabled, is it ?? > There are 4096 RSS indices in total, for each VF you can use max 32. > I guess you wanted to set no of hash bits to 5 instead of table size. > > SATHA>>> We enabled 8 queues for VF. Yes Macro name misleads it has to be > hash bits, will change this in next version No, I am not referring to any discrepancy in naming the macro. If you check your code - hw->rss_ind_tbl_size = NIC_MAX_RSS_IDR_TBL_SIZE; + hw->rss_ind_tbl_size = veb_enabled ? NIC_TNS_RSS_IDR_TBL_SIZE : + NIC_MAX_RSS_IDR_TBL_SIZE; You are setting RSS table size to 5, i.e RSS hash bits will be set to 2. Hence only 4 queues (not even 5 as i mentioned earlier). Please check 'nicvf_rss_init' you will understand what i am saying. Have you tested and observed pkts in all 8 queues ?
Re: [PATCH] lib: bitmap: introduce bitmap_find_next_zero_area_and_size
On 2016년 12월 27일 06:09, Michal Nazarewicz wrote: > On Mon, Dec 26 2016, Jaewon Kim wrote: >> There was no bitmap API which returns both next zero index and size of zeros >> from that index. > Is it really needed? Does it noticeably simplifies callers? Why can’t > caller get the size by themselves if they need it? Hi thank you for your comment. As some other functions, this is a helper function to use easily. Without this patch, we can get the size by using two bitmap functions. >> This is helpful to look fragmentation. This is an test code to look size of >> zeros. >> Test result is '10+9+994=>1013 found of total: 1024' >> >> unsigned long search_idx, found_idx, nr_found_tot; >> unsigned long bitmap_max; >> unsigned int nr_found; >> unsigned long *bitmap; >> >> search_idx = nr_found_tot = 0; >> bitmap_max = 1024; >> bitmap = kzalloc(BITS_TO_LONGS(bitmap_max) * sizeof(long), >> GFP_KERNEL); >> >> /* test bitmap_set offset, count */ >> bitmap_set(bitmap, 10, 1); >> bitmap_set(bitmap, 20, 10); >> >> for (;;) { >> found_idx = bitmap_find_next_zero_area_and_size(bitmap, >> bitmap_max, search_idx, _found); >> if (found_idx >= bitmap_max) >> break; >> if (nr_found_tot == 0) >> printk("%u", nr_found); >> else >> printk("+%u", nr_found); >> nr_found_tot += nr_found; >> search_idx = found_idx + nr_found; >> } >> printk("=>%lu found of total: %lu\n", nr_found_tot, bitmap_max); >> >> Signed-off-by: Jaewon Kim>> --- >> include/linux/bitmap.h | 6 ++ >> lib/bitmap.c | 25 + >> 2 files changed, 31 insertions(+) >> >> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h >> index 3b77588..b724a6c 100644 >> --- a/include/linux/bitmap.h >> +++ b/include/linux/bitmap.h >> @@ -46,6 +46,7 @@ >> * bitmap_clear(dst, pos, nbits)Clear specified bit area >> * bitmap_find_next_zero_area(buf, len, pos, n, mask) Find bit free >> area >> * bitmap_find_next_zero_area_off(buf, len, pos, n, mask) as above >> + * bitmap_find_next_zero_area_and_size(buf, len, pos, n, mask) Find >> bit free area and its size >> * bitmap_shift_right(dst, src, n, nbits) *dst = *src >> n >> * bitmap_shift_left(dst, src, n, nbits)*dst = *src << n >> * bitmap_remap(dst, src, old, new, nbits) *dst = map(old, new)(src) >> @@ -123,6 +124,11 @@ extern unsigned long >> bitmap_find_next_zero_area_off(unsigned long *map, >> unsigned long align_mask, >> unsigned long align_offset); >> >> +extern unsigned long bitmap_find_next_zero_area_and_size(unsigned long *map, >> + unsigned long size, >> + unsigned long start, >> + unsigned int *nr); >> + >> /** >> * bitmap_find_next_zero_area - find a contiguous aligned zero area >> * @map: The address to base the search on >> diff --git a/lib/bitmap.c b/lib/bitmap.c >> index 0b66f0e..d02817c 100644 >> --- a/lib/bitmap.c >> +++ b/lib/bitmap.c >> @@ -332,6 +332,31 @@ unsigned long bitmap_find_next_zero_area_off(unsigned >> long *map, >> } >> EXPORT_SYMBOL(bitmap_find_next_zero_area_off); >> >> +/** >> + * bitmap_find_next_zero_area_and_size - find a contiguous aligned zero area >> + * @map: The address to base the search on >> + * @size: The bitmap size in bits >> + * @start: The bitnumber to start searching at >> + * @nr: The number of zeroed bits we've found >> + */ >> +unsigned long bitmap_find_next_zero_area_and_size(unsigned long *map, >> + unsigned long size, >> + unsigned long start, >> + unsigned int *nr) >> +{ >> +unsigned long index, i; >> + >> +*nr = 0; >> +index = find_next_zero_bit(map, size, start); >> + >> +if (index >= size) >> +return index; >> +i = find_next_bit(map, size, index); >> +*nr = i - index; >> +return index; >> +} >> +EXPORT_SYMBOL(bitmap_find_next_zero_area_and_size); >> + >> /* >> * Bitmap printing & parsing functions: first version by Nadia Yvette >> Chambers, >> * second version by Paul Jackson, third by Joe Korty. >> -- >> 1.9.1 >>
Re: [PATCH] lib: bitmap: introduce bitmap_find_next_zero_area_and_size
On 2016년 12월 27일 06:09, Michal Nazarewicz wrote: > On Mon, Dec 26 2016, Jaewon Kim wrote: >> There was no bitmap API which returns both next zero index and size of zeros >> from that index. > Is it really needed? Does it noticeably simplifies callers? Why can’t > caller get the size by themselves if they need it? Hi thank you for your comment. As some other functions, this is a helper function to use easily. Without this patch, we can get the size by using two bitmap functions. >> This is helpful to look fragmentation. This is an test code to look size of >> zeros. >> Test result is '10+9+994=>1013 found of total: 1024' >> >> unsigned long search_idx, found_idx, nr_found_tot; >> unsigned long bitmap_max; >> unsigned int nr_found; >> unsigned long *bitmap; >> >> search_idx = nr_found_tot = 0; >> bitmap_max = 1024; >> bitmap = kzalloc(BITS_TO_LONGS(bitmap_max) * sizeof(long), >> GFP_KERNEL); >> >> /* test bitmap_set offset, count */ >> bitmap_set(bitmap, 10, 1); >> bitmap_set(bitmap, 20, 10); >> >> for (;;) { >> found_idx = bitmap_find_next_zero_area_and_size(bitmap, >> bitmap_max, search_idx, _found); >> if (found_idx >= bitmap_max) >> break; >> if (nr_found_tot == 0) >> printk("%u", nr_found); >> else >> printk("+%u", nr_found); >> nr_found_tot += nr_found; >> search_idx = found_idx + nr_found; >> } >> printk("=>%lu found of total: %lu\n", nr_found_tot, bitmap_max); >> >> Signed-off-by: Jaewon Kim >> --- >> include/linux/bitmap.h | 6 ++ >> lib/bitmap.c | 25 + >> 2 files changed, 31 insertions(+) >> >> diff --git a/include/linux/bitmap.h b/include/linux/bitmap.h >> index 3b77588..b724a6c 100644 >> --- a/include/linux/bitmap.h >> +++ b/include/linux/bitmap.h >> @@ -46,6 +46,7 @@ >> * bitmap_clear(dst, pos, nbits)Clear specified bit area >> * bitmap_find_next_zero_area(buf, len, pos, n, mask) Find bit free >> area >> * bitmap_find_next_zero_area_off(buf, len, pos, n, mask) as above >> + * bitmap_find_next_zero_area_and_size(buf, len, pos, n, mask) Find >> bit free area and its size >> * bitmap_shift_right(dst, src, n, nbits) *dst = *src >> n >> * bitmap_shift_left(dst, src, n, nbits)*dst = *src << n >> * bitmap_remap(dst, src, old, new, nbits) *dst = map(old, new)(src) >> @@ -123,6 +124,11 @@ extern unsigned long >> bitmap_find_next_zero_area_off(unsigned long *map, >> unsigned long align_mask, >> unsigned long align_offset); >> >> +extern unsigned long bitmap_find_next_zero_area_and_size(unsigned long *map, >> + unsigned long size, >> + unsigned long start, >> + unsigned int *nr); >> + >> /** >> * bitmap_find_next_zero_area - find a contiguous aligned zero area >> * @map: The address to base the search on >> diff --git a/lib/bitmap.c b/lib/bitmap.c >> index 0b66f0e..d02817c 100644 >> --- a/lib/bitmap.c >> +++ b/lib/bitmap.c >> @@ -332,6 +332,31 @@ unsigned long bitmap_find_next_zero_area_off(unsigned >> long *map, >> } >> EXPORT_SYMBOL(bitmap_find_next_zero_area_off); >> >> +/** >> + * bitmap_find_next_zero_area_and_size - find a contiguous aligned zero area >> + * @map: The address to base the search on >> + * @size: The bitmap size in bits >> + * @start: The bitnumber to start searching at >> + * @nr: The number of zeroed bits we've found >> + */ >> +unsigned long bitmap_find_next_zero_area_and_size(unsigned long *map, >> + unsigned long size, >> + unsigned long start, >> + unsigned int *nr) >> +{ >> +unsigned long index, i; >> + >> +*nr = 0; >> +index = find_next_zero_bit(map, size, start); >> + >> +if (index >= size) >> +return index; >> +i = find_next_bit(map, size, index); >> +*nr = i - index; >> +return index; >> +} >> +EXPORT_SYMBOL(bitmap_find_next_zero_area_and_size); >> + >> /* >> * Bitmap printing & parsing functions: first version by Nadia Yvette >> Chambers, >> * second version by Paul Jackson, third by Joe Korty. >> -- >> 1.9.1 >>
[PATCH v18 2/4] mailbox: mediatek: Add Mediatek CMDQ driver
This patch is first version of Mediatek Command Queue(CMDQ) driver. The CMDQ is used to help write registers with critical time limitation, such as updating display configuration during the vblank. It controls Global Command Engine (GCE) hardware to achieve this requirement. Currently, CMDQ only supports display related hardwares, but we expect it can be extended to other hardwares for future requirements. Signed-off-by: HS LiaoSigned-off-by: CK Hu --- drivers/mailbox/Kconfig | 10 + drivers/mailbox/Makefile | 2 + drivers/mailbox/mtk-cmdq-mailbox.c | 632 +++ include/linux/mailbox/mtk-cmdq-mailbox.h | 75 4 files changed, 719 insertions(+) create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index ceff415..9108dd4 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -152,4 +152,14 @@ config BCM_PDC_MBOX Mailbox implementation for the Broadcom PDC ring manager, which provides access to various offload engines on Broadcom SoCs. Say Y here if you want to use the Broadcom PDC. + +config MTK_CMDQ_MBOX + bool "MediaTek CMDQ Mailbox Support" + depends on ARM64 && ( ARCH_MEDIATEK || COMPILE_TEST ) + select MTK_INFRACFG + help + Say yes here to add support for the MediaTek Command Queue (CMDQ) + mailbox driver. The CMDQ is used to help read/write registers with + critical time limitation, such as updating display configuration + during the vblank. endif diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 7dde4f6..fad8965 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -31,3 +31,5 @@ obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o obj-$(CONFIG_BCM_PDC_MBOX) += bcm-pdc-mailbox.o obj-$(CONFIG_TEGRA_HSP_MBOX) += tegra-hsp.o + +obj-$(CONFIG_MTK_CMDQ_MBOX)+= mtk-cmdq-mailbox.o diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c new file mode 100644 index 000..8771e57 --- /dev/null +++ b/drivers/mailbox/mtk-cmdq-mailbox.c @@ -0,0 +1,632 @@ +/* + * Copyright (c) 2015 MediaTek Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define CMDQ_THR_MAX_COUNT 3 /* main, sub, general(misc) */ +#define CMDQ_OP_CODE_MASK (0xff << CMDQ_OP_CODE_SHIFT) +#define CMDQ_TIMEOUT_MS1000 +#define CMDQ_IRQ_MASK 0x +#define CMDQ_NUM_CMD(t)(t->cmd_buf_size / CMDQ_INST_SIZE) + +#define CMDQ_CURR_IRQ_STATUS 0x10 +#define CMDQ_THR_SLOT_CYCLES 0x30 + +#define CMDQ_THR_BASE 0x100 +#define CMDQ_THR_SIZE 0x80 +#define CMDQ_THR_WARM_RESET0x00 +#define CMDQ_THR_ENABLE_TASK 0x04 +#define CMDQ_THR_SUSPEND_TASK 0x08 +#define CMDQ_THR_CURR_STATUS 0x0c +#define CMDQ_THR_IRQ_STATUS0x10 +#define CMDQ_THR_IRQ_ENABLE0x14 +#define CMDQ_THR_CURR_ADDR 0x20 +#define CMDQ_THR_END_ADDR 0x24 +#define CMDQ_THR_WAIT_TOKEN0x30 + +#define CMDQ_THR_ENABLED 0x1 +#define CMDQ_THR_DISABLED 0x0 +#define CMDQ_THR_SUSPEND 0x1 +#define CMDQ_THR_RESUME0x0 +#define CMDQ_THR_STATUS_SUSPENDED BIT(1) +#define CMDQ_THR_DO_WARM_RESET BIT(0) +#define CMDQ_THR_ACTIVE_SLOT_CYCLES0x3200 +#define CMDQ_THR_IRQ_DONE 0x1 +#define CMDQ_THR_IRQ_ERROR 0x12 +#define CMDQ_THR_IRQ_EN(CMDQ_THR_IRQ_ERROR | CMDQ_THR_IRQ_DONE) +#define CMDQ_THR_IS_WAITINGBIT(31) + +#define CMDQ_JUMP_BY_OFFSET0x1000 +#define CMDQ_JUMP_BY_PA0x1001 + +struct cmdq_thread { + struct mbox_chan*chan; + void __iomem*base; + struct list_headtask_busy_list; + struct timer_list timeout; + boolatomic_exec; +}; + +struct cmdq_task { + struct cmdq *cmdq; + struct list_headlist_entry; + dma_addr_t pa_base; + struct cmdq_thread *thread; + struct cmdq_pkt *pkt; /* the packet
[PATCH v18 2/4] mailbox: mediatek: Add Mediatek CMDQ driver
This patch is first version of Mediatek Command Queue(CMDQ) driver. The CMDQ is used to help write registers with critical time limitation, such as updating display configuration during the vblank. It controls Global Command Engine (GCE) hardware to achieve this requirement. Currently, CMDQ only supports display related hardwares, but we expect it can be extended to other hardwares for future requirements. Signed-off-by: HS Liao Signed-off-by: CK Hu --- drivers/mailbox/Kconfig | 10 + drivers/mailbox/Makefile | 2 + drivers/mailbox/mtk-cmdq-mailbox.c | 632 +++ include/linux/mailbox/mtk-cmdq-mailbox.h | 75 4 files changed, 719 insertions(+) create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h diff --git a/drivers/mailbox/Kconfig b/drivers/mailbox/Kconfig index ceff415..9108dd4 100644 --- a/drivers/mailbox/Kconfig +++ b/drivers/mailbox/Kconfig @@ -152,4 +152,14 @@ config BCM_PDC_MBOX Mailbox implementation for the Broadcom PDC ring manager, which provides access to various offload engines on Broadcom SoCs. Say Y here if you want to use the Broadcom PDC. + +config MTK_CMDQ_MBOX + bool "MediaTek CMDQ Mailbox Support" + depends on ARM64 && ( ARCH_MEDIATEK || COMPILE_TEST ) + select MTK_INFRACFG + help + Say yes here to add support for the MediaTek Command Queue (CMDQ) + mailbox driver. The CMDQ is used to help read/write registers with + critical time limitation, such as updating display configuration + during the vblank. endif diff --git a/drivers/mailbox/Makefile b/drivers/mailbox/Makefile index 7dde4f6..fad8965 100644 --- a/drivers/mailbox/Makefile +++ b/drivers/mailbox/Makefile @@ -31,3 +31,5 @@ obj-$(CONFIG_HI6220_MBOX) += hi6220-mailbox.o obj-$(CONFIG_BCM_PDC_MBOX) += bcm-pdc-mailbox.o obj-$(CONFIG_TEGRA_HSP_MBOX) += tegra-hsp.o + +obj-$(CONFIG_MTK_CMDQ_MBOX)+= mtk-cmdq-mailbox.o diff --git a/drivers/mailbox/mtk-cmdq-mailbox.c b/drivers/mailbox/mtk-cmdq-mailbox.c new file mode 100644 index 000..8771e57 --- /dev/null +++ b/drivers/mailbox/mtk-cmdq-mailbox.c @@ -0,0 +1,632 @@ +/* + * Copyright (c) 2015 MediaTek Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#define CMDQ_THR_MAX_COUNT 3 /* main, sub, general(misc) */ +#define CMDQ_OP_CODE_MASK (0xff << CMDQ_OP_CODE_SHIFT) +#define CMDQ_TIMEOUT_MS1000 +#define CMDQ_IRQ_MASK 0x +#define CMDQ_NUM_CMD(t)(t->cmd_buf_size / CMDQ_INST_SIZE) + +#define CMDQ_CURR_IRQ_STATUS 0x10 +#define CMDQ_THR_SLOT_CYCLES 0x30 + +#define CMDQ_THR_BASE 0x100 +#define CMDQ_THR_SIZE 0x80 +#define CMDQ_THR_WARM_RESET0x00 +#define CMDQ_THR_ENABLE_TASK 0x04 +#define CMDQ_THR_SUSPEND_TASK 0x08 +#define CMDQ_THR_CURR_STATUS 0x0c +#define CMDQ_THR_IRQ_STATUS0x10 +#define CMDQ_THR_IRQ_ENABLE0x14 +#define CMDQ_THR_CURR_ADDR 0x20 +#define CMDQ_THR_END_ADDR 0x24 +#define CMDQ_THR_WAIT_TOKEN0x30 + +#define CMDQ_THR_ENABLED 0x1 +#define CMDQ_THR_DISABLED 0x0 +#define CMDQ_THR_SUSPEND 0x1 +#define CMDQ_THR_RESUME0x0 +#define CMDQ_THR_STATUS_SUSPENDED BIT(1) +#define CMDQ_THR_DO_WARM_RESET BIT(0) +#define CMDQ_THR_ACTIVE_SLOT_CYCLES0x3200 +#define CMDQ_THR_IRQ_DONE 0x1 +#define CMDQ_THR_IRQ_ERROR 0x12 +#define CMDQ_THR_IRQ_EN(CMDQ_THR_IRQ_ERROR | CMDQ_THR_IRQ_DONE) +#define CMDQ_THR_IS_WAITINGBIT(31) + +#define CMDQ_JUMP_BY_OFFSET0x1000 +#define CMDQ_JUMP_BY_PA0x1001 + +struct cmdq_thread { + struct mbox_chan*chan; + void __iomem*base; + struct list_headtask_busy_list; + struct timer_list timeout; + boolatomic_exec; +}; + +struct cmdq_task { + struct cmdq *cmdq; + struct list_headlist_entry; + dma_addr_t pa_base; + struct cmdq_thread *thread; + struct cmdq_pkt *pkt; /* the packet sent from mailbox client */ +}; + +struct
[PATCH v18 3/4] arm64: dts: mt8173: Add GCE node
This patch adds the device node of the GCE hardware for CMDQ module. Signed-off-by: HS Liao--- arch/arm64/boot/dts/mediatek/mt8173.dtsi | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi index 12e7027..9f93447 100644 --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi @@ -422,6 +422,16 @@ status = "disabled"; }; + gce: gce@10212000 { + compatible = "mediatek,mt8173-gce"; + reg = <0 0x10212000 0 0x1000>; + interrupts = ; + clocks = < CLK_INFRA_GCE>; + clock-names = "gce"; + + #mbox-cells = <2>; + }; + mipi_tx0: mipi-dphy@10215000 { compatible = "mediatek,mt8173-mipi-tx"; reg = <0 0x10215000 0 0x1000>; -- 1.9.1
[PATCH v18 3/4] arm64: dts: mt8173: Add GCE node
This patch adds the device node of the GCE hardware for CMDQ module. Signed-off-by: HS Liao --- arch/arm64/boot/dts/mediatek/mt8173.dtsi | 10 ++ 1 file changed, 10 insertions(+) diff --git a/arch/arm64/boot/dts/mediatek/mt8173.dtsi b/arch/arm64/boot/dts/mediatek/mt8173.dtsi index 12e7027..9f93447 100644 --- a/arch/arm64/boot/dts/mediatek/mt8173.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt8173.dtsi @@ -422,6 +422,16 @@ status = "disabled"; }; + gce: gce@10212000 { + compatible = "mediatek,mt8173-gce"; + reg = <0 0x10212000 0 0x1000>; + interrupts = ; + clocks = < CLK_INFRA_GCE>; + clock-names = "gce"; + + #mbox-cells = <2>; + }; + mipi_tx0: mipi-dphy@10215000 { compatible = "mediatek,mt8173-mipi-tx"; reg = <0 0x10215000 0 0x1000>; -- 1.9.1
[PATCH v18 4/4] soc: mediatek: Add Mediatek CMDQ helper
Add Mediatek CMDQ helper to create CMDQ packet and assemble GCE op code. Signed-off-by: HS Liao--- drivers/soc/mediatek/Kconfig | 11 ++ drivers/soc/mediatek/Makefile | 1 + drivers/soc/mediatek/mtk-cmdq-helper.c | 310 + include/linux/soc/mediatek/mtk-cmdq.h | 174 ++ 4 files changed, 496 insertions(+) create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig index 609bb34..726c09a 100644 --- a/drivers/soc/mediatek/Kconfig +++ b/drivers/soc/mediatek/Kconfig @@ -1,6 +1,17 @@ # # MediaTek SoC drivers # +config MTK_CMDQ + bool "MediaTek CMDQ Support" + depends on ARM64 && ( ARCH_MEDIATEK || COMPILE_TEST ) + select MTK_CMDQ_MBOX + select MTK_INFRACFG + help + Say yes here to add support for the MediaTek Command Queue (CMDQ) + driver. The CMDQ is used to help read/write registers with critical + time limitation, such as updating display configuration during the + vblank. + config MTK_INFRACFG bool "MediaTek INFRACFG Support" depends on ARCH_MEDIATEK || COMPILE_TEST diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile index 12998b0..64ce5ee 100644 --- a/drivers/soc/mediatek/Makefile +++ b/drivers/soc/mediatek/Makefile @@ -1,3 +1,4 @@ +obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c new file mode 100644 index 000..7809e65 --- /dev/null +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -0,0 +1,310 @@ +/* + * Copyright (c) 2015 MediaTek Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include + +#define CMDQ_SUBSYS_SHIFT 16 +#define CMDQ_ARG_A_WRITE_MASK 0x +#define CMDQ_WRITE_ENABLE_MASK BIT(0) +#define CMDQ_EOC_IRQ_ENBIT(0) +#define CMDQ_EOC_CMD ((u64)((CMDQ_CODE_EOC << CMDQ_OP_CODE_SHIFT)) \ + << 32 | CMDQ_EOC_IRQ_EN) + +struct cmdq_subsys { + u32 base; + int id; +}; + +static const struct cmdq_subsys gce_subsys[] = { + {0x1400, 1}, + {0x1401, 2}, + {0x1402, 3}, +}; + +static int cmdq_subsys_base_to_id(u32 base) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(gce_subsys); i++) + if (gce_subsys[i].base == base) + return gce_subsys[i].id; + return -EFAULT; +} + +static int cmdq_pkt_realloc_cmd_buffer(struct cmdq_pkt *pkt, size_t size) +{ + void *new_buf; + + new_buf = krealloc(pkt->va_base, size, GFP_KERNEL | __GFP_ZERO); + if (!new_buf) + return -ENOMEM; + pkt->va_base = new_buf; + pkt->buf_size = size; + return 0; +} + +struct cmdq_base *cmdq_register_device(struct device *dev) +{ + struct cmdq_base *cmdq_base; + struct resource res; + int subsys; + u32 base; + + if (of_address_to_resource(dev->of_node, 0, )) + return NULL; + base = (u32)res.start; + + subsys = cmdq_subsys_base_to_id(base >> 16); + if (subsys < 0) + return NULL; + + cmdq_base = devm_kmalloc(dev, sizeof(*cmdq_base), GFP_KERNEL); + if (!cmdq_base) + return NULL; + cmdq_base->subsys = subsys; + cmdq_base->base = base; + + return cmdq_base; +} +EXPORT_SYMBOL(cmdq_register_device); + +struct cmdq_client *cmdq_mbox_create(struct device *dev, int index) +{ + struct cmdq_client *client; + + client = kzalloc(sizeof(*client), GFP_KERNEL); + client->client.dev = dev; + client->client.tx_block = false; + client->chan = mbox_request_channel(>client, index); + return client; +} +EXPORT_SYMBOL(cmdq_mbox_create); + +void cmdq_mbox_destroy(struct cmdq_client *client) +{ + mbox_free_channel(client->chan); + kfree(client); +} +EXPORT_SYMBOL(cmdq_mbox_destroy); + +int cmdq_pkt_create(struct cmdq_pkt **pkt_ptr) +{ + struct cmdq_pkt *pkt; + int err; + + pkt = kzalloc(sizeof(*pkt), GFP_KERNEL); + if (!pkt) + return -ENOMEM; + err = cmdq_pkt_realloc_cmd_buffer(pkt, PAGE_SIZE); + if (err < 0) { + kfree(pkt); + return err; + } +
[PATCH v18 4/4] soc: mediatek: Add Mediatek CMDQ helper
Add Mediatek CMDQ helper to create CMDQ packet and assemble GCE op code. Signed-off-by: HS Liao --- drivers/soc/mediatek/Kconfig | 11 ++ drivers/soc/mediatek/Makefile | 1 + drivers/soc/mediatek/mtk-cmdq-helper.c | 310 + include/linux/soc/mediatek/mtk-cmdq.h | 174 ++ 4 files changed, 496 insertions(+) create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h diff --git a/drivers/soc/mediatek/Kconfig b/drivers/soc/mediatek/Kconfig index 609bb34..726c09a 100644 --- a/drivers/soc/mediatek/Kconfig +++ b/drivers/soc/mediatek/Kconfig @@ -1,6 +1,17 @@ # # MediaTek SoC drivers # +config MTK_CMDQ + bool "MediaTek CMDQ Support" + depends on ARM64 && ( ARCH_MEDIATEK || COMPILE_TEST ) + select MTK_CMDQ_MBOX + select MTK_INFRACFG + help + Say yes here to add support for the MediaTek Command Queue (CMDQ) + driver. The CMDQ is used to help read/write registers with critical + time limitation, such as updating display configuration during the + vblank. + config MTK_INFRACFG bool "MediaTek INFRACFG Support" depends on ARCH_MEDIATEK || COMPILE_TEST diff --git a/drivers/soc/mediatek/Makefile b/drivers/soc/mediatek/Makefile index 12998b0..64ce5ee 100644 --- a/drivers/soc/mediatek/Makefile +++ b/drivers/soc/mediatek/Makefile @@ -1,3 +1,4 @@ +obj-$(CONFIG_MTK_CMDQ) += mtk-cmdq-helper.o obj-$(CONFIG_MTK_INFRACFG) += mtk-infracfg.o obj-$(CONFIG_MTK_PMIC_WRAP) += mtk-pmic-wrap.o obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c new file mode 100644 index 000..7809e65 --- /dev/null +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -0,0 +1,310 @@ +/* + * Copyright (c) 2015 MediaTek Inc. + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License version 2 as + * published by the Free Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + */ + +#include +#include +#include +#include + +#define CMDQ_SUBSYS_SHIFT 16 +#define CMDQ_ARG_A_WRITE_MASK 0x +#define CMDQ_WRITE_ENABLE_MASK BIT(0) +#define CMDQ_EOC_IRQ_ENBIT(0) +#define CMDQ_EOC_CMD ((u64)((CMDQ_CODE_EOC << CMDQ_OP_CODE_SHIFT)) \ + << 32 | CMDQ_EOC_IRQ_EN) + +struct cmdq_subsys { + u32 base; + int id; +}; + +static const struct cmdq_subsys gce_subsys[] = { + {0x1400, 1}, + {0x1401, 2}, + {0x1402, 3}, +}; + +static int cmdq_subsys_base_to_id(u32 base) +{ + int i; + + for (i = 0; i < ARRAY_SIZE(gce_subsys); i++) + if (gce_subsys[i].base == base) + return gce_subsys[i].id; + return -EFAULT; +} + +static int cmdq_pkt_realloc_cmd_buffer(struct cmdq_pkt *pkt, size_t size) +{ + void *new_buf; + + new_buf = krealloc(pkt->va_base, size, GFP_KERNEL | __GFP_ZERO); + if (!new_buf) + return -ENOMEM; + pkt->va_base = new_buf; + pkt->buf_size = size; + return 0; +} + +struct cmdq_base *cmdq_register_device(struct device *dev) +{ + struct cmdq_base *cmdq_base; + struct resource res; + int subsys; + u32 base; + + if (of_address_to_resource(dev->of_node, 0, )) + return NULL; + base = (u32)res.start; + + subsys = cmdq_subsys_base_to_id(base >> 16); + if (subsys < 0) + return NULL; + + cmdq_base = devm_kmalloc(dev, sizeof(*cmdq_base), GFP_KERNEL); + if (!cmdq_base) + return NULL; + cmdq_base->subsys = subsys; + cmdq_base->base = base; + + return cmdq_base; +} +EXPORT_SYMBOL(cmdq_register_device); + +struct cmdq_client *cmdq_mbox_create(struct device *dev, int index) +{ + struct cmdq_client *client; + + client = kzalloc(sizeof(*client), GFP_KERNEL); + client->client.dev = dev; + client->client.tx_block = false; + client->chan = mbox_request_channel(>client, index); + return client; +} +EXPORT_SYMBOL(cmdq_mbox_create); + +void cmdq_mbox_destroy(struct cmdq_client *client) +{ + mbox_free_channel(client->chan); + kfree(client); +} +EXPORT_SYMBOL(cmdq_mbox_destroy); + +int cmdq_pkt_create(struct cmdq_pkt **pkt_ptr) +{ + struct cmdq_pkt *pkt; + int err; + + pkt = kzalloc(sizeof(*pkt), GFP_KERNEL); + if (!pkt) + return -ENOMEM; + err = cmdq_pkt_realloc_cmd_buffer(pkt, PAGE_SIZE); + if (err < 0) { + kfree(pkt); + return err; + } + *pkt_ptr = pkt; +
[PATCH v18 1/4] dt-bindings: soc: Add documentation for the MediaTek GCE unit
This adds documentation for the MediaTek Global Command Engine (GCE) unit found in MT8173 SoCs. Signed-off-by: HS LiaoAcked-by: Rob Herring --- .../devicetree/bindings/mailbox/mtk-gce.txt| 43 ++ 1 file changed, 43 insertions(+) create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt diff --git a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt new file mode 100644 index 000..d2d3ccb --- /dev/null +++ b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt @@ -0,0 +1,43 @@ +MediaTek GCE +=== + +The Global Command Engine (GCE) is used to help read/write registers with +critical time limitation, such as updating display configuration during the +vblank. The GCE can be used to implement the Command Queue (CMDQ) driver. + +CMDQ driver uses mailbox framework for communication. Please refer to +mailbox.txt for generic information about mailbox device-tree bindings. + +Required properties: +- compatible: Must be "mediatek,mt8173-gce" +- reg: Address range of the GCE unit +- interrupts: The interrupt signal from the GCE block +- clock: Clocks according to the common clock binding +- clock-names: Must be "gce" to stand for GCE clock +- #mbox-cells: Should be 2 + +Required properties for a client device: +- mboxes: client use mailbox to communicate with GCE, it should have this + property and list of phandle, mailbox channel specifiers, and atomic + execution flag. + +Example: + + gce: gce@10212000 { + compatible = "mediatek,mt8173-gce"; + reg = <0 0x10212000 0 0x1000>; + interrupts = ; + clocks = < CLK_INFRA_GCE>; + clock-names = "gce"; + + #mbox-cells = <2>; + }; + +Example for a client device: + + mmsys: clock-controller@1400 { + compatible = "mediatek,mt8173-mmsys"; + mboxes = < 0 1 /* main display with atomic execution */ + 1 1>; /* sub display with atomic execution */ + ... + }; -- 1.9.1
[PATCH v18 1/4] dt-bindings: soc: Add documentation for the MediaTek GCE unit
This adds documentation for the MediaTek Global Command Engine (GCE) unit found in MT8173 SoCs. Signed-off-by: HS Liao Acked-by: Rob Herring --- .../devicetree/bindings/mailbox/mtk-gce.txt| 43 ++ 1 file changed, 43 insertions(+) create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt diff --git a/Documentation/devicetree/bindings/mailbox/mtk-gce.txt b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt new file mode 100644 index 000..d2d3ccb --- /dev/null +++ b/Documentation/devicetree/bindings/mailbox/mtk-gce.txt @@ -0,0 +1,43 @@ +MediaTek GCE +=== + +The Global Command Engine (GCE) is used to help read/write registers with +critical time limitation, such as updating display configuration during the +vblank. The GCE can be used to implement the Command Queue (CMDQ) driver. + +CMDQ driver uses mailbox framework for communication. Please refer to +mailbox.txt for generic information about mailbox device-tree bindings. + +Required properties: +- compatible: Must be "mediatek,mt8173-gce" +- reg: Address range of the GCE unit +- interrupts: The interrupt signal from the GCE block +- clock: Clocks according to the common clock binding +- clock-names: Must be "gce" to stand for GCE clock +- #mbox-cells: Should be 2 + +Required properties for a client device: +- mboxes: client use mailbox to communicate with GCE, it should have this + property and list of phandle, mailbox channel specifiers, and atomic + execution flag. + +Example: + + gce: gce@10212000 { + compatible = "mediatek,mt8173-gce"; + reg = <0 0x10212000 0 0x1000>; + interrupts = ; + clocks = < CLK_INFRA_GCE>; + clock-names = "gce"; + + #mbox-cells = <2>; + }; + +Example for a client device: + + mmsys: clock-controller@1400 { + compatible = "mediatek,mt8173-mmsys"; + mboxes = < 0 1 /* main display with atomic execution */ + 1 1>; /* sub display with atomic execution */ + ... + }; -- 1.9.1
[PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
From: "Kweh, Hock Leong"If kernel module stmmac driver being loaded after OS booted, there is a race condition between stmmac_open() and stmmac_mdio_register(), which is invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as PHY not found and stmmac_open() failed: [ 473.919358] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): stmmac_dvr_probe: warning: cannot get CSR clock [ 473.919382] stmmaceth :01:00.0: no reset control found [ 473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42 [ 473.919429] stmmaceth :01:00.0: DMA HW capability register supported [ 473.919436] stmmaceth :01:00.0: RX Checksum Offload Engine supported [ 473.919443] stmmaceth :01:00.0: TX Checksum insertion supported [ 473.919451] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): Enable RX Mitigation via HW Watchdog Timer [ 473.921395] libphy: PHY stmmac-1:00 not found [ 473.921417] stmmaceth :01:00.0 eth0: Could not attach to PHY [ 473.921427] stmmaceth :01:00.0 eth0: stmmac_open: Cannot attach to PHY (error: -19) [ 473.959710] libphy: stmmac: probed [ 473.959724] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL (stmmac-1:00) active [ 473.959728] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL (stmmac-1:01) [ 473.959731] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL (stmmac-1:02) [ 473.959734] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL (stmmac-1:03) The resolution used wait_for_completion_interruptible() to synchronize stmmac_open() and stmmac_dvr_probe() to prevent the race condition happening. Signed-off-by: Kweh, Hock Leong --- drivers/net/ethernet/stmicro/stmmac/stmmac.h |1 + drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++ 2 files changed, 11 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h index eab04ae..5daf8a5 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h @@ -131,6 +131,7 @@ struct stmmac_priv { u32 rx_tail_addr; u32 tx_tail_addr; u32 mss; + struct completion probe_done; #ifdef CONFIG_DEBUG_FS struct dentry *dbgfs_dir; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index bb40382..28e85f6 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1770,6 +1770,14 @@ static int stmmac_open(struct net_device *dev) struct stmmac_priv *priv = netdev_priv(dev); int ret; + ret = wait_for_completion_interruptible(>probe_done); + if (ret) { + netdev_err(priv->dev, + "%s: Interrupted while waiting probe completion\n", + __func__); + return ret; + } + stmmac_check_ether_addr(priv); if (priv->hw->pcs != STMMAC_PCS_RGMII && @@ -3226,6 +3234,7 @@ int stmmac_dvr_probe(struct device *device, priv = netdev_priv(ndev); priv->device = device; priv->dev = ndev; + init_completion(>probe_done); stmmac_set_ethtool_ops(ndev); priv->pause = pause; @@ -3372,6 +3381,7 @@ int stmmac_dvr_probe(struct device *device, } } + complete_all(>probe_done); return 0; error_mdio_register: -- 1.7.9.5
[PATCH] net: stmmac: synchronize stmmac_open and stmmac_dvr_probe
From: "Kweh, Hock Leong" If kernel module stmmac driver being loaded after OS booted, there is a race condition between stmmac_open() and stmmac_mdio_register(), which is invoked inside stmmac_dvr_probe(), and the error is showed in dmesg log as PHY not found and stmmac_open() failed: [ 473.919358] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): stmmac_dvr_probe: warning: cannot get CSR clock [ 473.919382] stmmaceth :01:00.0: no reset control found [ 473.919412] stmmac - user ID: 0x10, Synopsys ID: 0x42 [ 473.919429] stmmaceth :01:00.0: DMA HW capability register supported [ 473.919436] stmmaceth :01:00.0: RX Checksum Offload Engine supported [ 473.919443] stmmaceth :01:00.0: TX Checksum insertion supported [ 473.919451] stmmaceth :01:00.0 (unnamed net_device) (uninitialized): Enable RX Mitigation via HW Watchdog Timer [ 473.921395] libphy: PHY stmmac-1:00 not found [ 473.921417] stmmaceth :01:00.0 eth0: Could not attach to PHY [ 473.921427] stmmaceth :01:00.0 eth0: stmmac_open: Cannot attach to PHY (error: -19) [ 473.959710] libphy: stmmac: probed [ 473.959724] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 0 IRQ POLL (stmmac-1:00) active [ 473.959728] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 1 IRQ POLL (stmmac-1:01) [ 473.959731] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 2 IRQ POLL (stmmac-1:02) [ 473.959734] stmmaceth :01:00.0 eth0: PHY ID 01410cc2 at 3 IRQ POLL (stmmac-1:03) The resolution used wait_for_completion_interruptible() to synchronize stmmac_open() and stmmac_dvr_probe() to prevent the race condition happening. Signed-off-by: Kweh, Hock Leong --- drivers/net/ethernet/stmicro/stmmac/stmmac.h |1 + drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 10 ++ 2 files changed, 11 insertions(+) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac.h b/drivers/net/ethernet/stmicro/stmmac/stmmac.h index eab04ae..5daf8a5 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac.h +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac.h @@ -131,6 +131,7 @@ struct stmmac_priv { u32 rx_tail_addr; u32 tx_tail_addr; u32 mss; + struct completion probe_done; #ifdef CONFIG_DEBUG_FS struct dentry *dbgfs_dir; diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index bb40382..28e85f6 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c @@ -1770,6 +1770,14 @@ static int stmmac_open(struct net_device *dev) struct stmmac_priv *priv = netdev_priv(dev); int ret; + ret = wait_for_completion_interruptible(>probe_done); + if (ret) { + netdev_err(priv->dev, + "%s: Interrupted while waiting probe completion\n", + __func__); + return ret; + } + stmmac_check_ether_addr(priv); if (priv->hw->pcs != STMMAC_PCS_RGMII && @@ -3226,6 +3234,7 @@ int stmmac_dvr_probe(struct device *device, priv = netdev_priv(ndev); priv->device = device; priv->dev = ndev; + init_completion(>probe_done); stmmac_set_ethtool_ops(ndev); priv->pause = pause; @@ -3372,6 +3381,7 @@ int stmmac_dvr_probe(struct device *device, } } + complete_all(>probe_done); return 0; error_mdio_register: -- 1.7.9.5
[PATCH v18 0/4] Mediatek MT8173 CMDQ support
Hi, This is Mediatek MT8173 Command Queue(CMDQ) driver. The CMDQ is used to help write registers with critical time limitation, such as updating display configuration during the vblank. It controls Global Command Engine (GCE) hardware to achieve this requirement. These patches have a build dependency on top of v4.10-rc1. Changes since v17: - rebase to v4.10-rc1 Best regards, HS Liao HS Liao (4): dt-bindings: soc: Add documentation for the MediaTek GCE unit mailbox: mediatek: Add Mediatek CMDQ driver arm64: dts: mt8173: Add GCE node soc: mediatek: Add Mediatek CMDQ helper .../devicetree/bindings/mailbox/mtk-gce.txt| 43 ++ arch/arm64/boot/dts/mediatek/mt8173.dtsi | 10 + drivers/mailbox/Kconfig| 10 + drivers/mailbox/Makefile | 2 + drivers/mailbox/mtk-cmdq-mailbox.c | 632 + drivers/soc/mediatek/Kconfig | 11 + drivers/soc/mediatek/Makefile | 1 + drivers/soc/mediatek/mtk-cmdq-helper.c | 310 ++ include/linux/mailbox/mtk-cmdq-mailbox.h | 75 +++ include/linux/soc/mediatek/mtk-cmdq.h | 174 ++ 10 files changed, 1268 insertions(+) create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h -- 1.9.1
[PATCH v18 0/4] Mediatek MT8173 CMDQ support
Hi, This is Mediatek MT8173 Command Queue(CMDQ) driver. The CMDQ is used to help write registers with critical time limitation, such as updating display configuration during the vblank. It controls Global Command Engine (GCE) hardware to achieve this requirement. These patches have a build dependency on top of v4.10-rc1. Changes since v17: - rebase to v4.10-rc1 Best regards, HS Liao HS Liao (4): dt-bindings: soc: Add documentation for the MediaTek GCE unit mailbox: mediatek: Add Mediatek CMDQ driver arm64: dts: mt8173: Add GCE node soc: mediatek: Add Mediatek CMDQ helper .../devicetree/bindings/mailbox/mtk-gce.txt| 43 ++ arch/arm64/boot/dts/mediatek/mt8173.dtsi | 10 + drivers/mailbox/Kconfig| 10 + drivers/mailbox/Makefile | 2 + drivers/mailbox/mtk-cmdq-mailbox.c | 632 + drivers/soc/mediatek/Kconfig | 11 + drivers/soc/mediatek/Makefile | 1 + drivers/soc/mediatek/mtk-cmdq-helper.c | 310 ++ include/linux/mailbox/mtk-cmdq-mailbox.h | 75 +++ include/linux/soc/mediatek/mtk-cmdq.h | 174 ++ 10 files changed, 1268 insertions(+) create mode 100644 Documentation/devicetree/bindings/mailbox/mtk-gce.txt create mode 100644 drivers/mailbox/mtk-cmdq-mailbox.c create mode 100644 drivers/soc/mediatek/mtk-cmdq-helper.c create mode 100644 include/linux/mailbox/mtk-cmdq-mailbox.h create mode 100644 include/linux/soc/mediatek/mtk-cmdq.h -- 1.9.1
Re: [PATCH] PCI: exynos: refactor exynos pcie driver
Hi Jingoo, On 24 December 2016 at 00:07, Jingoo Hanwrote: > On Friday, December 23, 2016 5:56 AM, Pankaj Dubey wrote: >> >> From: Niyas Ahmed S T >> >> Currently Exynos PCIe driver is only supported for Exynos5440 SoC. >> This patch does refactoring of Exynos PCIe driver to extend support >> for other Exynos SoC. >> >> Following are the main changes done via this patch: >> 1) It adds separate structs for memory, clock resources. > > What is the reason to separate structs for these? > Please add the reason to this commit message. > It will be helpful. > As we know current driver only supports exynos5440 specific PCIe controller, We also know for sure that different variant of Exynos SoC will have different hardware resources such as iomem, clks, regmap handles etc. for PCIe controller, so our intention behind separating these resources was to make exynos_pcie struct simple instead of making it complex. All data structs will be shared by all Exynos SoCs but different exynos_pcie_ops will give them freedom to use them as per SoC need. We have refactored current driver in such a way that, even with very minimal information about exynos5440 we can add support for new SoC's PCIe controller with maximum core reuse. >> 2) It add exynos_pcie_ops struct which will allow us to support the >> differences in resources in different Exynos SoC. > Thanks for review. Please let us know if you have any other concern, and it would be great if you can review this patch more thoroughly. Thanks, Pankaj > Good. I have no objection. > > Best regards, > Jingoo Han > >> >> No functional change intended. >> >> Signed-off-by: Niyas Ahmed S T >> Signed-off-by: Pankaj Dubey >> --- >> This patch set is prepared on top of Krzysztof's for-next and >> PCIe driver cleanup patch [1] by Jaehoon Chung. >> >> [1]: https://lkml.org/lkml/2016/12/19/44 >> >> >> drivers/pci/host/pci-exynos.c | 346 ++--- >> - >> 1 file changed, 217 insertions(+), 129 deletions(-) >> >> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c >> index 33562cf..2dc54f7 100644 >> --- a/drivers/pci/host/pci-exynos.c >> +++ b/drivers/pci/host/pci-exynos.c >> @@ -17,6 +17,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -28,16 +29,6 @@ >> >> #define to_exynos_pcie(x)container_of(x, struct exynos_pcie, pp) >> >> -struct exynos_pcie { >> - struct pcie_portpp; >> - void __iomem*elbi_base; /* DT 0th resource */ >> - void __iomem*phy_base; /* DT 1st resource */ >> - void __iomem*block_base;/* DT 2nd resource */ >> - int reset_gpio; >> - struct clk *clk; >> - struct clk *bus_clk; >> -}; >> - >> /* PCIe ELBI registers */ >> #define PCIE_IRQ_PULSE 0x000 >> #define IRQ_INTA_ASSERT BIT(0) >> @@ -102,6 +93,122 @@ struct exynos_pcie { >> #define PCIE_PHY_TRSV3_PD_TSVBIT(7) >> #define PCIE_PHY_TRSV3_LVCC 0x31c >> >> +struct exynos_pcie_mem_res { >> + void __iomem *elbi_base; /* DT 0th resource: PCIE CTRL */ >> + void __iomem *phy_base; /* DT 1st resource: PHY CTRL */ >> + void __iomem *block_base; /* DT 2nd resource: PHY ADDITIONAL CTRL >> */ >> +}; >> + >> +struct exynos_pcie_clk_res { >> + struct clk *clk; >> + struct clk *bus_clk; >> +}; >> + >> +struct exynos_pcie { >> + struct pcie_portpp; >> + struct exynos_pcie_mem_res *mem_res; >> + struct exynos_pcie_clk_res *clk_res; >> + const struct exynos_pcie_ops*ops; >> + int reset_gpio; >> +}; >> + >> +struct exynos_pcie_ops { >> + int (*get_mem_resources)(struct platform_device *pdev, >> + struct exynos_pcie *ep); >> + int (*get_clk_resources)(struct exynos_pcie *ep); >> + int (*init_clk_resources)(struct exynos_pcie *ep); >> + void (*deinit_clk_resources)(struct exynos_pcie *ep); >> +}; >> + >> +static int exynos5440_pcie_get_mem_resources(struct platform_device *pdev, >> + struct exynos_pcie *ep) >> +{ >> + struct resource *res; >> + struct device *dev = ep->pp.dev; >> + >> + ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL); >> + if (!ep->mem_res) >> + return -ENOMEM; >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + ep->mem_res->elbi_base = devm_ioremap_resource(dev, res); >> + if (IS_ERR(ep->mem_res->elbi_base)) >> + return PTR_ERR(ep->mem_res->elbi_base); >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); >> + ep->mem_res->phy_base = devm_ioremap_resource(dev, res); >> + if (IS_ERR(ep->mem_res->phy_base)) >> +
Re: [PATCH] PCI: exynos: refactor exynos pcie driver
Hi Jingoo, On 24 December 2016 at 00:07, Jingoo Han wrote: > On Friday, December 23, 2016 5:56 AM, Pankaj Dubey wrote: >> >> From: Niyas Ahmed S T >> >> Currently Exynos PCIe driver is only supported for Exynos5440 SoC. >> This patch does refactoring of Exynos PCIe driver to extend support >> for other Exynos SoC. >> >> Following are the main changes done via this patch: >> 1) It adds separate structs for memory, clock resources. > > What is the reason to separate structs for these? > Please add the reason to this commit message. > It will be helpful. > As we know current driver only supports exynos5440 specific PCIe controller, We also know for sure that different variant of Exynos SoC will have different hardware resources such as iomem, clks, regmap handles etc. for PCIe controller, so our intention behind separating these resources was to make exynos_pcie struct simple instead of making it complex. All data structs will be shared by all Exynos SoCs but different exynos_pcie_ops will give them freedom to use them as per SoC need. We have refactored current driver in such a way that, even with very minimal information about exynos5440 we can add support for new SoC's PCIe controller with maximum core reuse. >> 2) It add exynos_pcie_ops struct which will allow us to support the >> differences in resources in different Exynos SoC. > Thanks for review. Please let us know if you have any other concern, and it would be great if you can review this patch more thoroughly. Thanks, Pankaj > Good. I have no objection. > > Best regards, > Jingoo Han > >> >> No functional change intended. >> >> Signed-off-by: Niyas Ahmed S T >> Signed-off-by: Pankaj Dubey >> --- >> This patch set is prepared on top of Krzysztof's for-next and >> PCIe driver cleanup patch [1] by Jaehoon Chung. >> >> [1]: https://lkml.org/lkml/2016/12/19/44 >> >> >> drivers/pci/host/pci-exynos.c | 346 ++--- >> - >> 1 file changed, 217 insertions(+), 129 deletions(-) >> >> diff --git a/drivers/pci/host/pci-exynos.c b/drivers/pci/host/pci-exynos.c >> index 33562cf..2dc54f7 100644 >> --- a/drivers/pci/host/pci-exynos.c >> +++ b/drivers/pci/host/pci-exynos.c >> @@ -17,6 +17,7 @@ >> #include >> #include >> #include >> +#include >> #include >> #include >> #include >> @@ -28,16 +29,6 @@ >> >> #define to_exynos_pcie(x)container_of(x, struct exynos_pcie, pp) >> >> -struct exynos_pcie { >> - struct pcie_portpp; >> - void __iomem*elbi_base; /* DT 0th resource */ >> - void __iomem*phy_base; /* DT 1st resource */ >> - void __iomem*block_base;/* DT 2nd resource */ >> - int reset_gpio; >> - struct clk *clk; >> - struct clk *bus_clk; >> -}; >> - >> /* PCIe ELBI registers */ >> #define PCIE_IRQ_PULSE 0x000 >> #define IRQ_INTA_ASSERT BIT(0) >> @@ -102,6 +93,122 @@ struct exynos_pcie { >> #define PCIE_PHY_TRSV3_PD_TSVBIT(7) >> #define PCIE_PHY_TRSV3_LVCC 0x31c >> >> +struct exynos_pcie_mem_res { >> + void __iomem *elbi_base; /* DT 0th resource: PCIE CTRL */ >> + void __iomem *phy_base; /* DT 1st resource: PHY CTRL */ >> + void __iomem *block_base; /* DT 2nd resource: PHY ADDITIONAL CTRL >> */ >> +}; >> + >> +struct exynos_pcie_clk_res { >> + struct clk *clk; >> + struct clk *bus_clk; >> +}; >> + >> +struct exynos_pcie { >> + struct pcie_portpp; >> + struct exynos_pcie_mem_res *mem_res; >> + struct exynos_pcie_clk_res *clk_res; >> + const struct exynos_pcie_ops*ops; >> + int reset_gpio; >> +}; >> + >> +struct exynos_pcie_ops { >> + int (*get_mem_resources)(struct platform_device *pdev, >> + struct exynos_pcie *ep); >> + int (*get_clk_resources)(struct exynos_pcie *ep); >> + int (*init_clk_resources)(struct exynos_pcie *ep); >> + void (*deinit_clk_resources)(struct exynos_pcie *ep); >> +}; >> + >> +static int exynos5440_pcie_get_mem_resources(struct platform_device *pdev, >> + struct exynos_pcie *ep) >> +{ >> + struct resource *res; >> + struct device *dev = ep->pp.dev; >> + >> + ep->mem_res = devm_kzalloc(dev, sizeof(*ep->mem_res), GFP_KERNEL); >> + if (!ep->mem_res) >> + return -ENOMEM; >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 0); >> + ep->mem_res->elbi_base = devm_ioremap_resource(dev, res); >> + if (IS_ERR(ep->mem_res->elbi_base)) >> + return PTR_ERR(ep->mem_res->elbi_base); >> + >> + res = platform_get_resource(pdev, IORESOURCE_MEM, 1); >> + ep->mem_res->phy_base = devm_ioremap_resource(dev, res); >> + if (IS_ERR(ep->mem_res->phy_base)) >> + return PTR_ERR(ep->mem_res->phy_base); >> + >> + res = platform_get_resource(pdev,
Re: [alsa-devel] [PATCH 2/2] ASoC: Intel: boards: Add Baytrail RT5660 machine driver
On Tue, 2016-12-27 at 11:29 +0800, Shrirang Bagul wrote: > On Mon, 2016-12-19 at 16:44 +, Mark Brown wrote: > > On Mon, Dec 19, 2016 at 10:30:09AM -0600, Pierre-Louis Bossart wrote: > > > > > All this code seems to be largely a copy-paste of the bytcr_rt5640 machine > > > driver and the same comments would apply there. This patch did miss the > > > last > > > > Yes, there's a lot of room for cleanups in the existing code too (and of > > course if there's such a large amount of cut'n'paste that implies that > > there should be some code reuse going on). > > Thank you for the review and valuable comments. Following the discussion so > far, > I feel the proper way would be to adapt bytcr_rt5660 machine driver to manage > RT5660 codec. typo, should be bytcr_rt5640 > > > > > correction merged by Mark to deal with errors "ASoC: Intel: bytcr_rt5640: > > > fallback mechanism if MCLK is not enabled" and the same error handling > > > would > > > be needed. > > > > There was a cut back version of it I thought? > > Will try and include the MCLK fallback patch in ver. 2 of the patch. signature.asc Description: This is a digitally signed message part
Re: [alsa-devel] [PATCH 2/2] ASoC: Intel: boards: Add Baytrail RT5660 machine driver
On Tue, 2016-12-27 at 11:29 +0800, Shrirang Bagul wrote: > On Mon, 2016-12-19 at 16:44 +, Mark Brown wrote: > > On Mon, Dec 19, 2016 at 10:30:09AM -0600, Pierre-Louis Bossart wrote: > > > > > All this code seems to be largely a copy-paste of the bytcr_rt5640 machine > > > driver and the same comments would apply there. This patch did miss the > > > last > > > > Yes, there's a lot of room for cleanups in the existing code too (and of > > course if there's such a large amount of cut'n'paste that implies that > > there should be some code reuse going on). > > Thank you for the review and valuable comments. Following the discussion so > far, > I feel the proper way would be to adapt bytcr_rt5660 machine driver to manage > RT5660 codec. typo, should be bytcr_rt5640 > > > > > correction merged by Mark to deal with errors "ASoC: Intel: bytcr_rt5640: > > > fallback mechanism if MCLK is not enabled" and the same error handling > > > would > > > be needed. > > > > There was a cut back version of it I thought? > > Will try and include the MCLK fallback patch in ver. 2 of the patch. signature.asc Description: This is a digitally signed message part
Re: [PATCH 1/2] ASoC: rt5660: Add ACPI support
On Mon, 2016-12-19 at 15:44 +, Mark Brown wrote: > On Mon, Dec 19, 2016 at 09:51:46PM +0800, Shrirang Bagul wrote: > > > +static const struct acpi_gpio_params audio_wake_intr_gpio = { 0, 0, false > > }; > > +static const struct acpi_gpio_params lineout_mute_gpio = { 1, 0, true }; > > Can we please write these in a fashion more idiomatic for the kernel and > useful for human readers with named struct fields? Okay, will update in ver. 2 of the patch. Thanks. signature.asc Description: This is a digitally signed message part
Re: [PATCH 1/2] ASoC: rt5660: Add ACPI support
On Mon, 2016-12-19 at 15:44 +, Mark Brown wrote: > On Mon, Dec 19, 2016 at 09:51:46PM +0800, Shrirang Bagul wrote: > > > +static const struct acpi_gpio_params audio_wake_intr_gpio = { 0, 0, false > > }; > > +static const struct acpi_gpio_params lineout_mute_gpio = { 1, 0, true }; > > Can we please write these in a fashion more idiomatic for the kernel and > useful for human readers with named struct fields? Okay, will update in ver. 2 of the patch. Thanks. signature.asc Description: This is a digitally signed message part
Re: [alsa-devel] [PATCH 2/2] ASoC: Intel: boards: Add Baytrail RT5660 machine driver
On Mon, 2016-12-19 at 16:44 +, Mark Brown wrote: > On Mon, Dec 19, 2016 at 10:30:09AM -0600, Pierre-Louis Bossart wrote: > > > All this code seems to be largely a copy-paste of the bytcr_rt5640 machine > > driver and the same comments would apply there. This patch did miss the last > > Yes, there's a lot of room for cleanups in the existing code too (and of > course if there's such a large amount of cut'n'paste that implies that > there should be some code reuse going on). Thank you for the review and valuable comments. Following the discussion so far, I feel the proper way would be to adapt bytcr_rt5660 machine driver to manage RT5660 codec. > > > correction merged by Mark to deal with errors "ASoC: Intel: bytcr_rt5640: > > fallback mechanism if MCLK is not enabled" and the same error handling would > > be needed. > > There was a cut back version of it I thought? Will try and include the MCLK fallback patch in ver. 2 of the patch. signature.asc Description: This is a digitally signed message part
Re: [alsa-devel] [PATCH 2/2] ASoC: Intel: boards: Add Baytrail RT5660 machine driver
On Mon, 2016-12-19 at 16:44 +, Mark Brown wrote: > On Mon, Dec 19, 2016 at 10:30:09AM -0600, Pierre-Louis Bossart wrote: > > > All this code seems to be largely a copy-paste of the bytcr_rt5640 machine > > driver and the same comments would apply there. This patch did miss the last > > Yes, there's a lot of room for cleanups in the existing code too (and of > course if there's such a large amount of cut'n'paste that implies that > there should be some code reuse going on). Thank you for the review and valuable comments. Following the discussion so far, I feel the proper way would be to adapt bytcr_rt5660 machine driver to manage RT5660 codec. > > > correction merged by Mark to deal with errors "ASoC: Intel: bytcr_rt5640: > > fallback mechanism if MCLK is not enabled" and the same error handling would > > be needed. > > There was a cut back version of it I thought? Will try and include the MCLK fallback patch in ver. 2 of the patch. signature.asc Description: This is a digitally signed message part
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR
On Mon, Dec 26, 2016 at 6:24 PM, Kirill A. Shutemovwrote: > On Mon, Dec 26, 2016 at 06:06:01PM -0800, Andy Lutomirski wrote: >> On Mon, Dec 26, 2016 at 5:54 PM, Kirill A. Shutemov >> wrote: >> > This patch introduces new rlimit resource to manage maximum virtual >> > address available to userspace to map. >> > >> > On x86, 5-level paging enables 56-bit userspace virtual address space. >> > Not all user space is ready to handle wide addresses. It's known that >> > at least some JIT compilers use high bit in pointers to encode their >> > information. It collides with valid pointers with 5-level paging and >> > leads to crashes. >> > >> > The patch aims to address this compatibility issue. >> > >> > MM would use min(RLIMIT_VADDR, TASK_SIZE) as upper limit of virtual >> > address available to map by userspace. >> > >> > The default hard limit will be RLIM_INFINITY, which basically means that >> > TASK_SIZE limits available address space. >> > >> > The soft limit will also be RLIM_INFINITY everywhere, but the machine >> > with 5-level paging enabled. In this case, soft limit would be >> > (1UL << 47) - PAGE_SIZE. It’s current x86-64 TASK_SIZE_MAX with 4-level >> > paging which known to be safe >> > >> > New rlimit resource would follow usual semantics with regards to >> > inheritance: preserved on fork(2) and exec(2). This has potential to >> > break application if limits set too wide or too narrow, but this is not >> > uncommon for other resources (consider RLIMIT_DATA or RLIMIT_AS). >> > >> > As with other resources you can set the limit lower than current usage. >> > It would affect only future virtual address space allocations. >> > >> > Use-cases for new rlimit: >> > >> > - Bumping the soft limit to RLIM_INFINITY, allows current process all >> > its children to use addresses above 47-bits. >> > >> > - Bumping the soft limit to RLIM_INFINITY after fork(2), but before >> > exec(2) allows the child to use addresses above 47-bits. >> > >> > - Lowering the hard limit to 47-bits would prevent current process all >> > its children to use addresses above 47-bits, unless a process has >> > CAP_SYS_RESOURCES. >> > >> > - It’s also can be handy to lower hard or soft limit to arbitrary >> > address. User-mode emulation in QEMU may lower the limit to 32-bit >> > to emulate 32-bit machine on 64-bit host. >> >> I tend to think that this should be a personality or an ELF flag, not >> an rlimit. > > My plan was to implement ELF flag on top. Basically, ELF flag would mean > that we bump soft limit to hard limit on exec. > >> That way setuid works right. > > Um.. I probably miss background here. > If a setuid program depends on the lower limit, then a malicious program shouldn't be able to cause it to run with the higher limit. The personality code should already get this case right because personalities are reset when setuid happens. --Andy
Re: [RFC, PATCHv2 29/29] mm, x86: introduce RLIMIT_VADDR
On Mon, Dec 26, 2016 at 6:24 PM, Kirill A. Shutemov wrote: > On Mon, Dec 26, 2016 at 06:06:01PM -0800, Andy Lutomirski wrote: >> On Mon, Dec 26, 2016 at 5:54 PM, Kirill A. Shutemov >> wrote: >> > This patch introduces new rlimit resource to manage maximum virtual >> > address available to userspace to map. >> > >> > On x86, 5-level paging enables 56-bit userspace virtual address space. >> > Not all user space is ready to handle wide addresses. It's known that >> > at least some JIT compilers use high bit in pointers to encode their >> > information. It collides with valid pointers with 5-level paging and >> > leads to crashes. >> > >> > The patch aims to address this compatibility issue. >> > >> > MM would use min(RLIMIT_VADDR, TASK_SIZE) as upper limit of virtual >> > address available to map by userspace. >> > >> > The default hard limit will be RLIM_INFINITY, which basically means that >> > TASK_SIZE limits available address space. >> > >> > The soft limit will also be RLIM_INFINITY everywhere, but the machine >> > with 5-level paging enabled. In this case, soft limit would be >> > (1UL << 47) - PAGE_SIZE. It’s current x86-64 TASK_SIZE_MAX with 4-level >> > paging which known to be safe >> > >> > New rlimit resource would follow usual semantics with regards to >> > inheritance: preserved on fork(2) and exec(2). This has potential to >> > break application if limits set too wide or too narrow, but this is not >> > uncommon for other resources (consider RLIMIT_DATA or RLIMIT_AS). >> > >> > As with other resources you can set the limit lower than current usage. >> > It would affect only future virtual address space allocations. >> > >> > Use-cases for new rlimit: >> > >> > - Bumping the soft limit to RLIM_INFINITY, allows current process all >> > its children to use addresses above 47-bits. >> > >> > - Bumping the soft limit to RLIM_INFINITY after fork(2), but before >> > exec(2) allows the child to use addresses above 47-bits. >> > >> > - Lowering the hard limit to 47-bits would prevent current process all >> > its children to use addresses above 47-bits, unless a process has >> > CAP_SYS_RESOURCES. >> > >> > - It’s also can be handy to lower hard or soft limit to arbitrary >> > address. User-mode emulation in QEMU may lower the limit to 32-bit >> > to emulate 32-bit machine on 64-bit host. >> >> I tend to think that this should be a personality or an ELF flag, not >> an rlimit. > > My plan was to implement ELF flag on top. Basically, ELF flag would mean > that we bump soft limit to hard limit on exec. > >> That way setuid works right. > > Um.. I probably miss background here. > If a setuid program depends on the lower limit, then a malicious program shouldn't be able to cause it to run with the higher limit. The personality code should already get this case right because personalities are reset when setuid happens. --Andy
[PATCH v2] selftests/x86: Add a selftest for SYSRET to noncanonical addresses
SYSRET to a noncanonical address will blow up on Intel CPUs. Linux needs to prevent this from happening in two major cases, and the criteria will become more complicated when support for larger virtual address spaces is added. A fast-path SYSCALL will fallthrough to the following instruction using SYSRET without any particular checking. To prevent fallthrough to a noncanonical address, Linux prevents the highest canonical page from being mapped. This test case checks a variety of possible maximum addresses to make sure that either we can't map code there or that SYSCALL fallthrough works. A slow-path system call can return anywhere. Linux needs to make sure that, if the return address is non-canonical, it won't use SYSRET. This test cases causes sigreturn() to return to a variety of addresses (with RCX == RIP) and makes sure that nothing explodes. Some of this code comes from Kirill Shutemov. Changes from v1: - Get rid of some extra parentheses. - Test more corner cases. - Get rid of some duplicate '0x' printout. Cc: "Kirill A. Shutemov"Signed-off-by: Andy Lutomirski --- tools/testing/selftests/x86/Makefile | 2 +- tools/testing/selftests/x86/sysret_rip.c | 195 +++ 2 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/x86/sysret_rip.c diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index 8c1cb423cfe6..25d4067c11e4 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -10,7 +10,7 @@ TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_sysc TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso \ test_FCMOV test_FCOMI test_FISTTP \ vdso_restorer -TARGETS_C_64BIT_ONLY := fsgsbase +TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY) TARGETS_C_64BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_64BIT_ONLY) diff --git a/tools/testing/selftests/x86/sysret_rip.c b/tools/testing/selftests/x86/sysret_rip.c new file mode 100644 index ..d85ec5b3671c --- /dev/null +++ b/tools/testing/selftests/x86/sysret_rip.c @@ -0,0 +1,195 @@ +/* + * sigreturn.c - tests that x86 avoids Intel SYSRET pitfalls + * Copyright (c) 2014-2016 Andrew Lutomirski + * + * This program is free software; you can redistribute it and/or modify + * it under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#define _GNU_SOURCE + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +asm ( + ".pushsection \".text\", \"ax\"\n\t" + ".balign 4096\n\t" + "test_page: .globl test_page\n\t" + ".fill 4094,1,0xcc\n\t" + "test_syscall_insn:\n\t" + "syscall\n\t" + ".ifne . - test_page - 4096\n\t" + ".error \"test page is not one page long\"\n\t" + ".endif\n\t" + ".popsection" +); + +extern const char test_page[]; +static void const *current_test_page_addr = test_page; + +static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *), + int flags) +{ + struct sigaction sa; + memset(, 0, sizeof(sa)); + sa.sa_sigaction = handler; + sa.sa_flags = SA_SIGINFO | flags; + sigemptyset(_mask); + if (sigaction(sig, , 0)) + err(1, "sigaction"); +} + +static void clearhandler(int sig) +{ + struct sigaction sa; + memset(, 0, sizeof(sa)); + sa.sa_handler = SIG_DFL; + sigemptyset(_mask); + if (sigaction(sig, , 0)) + err(1, "sigaction"); +} + +/* State used by our signal handlers. */ +static gregset_t initial_regs; + +static volatile unsigned long rip; + +static void sigsegv_for_sigreturn_test(int sig, siginfo_t *info, void *ctx_void) +{ + ucontext_t *ctx = (ucontext_t*)ctx_void; + + if (rip != ctx->uc_mcontext.gregs[REG_RIP]) { + printf("[FAIL]\tRequested RIP=0x%lx but got RIP=0x%lx\n", + rip, (unsigned long)ctx->uc_mcontext.gregs[REG_RIP]); + fflush(stdout); + _exit(1); + } + + memcpy(>uc_mcontext.gregs, _regs, sizeof(gregset_t)); + + printf("[OK]\tGot SIGSEGV at RIP=0x%lx\n", rip); +} + +static void sigusr1(int sig, siginfo_t *info, void *ctx_void) +{ + ucontext_t *ctx = (ucontext_t*)ctx_void; + + memcpy(_regs, >uc_mcontext.gregs, sizeof(gregset_t)); +
[PATCH v2] selftests/x86: Add a selftest for SYSRET to noncanonical addresses
SYSRET to a noncanonical address will blow up on Intel CPUs. Linux needs to prevent this from happening in two major cases, and the criteria will become more complicated when support for larger virtual address spaces is added. A fast-path SYSCALL will fallthrough to the following instruction using SYSRET without any particular checking. To prevent fallthrough to a noncanonical address, Linux prevents the highest canonical page from being mapped. This test case checks a variety of possible maximum addresses to make sure that either we can't map code there or that SYSCALL fallthrough works. A slow-path system call can return anywhere. Linux needs to make sure that, if the return address is non-canonical, it won't use SYSRET. This test cases causes sigreturn() to return to a variety of addresses (with RCX == RIP) and makes sure that nothing explodes. Some of this code comes from Kirill Shutemov. Changes from v1: - Get rid of some extra parentheses. - Test more corner cases. - Get rid of some duplicate '0x' printout. Cc: "Kirill A. Shutemov" Signed-off-by: Andy Lutomirski --- tools/testing/selftests/x86/Makefile | 2 +- tools/testing/selftests/x86/sysret_rip.c | 195 +++ 2 files changed, 196 insertions(+), 1 deletion(-) create mode 100644 tools/testing/selftests/x86/sysret_rip.c diff --git a/tools/testing/selftests/x86/Makefile b/tools/testing/selftests/x86/Makefile index 8c1cb423cfe6..25d4067c11e4 100644 --- a/tools/testing/selftests/x86/Makefile +++ b/tools/testing/selftests/x86/Makefile @@ -10,7 +10,7 @@ TARGETS_C_BOTHBITS := single_step_syscall sysret_ss_attrs syscall_nt ptrace_sysc TARGETS_C_32BIT_ONLY := entry_from_vm86 syscall_arg_fault test_syscall_vdso unwind_vdso \ test_FCMOV test_FCOMI test_FISTTP \ vdso_restorer -TARGETS_C_64BIT_ONLY := fsgsbase +TARGETS_C_64BIT_ONLY := fsgsbase sysret_rip TARGETS_C_32BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_32BIT_ONLY) TARGETS_C_64BIT_ALL := $(TARGETS_C_BOTHBITS) $(TARGETS_C_64BIT_ONLY) diff --git a/tools/testing/selftests/x86/sysret_rip.c b/tools/testing/selftests/x86/sysret_rip.c new file mode 100644 index ..d85ec5b3671c --- /dev/null +++ b/tools/testing/selftests/x86/sysret_rip.c @@ -0,0 +1,195 @@ +/* + * sigreturn.c - tests that x86 avoids Intel SYSRET pitfalls + * Copyright (c) 2014-2016 Andrew Lutomirski + * + * This program is free software; you can redistribute it and/or modify + * it under the terms and conditions of the GNU General Public License, + * version 2, as published by the Free Software Foundation. + * + * This program is distributed in the hope it will be useful, but + * WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * General Public License for more details. + */ + +#define _GNU_SOURCE + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + + +asm ( + ".pushsection \".text\", \"ax\"\n\t" + ".balign 4096\n\t" + "test_page: .globl test_page\n\t" + ".fill 4094,1,0xcc\n\t" + "test_syscall_insn:\n\t" + "syscall\n\t" + ".ifne . - test_page - 4096\n\t" + ".error \"test page is not one page long\"\n\t" + ".endif\n\t" + ".popsection" +); + +extern const char test_page[]; +static void const *current_test_page_addr = test_page; + +static void sethandler(int sig, void (*handler)(int, siginfo_t *, void *), + int flags) +{ + struct sigaction sa; + memset(, 0, sizeof(sa)); + sa.sa_sigaction = handler; + sa.sa_flags = SA_SIGINFO | flags; + sigemptyset(_mask); + if (sigaction(sig, , 0)) + err(1, "sigaction"); +} + +static void clearhandler(int sig) +{ + struct sigaction sa; + memset(, 0, sizeof(sa)); + sa.sa_handler = SIG_DFL; + sigemptyset(_mask); + if (sigaction(sig, , 0)) + err(1, "sigaction"); +} + +/* State used by our signal handlers. */ +static gregset_t initial_regs; + +static volatile unsigned long rip; + +static void sigsegv_for_sigreturn_test(int sig, siginfo_t *info, void *ctx_void) +{ + ucontext_t *ctx = (ucontext_t*)ctx_void; + + if (rip != ctx->uc_mcontext.gregs[REG_RIP]) { + printf("[FAIL]\tRequested RIP=0x%lx but got RIP=0x%lx\n", + rip, (unsigned long)ctx->uc_mcontext.gregs[REG_RIP]); + fflush(stdout); + _exit(1); + } + + memcpy(>uc_mcontext.gregs, _regs, sizeof(gregset_t)); + + printf("[OK]\tGot SIGSEGV at RIP=0x%lx\n", rip); +} + +static void sigusr1(int sig, siginfo_t *info, void *ctx_void) +{ + ucontext_t *ctx = (ucontext_t*)ctx_void; + + memcpy(_regs, >uc_mcontext.gregs, sizeof(gregset_t)); + + /* Set IP and CX to match so that SYSRET
Re: [PATCH net 1/9] virtio-net: remove the warning before XDP linearizing
On 2016年12月24日 03:31, Daniel Borkmann wrote: Hi Jason, On 12/23/2016 03:37 PM, Jason Wang wrote: Since we use EWMA to estimate the size of rx buffer. When rx buffer size is underestimated, it's usual to have a packet with more than one buffers. Consider this is not a bug, remove the warning and correct the comment before XDP linearizing. Cc: John FastabendSigned-off-by: Jason Wang --- drivers/net/virtio_net.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 08327e0..1067253 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -552,14 +552,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, struct page *xdp_page; u32 act; -/* No known backend devices should send packets with - * more than a single buffer when XDP conditions are - * met. However it is not strictly illegal so the case - * is handled as an exception and a warning is thrown. - */ +/* This happens when rx buffer size is underestimated */ if (unlikely(num_buf > 1)) { -bpf_warn_invalid_xdp_buffer(); Could you also remove the bpf_warn_invalid_xdp_buffer(), which got added just for this? Thanks. Posted. Thanks
Re: [PATCH net 1/9] virtio-net: remove the warning before XDP linearizing
On 2016年12月24日 03:31, Daniel Borkmann wrote: Hi Jason, On 12/23/2016 03:37 PM, Jason Wang wrote: Since we use EWMA to estimate the size of rx buffer. When rx buffer size is underestimated, it's usual to have a packet with more than one buffers. Consider this is not a bug, remove the warning and correct the comment before XDP linearizing. Cc: John Fastabend Signed-off-by: Jason Wang --- drivers/net/virtio_net.c | 8 +--- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index 08327e0..1067253 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -552,14 +552,8 @@ static struct sk_buff *receive_mergeable(struct net_device *dev, struct page *xdp_page; u32 act; -/* No known backend devices should send packets with - * more than a single buffer when XDP conditions are - * met. However it is not strictly illegal so the case - * is handled as an exception and a warning is thrown. - */ +/* This happens when rx buffer size is underestimated */ if (unlikely(num_buf > 1)) { -bpf_warn_invalid_xdp_buffer(); Could you also remove the bpf_warn_invalid_xdp_buffer(), which got added just for this? Thanks. Posted. Thanks
Re: [PATCH 2/2] usb: host: xhci: Handle the right timeout command
Hi, On 21 December 2016 at 21:00, Mathias Nymanwrote: > On 21.12.2016 04:22, Baolin Wang wrote: >> >> Hi Mathias, >> >> On 20 December 2016 at 23:13, Mathias Nyman >> wrote: >>> >>> On 20.12.2016 09:30, Baolin Wang wrote: >>> ... >>> >>> Alright, I gathered all current work related to xhci races and timeouts >>> and put them into a branch: >>> >>> git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git >>> timeout_race_fixes >>> >>> Its based on 4.9 >>> It includes a few other patches just to avoid conflicts and make my life >>> easier >>> >>> Interesting patches are: >>> >>> ee4eb91 xhci: remove unnecessary check for pending timer >>> 0cba67d xhci: detect stop endpoint race using pending timer instead of >>> counter. >>> 4f2535f xhci: Handle command completion and timeout race >>> b9d00d7 usb: host: xhci: Fix possible wild pointer when handling abort >>> command >>> 529a5a0 usb: xhci: fix possible wild pointer >>> 4766555 xhci: Fix race related to abort operation >>> de834a3 xhci: Use delayed_work instead of timer for command timeout >>> 69973b8 Linux 4.9 >>> >>> The fixes for command queue races will go to usb-linus and stable, the >>> reworks for stop ep watchdog timer will go to usb-next. >> >> >> How about applying below patch in your 'timeout_race_fixes' branch? >> [PATCH] usb: host: xhci: Clean up commands when stop endpoint command is >> timeout >> https://lkml.org/lkml/2016/12/13/94 >> > > usb_hc_died() should eventyally end up calling xhci_mem_cleanup() > which will cleanup the command queue. But I need to look into that usb_hc_died() did not call xhci_mem_cleanup() to clean up command queue, it just disconnects all children devices attached on the dying hub. I did not find where it will clean up the command queue when issuing usb_hc_died(). Also before issuing usb_hc_died() in xhci_handle_command_timeout(), we will call xhci_cleanup_command_queue(). I think it should same as in xhci_stop_endpoint_command_watchdog(). -- Baolin.wang Best Regards
Re: [PATCH 2/2] usb: host: xhci: Handle the right timeout command
Hi, On 21 December 2016 at 21:00, Mathias Nyman wrote: > On 21.12.2016 04:22, Baolin Wang wrote: >> >> Hi Mathias, >> >> On 20 December 2016 at 23:13, Mathias Nyman >> wrote: >>> >>> On 20.12.2016 09:30, Baolin Wang wrote: >>> ... >>> >>> Alright, I gathered all current work related to xhci races and timeouts >>> and put them into a branch: >>> >>> git://git.kernel.org/pub/scm/linux/kernel/git/mnyman/xhci.git >>> timeout_race_fixes >>> >>> Its based on 4.9 >>> It includes a few other patches just to avoid conflicts and make my life >>> easier >>> >>> Interesting patches are: >>> >>> ee4eb91 xhci: remove unnecessary check for pending timer >>> 0cba67d xhci: detect stop endpoint race using pending timer instead of >>> counter. >>> 4f2535f xhci: Handle command completion and timeout race >>> b9d00d7 usb: host: xhci: Fix possible wild pointer when handling abort >>> command >>> 529a5a0 usb: xhci: fix possible wild pointer >>> 4766555 xhci: Fix race related to abort operation >>> de834a3 xhci: Use delayed_work instead of timer for command timeout >>> 69973b8 Linux 4.9 >>> >>> The fixes for command queue races will go to usb-linus and stable, the >>> reworks for stop ep watchdog timer will go to usb-next. >> >> >> How about applying below patch in your 'timeout_race_fixes' branch? >> [PATCH] usb: host: xhci: Clean up commands when stop endpoint command is >> timeout >> https://lkml.org/lkml/2016/12/13/94 >> > > usb_hc_died() should eventyally end up calling xhci_mem_cleanup() > which will cleanup the command queue. But I need to look into that usb_hc_died() did not call xhci_mem_cleanup() to clean up command queue, it just disconnects all children devices attached on the dying hub. I did not find where it will clean up the command queue when issuing usb_hc_died(). Also before issuing usb_hc_died() in xhci_handle_command_timeout(), we will call xhci_cleanup_command_queue(). I think it should same as in xhci_stop_endpoint_command_watchdog(). -- Baolin.wang Best Regards
Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler
Hi, On 27 December 2016 at 10:39, Lu Baoluwrote: > Hi, > > On 12/26/2016 04:01 PM, Baolin Wang wrote: >> On some platfroms(like x86 platform), when one core is running the USB gadget >> irq thread handler by dwc3_thread_interrupt(), meanwhile another core also >> can >> respond other interrupts from dwc3 controller and modify the event buffer by >> dwc3_interrupt() function, that will cause getting the wrong event count in >> irq thread handler to make the USB function abnormal. >> >> We should add spin_lock/unlock() in dwc3_check_event_buf() to avoid this >> race. > > Why not spin_lock_irq ones? This lock seems to be used in both > normal and interrupt threads. Or, I missed anything? I assumed there are no nested interrupts, when one core is running at interrupt context, then it can not respond any other interrupts, which means we don't need to disable local IRQ now, right? -- Baolin.wang Best Regards
Re: [PATCH] usb: dwc3: gadget: Avoid race between dwc3 interrupt handler and irq thread handler
Hi, On 27 December 2016 at 10:39, Lu Baolu wrote: > Hi, > > On 12/26/2016 04:01 PM, Baolin Wang wrote: >> On some platfroms(like x86 platform), when one core is running the USB gadget >> irq thread handler by dwc3_thread_interrupt(), meanwhile another core also >> can >> respond other interrupts from dwc3 controller and modify the event buffer by >> dwc3_interrupt() function, that will cause getting the wrong event count in >> irq thread handler to make the USB function abnormal. >> >> We should add spin_lock/unlock() in dwc3_check_event_buf() to avoid this >> race. > > Why not spin_lock_irq ones? This lock seems to be used in both > normal and interrupt threads. Or, I missed anything? I assumed there are no nested interrupts, when one core is running at interrupt context, then it can not respond any other interrupts, which means we don't need to disable local IRQ now, right? -- Baolin.wang Best Regards
Re: [igvt-g-dev] [PATCH 1/1] drm/i915/gvt: verify functions types in new_mmio_info()
On 2016.12.26 14:52:23 +0100, Nicolas Iooss wrote: > The current prototype of new_mmio_info() uses void* for parameters read > and write, which are functions with precise calling conventions > (argument types and return type). Write down these conventions in > new_mmio_info() definition. > > This has been reported by the following warnings when clang is used to > build the kernel: > > drivers/gpu/drm/i915/gvt/handlers.c:124:21: error: pointer type > mismatch ('void *' and 'int (*)(struct intel_vgpu *, unsigned int, > void *, unsigned int)') [-Werror,-Wpointer-type-mismatch] > info->read = read ? read : intel_vgpu_default_mmio_read; > ^ > drivers/gpu/drm/i915/gvt/handlers.c:125:23: error: pointer type > mismatch ('void *' and 'int (*)(struct intel_vgpu *, unsigned int, > void *, unsigned int)') [-Werror,-Wpointer-type-mismatch] > info->write = write ? write : intel_vgpu_default_mmio_write; > ^ ~ ~ > > This allows the compiler to detect that sbi_ctl_mmio_write() returns a > "bool" value instead of an expected "int" one. Fix this. > Looks good to me. Will queue this up. Thanks > Signed-off-by: Nicolas Iooss> --- > drivers/gpu/drm/i915/gvt/handlers.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c > b/drivers/gpu/drm/i915/gvt/handlers.c > index 522809710312..052e57124c0a 100644 > --- a/drivers/gpu/drm/i915/gvt/handlers.c > +++ b/drivers/gpu/drm/i915/gvt/handlers.c > @@ -93,7 +93,8 @@ static void write_vreg(struct intel_vgpu *vgpu, unsigned > int offset, > static int new_mmio_info(struct intel_gvt *gvt, > u32 offset, u32 flags, u32 size, > u32 addr_mask, u32 ro_mask, u32 device, > - void *read, void *write) > + int (*read)(struct intel_vgpu *, unsigned int, void *, unsigned > int), > + int (*write)(struct intel_vgpu *, unsigned int, void *, > unsigned int)) > { > struct intel_gvt_mmio_info *info, *p; > u32 start, end, i; > @@ -974,7 +975,7 @@ static int sbi_data_mmio_read(struct intel_vgpu *vgpu, > unsigned int offset, > return 0; > } > > -static bool sbi_ctl_mmio_write(struct intel_vgpu *vgpu, unsigned int offset, > +static int sbi_ctl_mmio_write(struct intel_vgpu *vgpu, unsigned int offset, > void *p_data, unsigned int bytes) > { > u32 data; > -- > 2.11.0 > > ___ > igvt-g-dev mailing list > igvt-g-...@lists.01.org > https://lists.01.org/mailman/listinfo/igvt-g-dev -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature
Re: [igvt-g-dev] [PATCH 1/1] drm/i915/gvt: verify functions types in new_mmio_info()
On 2016.12.26 14:52:23 +0100, Nicolas Iooss wrote: > The current prototype of new_mmio_info() uses void* for parameters read > and write, which are functions with precise calling conventions > (argument types and return type). Write down these conventions in > new_mmio_info() definition. > > This has been reported by the following warnings when clang is used to > build the kernel: > > drivers/gpu/drm/i915/gvt/handlers.c:124:21: error: pointer type > mismatch ('void *' and 'int (*)(struct intel_vgpu *, unsigned int, > void *, unsigned int)') [-Werror,-Wpointer-type-mismatch] > info->read = read ? read : intel_vgpu_default_mmio_read; > ^ > drivers/gpu/drm/i915/gvt/handlers.c:125:23: error: pointer type > mismatch ('void *' and 'int (*)(struct intel_vgpu *, unsigned int, > void *, unsigned int)') [-Werror,-Wpointer-type-mismatch] > info->write = write ? write : intel_vgpu_default_mmio_write; > ^ ~ ~ > > This allows the compiler to detect that sbi_ctl_mmio_write() returns a > "bool" value instead of an expected "int" one. Fix this. > Looks good to me. Will queue this up. Thanks > Signed-off-by: Nicolas Iooss > --- > drivers/gpu/drm/i915/gvt/handlers.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gvt/handlers.c > b/drivers/gpu/drm/i915/gvt/handlers.c > index 522809710312..052e57124c0a 100644 > --- a/drivers/gpu/drm/i915/gvt/handlers.c > +++ b/drivers/gpu/drm/i915/gvt/handlers.c > @@ -93,7 +93,8 @@ static void write_vreg(struct intel_vgpu *vgpu, unsigned > int offset, > static int new_mmio_info(struct intel_gvt *gvt, > u32 offset, u32 flags, u32 size, > u32 addr_mask, u32 ro_mask, u32 device, > - void *read, void *write) > + int (*read)(struct intel_vgpu *, unsigned int, void *, unsigned > int), > + int (*write)(struct intel_vgpu *, unsigned int, void *, > unsigned int)) > { > struct intel_gvt_mmio_info *info, *p; > u32 start, end, i; > @@ -974,7 +975,7 @@ static int sbi_data_mmio_read(struct intel_vgpu *vgpu, > unsigned int offset, > return 0; > } > > -static bool sbi_ctl_mmio_write(struct intel_vgpu *vgpu, unsigned int offset, > +static int sbi_ctl_mmio_write(struct intel_vgpu *vgpu, unsigned int offset, > void *p_data, unsigned int bytes) > { > u32 data; > -- > 2.11.0 > > ___ > igvt-g-dev mailing list > igvt-g-...@lists.01.org > https://lists.01.org/mailman/listinfo/igvt-g-dev -- Open Source Technology Center, Intel ltd. $gpg --keyserver wwwkeys.pgp.net --recv-keys 4D781827 signature.asc Description: PGP signature
[PATCH net] net: xdp: remove unused bfp_warn_invalid_xdp_buffer()
After commit 73b62bd085f4737679ea9afc7867fa5f99ba7d1b ("virtio-net: remove the warning before XDP linearizing"), there's no users for bpf_warn_invalid_xdp_buffer(), so remove it. This is a revert for commit f23bc46c30ca5ef58b8549434899fcbac41b2cfc. Cc: Daniel BorkmannCc: John Fastabend Signed-off-by: Jason Wang --- include/linux/filter.h | 1 - net/core/filter.c | 6 -- 2 files changed, 7 deletions(-) diff --git a/include/linux/filter.h b/include/linux/filter.h index 7023142..a0934e6 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -610,7 +610,6 @@ bool bpf_helper_changes_pkt_data(void *func); struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off, const struct bpf_insn *patch, u32 len); void bpf_warn_invalid_xdp_action(u32 act); -void bpf_warn_invalid_xdp_buffer(void); #ifdef CONFIG_BPF_JIT extern int bpf_jit_enable; diff --git a/net/core/filter.c b/net/core/filter.c index 7190bd6..b146170 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2972,12 +2972,6 @@ void bpf_warn_invalid_xdp_action(u32 act) } EXPORT_SYMBOL_GPL(bpf_warn_invalid_xdp_action); -void bpf_warn_invalid_xdp_buffer(void) -{ - WARN_ONCE(1, "Illegal XDP buffer encountered, expect throughput degradation\n"); -} -EXPORT_SYMBOL_GPL(bpf_warn_invalid_xdp_buffer); - static u32 sk_filter_convert_ctx_access(enum bpf_access_type type, int dst_reg, int src_reg, int ctx_off, struct bpf_insn *insn_buf, -- 2.7.4
[PATCH net] net: xdp: remove unused bfp_warn_invalid_xdp_buffer()
After commit 73b62bd085f4737679ea9afc7867fa5f99ba7d1b ("virtio-net: remove the warning before XDP linearizing"), there's no users for bpf_warn_invalid_xdp_buffer(), so remove it. This is a revert for commit f23bc46c30ca5ef58b8549434899fcbac41b2cfc. Cc: Daniel Borkmann Cc: John Fastabend Signed-off-by: Jason Wang --- include/linux/filter.h | 1 - net/core/filter.c | 6 -- 2 files changed, 7 deletions(-) diff --git a/include/linux/filter.h b/include/linux/filter.h index 7023142..a0934e6 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -610,7 +610,6 @@ bool bpf_helper_changes_pkt_data(void *func); struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off, const struct bpf_insn *patch, u32 len); void bpf_warn_invalid_xdp_action(u32 act); -void bpf_warn_invalid_xdp_buffer(void); #ifdef CONFIG_BPF_JIT extern int bpf_jit_enable; diff --git a/net/core/filter.c b/net/core/filter.c index 7190bd6..b146170 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -2972,12 +2972,6 @@ void bpf_warn_invalid_xdp_action(u32 act) } EXPORT_SYMBOL_GPL(bpf_warn_invalid_xdp_action); -void bpf_warn_invalid_xdp_buffer(void) -{ - WARN_ONCE(1, "Illegal XDP buffer encountered, expect throughput degradation\n"); -} -EXPORT_SYMBOL_GPL(bpf_warn_invalid_xdp_buffer); - static u32 sk_filter_convert_ctx_access(enum bpf_access_type type, int dst_reg, int src_reg, int ctx_off, struct bpf_insn *insn_buf, -- 2.7.4
[PATCH 1/2] extcon: adc-jack: Use the internal data instead of using struct extcon_dev
This patch uses the internal dev instance instead of using the field of struct extcon_dev because the core structure (extcon_dev) of extcon have to be touched by only extcon core driver. Signed-off-by: Chanwoo Choi--- drivers/extcon/extcon-adc-jack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c index bc538708c753..6f6537ab0a79 100644 --- a/drivers/extcon/extcon-adc-jack.c +++ b/drivers/extcon/extcon-adc-jack.c @@ -67,7 +67,7 @@ static void adc_jack_handler(struct work_struct *work) ret = iio_read_channel_raw(data->chan, _val); if (ret < 0) { - dev_err(>edev->dev, "read channel() error: %d\n", ret); + dev_err(data->dev, "read channel() error: %d\n", ret); return; } -- 1.9.1
[PATCH 2/2] extcon: Move defintion of struct extcon_dev to driver/extcon directory
This patch moves the 'struct extcon_dev' of extcon subsystem to driver/extcon/extcon.h header file because the struct extcon_dev have to be handled by extcon API to guarantee the consistency of strcut extcon_dev. If external drivers are able to touch the struct extcon_dev directly, it might cause the critical and unknown problem. Signed-off-by: Chanwoo Choi--- drivers/extcon/devres.c | 2 +- drivers/extcon/extcon.c | 3 ++- drivers/extcon/extcon.h | 62 + include/linux/extcon.h | 57 + 4 files changed, 66 insertions(+), 58 deletions(-) create mode 100644 drivers/extcon/extcon.h diff --git a/drivers/extcon/devres.c b/drivers/extcon/devres.c index e686acd1c459..b40eb1805927 100644 --- a/drivers/extcon/devres.c +++ b/drivers/extcon/devres.c @@ -14,7 +14,7 @@ * GNU General Public License for more details. */ -#include +#include "extcon.h" static int devm_extcon_dev_match(struct device *dev, void *res, void *data) { diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c index d0e367959c91..591582b0d2b7 100644 --- a/drivers/extcon/extcon.c +++ b/drivers/extcon/extcon.c @@ -30,11 +30,12 @@ #include #include #include -#include #include #include #include +#include "extcon.h" + #define SUPPORTED_CABLE_MAX32 #define CABLE_NAME_MAX 30 diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h new file mode 100644 index ..993ddccafe11 --- /dev/null +++ b/drivers/extcon/extcon.h @@ -0,0 +1,62 @@ +#ifndef __LINUX_EXTCON_INTERNAL_H__ +#define __LINUX_EXTCON_INTERNAL_H__ + +#include + +/** + * struct extcon_dev - An extcon device represents one external connector. + * @name: The name of this extcon device. Parent device name is + * used if NULL. + * @supported_cable: Array of supported cable names ending with EXTCON_NONE. + * If supported_cable is NULL, cable name related APIs + * are disabled. + * @mutually_exclusive:Array of mutually exclusive set of cables that cannot + * be attached simultaneously. The array should be + * ending with NULL or be NULL (no mutually exclusive + * cables). For example, if it is { 0x7, 0x30, 0}, then, + * {0, 1}, {0, 1, 2}, {0, 2}, {1, 2}, or {4, 5} cannot + * be attached simulataneously. {0x7, 0} is equivalent to + * {0x3, 0x6, 0x5, 0}. If it is {0x, 0}, there + * can be no simultaneous connections. + * @dev: Device of this extcon. + * @state: Attach/detach state of this extcon. Do not provide at + * register-time. + * @nh:Notifier for the state change events from this extcon + * @entry: To support list of extcon devices so that users can + * search for extcon devices based on the extcon name. + * @lock: + * @max_supported: Internal value to store the number of cables. + * @extcon_dev_type: Device_type struct to provide attribute_groups + * customized for each extcon device. + * @cables:Sysfs subdirectories. Each represents one cable. + * + * In most cases, users only need to provide "User initializing data" of + * this struct when registering an extcon. In some exceptional cases, + * optional callbacks may be needed. However, the values in "internal data" + * are overwritten by register function. + */ +struct extcon_dev { + /* Optional user initializing data */ + const char *name; + const unsigned int *supported_cable; + const u32 *mutually_exclusive; + + /* Internal data. Please do not set. */ + struct device dev; + struct raw_notifier_head *nh; + struct list_head entry; + int max_supported; + spinlock_t lock;/* could be called by irq handler */ + u32 state; + + /* /sys/class/extcon/.../cable.n/... */ + struct device_type extcon_dev_type; + struct extcon_cable *cables; + + /* /sys/class/extcon/.../mutually_exclusive/... */ + struct attribute_group attr_g_muex; + struct attribute **attrs_muex; + struct device_attribute *d_attrs_muex; +}; + +#endif /* __LINUX_EXTCON_INTERNAL_H__ */ diff --git a/include/linux/extcon.h b/include/linux/extcon.h index 00201230bea5..d57e52443841 100644 --- a/include/linux/extcon.h +++ b/include/linux/extcon.h @@ -167,62 +167,7 @@ }; struct extcon_cable; - -/** - * struct extcon_dev - An extcon device represents one external connector. - * @name: The name of this extcon device. Parent device name is - * used if NULL. - * @supported_cable: Array of supported cable names ending with EXTCON_NONE. - * If supported_cable is NULL, cable name
[PATCH 1/2] extcon: adc-jack: Use the internal data instead of using struct extcon_dev
This patch uses the internal dev instance instead of using the field of struct extcon_dev because the core structure (extcon_dev) of extcon have to be touched by only extcon core driver. Signed-off-by: Chanwoo Choi --- drivers/extcon/extcon-adc-jack.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/extcon/extcon-adc-jack.c b/drivers/extcon/extcon-adc-jack.c index bc538708c753..6f6537ab0a79 100644 --- a/drivers/extcon/extcon-adc-jack.c +++ b/drivers/extcon/extcon-adc-jack.c @@ -67,7 +67,7 @@ static void adc_jack_handler(struct work_struct *work) ret = iio_read_channel_raw(data->chan, _val); if (ret < 0) { - dev_err(>edev->dev, "read channel() error: %d\n", ret); + dev_err(data->dev, "read channel() error: %d\n", ret); return; } -- 1.9.1
[PATCH 2/2] extcon: Move defintion of struct extcon_dev to driver/extcon directory
This patch moves the 'struct extcon_dev' of extcon subsystem to driver/extcon/extcon.h header file because the struct extcon_dev have to be handled by extcon API to guarantee the consistency of strcut extcon_dev. If external drivers are able to touch the struct extcon_dev directly, it might cause the critical and unknown problem. Signed-off-by: Chanwoo Choi --- drivers/extcon/devres.c | 2 +- drivers/extcon/extcon.c | 3 ++- drivers/extcon/extcon.h | 62 + include/linux/extcon.h | 57 + 4 files changed, 66 insertions(+), 58 deletions(-) create mode 100644 drivers/extcon/extcon.h diff --git a/drivers/extcon/devres.c b/drivers/extcon/devres.c index e686acd1c459..b40eb1805927 100644 --- a/drivers/extcon/devres.c +++ b/drivers/extcon/devres.c @@ -14,7 +14,7 @@ * GNU General Public License for more details. */ -#include +#include "extcon.h" static int devm_extcon_dev_match(struct device *dev, void *res, void *data) { diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c index d0e367959c91..591582b0d2b7 100644 --- a/drivers/extcon/extcon.c +++ b/drivers/extcon/extcon.c @@ -30,11 +30,12 @@ #include #include #include -#include #include #include #include +#include "extcon.h" + #define SUPPORTED_CABLE_MAX32 #define CABLE_NAME_MAX 30 diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h new file mode 100644 index ..993ddccafe11 --- /dev/null +++ b/drivers/extcon/extcon.h @@ -0,0 +1,62 @@ +#ifndef __LINUX_EXTCON_INTERNAL_H__ +#define __LINUX_EXTCON_INTERNAL_H__ + +#include + +/** + * struct extcon_dev - An extcon device represents one external connector. + * @name: The name of this extcon device. Parent device name is + * used if NULL. + * @supported_cable: Array of supported cable names ending with EXTCON_NONE. + * If supported_cable is NULL, cable name related APIs + * are disabled. + * @mutually_exclusive:Array of mutually exclusive set of cables that cannot + * be attached simultaneously. The array should be + * ending with NULL or be NULL (no mutually exclusive + * cables). For example, if it is { 0x7, 0x30, 0}, then, + * {0, 1}, {0, 1, 2}, {0, 2}, {1, 2}, or {4, 5} cannot + * be attached simulataneously. {0x7, 0} is equivalent to + * {0x3, 0x6, 0x5, 0}. If it is {0x, 0}, there + * can be no simultaneous connections. + * @dev: Device of this extcon. + * @state: Attach/detach state of this extcon. Do not provide at + * register-time. + * @nh:Notifier for the state change events from this extcon + * @entry: To support list of extcon devices so that users can + * search for extcon devices based on the extcon name. + * @lock: + * @max_supported: Internal value to store the number of cables. + * @extcon_dev_type: Device_type struct to provide attribute_groups + * customized for each extcon device. + * @cables:Sysfs subdirectories. Each represents one cable. + * + * In most cases, users only need to provide "User initializing data" of + * this struct when registering an extcon. In some exceptional cases, + * optional callbacks may be needed. However, the values in "internal data" + * are overwritten by register function. + */ +struct extcon_dev { + /* Optional user initializing data */ + const char *name; + const unsigned int *supported_cable; + const u32 *mutually_exclusive; + + /* Internal data. Please do not set. */ + struct device dev; + struct raw_notifier_head *nh; + struct list_head entry; + int max_supported; + spinlock_t lock;/* could be called by irq handler */ + u32 state; + + /* /sys/class/extcon/.../cable.n/... */ + struct device_type extcon_dev_type; + struct extcon_cable *cables; + + /* /sys/class/extcon/.../mutually_exclusive/... */ + struct attribute_group attr_g_muex; + struct attribute **attrs_muex; + struct device_attribute *d_attrs_muex; +}; + +#endif /* __LINUX_EXTCON_INTERNAL_H__ */ diff --git a/include/linux/extcon.h b/include/linux/extcon.h index 00201230bea5..d57e52443841 100644 --- a/include/linux/extcon.h +++ b/include/linux/extcon.h @@ -167,62 +167,7 @@ }; struct extcon_cable; - -/** - * struct extcon_dev - An extcon device represents one external connector. - * @name: The name of this extcon device. Parent device name is - * used if NULL. - * @supported_cable: Array of supported cable names ending with EXTCON_NONE. - * If supported_cable is NULL, cable name related APIs - *