Re: [PATCH v5 0/6] Introduce ZONE_CMA

[Aneesh Kumar K.V]

Joonsoo Kim  writes:

> On Tue, Aug 30, 2016 at 04:09:37PM +0530, Aneesh Kumar K.V wrote:
>> Joonsoo Kim  writes:
>> 
>> > 2016-08-29 18:27 GMT+09:00 Aneesh Kumar K.V 
>> > :
>> >> js1...@gmail.com writes:
>> >>
>> >>> From: Joonsoo Kim 
>> >>>
>> >>> Hello,
>> >>>
>> >>> Changes from v4
>> >>> o Rebase on next-20160825
>> >>> o Add general fix patch for lowmem reserve
>> >>> o Fix lowmem reserve ratio
>> >>> o Fix zone span optimizaion per Vlastimil
>> >>> o Fix pageset initialization
>> >>> o Change invocation timing on cma_init_reserved_areas()
>> >>
>> >> I don't see much information regarding how we interleave between
>> >> ZONE_CMA and other zones for movable allocation. Is that explained in
>> >> any of the patch ? The fair zone allocator got removed by
>> >> e6cbd7f2efb433d717af72aa8510a9db6f7a7e05
>> >
>> > Interleaving would not work since the fair zone allocator policy is 
>> > removed.
>> > I don't think that it's a big problem because it is just matter of
>> > timing to fill
>> > up the memory. Eventually, memory on ZONE_CMA will be fully used in
>> > any case.
>> 
>> Does that mean a CMA allocation will now be slower because in most case we
>> will need to reclaim ? The zone list will now have ZONE_CMA in the
>> beginning right ?
>
> ZONE_CMA will be used first but I don't think that CMA allocation will
> be slower. In most case, memory would be fully used (usually
> by page cache). So, we need reclaim or migration in any case.

Considering that the upstream kernel doesn't allow migration of THP
pages, this would mean that migrate will fail in most case if we have
THP enabled and the THP allocation request got satisfied via ZONE_CMA.
Isn't that going to be a problem ?

-aneesh

Re: [PATCH v5 0/6] Introduce ZONE_CMA

[Aneesh Kumar K.V]

Joonsoo Kim  writes:

> On Tue, Aug 30, 2016 at 04:09:37PM +0530, Aneesh Kumar K.V wrote:
>> Joonsoo Kim  writes:
>> 
>> > 2016-08-29 18:27 GMT+09:00 Aneesh Kumar K.V 
>> > :
>> >> js1...@gmail.com writes:
>> >>
>> >>> From: Joonsoo Kim 
>> >>>
>> >>> Hello,
>> >>>
>> >>> Changes from v4
>> >>> o Rebase on next-20160825
>> >>> o Add general fix patch for lowmem reserve
>> >>> o Fix lowmem reserve ratio
>> >>> o Fix zone span optimizaion per Vlastimil
>> >>> o Fix pageset initialization
>> >>> o Change invocation timing on cma_init_reserved_areas()
>> >>
>> >> I don't see much information regarding how we interleave between
>> >> ZONE_CMA and other zones for movable allocation. Is that explained in
>> >> any of the patch ? The fair zone allocator got removed by
>> >> e6cbd7f2efb433d717af72aa8510a9db6f7a7e05
>> >
>> > Interleaving would not work since the fair zone allocator policy is 
>> > removed.
>> > I don't think that it's a big problem because it is just matter of
>> > timing to fill
>> > up the memory. Eventually, memory on ZONE_CMA will be fully used in
>> > any case.
>> 
>> Does that mean a CMA allocation will now be slower because in most case we
>> will need to reclaim ? The zone list will now have ZONE_CMA in the
>> beginning right ?
>
> ZONE_CMA will be used first but I don't think that CMA allocation will
> be slower. In most case, memory would be fully used (usually
> by page cache). So, we need reclaim or migration in any case.

Considering that the upstream kernel doesn't allow migration of THP
pages, this would mean that migrate will fail in most case if we have
THP enabled and the THP allocation request got satisfied via ZONE_CMA.
Isn't that going to be a problem ?

-aneesh

RE: [PATCH v3 kernel 0/7] Extend virtio-balloon for fast (de)inflating & fast live migration

[Li, Liang Z]

> Subject: Re: [PATCH v3 kernel 0/7] Extend virtio-balloon for fast 
> (de)inflating
> & fast live migration
> 
> 2016-08-08 14:35 GMT+08:00 Liang Li :
> > This patch set contains two parts of changes to the virtio-balloon.
> >
> > One is the change for speeding up the inflating & deflating process,
> > the main idea of this optimization is to use bitmap to send the page
> > information to host instead of the PFNs, to reduce the overhead of
> > virtio data transmission, address translation and madvise(). This can
> > help to improve the performance by about 85%.
> >
> > Another change is for speeding up live migration. By skipping process
> > guest's free pages in the first round of data copy, to reduce needless
> > data processing, this can help to save quite a lot of CPU cycles and
> > network bandwidth. We put guest's free page information in bitmap and
> > send it to host with the virt queue of virtio-balloon. For an idle 8GB
> > guest, this can help to shorten the total live migration time from
> > 2Sec to about 500ms in the 10Gbps network environment.
> 
> I just read the slides of this feature for recent kvm forum, the cloud
> providers more care about live migration downtime to avoid customers'
> perception than total time, however, this feature will increase downtime
> when acquire the benefit of reducing total time, maybe it will be more
> acceptable if there is no downside for downtime.
> 
> Regards,
> Wanpeng Li

In theory, there is no factor that will increase the downtime. There is no 
additional operation
and no more data copy during the stop and copy stage. But in the test, the 
downtime increases
and this can be reproduced. I think the busy network line maybe the reason for 
this. With this
 optimization, a huge amount of data is written to the socket in a shorter 
time, so some of the write
operation may need to wait. Without this optimization, zero page checking takes 
more time,
the network is not so busy.

If the guest is not an idle one, I think the gap of the downtime will not so 
obvious.  Anyway, the
downtime is still less than the  max_down_time set by the user.

Thanks!
Liang

RE: [PATCH v3 kernel 0/7] Extend virtio-balloon for fast (de)inflating & fast live migration

[Li, Liang Z]

> Subject: Re: [PATCH v3 kernel 0/7] Extend virtio-balloon for fast 
> (de)inflating
> & fast live migration
> 
> 2016-08-08 14:35 GMT+08:00 Liang Li :
> > This patch set contains two parts of changes to the virtio-balloon.
> >
> > One is the change for speeding up the inflating & deflating process,
> > the main idea of this optimization is to use bitmap to send the page
> > information to host instead of the PFNs, to reduce the overhead of
> > virtio data transmission, address translation and madvise(). This can
> > help to improve the performance by about 85%.
> >
> > Another change is for speeding up live migration. By skipping process
> > guest's free pages in the first round of data copy, to reduce needless
> > data processing, this can help to save quite a lot of CPU cycles and
> > network bandwidth. We put guest's free page information in bitmap and
> > send it to host with the virt queue of virtio-balloon. For an idle 8GB
> > guest, this can help to shorten the total live migration time from
> > 2Sec to about 500ms in the 10Gbps network environment.
> 
> I just read the slides of this feature for recent kvm forum, the cloud
> providers more care about live migration downtime to avoid customers'
> perception than total time, however, this feature will increase downtime
> when acquire the benefit of reducing total time, maybe it will be more
> acceptable if there is no downside for downtime.
> 
> Regards,
> Wanpeng Li

In theory, there is no factor that will increase the downtime. There is no 
additional operation
and no more data copy during the stop and copy stage. But in the test, the 
downtime increases
and this can be reproduced. I think the busy network line maybe the reason for 
this. With this
 optimization, a huge amount of data is written to the socket in a shorter 
time, so some of the write
operation may need to wait. Without this optimization, zero page checking takes 
more time,
the network is not so busy.

If the guest is not an idle one, I think the gap of the downtime will not so 
obvious.  Anyway, the
downtime is still less than the  max_down_time set by the user.

Thanks!
Liang

Greetings

[Mrs Julie Leach]

You are a recipient to Mrs Julie Leach Donation of $3 million USD. Contact 
(julieleac...@gmail.com) for claims.

Greetings

[Mrs Julie Leach]

You are a recipient to Mrs Julie Leach Donation of $3 million USD. Contact 
(julieleac...@gmail.com) for claims.

Re: [PATCH V4] leds: trigger: Introduce an USB port trigger

[Rafał Miłecki]

On 31 August 2016 at 21:00, Rafał Miłecki  wrote:
> On 31 August 2016 at 20:23, Alan Stern  wrote:
>> On Tue, 30 Aug 2016, Rafał Miłecki wrote:
>>> Not really as it won't cover some pretty common use cases. Many home
>>> routers have few USB ports (2-5) and only 1 USB LED. It has to be
>>> possible to assign few USB ports to a single LED (trigger). That way
>>> LED should be turned on (and kept on) if there is at least 1 USB
>>> device connected. You obviously can't do:
>>> echo "usb1-1 usb1-2 usb2-1" > /sys/class/leds/foo/trigger
>>>
>>> This was already brought up by Rob (who mentioned CPU trigger) and I
>>> replied him pretty much the same way in:
>>> https://lkml.org/lkml/2016/7/29/38
>>> (reply starts with "Anyway, the serious limitation I see").
>>
>> The code for a bunch of triggers must already be written.  What would
>> the user do if he wanted to flash a single LED in response to both
>> CPU activity and MTD activity?  If not
>>
>> echo "cpu mtd" >/sys/class/leds/foo/trigger
>>
>> then what?
>
> Well, it sounds like a new feature then. Shall we add an extra API
> with a request function for turning LED on? It could internally count
> how many requests were raised and keep LED on as long as there is at
> least 1 left. I guess we should implement it in trigger "subsystem"
> (if I can call it so). Does it sound like a good plan?

I'm pretty sure noone ever planned to have more than 1 trigger
assigned to a single LED. I just realized there will be a problem with
proposed solution: sysfs files conflict.

Consider 2 existing triggers for a moment:
1) oneshot: it creates following sysfs files:
/sys/class/leds/foo/delay_on
/sys/class/leds/foo/delay_off
/sys/class/leds/foo/invert
/sys/class/leds/foo/shot
2) timer: it creates following sysfs files:
/sys/class/leds/foo/delay_on
/sys/class/leds/foo/delay_off

Activating both of them will probably cause a WARNING in sysfs. They
can't coexist :(

We should probably have per-trigger subdirs, e.g.:
/sys/class/leds/foo/trigger-oneshot/delay_on
/sys/class/leds/foo/trigger-oneshot/delay_off
/sys/class/leds/foo/trigger-oneshot/invert
/sys/class/leds/foo/trigger-oneshot/shot
/sys/class/leds/foo/trigger-timer/delay_on
/sys/class/leds/foo/trigger-timer/delay_off
but implementing it now would break the ABI.

One workaround I can see is doing triggers V2, they:
1) Would put sysfs files in /sys/class/leds/foo/trigger-bar/
2) Use new API for *requesting* LED to be on/off
3) There would be a counter of requests in V2 API
4) Multiple triggers V2 would be allowed to be used (assigned) at the same time

-- 
Rafał

Re: [PATCH V4] leds: trigger: Introduce an USB port trigger

[Rafał Miłecki]

On 31 August 2016 at 21:00, Rafał Miłecki  wrote:
> On 31 August 2016 at 20:23, Alan Stern  wrote:
>> On Tue, 30 Aug 2016, Rafał Miłecki wrote:
>>> Not really as it won't cover some pretty common use cases. Many home
>>> routers have few USB ports (2-5) and only 1 USB LED. It has to be
>>> possible to assign few USB ports to a single LED (trigger). That way
>>> LED should be turned on (and kept on) if there is at least 1 USB
>>> device connected. You obviously can't do:
>>> echo "usb1-1 usb1-2 usb2-1" > /sys/class/leds/foo/trigger
>>>
>>> This was already brought up by Rob (who mentioned CPU trigger) and I
>>> replied him pretty much the same way in:
>>> https://lkml.org/lkml/2016/7/29/38
>>> (reply starts with "Anyway, the serious limitation I see").
>>
>> The code for a bunch of triggers must already be written.  What would
>> the user do if he wanted to flash a single LED in response to both
>> CPU activity and MTD activity?  If not
>>
>> echo "cpu mtd" >/sys/class/leds/foo/trigger
>>
>> then what?
>
> Well, it sounds like a new feature then. Shall we add an extra API
> with a request function for turning LED on? It could internally count
> how many requests were raised and keep LED on as long as there is at
> least 1 left. I guess we should implement it in trigger "subsystem"
> (if I can call it so). Does it sound like a good plan?

I'm pretty sure noone ever planned to have more than 1 trigger
assigned to a single LED. I just realized there will be a problem with
proposed solution: sysfs files conflict.

Consider 2 existing triggers for a moment:
1) oneshot: it creates following sysfs files:
/sys/class/leds/foo/delay_on
/sys/class/leds/foo/delay_off
/sys/class/leds/foo/invert
/sys/class/leds/foo/shot
2) timer: it creates following sysfs files:
/sys/class/leds/foo/delay_on
/sys/class/leds/foo/delay_off

Activating both of them will probably cause a WARNING in sysfs. They
can't coexist :(

We should probably have per-trigger subdirs, e.g.:
/sys/class/leds/foo/trigger-oneshot/delay_on
/sys/class/leds/foo/trigger-oneshot/delay_off
/sys/class/leds/foo/trigger-oneshot/invert
/sys/class/leds/foo/trigger-oneshot/shot
/sys/class/leds/foo/trigger-timer/delay_on
/sys/class/leds/foo/trigger-timer/delay_off
but implementing it now would break the ABI.

One workaround I can see is doing triggers V2, they:
1) Would put sysfs files in /sys/class/leds/foo/trigger-bar/
2) Use new API for *requesting* LED to be on/off
3) There would be a counter of requests in V2 API
4) Multiple triggers V2 would be allowed to be used (assigned) at the same time

-- 
Rafał

RE: [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device for legacy interrupts.

[Bharat Kumar Gogada]

>  Hi Bharat,
> > @@ -561,7 +561,7 @@ static int nwl_pcie_init_irq_domain(struct
> > nwl_pcie
>  *pcie)
> > }
> >
> > pcie->legacy_irq_domain = irq_domain_add_linear(legacy_intc_node,
> > -   INTX_NUM,
> > +   INTX_NUM + 1,
> > _domain_ops,
> > pcie);
> 
>  This feels like the wrong thing to do. You have INTX_NUM irqs, so
>  the domain allocation should reflect this. On the other hand, the
>  way the driver currently deals with mappings is quite broken
>  (consistently adding 1 to
> >> the HW interrupt).
> 
> >>> Hi Marc,
> >>>
> >>> Without above change I get following crash in kernel while booting.
> >>>
> >>> [2.441684] error: hwirq 0x4 is too large for dummy
> >>>
> >>> [2.441694] [ cut here ]
> >>>
> >>> [2.441698] WARNING: at kernel/irq/irqdomain.c:344
> >>>
> >>> [2.441702] Modules linked in:
> >>>
> >>> [2.441706]
> >>>
> >>> [2.441714] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.4.0 #8
> >>>
> >>> [2.441718] Hardware name: xlnx,zynqmp (DT)
> >>>
> >>> [2.441723] task: ffc071886b80 ti: ffc071888000 task.ti:
> >> ffc071888000
> >>>
> >>> [2.441732] PC is at irq_domain_associate+0x138/0x1c0
> >>>
> >>> [2.441738] LR is at irq_domain_associate+0x138/0x1c0
> >>>
> >>> In kernel/irq/irqdomain.c function irq_domain_associate
> >>>
> >>> if (WARN(hwirq >= domain->hwirq_max,
> >>>  "error: hwirq 0x%x is too large for %s\n", (int)hwirq, 
> >>> domain-
> >name))
> >>> return -EINVAL;
> >>>
> >>> Here the hwirq and hwirq_max are equal to 4 without the above
> >>> condition
> >> (INTX_NUM + 1) due to which crash is coming.
> >>> This is happening as the legacy interrupts are starting from 1 (INTA).
> >>
> >> I understood that. I'm still persisting in saying that you have the wrong 
> >> fix.
> >>
> >> Your domain should always allocate many interrupts as you have
> >> interrupt sources. These interrupts (hwirq) should be numbered from 0 to 
> >> (n-
> 1).
> >
> > Agreed, but here comes the problem the hwirq for legacy interrupts
> > will start at 0x1 to 0x4 (INTA to INTD) and these values are as per
> > PCIe specification for legacy interrupts. So these cannot be numbered
> > from 0. So when 0x4 (INTD) for a multi-function device comes the crash
> > occurs.
> 
> So who provides this hwirq? Who calls irq_domain_associate() with hwirq set to
> 4?
> 
PCIe subsystem invokes pcibios_add_device function in arch/arm64/kernel/pci.c 
for every pci device.
The purpose of this function is to assign dev->irq using 
of_irq_parse_and_map_pci.
of_irq_parse_and_map_pci invokes of_irq_parse_pci where it reads 
PCI_INTERRUPT_PIN from configuration space and saves it
in parameter of struct of_phandle_args.
This structure is passed to irq_create_of_mapping where it invokes 
irq_create_fwspec_mapping.
irq_create_fwspec_mapping invokes irq_domain_translate and gets hwirq, here the 
above saved PCI_INTERRUPT_PIN value is assigned 
to hwirq (*hwirq = fwspec->param[0]).
And then using this hwirq irq_create_mapping -> irq_domain_associate were 
invoked and mapping is created for virtual irq with this hwirq.
So for any end point PCI_INTERRUPT_PIN value starts from 0x1 to 0x4 and so 
hwirq starts from 0x1 to 0x4.

So the values are more generic w.r.t to protocol, that's why hwirq will range 
from 0x1 to 0x4. 
And then if you check pcie-altera.c they are doing this adding one in their 
handler and while creating legacy domain.
 
Thanks & Regards,
Bharat
 

 

RE: [PATCH 3/3] PCI: Xilinx NWL PCIe: Fix Error for multi function device for legacy interrupts.

[Bharat Kumar Gogada]

>  Hi Bharat,
> > @@ -561,7 +561,7 @@ static int nwl_pcie_init_irq_domain(struct
> > nwl_pcie
>  *pcie)
> > }
> >
> > pcie->legacy_irq_domain = irq_domain_add_linear(legacy_intc_node,
> > -   INTX_NUM,
> > +   INTX_NUM + 1,
> > _domain_ops,
> > pcie);
> 
>  This feels like the wrong thing to do. You have INTX_NUM irqs, so
>  the domain allocation should reflect this. On the other hand, the
>  way the driver currently deals with mappings is quite broken
>  (consistently adding 1 to
> >> the HW interrupt).
> 
> >>> Hi Marc,
> >>>
> >>> Without above change I get following crash in kernel while booting.
> >>>
> >>> [2.441684] error: hwirq 0x4 is too large for dummy
> >>>
> >>> [2.441694] [ cut here ]
> >>>
> >>> [2.441698] WARNING: at kernel/irq/irqdomain.c:344
> >>>
> >>> [2.441702] Modules linked in:
> >>>
> >>> [2.441706]
> >>>
> >>> [2.441714] CPU: 1 PID: 1 Comm: swapper/0 Not tainted 4.4.0 #8
> >>>
> >>> [2.441718] Hardware name: xlnx,zynqmp (DT)
> >>>
> >>> [2.441723] task: ffc071886b80 ti: ffc071888000 task.ti:
> >> ffc071888000
> >>>
> >>> [2.441732] PC is at irq_domain_associate+0x138/0x1c0
> >>>
> >>> [2.441738] LR is at irq_domain_associate+0x138/0x1c0
> >>>
> >>> In kernel/irq/irqdomain.c function irq_domain_associate
> >>>
> >>> if (WARN(hwirq >= domain->hwirq_max,
> >>>  "error: hwirq 0x%x is too large for %s\n", (int)hwirq, 
> >>> domain-
> >name))
> >>> return -EINVAL;
> >>>
> >>> Here the hwirq and hwirq_max are equal to 4 without the above
> >>> condition
> >> (INTX_NUM + 1) due to which crash is coming.
> >>> This is happening as the legacy interrupts are starting from 1 (INTA).
> >>
> >> I understood that. I'm still persisting in saying that you have the wrong 
> >> fix.
> >>
> >> Your domain should always allocate many interrupts as you have
> >> interrupt sources. These interrupts (hwirq) should be numbered from 0 to 
> >> (n-
> 1).
> >
> > Agreed, but here comes the problem the hwirq for legacy interrupts
> > will start at 0x1 to 0x4 (INTA to INTD) and these values are as per
> > PCIe specification for legacy interrupts. So these cannot be numbered
> > from 0. So when 0x4 (INTD) for a multi-function device comes the crash
> > occurs.
> 
> So who provides this hwirq? Who calls irq_domain_associate() with hwirq set to
> 4?
> 
PCIe subsystem invokes pcibios_add_device function in arch/arm64/kernel/pci.c 
for every pci device.
The purpose of this function is to assign dev->irq using 
of_irq_parse_and_map_pci.
of_irq_parse_and_map_pci invokes of_irq_parse_pci where it reads 
PCI_INTERRUPT_PIN from configuration space and saves it
in parameter of struct of_phandle_args.
This structure is passed to irq_create_of_mapping where it invokes 
irq_create_fwspec_mapping.
irq_create_fwspec_mapping invokes irq_domain_translate and gets hwirq, here the 
above saved PCI_INTERRUPT_PIN value is assigned 
to hwirq (*hwirq = fwspec->param[0]).
And then using this hwirq irq_create_mapping -> irq_domain_associate were 
invoked and mapping is created for virtual irq with this hwirq.
So for any end point PCI_INTERRUPT_PIN value starts from 0x1 to 0x4 and so 
hwirq starts from 0x1 to 0x4.

So the values are more generic w.r.t to protocol, that's why hwirq will range 
from 0x1 to 0x4. 
And then if you check pcie-altera.c they are doing this adding one in their 
handler and while creating legacy domain.
 
Thanks & Regards,
Bharat
 

 

Re: [PATCH 1/4] kernel: add a helper to get an owning user namespace for a namespace

[Serge E. Hallyn]

On Wed, Aug 31, 2016 at 01:38:35PM -0700, Andrey Vagin wrote:
> On Tue, Aug 30, 2016 at 7:56 PM, Serge E. Hallyn  wrote:
> > On Fri, Aug 26, 2016 at 04:08:08PM -0700, Andrei Vagin wrote:
> >> +struct ns_common *ns_get_owner(struct ns_common *ns)
> >> +{
> >> + struct user_namespace *my_user_ns = current_user_ns();
> >> + struct user_namespace *owner, *p;
> >> +
> >> + /* See if the owner is in the current user namespace */
> >> + owner = p = ns->ops->get_owner(ns);
> >> + for (;;) {
> >> + if (!p)
> >> + return ERR_PTR(-EPERM);
> >> + if (p == my_user_ns)
> >> + break;
> >> + p = p->parent;
> >> + }
> >> +
> >> + return _user_ns(owner)->ns;
> >
> > get_user_ns() bumps the owner's refcount.  I don't see where
> > this is being dropped, especially when ns_ioctl() uses it in
> > the next patch.
> 
> It is dropped in __ns_get_path if a namespace has a dentry, otherwise
> it is dropped from nsfs_evict.
> 
> static void *__ns_get_path(struct path *path, struct ns_common *ns)
> |return -EPERM;
> ...
> ns->ops->put(ns); 
>  |
> got_it:
> |/* See if the owner is in the current user namespace
> */
> path->mnt = mnt;
> |owner = p = ns->ops->get_owner(ns);
> path->dentry = dentry;
> |for (;;) {
> return NULL;
> ...
> 
> static void nsfs_evict(struct inode *inode)   
>  |
> {
> |if (!ns_capable(user_ns, CAP_SYS_ADMIN))
> struct ns_common *ns = inode->i_private;
> |return -EPERM;
> clear_inode(inode);   
>  |
> ns->ops->put(ns);
> |cred = prepare_creds();
> }

Gotcha, thanks.

Re: [PATCH 1/4] kernel: add a helper to get an owning user namespace for a namespace

[Serge E. Hallyn]

On Wed, Aug 31, 2016 at 01:38:35PM -0700, Andrey Vagin wrote:
> On Tue, Aug 30, 2016 at 7:56 PM, Serge E. Hallyn  wrote:
> > On Fri, Aug 26, 2016 at 04:08:08PM -0700, Andrei Vagin wrote:
> >> +struct ns_common *ns_get_owner(struct ns_common *ns)
> >> +{
> >> + struct user_namespace *my_user_ns = current_user_ns();
> >> + struct user_namespace *owner, *p;
> >> +
> >> + /* See if the owner is in the current user namespace */
> >> + owner = p = ns->ops->get_owner(ns);
> >> + for (;;) {
> >> + if (!p)
> >> + return ERR_PTR(-EPERM);
> >> + if (p == my_user_ns)
> >> + break;
> >> + p = p->parent;
> >> + }
> >> +
> >> + return _user_ns(owner)->ns;
> >
> > get_user_ns() bumps the owner's refcount.  I don't see where
> > this is being dropped, especially when ns_ioctl() uses it in
> > the next patch.
> 
> It is dropped in __ns_get_path if a namespace has a dentry, otherwise
> it is dropped from nsfs_evict.
> 
> static void *__ns_get_path(struct path *path, struct ns_common *ns)
> |return -EPERM;
> ...
> ns->ops->put(ns); 
>  |
> got_it:
> |/* See if the owner is in the current user namespace
> */
> path->mnt = mnt;
> |owner = p = ns->ops->get_owner(ns);
> path->dentry = dentry;
> |for (;;) {
> return NULL;
> ...
> 
> static void nsfs_evict(struct inode *inode)   
>  |
> {
> |if (!ns_capable(user_ns, CAP_SYS_ADMIN))
> struct ns_common *ns = inode->i_private;
> |return -EPERM;
> clear_inode(inode);   
>  |
> ns->ops->put(ns);
> |cred = prepare_creds();
> }

Gotcha, thanks.

Re: [PATCH v5 2/2] phy: add a driver for the Rockchip SoC internal PCIe PHY

[Kishon Vijay Abraham I]

On Wednesday 31 August 2016 07:38 PM, Heiko Stübner wrote:
> Hi,
> 
> Am Samstag, 20. August 2016, 10:53:37 schrieb Shawn Lin:
>> This patch to add a generic PHY driver for rockchip PCIe PHY.
>> Access the PHY via registers provided by GRF (general register
>> files) module.
>>
>> Signed-off-by: Shawn Lin 
> 
> seems I'm late to the party, but when looking if I can apply the pcie-
> devicetree patches, I found that the phy is still pending.
> 
> Apart from some error-message nitpicks below, this looks ok to me. I don't 
> know enough about the actual pci phy part though.
> 
> Kishon, is this on your radar?

yes.. can the nipicks be fixed and posted asap?

Thanks
Kishon

> 
> [...]
> 
>> +static int rockchip_pcie_phy_power_off(struct phy *phy)
>> +{
>> +struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
>> +int err = 0;
>> +
>> +err = reset_control_assert(rk_phy->phy_rst);
>> +if (err) {
>> +pr_err("assert phy_rst err %d\n", err);
> 
>   dev_err(phy->dev, ...)
> 
> probably the same for all other pr_err invocations
> 
> 
>> +return err;
>> +}
>> +
>> +return 0;
>> +}
> 
> [...]
> 
>> +static const struct of_device_id rockchip_pcie_phy_dt_ids[] = {
>> +{
>> +.compatible = "rockchip,rk3399-pcie-phy",
>> +.data = _pcie_data,
>> +},
>> +{}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, rockchip_pcie_phy_dt_ids);
>> +
>> +static int rockchip_pcie_phy_probe(struct platform_device *pdev)
>> +{
>> +struct device *dev = >dev;
>> +struct rockchip_pcie_phy *rk_phy;
>> +struct phy *generic_phy;
>> +struct phy_provider *phy_provider;
>> +struct regmap *grf;
>> +const struct of_device_id *of_id;
>> +
>> +grf = syscon_node_to_regmap(dev->parent->of_node);
>> +if (IS_ERR(grf)) {
>> +dev_err(dev, "Missing rockchip,grf property\n");
> 
>   dev_err(dev, "Cannot find GRF syscon\n");
> 
> 
> Heiko
> 

Re: [PATCH v5 2/2] phy: add a driver for the Rockchip SoC internal PCIe PHY

[Kishon Vijay Abraham I]

On Wednesday 31 August 2016 07:38 PM, Heiko Stübner wrote:
> Hi,
> 
> Am Samstag, 20. August 2016, 10:53:37 schrieb Shawn Lin:
>> This patch to add a generic PHY driver for rockchip PCIe PHY.
>> Access the PHY via registers provided by GRF (general register
>> files) module.
>>
>> Signed-off-by: Shawn Lin 
> 
> seems I'm late to the party, but when looking if I can apply the pcie-
> devicetree patches, I found that the phy is still pending.
> 
> Apart from some error-message nitpicks below, this looks ok to me. I don't 
> know enough about the actual pci phy part though.
> 
> Kishon, is this on your radar?

yes.. can the nipicks be fixed and posted asap?

Thanks
Kishon

> 
> [...]
> 
>> +static int rockchip_pcie_phy_power_off(struct phy *phy)
>> +{
>> +struct rockchip_pcie_phy *rk_phy = phy_get_drvdata(phy);
>> +int err = 0;
>> +
>> +err = reset_control_assert(rk_phy->phy_rst);
>> +if (err) {
>> +pr_err("assert phy_rst err %d\n", err);
> 
>   dev_err(phy->dev, ...)
> 
> probably the same for all other pr_err invocations
> 
> 
>> +return err;
>> +}
>> +
>> +return 0;
>> +}
> 
> [...]
> 
>> +static const struct of_device_id rockchip_pcie_phy_dt_ids[] = {
>> +{
>> +.compatible = "rockchip,rk3399-pcie-phy",
>> +.data = _pcie_data,
>> +},
>> +{}
>> +};
>> +
>> +MODULE_DEVICE_TABLE(of, rockchip_pcie_phy_dt_ids);
>> +
>> +static int rockchip_pcie_phy_probe(struct platform_device *pdev)
>> +{
>> +struct device *dev = >dev;
>> +struct rockchip_pcie_phy *rk_phy;
>> +struct phy *generic_phy;
>> +struct phy_provider *phy_provider;
>> +struct regmap *grf;
>> +const struct of_device_id *of_id;
>> +
>> +grf = syscon_node_to_regmap(dev->parent->of_node);
>> +if (IS_ERR(grf)) {
>> +dev_err(dev, "Missing rockchip,grf property\n");
> 
>   dev_err(dev, "Cannot find GRF syscon\n");
> 
> 
> Heiko
> 

Re: [PATCH v2] arm64: defconfig: enable common modules for power management

[Leo Yan]

Hi Catalin, Will,

On Thu, Sep 01, 2016 at 12:51:09PM +0800, Leo Yan wrote:
> Enable common modules for power management; one is to enable
> CPUFREQ_DT driver; the driver is used by many platforms by passing OPP
> table from device tree.
> 
> Also enables thermal related drivers. Firstly we need enable
> configuration CPU_THERMAL for CPU cooling device driver, this will bind
> thermal zone with CPU cooling device; and enable 'power allocator'
> thermal governor.

This patch is an updated version for [1] with enabling "power
allocator". Sorry for regression.

[1] 
http://archive.arm.linux.org.uk/lurker/message/20160831.085017.a42c57fe.en.html

Thanks,
Leo Yan

> 
> Signed-off-by: Leo Yan 
> ---
>  arch/arm64/configs/defconfig | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index eadf485..c4f5948 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -82,6 +82,7 @@ CONFIG_COMPAT=y
>  CONFIG_CPU_IDLE=y
>  CONFIG_ARM_CPUIDLE=y
>  CONFIG_CPU_FREQ=y
> +CONFIG_CPUFREQ_DT=y
>  CONFIG_ARM_BIG_LITTLE_CPUFREQ=y
>  CONFIG_ARM_SCPI_CPUFREQ=y
>  CONFIG_NET=y
> @@ -252,6 +253,8 @@ CONFIG_SENSORS_INA2XX=m
>  CONFIG_SENSORS_ARM_SCPI=y
>  CONFIG_THERMAL=y
>  CONFIG_THERMAL_EMULATION=y
> +CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
> +CONFIG_CPU_THERMAL=y
>  CONFIG_EXYNOS_THERMAL=y
>  CONFIG_WATCHDOG=y
>  CONFIG_RENESAS_WDT=y
> -- 
> 1.9.1
> 

Re: [PATCH v2] arm64: defconfig: enable common modules for power management

[Leo Yan]

Hi Catalin, Will,

On Thu, Sep 01, 2016 at 12:51:09PM +0800, Leo Yan wrote:
> Enable common modules for power management; one is to enable
> CPUFREQ_DT driver; the driver is used by many platforms by passing OPP
> table from device tree.
> 
> Also enables thermal related drivers. Firstly we need enable
> configuration CPU_THERMAL for CPU cooling device driver, this will bind
> thermal zone with CPU cooling device; and enable 'power allocator'
> thermal governor.

This patch is an updated version for [1] with enabling "power
allocator". Sorry for regression.

[1] 
http://archive.arm.linux.org.uk/lurker/message/20160831.085017.a42c57fe.en.html

Thanks,
Leo Yan

> 
> Signed-off-by: Leo Yan 
> ---
>  arch/arm64/configs/defconfig | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
> index eadf485..c4f5948 100644
> --- a/arch/arm64/configs/defconfig
> +++ b/arch/arm64/configs/defconfig
> @@ -82,6 +82,7 @@ CONFIG_COMPAT=y
>  CONFIG_CPU_IDLE=y
>  CONFIG_ARM_CPUIDLE=y
>  CONFIG_CPU_FREQ=y
> +CONFIG_CPUFREQ_DT=y
>  CONFIG_ARM_BIG_LITTLE_CPUFREQ=y
>  CONFIG_ARM_SCPI_CPUFREQ=y
>  CONFIG_NET=y
> @@ -252,6 +253,8 @@ CONFIG_SENSORS_INA2XX=m
>  CONFIG_SENSORS_ARM_SCPI=y
>  CONFIG_THERMAL=y
>  CONFIG_THERMAL_EMULATION=y
> +CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
> +CONFIG_CPU_THERMAL=y
>  CONFIG_EXYNOS_THERMAL=y
>  CONFIG_WATCHDOG=y
>  CONFIG_RENESAS_WDT=y
> -- 
> 1.9.1
> 

[PATCH v2] arm64: defconfig: enable common modules for power management

[Leo Yan]

Enable common modules for power management; one is to enable
CPUFREQ_DT driver; the driver is used by many platforms by passing OPP
table from device tree.

Also enables thermal related drivers. Firstly we need enable
configuration CPU_THERMAL for CPU cooling device driver, this will bind
thermal zone with CPU cooling device; and enable 'power allocator'
thermal governor.

Signed-off-by: Leo Yan 
---
 arch/arm64/configs/defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index eadf485..c4f5948 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -82,6 +82,7 @@ CONFIG_COMPAT=y
 CONFIG_CPU_IDLE=y
 CONFIG_ARM_CPUIDLE=y
 CONFIG_CPU_FREQ=y
+CONFIG_CPUFREQ_DT=y
 CONFIG_ARM_BIG_LITTLE_CPUFREQ=y
 CONFIG_ARM_SCPI_CPUFREQ=y
 CONFIG_NET=y
@@ -252,6 +253,8 @@ CONFIG_SENSORS_INA2XX=m
 CONFIG_SENSORS_ARM_SCPI=y
 CONFIG_THERMAL=y
 CONFIG_THERMAL_EMULATION=y
+CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
+CONFIG_CPU_THERMAL=y
 CONFIG_EXYNOS_THERMAL=y
 CONFIG_WATCHDOG=y
 CONFIG_RENESAS_WDT=y
-- 
1.9.1

[PATCH v2] arm64: defconfig: enable common modules for power management

[Leo Yan]

Enable common modules for power management; one is to enable
CPUFREQ_DT driver; the driver is used by many platforms by passing OPP
table from device tree.

Also enables thermal related drivers. Firstly we need enable
configuration CPU_THERMAL for CPU cooling device driver, this will bind
thermal zone with CPU cooling device; and enable 'power allocator'
thermal governor.

Signed-off-by: Leo Yan 
---
 arch/arm64/configs/defconfig | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index eadf485..c4f5948 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -82,6 +82,7 @@ CONFIG_COMPAT=y
 CONFIG_CPU_IDLE=y
 CONFIG_ARM_CPUIDLE=y
 CONFIG_CPU_FREQ=y
+CONFIG_CPUFREQ_DT=y
 CONFIG_ARM_BIG_LITTLE_CPUFREQ=y
 CONFIG_ARM_SCPI_CPUFREQ=y
 CONFIG_NET=y
@@ -252,6 +253,8 @@ CONFIG_SENSORS_INA2XX=m
 CONFIG_SENSORS_ARM_SCPI=y
 CONFIG_THERMAL=y
 CONFIG_THERMAL_EMULATION=y
+CONFIG_THERMAL_GOV_POWER_ALLOCATOR=y
+CONFIG_CPU_THERMAL=y
 CONFIG_EXYNOS_THERMAL=y
 CONFIG_WATCHDOG=y
 CONFIG_RENESAS_WDT=y
-- 
1.9.1

Re: [PATCH v3 kernel 0/7] Extend virtio-balloon for fast (de)inflating & fast live migration

[Wanpeng Li]

2016-08-08 14:35 GMT+08:00 Liang Li :
> This patch set contains two parts of changes to the virtio-balloon.
>
> One is the change for speeding up the inflating & deflating process,
> the main idea of this optimization is to use bitmap to send the page
> information to host instead of the PFNs, to reduce the overhead of
> virtio data transmission, address translation and madvise(). This can
> help to improve the performance by about 85%.
>
> Another change is for speeding up live migration. By skipping process
> guest's free pages in the first round of data copy, to reduce needless
> data processing, this can help to save quite a lot of CPU cycles and
> network bandwidth. We put guest's free page information in bitmap and
> send it to host with the virt queue of virtio-balloon. For an idle 8GB
> guest, this can help to shorten the total live migration time from 2Sec
> to about 500ms in the 10Gbps network environment.

I just read the slides of this feature for recent kvm forum, the cloud
providers more care about live migration downtime to avoid customers'
perception than total time, however, this feature will increase
downtime when acquire the benefit of reducing total time, maybe it
will be more acceptable if there is no downside for downtime.

Regards,
Wanpeng Li

Re: [PATCH v3 kernel 0/7] Extend virtio-balloon for fast (de)inflating & fast live migration

[Wanpeng Li]

2016-08-08 14:35 GMT+08:00 Liang Li :
> This patch set contains two parts of changes to the virtio-balloon.
>
> One is the change for speeding up the inflating & deflating process,
> the main idea of this optimization is to use bitmap to send the page
> information to host instead of the PFNs, to reduce the overhead of
> virtio data transmission, address translation and madvise(). This can
> help to improve the performance by about 85%.
>
> Another change is for speeding up live migration. By skipping process
> guest's free pages in the first round of data copy, to reduce needless
> data processing, this can help to save quite a lot of CPU cycles and
> network bandwidth. We put guest's free page information in bitmap and
> send it to host with the virt queue of virtio-balloon. For an idle 8GB
> guest, this can help to shorten the total live migration time from 2Sec
> to about 500ms in the 10Gbps network environment.

I just read the slides of this feature for recent kvm forum, the cloud
providers more care about live migration downtime to avoid customers'
perception than total time, however, this feature will increase
downtime when acquire the benefit of reducing total time, maybe it
will be more acceptable if there is no downside for downtime.

Regards,
Wanpeng Li

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Doug Anderson]

Hi,

On Wed, Aug 31, 2016 at 7:29 PM, Ziyuan Xu  wrote:
>> This is fine to pick up _only_ if you don't care about suspend/resume.
>> If you care about suspend/resume then someone needs to first write a
>> patch that will re-init all "corecfg" values after power is turned on.
>
>
> Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we
> don't need to strore/re-init it after resume.
> corecfg_clockmultiplier is only used to fetch host->clk_mul, and
> host->clk_mul has been a fixed value at run-time, unless driver unbind.
> The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to check
> the xin_clk at probe time, we don't reference it at run-time.
> BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC works
> fine.

I guess I don't actually know how the corecfg_clockmultiplier and
corecfg_baseclkfreq fields are actually used, but I presume that they
actually do something useful and aren't used to just communicate back
to software?

I know that:

1. If I don't pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are still fine after
suspend / resume.

2. If I do pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are wrong after
suspend/resume (tested by reading /dev/mem directly from userspace
after suspend/resume).


Are you saying that it is unimportant that corecfg_clockmultiplier and
corecfg_baseclkfreq are wrong?

>> Technically I think this should probably use "pm runtime" and not
>> normal suspend/resume hooks.  Any time we end up pm runtime suspended
>> then I think our power will go off (because of genpd?) and we need to
>> restore values.
>
>
> I understand your consideration. BUT genpd is in charge of on/off pd if the
> corresponding device node has "power-domains" property. RPM is unnecessary
> for this situation, we will not use autosuspend, right?
>
> @shawn, what's your opinion?

I haven't dug.  If Runtime PM isn't enabled for sdhci-of-arasan then I
guess we can just worry about suspend/resume, though.

-Doug

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Doug Anderson]

Hi,

On Wed, Aug 31, 2016 at 7:29 PM, Ziyuan Xu  wrote:
>> This is fine to pick up _only_ if you don't care about suspend/resume.
>> If you care about suspend/resume then someone needs to first write a
>> patch that will re-init all "corecfg" values after power is turned on.
>
>
> Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we
> don't need to strore/re-init it after resume.
> corecfg_clockmultiplier is only used to fetch host->clk_mul, and
> host->clk_mul has been a fixed value at run-time, unless driver unbind.
> The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to check
> the xin_clk at probe time, we don't reference it at run-time.
> BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC works
> fine.

I guess I don't actually know how the corecfg_clockmultiplier and
corecfg_baseclkfreq fields are actually used, but I presume that they
actually do something useful and aren't used to just communicate back
to software?

I know that:

1. If I don't pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are still fine after
suspend / resume.

2. If I do pick this patch and I suspend/resume,
corecfg_clockmultiplier and corecfg_baseclkfreq are wrong after
suspend/resume (tested by reading /dev/mem directly from userspace
after suspend/resume).


Are you saying that it is unimportant that corecfg_clockmultiplier and
corecfg_baseclkfreq are wrong?

>> Technically I think this should probably use "pm runtime" and not
>> normal suspend/resume hooks.  Any time we end up pm runtime suspended
>> then I think our power will go off (because of genpd?) and we need to
>> restore values.
>
>
> I understand your consideration. BUT genpd is in charge of on/off pd if the
> corresponding device node has "power-domains" property. RPM is unnecessary
> for this situation, we will not use autosuspend, right?
>
> @shawn, what's your opinion?

I haven't dug.  If Runtime PM isn't enabled for sdhci-of-arasan then I
guess we can just worry about suspend/resume, though.

-Doug

Re: [v10,2/2] PCI: Rockchip: Add Rockchip PCIe controller support

[Guenter Roeck]

On 08/31/2016 08:39 PM, Shawn Lin wrote:

Hi Guenter,

Thanks for your review, and I think it still not too late
for nitpicking as it isn't merged to next branch. :)

We have amend the code a bit, so probably we fixed some of
the minor issues against V10. But some of them are really
personal taste, if you still insist on that, I will be ok
to modify them.



Take it as suggestions; I won't insist on anything.

Guenter

Re: [v10,2/2] PCI: Rockchip: Add Rockchip PCIe controller support

[Guenter Roeck]

On 08/31/2016 08:39 PM, Shawn Lin wrote:

Hi Guenter,

Thanks for your review, and I think it still not too late
for nitpicking as it isn't merged to next branch. :)

We have amend the code a bit, so probably we fixed some of
the minor issues against V10. But some of them are really
personal taste, if you still insist on that, I will be ok
to modify them.



Take it as suggestions; I won't insist on anything.

Guenter

Re: [patch v3.18+ regression fix] sched: Further improve spurious CPU_IDLE active migrations

[Mike Galbraith]

On Wed, 2016-08-31 at 17:52 +0200, Vincent Guittot wrote:
> On 31 August 2016 at 12:36, Mike Galbraith  wrote:
> > On Wed, 2016-08-31 at 12:18 +0200, Mike Galbraith wrote:
> > > On Wed, 2016-08-31 at 12:01 +0200, Peter Zijlstra wrote:
> > 
> > > > So 43f4d66637bc ("sched: Improve sysbench performance by fixing spurious
> > > > active migration") 's +1 made sense in that its a tie breaker. If you
> > > > have 3 tasks on 2 groups, one group will have to have 2 tasks, and
> > > > bouncing the one task around just isn't going to help _anything_.
> > > 
> > > Yeah, but frequently tasks don't come in ones, so, you end up with an
> > > endless tug of war between LB ripping communicating buddies apart, and
> > > select_idle_sibling() pulling them back together.. bouncing cow
> > > syndrome.
> > 
> 
> replacing +1 by +2 fixes this use case that involves 2 threads but
> similar behavior can happen with 3 tasks on system with 4 cores per MC
> as an example
> 
> IIUC, you have on
> - one side, periodic load balance that spreads the 2 tasks in the system
> - on the other side, wake up path that moves the task back in the same MC.

Yup.

> Isn't your regression more linked to spurious migration than where the
> task is scheduled ? I don't see any direct relation between the client
> and the server in this netperf test, isn't it ?

 netperf  4360 [004]  1207.865265:   sched:sched_wakeup: 
netserver:4361 [120] success=1 CPU:002
 netperf  4360 [004]  1207.865274:   sched:sched_wakeup: 
netserver:4361 [120] success=1 CPU:002
 netperf  4360 [004]  1207.865280:   sched:sched_wakeup: 
netserver:4361 [120] success=1 CPU:002
   netserver  4361 [002]  1207.865313:   sched:sched_wakeup: 
netperf:4360 [120] success=1 CPU:004
 netperf  4360 [004]  1207.865340:   sched:sched_wakeup: 
kworker/u16:4:89 [120] success=1 CPU:000
 netperf  4360 [004]  1207.865345:   sched:sched_wakeup: 
kworker/u16:5:90 [120] success=1 CPU:006
 netperf  4360 [004]  1207.865355:   sched:sched_wakeup: 
kworker/u16:5:90 [120] success=1 CPU:006
 netperf  4360 [004]  1207.865357:   sched:sched_wakeup: 
kworker/u16:4:89 [120] success=1 CPU:000
 netperf  4360 [004]  1207.865369:   sched:sched_wakeup: 
netserver:4361 [120] success=1 CPU:002
   netserver  4361 [002]  1207.865377:   sched:sched_wakeup: 
netperf:4360 [120] success=1 CPU:004
 netperf  4360 [004]  1207.865476:   sched:sched_wakeup: perf:4359 
[120] success=1 CPU:003

It's not limited to this load, anything at all that is communicating
will do the same on these or similar processors.

This trying to be perfect looks like a booboo to me, as we are now
specifically asking our left hand undo what our right hand did to crank
up throughput.  For the diagnosed processor at least, one of those
hands definitely wants to be slapped.

This doesn't seem to be an issue for L3 equipped CPUs, but perhaps is
for some even modern processors, dunno (the boxen where regression was
detected are far from new).

> we could either remove the condition which tries to keep an even
> number of tasks in each group until busiest group becomes overloaded
> but it means that unrelated tasks may have to share same resources
> or we could try to prevent the migration at wake up. I was looking at
> wake_affine which seems to choose local cpu  when both prev and local
> cpu are idle. I wonder if local cpu is  really a better choice when
> both are idle

I don't see a great alternative to turning it off off the top of my
head, at least for processors with multiple LLCs.  Yeah, unrelated
tasks could end up sharing a cache needlessly, but will that hurt as
badly as tasks not munching tasty hot data definitely does?

-Mike

Re: [patch v3.18+ regression fix] sched: Further improve spurious CPU_IDLE active migrations

[Mike Galbraith]

On Wed, 2016-08-31 at 17:52 +0200, Vincent Guittot wrote:
> On 31 August 2016 at 12:36, Mike Galbraith  wrote:
> > On Wed, 2016-08-31 at 12:18 +0200, Mike Galbraith wrote:
> > > On Wed, 2016-08-31 at 12:01 +0200, Peter Zijlstra wrote:
> > 
> > > > So 43f4d66637bc ("sched: Improve sysbench performance by fixing spurious
> > > > active migration") 's +1 made sense in that its a tie breaker. If you
> > > > have 3 tasks on 2 groups, one group will have to have 2 tasks, and
> > > > bouncing the one task around just isn't going to help _anything_.
> > > 
> > > Yeah, but frequently tasks don't come in ones, so, you end up with an
> > > endless tug of war between LB ripping communicating buddies apart, and
> > > select_idle_sibling() pulling them back together.. bouncing cow
> > > syndrome.
> > 
> 
> replacing +1 by +2 fixes this use case that involves 2 threads but
> similar behavior can happen with 3 tasks on system with 4 cores per MC
> as an example
> 
> IIUC, you have on
> - one side, periodic load balance that spreads the 2 tasks in the system
> - on the other side, wake up path that moves the task back in the same MC.

Yup.

> Isn't your regression more linked to spurious migration than where the
> task is scheduled ? I don't see any direct relation between the client
> and the server in this netperf test, isn't it ?

 netperf  4360 [004]  1207.865265:   sched:sched_wakeup: 
netserver:4361 [120] success=1 CPU:002
 netperf  4360 [004]  1207.865274:   sched:sched_wakeup: 
netserver:4361 [120] success=1 CPU:002
 netperf  4360 [004]  1207.865280:   sched:sched_wakeup: 
netserver:4361 [120] success=1 CPU:002
   netserver  4361 [002]  1207.865313:   sched:sched_wakeup: 
netperf:4360 [120] success=1 CPU:004
 netperf  4360 [004]  1207.865340:   sched:sched_wakeup: 
kworker/u16:4:89 [120] success=1 CPU:000
 netperf  4360 [004]  1207.865345:   sched:sched_wakeup: 
kworker/u16:5:90 [120] success=1 CPU:006
 netperf  4360 [004]  1207.865355:   sched:sched_wakeup: 
kworker/u16:5:90 [120] success=1 CPU:006
 netperf  4360 [004]  1207.865357:   sched:sched_wakeup: 
kworker/u16:4:89 [120] success=1 CPU:000
 netperf  4360 [004]  1207.865369:   sched:sched_wakeup: 
netserver:4361 [120] success=1 CPU:002
   netserver  4361 [002]  1207.865377:   sched:sched_wakeup: 
netperf:4360 [120] success=1 CPU:004
 netperf  4360 [004]  1207.865476:   sched:sched_wakeup: perf:4359 
[120] success=1 CPU:003

It's not limited to this load, anything at all that is communicating
will do the same on these or similar processors.

This trying to be perfect looks like a booboo to me, as we are now
specifically asking our left hand undo what our right hand did to crank
up throughput.  For the diagnosed processor at least, one of those
hands definitely wants to be slapped.

This doesn't seem to be an issue for L3 equipped CPUs, but perhaps is
for some even modern processors, dunno (the boxen where regression was
detected are far from new).

> we could either remove the condition which tries to keep an even
> number of tasks in each group until busiest group becomes overloaded
> but it means that unrelated tasks may have to share same resources
> or we could try to prevent the migration at wake up. I was looking at
> wake_affine which seems to choose local cpu  when both prev and local
> cpu are idle. I wonder if local cpu is  really a better choice when
> both are idle

I don't see a great alternative to turning it off off the top of my
head, at least for processors with multiple LLCs.  Yeah, unrelated
tasks could end up sharing a cache needlessly, but will that hurt as
badly as tasks not munching tasty hot data definitely does?

-Mike

Re: [PATCH v3 0/5] net/usb: asix driver improvements

[David Miller]

From: robert.f...@collabora.com
Date: Mon, 29 Aug 2016 09:32:14 -0400

> This is a resubmission of v3, since the netdev
> mailinlist was not sent the previous submission.
> 
> This series improves power management of the asix driver.
 ...

Series applied, thanks.

Re: [PATCH v3 0/5] net/usb: asix driver improvements

[David Miller]

From: robert.f...@collabora.com
Date: Mon, 29 Aug 2016 09:32:14 -0400

> This is a resubmission of v3, since the netdev
> mailinlist was not sent the previous submission.
> 
> This series improves power management of the asix driver.
 ...

Series applied, thanks.

Re: [PATCH] mISDN: mark symbols static where possible

[David Miller]

Three different patches all with the same Subject line, so I can't
apply this stuff.

You must make the subject lines unique so that someone reading
the "git shortlog" can tell what is different in each change.

Re: [PATCH] mISDN: mark symbols static where possible

[David Miller]

Three different patches all with the same Subject line, so I can't
apply this stuff.

You must make the subject lines unique so that someone reading
the "git shortlog" can tell what is different in each change.

[PATCH 1/1] ARM: dts: sun8i: Add dts file for the NanoPi NEO SBC

[james]

From: James Pettigrew 

The NanoPi NEO is a minimal H3 based SBC. It comes with 256/512M RAM, a
micro SD slot, 10/100Mbit ethernet and a single USB-A port.

Signed-off-by: James Pettigrew 
---
 arch/arm/boot/dts/Makefile|   1 +
 arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts | 126 ++
 2 files changed, 127 insertions(+)
 create mode 100644 arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 7dd4a07..a35f986 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -792,6 +792,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
sun8i-a83t-allwinner-h8homlet-v2.dtb \
sun8i-a83t-cubietruck-plus.dtb \
sun8i-h3-bananapi-m2-plus.dtb \
+   sun8i-h3-nanopi-neo.dtb \
sun8i-h3-orangepi-2.dtb \
sun8i-h3-orangepi-lite.dtb \
sun8i-h3-orangepi-one.dtb \
diff --git a/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts 
b/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts
new file mode 100644
index 000..3a9eb19
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts
@@ -0,0 +1,126 @@
+/*
+ * Copyright (C) 2016 James Pettigrew 
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file 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 file 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "sun8i-h3.dtsi"
+#include "sunxi-common-regulators.dtsi"
+
+#include 
+#include 
+#include 
+
+/ {
+   model = "FriendlyARM NanoPi NEO";
+   compatible = "friendlyarm,nanopi-neo", "allwinner,sun8i-h3";
+
+   aliases {
+   serial0 = 
+   };
+
+   chosen {
+   stdout-path = "serial0:115200n8";
+   };
+
+   leds {
+   compatible = "gpio-leds";
+   pinctrl-names = "default";
+   pinctrl-0 = <_opc>, <_r_opc>;
+
+   pwr_led {
+   label = "nanopi:green:pwr";
+   gpios = <_pio 0 10 GPIO_ACTIVE_HIGH>;
+   default-state = "on";
+   };
+
+   status_led {
+   label = "nanopi:blue:status";
+   gpios = < 0 10 GPIO_ACTIVE_HIGH>;
+   };
+   };
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins_a>, <_cd_pin>;
+   vmmc-supply = <_vcc3v3>;
+   bus-width = <4>;
+   cd-gpios = < 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
+   cd-inverted;
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   leds_opc: led_pins@0 {
+   allwinner,pins = "PA10";
+   allwinner,function = "gpio_out";
+   allwinner,drive = ;
+   allwinner,pull = ;
+   };
+};
+
+_pio {
+   leds_r_opc: led_pins@0 {
+   allwinner,pins = "PL10";
+   allwinner,function = "gpio_out";
+   allwinner,drive = ;
+   allwinner,pull = ;
+   };
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins_a>;
+   status = "okay";
+};
+
+ {
+   /* USB VBUS is 

[PATCH 1/1] ARM: dts: sun8i: Add dts file for the NanoPi NEO SBC

[james]

From: James Pettigrew 

The NanoPi NEO is a minimal H3 based SBC. It comes with 256/512M RAM, a
micro SD slot, 10/100Mbit ethernet and a single USB-A port.

Signed-off-by: James Pettigrew 
---
 arch/arm/boot/dts/Makefile|   1 +
 arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts | 126 ++
 2 files changed, 127 insertions(+)
 create mode 100644 arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts

diff --git a/arch/arm/boot/dts/Makefile b/arch/arm/boot/dts/Makefile
index 7dd4a07..a35f986 100644
--- a/arch/arm/boot/dts/Makefile
+++ b/arch/arm/boot/dts/Makefile
@@ -792,6 +792,7 @@ dtb-$(CONFIG_MACH_SUN8I) += \
sun8i-a83t-allwinner-h8homlet-v2.dtb \
sun8i-a83t-cubietruck-plus.dtb \
sun8i-h3-bananapi-m2-plus.dtb \
+   sun8i-h3-nanopi-neo.dtb \
sun8i-h3-orangepi-2.dtb \
sun8i-h3-orangepi-lite.dtb \
sun8i-h3-orangepi-one.dtb \
diff --git a/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts 
b/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts
new file mode 100644
index 000..3a9eb19
--- /dev/null
+++ b/arch/arm/boot/dts/sun8i-h3-nanopi-neo.dts
@@ -0,0 +1,126 @@
+/*
+ * Copyright (C) 2016 James Pettigrew 
+ *
+ * This file is dual-licensed: you can use it either under the terms
+ * of the GPL or the X11 license, at your option. Note that this dual
+ * licensing only applies to this file, and not this project as a
+ * whole.
+ *
+ *  a) This file 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 file 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.
+ *
+ * Or, alternatively,
+ *
+ *  b) Permission is hereby granted, free of charge, to any person
+ * obtaining a copy of this software and associated documentation
+ * files (the "Software"), to deal in the Software without
+ * restriction, including without limitation the rights to use,
+ * copy, modify, merge, publish, distribute, sublicense, and/or
+ * sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following
+ * conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
+ * OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
+ * HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
+ * WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
+ * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
+ * OTHER DEALINGS IN THE SOFTWARE.
+ */
+
+/dts-v1/;
+#include "sun8i-h3.dtsi"
+#include "sunxi-common-regulators.dtsi"
+
+#include 
+#include 
+#include 
+
+/ {
+   model = "FriendlyARM NanoPi NEO";
+   compatible = "friendlyarm,nanopi-neo", "allwinner,sun8i-h3";
+
+   aliases {
+   serial0 = 
+   };
+
+   chosen {
+   stdout-path = "serial0:115200n8";
+   };
+
+   leds {
+   compatible = "gpio-leds";
+   pinctrl-names = "default";
+   pinctrl-0 = <_opc>, <_r_opc>;
+
+   pwr_led {
+   label = "nanopi:green:pwr";
+   gpios = <_pio 0 10 GPIO_ACTIVE_HIGH>;
+   default-state = "on";
+   };
+
+   status_led {
+   label = "nanopi:blue:status";
+   gpios = < 0 10 GPIO_ACTIVE_HIGH>;
+   };
+   };
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins_a>, <_cd_pin>;
+   vmmc-supply = <_vcc3v3>;
+   bus-width = <4>;
+   cd-gpios = < 5 6 GPIO_ACTIVE_HIGH>; /* PF6 */
+   cd-inverted;
+   status = "okay";
+};
+
+ {
+   status = "okay";
+};
+
+ {
+   leds_opc: led_pins@0 {
+   allwinner,pins = "PA10";
+   allwinner,function = "gpio_out";
+   allwinner,drive = ;
+   allwinner,pull = ;
+   };
+};
+
+_pio {
+   leds_r_opc: led_pins@0 {
+   allwinner,pins = "PL10";
+   allwinner,function = "gpio_out";
+   allwinner,drive = ;
+   allwinner,pull = ;
+   };
+};
+
+ {
+   pinctrl-names = "default";
+   pinctrl-0 = <_pins_a>;
+   status = "okay";
+};
+
+ {
+   /* USB VBUS is always on */
+   status = "okay";
+};
-- 
1.9.1

Re: [PATCH 2/8] writeback: add wbc_to_write_flags()

[Jens Axboe]

On 08/31/2016 05:32 PM, Omar Sandoval wrote:

On Wed, Aug 31, 2016 at 11:05:45AM -0600, Jens Axboe wrote:

Add wbc_to_write_flags(), which returns the write modifier flags to use,
based on a struct writeback_control. No functional changes in this
patch, but it prepares us for factoring other wbc fields for write type.

Signed-off-by: Jens Axboe 
Reviewed-by: Jan Kara 


[snip]


diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index fc1e16c25a29..e1fc25172397 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -100,6 +100,14 @@ struct writeback_control {
 #endif
 };

+static inline int wbc_to_write_flags(struct writeback_control *wbc)
+{
+   if (wbc->sync_mode == WB_SYNC_ALL)
+   return WRITE_SYNC;
+
+   return WRITE;


I think this should be `return 0;` after the op/flags split. WRITE == 1,
so this would get interpreted as REQ_FAILFAST_DEV in bi_opf.


Good catch, thanks! Fixed up.

--
Jens Axboe

Re: [PATCH 2/8] writeback: add wbc_to_write_flags()

[Jens Axboe]

On 08/31/2016 05:32 PM, Omar Sandoval wrote:

On Wed, Aug 31, 2016 at 11:05:45AM -0600, Jens Axboe wrote:

Add wbc_to_write_flags(), which returns the write modifier flags to use,
based on a struct writeback_control. No functional changes in this
patch, but it prepares us for factoring other wbc fields for write type.

Signed-off-by: Jens Axboe 
Reviewed-by: Jan Kara 


[snip]


diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index fc1e16c25a29..e1fc25172397 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -100,6 +100,14 @@ struct writeback_control {
 #endif
 };

+static inline int wbc_to_write_flags(struct writeback_control *wbc)
+{
+   if (wbc->sync_mode == WB_SYNC_ALL)
+   return WRITE_SYNC;
+
+   return WRITE;


I think this should be `return 0;` after the op/flags split. WRITE == 1,
so this would get interpreted as REQ_FAILFAST_DEV in bi_opf.


Good catch, thanks! Fixed up.

--
Jens Axboe

Re: [PATCH 2/4] cpu: expose pm_qos_resume_latency for each cpu

[Alex Shi]

Few commits and patch changed according to Greg's comments.

Regards
Alex



>From 186c534b0b8b9649fbfce05b0b4f90f764c571a4 Mon Sep 17 00:00:00 2001
From: Alex Shi 
Date: Tue, 16 Aug 2016 15:29:01 +0800
Subject: [PATCH 2/4] cpu: expose pm_qos_resume_latency for each cpu

Adding /sys/devices/system/cpu/cpux/power/pm_qos_resume_latency_us for
each of cpus. The pm_qos_resume_latency usage defined in
Documentation/ABI/testing/sysfs-devices-power

The cpu-dma PM QoS constraint impacts all the cpus in the system. There
is no way to let the user to choose a PM QoS constraint per cpu.

The following patch exposes to the userspace a per cpu based sysfs file
in order to let the userspace to change the value of the PM QoS latency
constraint.

This change is inoperative in its form and the cpuidle governors have to
take into account the per cpu latency constraint in addition to the
global cpu-dma latency constraint in order to operate properly.

Signed-off-by: Alex Shi 
To: linux-kernel@vger.kernel.org
To: Greg Kroah-Hartman 
Cc: linux...@vger.kernel.org
Cc: Ulf Hansson 
Cc: Daniel Lezcano 
---
 drivers/base/cpu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 4c28e1a..2c3b359 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "base.h"
 
@@ -376,6 +377,7 @@ int register_cpu(struct cpu *cpu, int num)
 
per_cpu(cpu_sys_devices, num) = >dev;
register_cpu_under_node(num, cpu_to_node(num));
+   dev_pm_qos_expose_latency_limit(>dev, 0);
 
return 0;
 }
-- 
2.8.1.101.g72d917a


 

Re: [PATCH 2/4] cpu: expose pm_qos_resume_latency for each cpu

[Alex Shi]

Few commits and patch changed according to Greg's comments.

Regards
Alex



>From 186c534b0b8b9649fbfce05b0b4f90f764c571a4 Mon Sep 17 00:00:00 2001
From: Alex Shi 
Date: Tue, 16 Aug 2016 15:29:01 +0800
Subject: [PATCH 2/4] cpu: expose pm_qos_resume_latency for each cpu

Adding /sys/devices/system/cpu/cpux/power/pm_qos_resume_latency_us for
each of cpus. The pm_qos_resume_latency usage defined in
Documentation/ABI/testing/sysfs-devices-power

The cpu-dma PM QoS constraint impacts all the cpus in the system. There
is no way to let the user to choose a PM QoS constraint per cpu.

The following patch exposes to the userspace a per cpu based sysfs file
in order to let the userspace to change the value of the PM QoS latency
constraint.

This change is inoperative in its form and the cpuidle governors have to
take into account the per cpu latency constraint in addition to the
global cpu-dma latency constraint in order to operate properly.

Signed-off-by: Alex Shi 
To: linux-kernel@vger.kernel.org
To: Greg Kroah-Hartman 
Cc: linux...@vger.kernel.org
Cc: Ulf Hansson 
Cc: Daniel Lezcano 
---
 drivers/base/cpu.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 4c28e1a..2c3b359 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -17,6 +17,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include "base.h"
 
@@ -376,6 +377,7 @@ int register_cpu(struct cpu *cpu, int num)
 
per_cpu(cpu_sys_devices, num) = >dev;
register_cpu_under_node(num, cpu_to_node(num));
+   dev_pm_qos_expose_latency_limit(>dev, 0);
 
return 0;
 }
-- 
2.8.1.101.g72d917a


 

Re: [PATCH 5/5] net: axienet: constify ethtool_ops structures

[David Miller]

From: Julia Lawall 
Date: Thu,  1 Sep 2016 00:21:23 +0200

> Check for ethtool_ops structures that are only stored in the ethtool_ops
> field of a net_device structure or passed as the second argument to
> netdev_set_default_ethtool_ops.  These contexts are declared const, so
> ethtool_ops structures that have these properties can be declared as const
> also.
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
 ...
> Suggested-by: Stephen Hemminger 
> 
> Signed-off-by: Julia Lawall 

Applied.

Re: [PATCH 5/5] net: axienet: constify ethtool_ops structures

[David Miller]

From: Julia Lawall 
Date: Thu,  1 Sep 2016 00:21:23 +0200

> Check for ethtool_ops structures that are only stored in the ethtool_ops
> field of a net_device structure or passed as the second argument to
> netdev_set_default_ethtool_ops.  These contexts are declared const, so
> ethtool_ops structures that have these properties can be declared as const
> also.
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
 ...
> Suggested-by: Stephen Hemminger 
> 
> Signed-off-by: Julia Lawall 

Applied.

Re: [PATCH 1/5] net: mediatek: constify ethtool_ops structures

[David Miller]

From: Julia Lawall 
Date: Thu,  1 Sep 2016 00:21:19 +0200

> Check for ethtool_ops structures that are only stored in the ethtool_ops
> field of a net_device structure or passed as the second argument to
> netdev_set_default_ethtool_ops.  These contexts are declared const, so
> ethtool_ops structures that have these properties can be declared as const
> also.
 ...
> Suggested-by: Stephen Hemminger 
> 
> Signed-off-by: Julia Lawall 

Applied.

Re: [PATCH 1/5] net: mediatek: constify ethtool_ops structures

[David Miller]

From: Julia Lawall 
Date: Thu,  1 Sep 2016 00:21:19 +0200

> Check for ethtool_ops structures that are only stored in the ethtool_ops
> field of a net_device structure or passed as the second argument to
> netdev_set_default_ethtool_ops.  These contexts are declared const, so
> ethtool_ops structures that have these properties can be declared as const
> also.
 ...
> Suggested-by: Stephen Hemminger 
> 
> Signed-off-by: Julia Lawall 

Applied.

Re: [PATCH 4/5] r8152: constify ethtool_ops structures

[David Miller]

From: Julia Lawall 
Date: Thu,  1 Sep 2016 00:21:22 +0200

> Check for ethtool_ops structures that are only stored in the ethtool_ops
> field of a net_device structure or passed as the second argument to
> netdev_set_default_ethtool_ops.  These contexts are declared const, so
> ethtool_ops structures that have these properties can be declared as const
> also.
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
 ...
> Suggested-by: Stephen Hemminger 
> 
> Signed-off-by: Julia Lawall 

Applied.

Re: [PATCH 4/5] r8152: constify ethtool_ops structures

[David Miller]

From: Julia Lawall 
Date: Thu,  1 Sep 2016 00:21:22 +0200

> Check for ethtool_ops structures that are only stored in the ethtool_ops
> field of a net_device structure or passed as the second argument to
> netdev_set_default_ethtool_ops.  These contexts are declared const, so
> ethtool_ops structures that have these properties can be declared as const
> also.
> 
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
 ...
> Suggested-by: Stephen Hemminger 
> 
> Signed-off-by: Julia Lawall 

Applied.

Re: [PATCH 2/4] cpu: expose pm_qos_resume_latency for each cpu

[Alex Shi]

On 09/01/2016 11:39 AM, Alex Shi wrote:
> User can set values on each of cpu, like limit 100ms on cpu0, that means
> the cpu0 response time should be in 100ms in possible idle. It similar
> with DMA_LATENCY, but that request is for all cpu. This is just for
> particular cpu, like a interrupt pined CPU.

Sorry for typo!
s/100ms/100us/

> 
> echo 100 > /sys/devices/system/cpu/cpu0/power/pm_qos_resume_latency_us
>> > 

Re: [PATCH 2/4] cpu: expose pm_qos_resume_latency for each cpu

[Alex Shi]

On 09/01/2016 11:39 AM, Alex Shi wrote:
> User can set values on each of cpu, like limit 100ms on cpu0, that means
> the cpu0 response time should be in 100ms in possible idle. It similar
> with DMA_LATENCY, but that request is for all cpu. This is just for
> particular cpu, like a interrupt pined CPU.

Sorry for typo!
s/100ms/100us/

> 
> echo 100 > /sys/devices/system/cpu/cpu0/power/pm_qos_resume_latency_us
>> > 

Re: [PATCH 04/13] perf/core: Extend perf_output_sample_regs() to include perf_arch_regs

[Madhavan Srinivasan]

On Tuesday 30 August 2016 09:41 PM, Nilay Vaish wrote:

On 28 August 2016 at 16:00, Madhavan Srinivasan
 wrote:

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 274288819829..e16bf4d057d1 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5371,16 +5371,24 @@ u64 __attribute__((weak)) perf_arch_reg_value(struct 
perf_arch_regs *regs,

  static void
  perf_output_sample_regs(struct perf_output_handle *handle,
-   struct pt_regs *regs, u64 mask)
+   struct perf_regs *regs, u64 mask)
  {
 int bit;
 DECLARE_BITMAP(_mask, 64);
+   u64 arch_regs_mask = regs->arch_regs_mask;

 bitmap_from_u64(_mask, mask);
 for_each_set_bit(bit, _mask, sizeof(mask) * BITS_PER_BYTE) {
 u64 val;

-   val = perf_reg_value(regs, bit);
+   val = perf_reg_value(regs->regs, bit);
+   perf_output_put(handle, val);
+   }
+
+   bitmap_from_u64(_mask, arch_regs_mask);
+   for_each_set_bit(bit, _mask, sizeof(mask) * BITS_PER_BYTE) {
+   u64 val;
+   val = perf_arch_reg_value(regs->arch_regs, bit);
 perf_output_put(handle, val);
 }
  }
@@ -5792,7 +5800,7 @@ void perf_output_sample(struct perf_output_handle *handle,
 if (abi) {
 u64 mask = event->attr.sample_regs_user;
 perf_output_sample_regs(handle,
-   data->regs_user.regs,
+   >regs_user,
 mask);
 }
 }
@@ -5827,7 +5835,7 @@ void perf_output_sample(struct perf_output_handle *handle,
 u64 mask = event->attr.sample_regs_intr;

 perf_output_sample_regs(handle,
-   data->regs_intr.regs,
+   >regs_intr,
 mask);
 }
 }
--
2.7.4


I would like to suggest a slightly different version.  Would it make
more sense to have something like following:


I agree we are outputting two different structures, but since we use the
INTR_REG infrastructure to dump the arch pmu registers, I preferred to
extend perf_output_sample_regs. But I guess I can break it up.

Maddy



@@ -5792,7 +5800,7 @@ void perf_output_sample(struct perf_output_handle *handle,
  if (abi) {
 u64 mask = event->attr.sample_regs_user;
 perf_output_sample_regs(handle,
 data->regs_user.regs,
 mask);
 }
+
+  if (arch_regs_mask) {
+   perf_output_pmu_regs(handle,
data->regs_users.arch_regs, arch_regs_mask);
+  }
 }


Somehow I don't like outputting the two sets of registers through the
same function call.

--
Nilay

Re: [PATCH 04/13] perf/core: Extend perf_output_sample_regs() to include perf_arch_regs

[Madhavan Srinivasan]

On Tuesday 30 August 2016 09:41 PM, Nilay Vaish wrote:

On 28 August 2016 at 16:00, Madhavan Srinivasan
 wrote:

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 274288819829..e16bf4d057d1 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -5371,16 +5371,24 @@ u64 __attribute__((weak)) perf_arch_reg_value(struct 
perf_arch_regs *regs,

  static void
  perf_output_sample_regs(struct perf_output_handle *handle,
-   struct pt_regs *regs, u64 mask)
+   struct perf_regs *regs, u64 mask)
  {
 int bit;
 DECLARE_BITMAP(_mask, 64);
+   u64 arch_regs_mask = regs->arch_regs_mask;

 bitmap_from_u64(_mask, mask);
 for_each_set_bit(bit, _mask, sizeof(mask) * BITS_PER_BYTE) {
 u64 val;

-   val = perf_reg_value(regs, bit);
+   val = perf_reg_value(regs->regs, bit);
+   perf_output_put(handle, val);
+   }
+
+   bitmap_from_u64(_mask, arch_regs_mask);
+   for_each_set_bit(bit, _mask, sizeof(mask) * BITS_PER_BYTE) {
+   u64 val;
+   val = perf_arch_reg_value(regs->arch_regs, bit);
 perf_output_put(handle, val);
 }
  }
@@ -5792,7 +5800,7 @@ void perf_output_sample(struct perf_output_handle *handle,
 if (abi) {
 u64 mask = event->attr.sample_regs_user;
 perf_output_sample_regs(handle,
-   data->regs_user.regs,
+   >regs_user,
 mask);
 }
 }
@@ -5827,7 +5835,7 @@ void perf_output_sample(struct perf_output_handle *handle,
 u64 mask = event->attr.sample_regs_intr;

 perf_output_sample_regs(handle,
-   data->regs_intr.regs,
+   >regs_intr,
 mask);
 }
 }
--
2.7.4


I would like to suggest a slightly different version.  Would it make
more sense to have something like following:


I agree we are outputting two different structures, but since we use the
INTR_REG infrastructure to dump the arch pmu registers, I preferred to
extend perf_output_sample_regs. But I guess I can break it up.

Maddy



@@ -5792,7 +5800,7 @@ void perf_output_sample(struct perf_output_handle *handle,
  if (abi) {
 u64 mask = event->attr.sample_regs_user;
 perf_output_sample_regs(handle,
 data->regs_user.regs,
 mask);
 }
+
+  if (arch_regs_mask) {
+   perf_output_pmu_regs(handle,
data->regs_users.arch_regs, arch_regs_mask);
+  }
 }


Somehow I don't like outputting the two sets of registers through the
same function call.

--
Nilay

[PATCH 6/7] serial: 8250_fintek: Add F81866 Support

[Ji-Ze Hong (Peter Hong)]

Fintek F81866 is a LPC to 6 UARTs SuperIO. It has fully functional UARTs
likes F81216H. It's also need check the IRQ mode with system assigned,
but the configuration is not the same with F81216 series.

F81866 IRQ Mode setting:
0xf0
Bit1: IRQ_MODE0
Bit0: Share mode (always on)
0xf6
Bit3: IRQ_MODE1

Level/Low: IRQ_MODE0:0, IRQ_MODE1:0
Edge/High: IRQ_MODE0:1, IRQ_MODE1:0

The following list is brief descriptions of F81866:

F81866 (1010)
9Bit/High baud rate(not implements with mainline)
RS485, 128Bytes FIFO (implemented)

Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 75 ---
 1 file changed, 69 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index a62cfdd..1194e57 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -21,6 +21,7 @@
 #define EXIT_KEY 0xAA
 #define CHIP_ID1  0x20
 #define CHIP_ID2  0x21
+#define CHIP_ID_F81866 0x1010
 #define CHIP_ID_F81216AD 0x1602
 #define CHIP_ID_F81216H 0x0501
 #define CHIP_ID_F81216 0x0802
@@ -50,6 +51,26 @@
 #define RXFTHR_MODE_MASK   (BIT(5) | BIT(4))
 #define RXFTHR_MODE_4X BIT(5)
 
+#define F81216_LDN_LOW 0x0
+#define F81216_LDN_HIGH0x4
+
+/*
+ * F81866 registers
+ *
+ * The IRQ setting mode of F81866 is not the same with F81216 series.
+ * Level/Low: IRQ_MODE0:0, IRQ_MODE1:0
+ * Edge/High: IRQ_MODE0:1, IRQ_MODE1:0
+ */
+#define F81866_IRQ_MODE0xf0
+#define F81866_IRQ_SHARE   BIT(0)
+#define F81866_IRQ_MODE0   BIT(1)
+
+#define F81866_FIFO_CTRL   FIFO_CTRL
+#define F81866_IRQ_MODE1   BIT(3)
+
+#define F81866_LDN_LOW 0x10
+#define F81866_LDN_HIGH0x16
+
 struct fintek_8250 {
u16 pid;
u16 base_port;
@@ -109,6 +130,7 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
chip |= sio_read_reg(pdata, CHIP_ID2) << 8;
 
switch (chip) {
+   case CHIP_ID_F81866:
case CHIP_ID_F81216AD:
case CHIP_ID_F81216H:
case CHIP_ID_F81216:
@@ -121,6 +143,26 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
return 0;
 }
 
+static int fintek_8250_get_ldn_range(struct fintek_8250 *pdata, int *min,
+int *max)
+{
+   switch (pdata->pid) {
+   case CHIP_ID_F81866:
+   *min = F81866_LDN_LOW;
+   *max = F81866_LDN_HIGH;
+   return 0;
+
+   case CHIP_ID_F81216AD:
+   case CHIP_ID_F81216H:
+   case CHIP_ID_F81216:
+   *min = F81216_LDN_LOW;
+   *max = F81216_LDN_HIGH;
+   return 0;
+   }
+
+   return -ENODEV;
+}
+
 static int fintek_8250_rs485_config(struct uart_port *port,
  struct serial_rs485 *rs485)
 {
@@ -175,6 +217,7 @@ static void fintek_8250_set_max_fifo(struct fintek_8250 
*pdata)
default: /* Default 16Bytes FIFO */
return;
 
+   case CHIP_ID_F81866:
case CHIP_ID_F81216H: /* 128Bytes FIFO */
sio_write_mask_reg(pdata, FIFO_CTRL,
   FIFO_MODE_MASK | RXFTHR_MODE_MASK,
@@ -186,9 +229,27 @@ static void fintek_8250_set_max_fifo(struct fintek_8250 
*pdata)
 static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool is_level)
 {
sio_write_reg(pdata, LDN, pdata->index);
-   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_SHARE, IRQ_SHARE);
-   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_MODE_MASK,
-  is_level ? IRQ_LEVEL_LOW : IRQ_EDGE_HIGH);
+
+   switch (pdata->pid) {
+   case CHIP_ID_F81866:
+   sio_write_mask_reg(pdata, F81866_FIFO_CTRL, F81866_IRQ_MODE1,
+  0);
+   sio_write_mask_reg(pdata, F81866_IRQ_MODE, F81866_IRQ_SHARE,
+  F81866_IRQ_SHARE);
+   sio_write_mask_reg(pdata, F81866_IRQ_MODE,
+  F81866_IRQ_MODE0,
+  is_level ? 0 : F81866_IRQ_MODE0);
+   break;
+
+   case CHIP_ID_F81216AD:
+   case CHIP_ID_F81216H:
+   case CHIP_ID_F81216:
+   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_SHARE,
+  IRQ_SHARE);
+   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_MODE_MASK,
+  is_level ? IRQ_LEVEL_LOW : IRQ_EDGE_HIGH);
+   break;
+   }
 }
 
 static int find_base_port(struct fintek_8250 *pdata, u16 io_address,
@@ -198,7 +259,7 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address,
static const u8 keys[] = {0x77, 0xa0, 0x87, 0x67};
struct irq_data *irq_data;
bool level_mode = false;
-   

[PATCH 6/7] serial: 8250_fintek: Add F81866 Support

[Ji-Ze Hong (Peter Hong)]

Fintek F81866 is a LPC to 6 UARTs SuperIO. It has fully functional UARTs
likes F81216H. It's also need check the IRQ mode with system assigned,
but the configuration is not the same with F81216 series.

F81866 IRQ Mode setting:
0xf0
Bit1: IRQ_MODE0
Bit0: Share mode (always on)
0xf6
Bit3: IRQ_MODE1

Level/Low: IRQ_MODE0:0, IRQ_MODE1:0
Edge/High: IRQ_MODE0:1, IRQ_MODE1:0

The following list is brief descriptions of F81866:

F81866 (1010)
9Bit/High baud rate(not implements with mainline)
RS485, 128Bytes FIFO (implemented)

Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 75 ---
 1 file changed, 69 insertions(+), 6 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index a62cfdd..1194e57 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -21,6 +21,7 @@
 #define EXIT_KEY 0xAA
 #define CHIP_ID1  0x20
 #define CHIP_ID2  0x21
+#define CHIP_ID_F81866 0x1010
 #define CHIP_ID_F81216AD 0x1602
 #define CHIP_ID_F81216H 0x0501
 #define CHIP_ID_F81216 0x0802
@@ -50,6 +51,26 @@
 #define RXFTHR_MODE_MASK   (BIT(5) | BIT(4))
 #define RXFTHR_MODE_4X BIT(5)
 
+#define F81216_LDN_LOW 0x0
+#define F81216_LDN_HIGH0x4
+
+/*
+ * F81866 registers
+ *
+ * The IRQ setting mode of F81866 is not the same with F81216 series.
+ * Level/Low: IRQ_MODE0:0, IRQ_MODE1:0
+ * Edge/High: IRQ_MODE0:1, IRQ_MODE1:0
+ */
+#define F81866_IRQ_MODE0xf0
+#define F81866_IRQ_SHARE   BIT(0)
+#define F81866_IRQ_MODE0   BIT(1)
+
+#define F81866_FIFO_CTRL   FIFO_CTRL
+#define F81866_IRQ_MODE1   BIT(3)
+
+#define F81866_LDN_LOW 0x10
+#define F81866_LDN_HIGH0x16
+
 struct fintek_8250 {
u16 pid;
u16 base_port;
@@ -109,6 +130,7 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
chip |= sio_read_reg(pdata, CHIP_ID2) << 8;
 
switch (chip) {
+   case CHIP_ID_F81866:
case CHIP_ID_F81216AD:
case CHIP_ID_F81216H:
case CHIP_ID_F81216:
@@ -121,6 +143,26 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
return 0;
 }
 
+static int fintek_8250_get_ldn_range(struct fintek_8250 *pdata, int *min,
+int *max)
+{
+   switch (pdata->pid) {
+   case CHIP_ID_F81866:
+   *min = F81866_LDN_LOW;
+   *max = F81866_LDN_HIGH;
+   return 0;
+
+   case CHIP_ID_F81216AD:
+   case CHIP_ID_F81216H:
+   case CHIP_ID_F81216:
+   *min = F81216_LDN_LOW;
+   *max = F81216_LDN_HIGH;
+   return 0;
+   }
+
+   return -ENODEV;
+}
+
 static int fintek_8250_rs485_config(struct uart_port *port,
  struct serial_rs485 *rs485)
 {
@@ -175,6 +217,7 @@ static void fintek_8250_set_max_fifo(struct fintek_8250 
*pdata)
default: /* Default 16Bytes FIFO */
return;
 
+   case CHIP_ID_F81866:
case CHIP_ID_F81216H: /* 128Bytes FIFO */
sio_write_mask_reg(pdata, FIFO_CTRL,
   FIFO_MODE_MASK | RXFTHR_MODE_MASK,
@@ -186,9 +229,27 @@ static void fintek_8250_set_max_fifo(struct fintek_8250 
*pdata)
 static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool is_level)
 {
sio_write_reg(pdata, LDN, pdata->index);
-   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_SHARE, IRQ_SHARE);
-   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_MODE_MASK,
-  is_level ? IRQ_LEVEL_LOW : IRQ_EDGE_HIGH);
+
+   switch (pdata->pid) {
+   case CHIP_ID_F81866:
+   sio_write_mask_reg(pdata, F81866_FIFO_CTRL, F81866_IRQ_MODE1,
+  0);
+   sio_write_mask_reg(pdata, F81866_IRQ_MODE, F81866_IRQ_SHARE,
+  F81866_IRQ_SHARE);
+   sio_write_mask_reg(pdata, F81866_IRQ_MODE,
+  F81866_IRQ_MODE0,
+  is_level ? 0 : F81866_IRQ_MODE0);
+   break;
+
+   case CHIP_ID_F81216AD:
+   case CHIP_ID_F81216H:
+   case CHIP_ID_F81216:
+   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_SHARE,
+  IRQ_SHARE);
+   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_MODE_MASK,
+  is_level ? IRQ_LEVEL_LOW : IRQ_EDGE_HIGH);
+   break;
+   }
 }
 
 static int find_base_port(struct fintek_8250 *pdata, u16 io_address,
@@ -198,7 +259,7 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address,
static const u8 keys[] = {0x77, 0xa0, 0x87, 0x67};
struct irq_data *irq_data;
bool level_mode = false;
-   int i, j, k;
+   int i, j, 

[PATCH 7/7] serial: 8250_fintek: Add F81865 Support

[Ji-Ze Hong (Peter Hong)]

Fintek F81865 is a LPC to 6 UARTs SuperIO. It has less functional UARTs
likes F81866. It's also need check the IRQ mode with system assigned,
but the configuration is not the same with F81216 series.

F81865 IRQ Mode setting:
0xf0
Bit1: IRQ_MODE0
Bit0: Share mode (always on)

Level/Low: IRQ_MODE0:0
Edge/High: IRQ_MODE0:1

The following list is brief descriptions of F81865:

F81865 (0704)
9Bit(not implements with mainline)
RS485(implemented)

Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index 1194e57..3fb9ab6 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -21,6 +21,7 @@
 #define EXIT_KEY 0xAA
 #define CHIP_ID1  0x20
 #define CHIP_ID2  0x21
+#define CHIP_ID_F81865 0x0407
 #define CHIP_ID_F81866 0x1010
 #define CHIP_ID_F81216AD 0x1602
 #define CHIP_ID_F81216H 0x0501
@@ -130,6 +131,7 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
chip |= sio_read_reg(pdata, CHIP_ID2) << 8;
 
switch (chip) {
+   case CHIP_ID_F81865:
case CHIP_ID_F81866:
case CHIP_ID_F81216AD:
case CHIP_ID_F81216H:
@@ -147,6 +149,7 @@ static int fintek_8250_get_ldn_range(struct fintek_8250 
*pdata, int *min,
 int *max)
 {
switch (pdata->pid) {
+   case CHIP_ID_F81865:
case CHIP_ID_F81866:
*min = F81866_LDN_LOW;
*max = F81866_LDN_HIGH;
@@ -231,9 +234,12 @@ static void fintek_8250_set_irq_mode(struct fintek_8250 
*pdata, bool is_level)
sio_write_reg(pdata, LDN, pdata->index);
 
switch (pdata->pid) {
+   case CHIP_ID_F81865:
case CHIP_ID_F81866:
-   sio_write_mask_reg(pdata, F81866_FIFO_CTRL, F81866_IRQ_MODE1,
-  0);
+   if (pdata->pid == CHIP_ID_F81866)
+   sio_write_mask_reg(pdata, F81866_FIFO_CTRL,
+  F81866_IRQ_MODE1, 0);
+
sio_write_mask_reg(pdata, F81866_IRQ_MODE, F81866_IRQ_SHARE,
   F81866_IRQ_SHARE);
sio_write_mask_reg(pdata, F81866_IRQ_MODE,
@@ -312,6 +318,7 @@ static void fintek_8250_set_rs485_handler(struct 
uart_8250_port *uart)
default: /* No RS485 Auto direction functional */
break;
 
+   case CHIP_ID_F81865:
case CHIP_ID_F81866:
case CHIP_ID_F81216AD:
case CHIP_ID_F81216H:
-- 
1.9.1

[PATCH 1/7] serial: 8250_fintek: Refactoring read/write method

[Ji-Ze Hong (Peter Hong)]

If we need to access SuperIO registers, It should write register offset
to base_addr and read/write value to base_addr + 1 to perform read/write.
We can make it more simply with write/read functions.

This patch add sio_read_reg()/sio_write_reg()/sio_write_mask_reg() to
reduce SuperIO register operation with lot of outb()/inb().

Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 73 ++-
 1 file changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index 0facc78..2ae846e 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -49,6 +49,27 @@ struct fintek_8250 {
u8 key;
 };
 
+static u8 sio_read_reg(struct fintek_8250 *pdata, u8 reg)
+{
+   outb(reg, pdata->base_port + ADDR_PORT);
+   return inb(pdata->base_port + DATA_PORT);
+}
+
+static void sio_write_reg(struct fintek_8250 *pdata, u8 reg, u8 data)
+{
+   outb(reg, pdata->base_port + ADDR_PORT);
+   outb(data, pdata->base_port + DATA_PORT);
+}
+
+static void sio_write_mask_reg(struct fintek_8250 *pdata, u8 reg, u8 mask,
+  u8 data)
+{
+   u8 tmp;
+
+   tmp = (sio_read_reg(pdata, reg) & ~mask) | (mask & data);
+   sio_write_reg(pdata, reg, tmp);
+}
+
 static int fintek_8250_enter_key(u16 base_port, u8 key)
 {
if (!request_muxed_region(base_port, 2, "8250_fintek"))
@@ -66,22 +87,18 @@ static void fintek_8250_exit_key(u16 base_port)
release_region(base_port + ADDR_PORT, 2);
 }
 
-static int fintek_8250_check_id(u16 base_port)
+static int fintek_8250_check_id(struct fintek_8250 *pdata)
 {
u16 chip;
 
-   outb(VENDOR_ID1, base_port + ADDR_PORT);
-   if (inb(base_port + DATA_PORT) != VENDOR_ID1_VAL)
+   if (sio_read_reg(pdata, VENDOR_ID1) != VENDOR_ID1_VAL)
return -ENODEV;
 
-   outb(VENDOR_ID2, base_port + ADDR_PORT);
-   if (inb(base_port + DATA_PORT) != VENDOR_ID2_VAL)
+   if (sio_read_reg(pdata, VENDOR_ID2) != VENDOR_ID2_VAL)
return -ENODEV;
 
-   outb(CHIP_ID1, base_port + ADDR_PORT);
-   chip = inb(base_port + DATA_PORT);
-   outb(CHIP_ID2, base_port + ADDR_PORT);
-   chip |= inb(base_port + DATA_PORT) << 8;
+   chip = sio_read_reg(pdata, CHIP_ID1);
+   chip |= sio_read_reg(pdata, CHIP_ID2) << 8;
 
if (chip != CHIP_ID_0 && chip != CHIP_ID_1)
return -ENODEV;
@@ -128,10 +145,8 @@ static int fintek_8250_rs485_config(struct uart_port *port,
if (fintek_8250_enter_key(pdata->base_port, pdata->key))
return -EBUSY;
 
-   outb(LDN, pdata->base_port + ADDR_PORT);
-   outb(pdata->index, pdata->base_port + DATA_PORT);
-   outb(RS485, pdata->base_port + ADDR_PORT);
-   outb(config, pdata->base_port + DATA_PORT);
+   sio_write_reg(pdata, LDN, pdata->index);
+   sio_write_reg(pdata, RS485, config);
fintek_8250_exit_key(pdata->base_port);
 
port->rs485 = *rs485;
@@ -147,10 +162,12 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address)
 
for (i = 0; i < ARRAY_SIZE(addr); i++) {
for (j = 0; j < ARRAY_SIZE(keys); j++) {
+   pdata->base_port = addr[i];
+   pdata->key = keys[j];
 
if (fintek_8250_enter_key(addr[i], keys[j]))
continue;
-   if (fintek_8250_check_id(addr[i])) {
+   if (fintek_8250_check_id(pdata)) {
fintek_8250_exit_key(addr[i]);
continue;
}
@@ -158,19 +175,13 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address)
for (k = 0; k < 4; k++) {
u16 aux;
 
-   outb(LDN, addr[i] + ADDR_PORT);
-   outb(k, addr[i] + DATA_PORT);
-
-   outb(IO_ADDR1, addr[i] + ADDR_PORT);
-   aux = inb(addr[i] + DATA_PORT);
-   outb(IO_ADDR2, addr[i] + ADDR_PORT);
-   aux |= inb(addr[i] + DATA_PORT) << 8;
+   sio_write_reg(pdata, LDN, k);
+   aux = sio_read_reg(pdata, IO_ADDR1);
+   aux |= sio_read_reg(pdata, IO_ADDR2) << 8;
if (aux != io_address)
continue;
 
fintek_8250_exit_key(addr[i]);
-   pdata->key = keys[j];
-   pdata->base_port = addr[i];
pdata->index = k;
 
return 0;
@@ -186,24 +197,16 @@ static int 

[PATCH 2/7] serial: 8250_fintek: Set IRQ Mode when port probed

[Ji-Ze Hong (Peter Hong)]

Set IRQ Mode when port probed in find_base_port()

It should hold the IO port premission via fintek_8250_enter_key() and
release via fintek_8250_exit_key() when we configure the SuperIO.

This patch will move all SuperIO configure operations to find_base_port()
to reduce fintek_8250_enter_key()/fintek_8250_exit_key() usage.

Suggested-by: Ricardo Ribalda Delgado 
Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 36 ++-
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index 2ae846e..cd8903f 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -49,6 +49,9 @@ struct fintek_8250 {
u8 key;
 };
 
+static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata,
+bool level_mode);
+
 static u8 sio_read_reg(struct fintek_8250 *pdata, u8 reg)
 {
outb(reg, pdata->base_port + ADDR_PORT);
@@ -154,10 +157,13 @@ static int fintek_8250_rs485_config(struct uart_port 
*port,
return 0;
 }
 
-static int find_base_port(struct fintek_8250 *pdata, u16 io_address)
+static int find_base_port(struct fintek_8250 *pdata, u16 io_address,
+ unsigned int irq)
 {
static const u16 addr[] = {0x4e, 0x2e};
static const u8 keys[] = {0x77, 0xa0, 0x87, 0x67};
+   struct irq_data *irq_data;
+   bool level_mode = false;
int i, j, k;
 
for (i = 0; i < ARRAY_SIZE(addr); i++) {
@@ -181,9 +187,16 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address)
if (aux != io_address)
continue;
 
-   fintek_8250_exit_key(addr[i]);
pdata->index = k;
 
+   irq_data = irq_get_irq_data(irq);
+   if (irq_data)
+   level_mode =
+   irqd_is_level_type(irq_data);
+
+   fintek_8250_set_irq_mode(pdata, level_mode);
+   fintek_8250_exit_key(addr[i]);
+
return 0;
}
 
@@ -194,31 +207,20 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address)
return -ENODEV;
 }
 
-static int fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool level_mode)
+static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool is_level)
 {
-   int status;
-
-   status = fintek_8250_enter_key(pdata->base_port, pdata->key);
-   if (status)
-   return status;
-
sio_write_reg(pdata, LDN, pdata->index);
sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_SHARE, IRQ_SHARE);
sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_MODE_MASK,
-  level_mode ? IRQ_LEVEL_LOW : IRQ_EDGE_HIGH);
-
-   fintek_8250_exit_key(pdata->base_port);
-   return 0;
+  is_level ? IRQ_LEVEL_LOW : IRQ_EDGE_HIGH);
 }
 
 int fintek_8250_probe(struct uart_8250_port *uart)
 {
struct fintek_8250 *pdata;
struct fintek_8250 probe_data;
-   struct irq_data *irq_data = irq_get_irq_data(uart->port.irq);
-   bool level_mode = irqd_is_level_type(irq_data);
 
-   if (find_base_port(_data, uart->port.iobase))
+   if (find_base_port(_data, uart->port.iobase, uart->port.irq))
return -ENODEV;
 
pdata = devm_kzalloc(uart->port.dev, sizeof(*pdata), GFP_KERNEL);
@@ -229,5 +231,5 @@ int fintek_8250_probe(struct uart_8250_port *uart)
uart->port.rs485_config = fintek_8250_rs485_config;
uart->port.private_data = pdata;
 
-   return fintek_8250_set_irq_mode(pdata, level_mode);
+   return 0;
 }
-- 
1.9.1

[PATCH 4/7] serial: 8250_fintek: Rearrange function

[Ji-Ze Hong (Peter Hong)]

We change the position of fintek_8250_set_irq_mode() above the
find_base_port() to eliminate the prototype define.

Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index 5625203..921f742 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -56,9 +56,6 @@ struct fintek_8250 {
u8 key;
 };
 
-static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata,
-bool level_mode);
-
 static u8 sio_read_reg(struct fintek_8250 *pdata, u8 reg)
 {
outb(reg, pdata->base_port + ADDR_PORT);
@@ -179,6 +176,14 @@ static void fintek_8250_set_max_fifo(struct fintek_8250 
*pdata)
}
 }
 
+static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool is_level)
+{
+   sio_write_reg(pdata, LDN, pdata->index);
+   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_SHARE, IRQ_SHARE);
+   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_MODE_MASK,
+  is_level ? IRQ_LEVEL_LOW : IRQ_EDGE_HIGH);
+}
+
 static int find_base_port(struct fintek_8250 *pdata, u16 io_address,
  unsigned int irq)
 {
@@ -230,14 +235,6 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address,
return -ENODEV;
 }
 
-static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool is_level)
-{
-   sio_write_reg(pdata, LDN, pdata->index);
-   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_SHARE, IRQ_SHARE);
-   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_MODE_MASK,
-  is_level ? IRQ_LEVEL_LOW : IRQ_EDGE_HIGH);
-}
-
 int fintek_8250_probe(struct uart_8250_port *uart)
 {
struct fintek_8250 *pdata;
-- 
1.9.1

[PATCH 5/7] serial: 8250_fintek: Add F81216 Support

[Ji-Ze Hong (Peter Hong)]

Fintek F81216 is a LPC to 4 UARTs device. It's the F81216 series but
support less functional than F81216AD/F81216H

The following list is brief descriptions of F81216 series:

F81216H (0105)
9Bit/High baud rate(not implements with mainline)
RS485, 128Bytes FIFO (implemented)

F81216AD (0216)
9Bit(not implements with mainline)
RS485(implemented)

F81216 (0208)
basically 16550A

Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 26 --
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index 921f742..a62cfdd 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -23,6 +23,7 @@
 #define CHIP_ID2  0x21
 #define CHIP_ID_F81216AD 0x1602
 #define CHIP_ID_F81216H 0x0501
+#define CHIP_ID_F81216 0x0802
 #define VENDOR_ID1 0x23
 #define VENDOR_ID1_VAL 0x19
 #define VENDOR_ID2 0x24
@@ -107,8 +108,14 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
chip = sio_read_reg(pdata, CHIP_ID1);
chip |= sio_read_reg(pdata, CHIP_ID2) << 8;
 
-   if (chip != CHIP_ID_F81216AD && chip != CHIP_ID_F81216H)
+   switch (chip) {
+   case CHIP_ID_F81216AD:
+   case CHIP_ID_F81216H:
+   case CHIP_ID_F81216:
+   break;
+   default:
return -ENODEV;
+   }
 
pdata->pid = chip;
return 0;
@@ -235,6 +242,21 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address,
return -ENODEV;
 }
 
+static void fintek_8250_set_rs485_handler(struct uart_8250_port *uart)
+{
+   struct fintek_8250 *pdata = uart->port.private_data;
+
+   switch (pdata->pid) {
+   default: /* No RS485 Auto direction functional */
+   break;
+
+   case CHIP_ID_F81216AD:
+   case CHIP_ID_F81216H:
+   uart->port.rs485_config = fintek_8250_rs485_config;
+   break;
+   }
+}
+
 int fintek_8250_probe(struct uart_8250_port *uart)
 {
struct fintek_8250 *pdata;
@@ -248,8 +270,8 @@ int fintek_8250_probe(struct uart_8250_port *uart)
return -ENOMEM;
 
memcpy(pdata, _data, sizeof(probe_data));
-   uart->port.rs485_config = fintek_8250_rs485_config;
uart->port.private_data = pdata;
+   fintek_8250_set_rs485_handler(uart);
 
return 0;
 }
-- 
1.9.1

[PATCH 7/7] serial: 8250_fintek: Add F81865 Support

[Ji-Ze Hong (Peter Hong)]

Fintek F81865 is a LPC to 6 UARTs SuperIO. It has less functional UARTs
likes F81866. It's also need check the IRQ mode with system assigned,
but the configuration is not the same with F81216 series.

F81865 IRQ Mode setting:
0xf0
Bit1: IRQ_MODE0
Bit0: Share mode (always on)

Level/Low: IRQ_MODE0:0
Edge/High: IRQ_MODE0:1

The following list is brief descriptions of F81865:

F81865 (0704)
9Bit(not implements with mainline)
RS485(implemented)

Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index 1194e57..3fb9ab6 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -21,6 +21,7 @@
 #define EXIT_KEY 0xAA
 #define CHIP_ID1  0x20
 #define CHIP_ID2  0x21
+#define CHIP_ID_F81865 0x0407
 #define CHIP_ID_F81866 0x1010
 #define CHIP_ID_F81216AD 0x1602
 #define CHIP_ID_F81216H 0x0501
@@ -130,6 +131,7 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
chip |= sio_read_reg(pdata, CHIP_ID2) << 8;
 
switch (chip) {
+   case CHIP_ID_F81865:
case CHIP_ID_F81866:
case CHIP_ID_F81216AD:
case CHIP_ID_F81216H:
@@ -147,6 +149,7 @@ static int fintek_8250_get_ldn_range(struct fintek_8250 
*pdata, int *min,
 int *max)
 {
switch (pdata->pid) {
+   case CHIP_ID_F81865:
case CHIP_ID_F81866:
*min = F81866_LDN_LOW;
*max = F81866_LDN_HIGH;
@@ -231,9 +234,12 @@ static void fintek_8250_set_irq_mode(struct fintek_8250 
*pdata, bool is_level)
sio_write_reg(pdata, LDN, pdata->index);
 
switch (pdata->pid) {
+   case CHIP_ID_F81865:
case CHIP_ID_F81866:
-   sio_write_mask_reg(pdata, F81866_FIFO_CTRL, F81866_IRQ_MODE1,
-  0);
+   if (pdata->pid == CHIP_ID_F81866)
+   sio_write_mask_reg(pdata, F81866_FIFO_CTRL,
+  F81866_IRQ_MODE1, 0);
+
sio_write_mask_reg(pdata, F81866_IRQ_MODE, F81866_IRQ_SHARE,
   F81866_IRQ_SHARE);
sio_write_mask_reg(pdata, F81866_IRQ_MODE,
@@ -312,6 +318,7 @@ static void fintek_8250_set_rs485_handler(struct 
uart_8250_port *uart)
default: /* No RS485 Auto direction functional */
break;
 
+   case CHIP_ID_F81865:
case CHIP_ID_F81866:
case CHIP_ID_F81216AD:
case CHIP_ID_F81216H:
-- 
1.9.1

[PATCH 1/7] serial: 8250_fintek: Refactoring read/write method

[Ji-Ze Hong (Peter Hong)]

If we need to access SuperIO registers, It should write register offset
to base_addr and read/write value to base_addr + 1 to perform read/write.
We can make it more simply with write/read functions.

This patch add sio_read_reg()/sio_write_reg()/sio_write_mask_reg() to
reduce SuperIO register operation with lot of outb()/inb().

Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 73 ++-
 1 file changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index 0facc78..2ae846e 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -49,6 +49,27 @@ struct fintek_8250 {
u8 key;
 };
 
+static u8 sio_read_reg(struct fintek_8250 *pdata, u8 reg)
+{
+   outb(reg, pdata->base_port + ADDR_PORT);
+   return inb(pdata->base_port + DATA_PORT);
+}
+
+static void sio_write_reg(struct fintek_8250 *pdata, u8 reg, u8 data)
+{
+   outb(reg, pdata->base_port + ADDR_PORT);
+   outb(data, pdata->base_port + DATA_PORT);
+}
+
+static void sio_write_mask_reg(struct fintek_8250 *pdata, u8 reg, u8 mask,
+  u8 data)
+{
+   u8 tmp;
+
+   tmp = (sio_read_reg(pdata, reg) & ~mask) | (mask & data);
+   sio_write_reg(pdata, reg, tmp);
+}
+
 static int fintek_8250_enter_key(u16 base_port, u8 key)
 {
if (!request_muxed_region(base_port, 2, "8250_fintek"))
@@ -66,22 +87,18 @@ static void fintek_8250_exit_key(u16 base_port)
release_region(base_port + ADDR_PORT, 2);
 }
 
-static int fintek_8250_check_id(u16 base_port)
+static int fintek_8250_check_id(struct fintek_8250 *pdata)
 {
u16 chip;
 
-   outb(VENDOR_ID1, base_port + ADDR_PORT);
-   if (inb(base_port + DATA_PORT) != VENDOR_ID1_VAL)
+   if (sio_read_reg(pdata, VENDOR_ID1) != VENDOR_ID1_VAL)
return -ENODEV;
 
-   outb(VENDOR_ID2, base_port + ADDR_PORT);
-   if (inb(base_port + DATA_PORT) != VENDOR_ID2_VAL)
+   if (sio_read_reg(pdata, VENDOR_ID2) != VENDOR_ID2_VAL)
return -ENODEV;
 
-   outb(CHIP_ID1, base_port + ADDR_PORT);
-   chip = inb(base_port + DATA_PORT);
-   outb(CHIP_ID2, base_port + ADDR_PORT);
-   chip |= inb(base_port + DATA_PORT) << 8;
+   chip = sio_read_reg(pdata, CHIP_ID1);
+   chip |= sio_read_reg(pdata, CHIP_ID2) << 8;
 
if (chip != CHIP_ID_0 && chip != CHIP_ID_1)
return -ENODEV;
@@ -128,10 +145,8 @@ static int fintek_8250_rs485_config(struct uart_port *port,
if (fintek_8250_enter_key(pdata->base_port, pdata->key))
return -EBUSY;
 
-   outb(LDN, pdata->base_port + ADDR_PORT);
-   outb(pdata->index, pdata->base_port + DATA_PORT);
-   outb(RS485, pdata->base_port + ADDR_PORT);
-   outb(config, pdata->base_port + DATA_PORT);
+   sio_write_reg(pdata, LDN, pdata->index);
+   sio_write_reg(pdata, RS485, config);
fintek_8250_exit_key(pdata->base_port);
 
port->rs485 = *rs485;
@@ -147,10 +162,12 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address)
 
for (i = 0; i < ARRAY_SIZE(addr); i++) {
for (j = 0; j < ARRAY_SIZE(keys); j++) {
+   pdata->base_port = addr[i];
+   pdata->key = keys[j];
 
if (fintek_8250_enter_key(addr[i], keys[j]))
continue;
-   if (fintek_8250_check_id(addr[i])) {
+   if (fintek_8250_check_id(pdata)) {
fintek_8250_exit_key(addr[i]);
continue;
}
@@ -158,19 +175,13 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address)
for (k = 0; k < 4; k++) {
u16 aux;
 
-   outb(LDN, addr[i] + ADDR_PORT);
-   outb(k, addr[i] + DATA_PORT);
-
-   outb(IO_ADDR1, addr[i] + ADDR_PORT);
-   aux = inb(addr[i] + DATA_PORT);
-   outb(IO_ADDR2, addr[i] + ADDR_PORT);
-   aux |= inb(addr[i] + DATA_PORT) << 8;
+   sio_write_reg(pdata, LDN, k);
+   aux = sio_read_reg(pdata, IO_ADDR1);
+   aux |= sio_read_reg(pdata, IO_ADDR2) << 8;
if (aux != io_address)
continue;
 
fintek_8250_exit_key(addr[i]);
-   pdata->key = keys[j];
-   pdata->base_port = addr[i];
pdata->index = k;
 
return 0;
@@ -186,24 +197,16 @@ static int find_base_port(struct fintek_8250 *pdata, 

[PATCH 2/7] serial: 8250_fintek: Set IRQ Mode when port probed

[Ji-Ze Hong (Peter Hong)]

Set IRQ Mode when port probed in find_base_port()

It should hold the IO port premission via fintek_8250_enter_key() and
release via fintek_8250_exit_key() when we configure the SuperIO.

This patch will move all SuperIO configure operations to find_base_port()
to reduce fintek_8250_enter_key()/fintek_8250_exit_key() usage.

Suggested-by: Ricardo Ribalda Delgado 
Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 36 ++-
 1 file changed, 19 insertions(+), 17 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index 2ae846e..cd8903f 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -49,6 +49,9 @@ struct fintek_8250 {
u8 key;
 };
 
+static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata,
+bool level_mode);
+
 static u8 sio_read_reg(struct fintek_8250 *pdata, u8 reg)
 {
outb(reg, pdata->base_port + ADDR_PORT);
@@ -154,10 +157,13 @@ static int fintek_8250_rs485_config(struct uart_port 
*port,
return 0;
 }
 
-static int find_base_port(struct fintek_8250 *pdata, u16 io_address)
+static int find_base_port(struct fintek_8250 *pdata, u16 io_address,
+ unsigned int irq)
 {
static const u16 addr[] = {0x4e, 0x2e};
static const u8 keys[] = {0x77, 0xa0, 0x87, 0x67};
+   struct irq_data *irq_data;
+   bool level_mode = false;
int i, j, k;
 
for (i = 0; i < ARRAY_SIZE(addr); i++) {
@@ -181,9 +187,16 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address)
if (aux != io_address)
continue;
 
-   fintek_8250_exit_key(addr[i]);
pdata->index = k;
 
+   irq_data = irq_get_irq_data(irq);
+   if (irq_data)
+   level_mode =
+   irqd_is_level_type(irq_data);
+
+   fintek_8250_set_irq_mode(pdata, level_mode);
+   fintek_8250_exit_key(addr[i]);
+
return 0;
}
 
@@ -194,31 +207,20 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address)
return -ENODEV;
 }
 
-static int fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool level_mode)
+static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool is_level)
 {
-   int status;
-
-   status = fintek_8250_enter_key(pdata->base_port, pdata->key);
-   if (status)
-   return status;
-
sio_write_reg(pdata, LDN, pdata->index);
sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_SHARE, IRQ_SHARE);
sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_MODE_MASK,
-  level_mode ? IRQ_LEVEL_LOW : IRQ_EDGE_HIGH);
-
-   fintek_8250_exit_key(pdata->base_port);
-   return 0;
+  is_level ? IRQ_LEVEL_LOW : IRQ_EDGE_HIGH);
 }
 
 int fintek_8250_probe(struct uart_8250_port *uart)
 {
struct fintek_8250 *pdata;
struct fintek_8250 probe_data;
-   struct irq_data *irq_data = irq_get_irq_data(uart->port.irq);
-   bool level_mode = irqd_is_level_type(irq_data);
 
-   if (find_base_port(_data, uart->port.iobase))
+   if (find_base_port(_data, uart->port.iobase, uart->port.irq))
return -ENODEV;
 
pdata = devm_kzalloc(uart->port.dev, sizeof(*pdata), GFP_KERNEL);
@@ -229,5 +231,5 @@ int fintek_8250_probe(struct uart_8250_port *uart)
uart->port.rs485_config = fintek_8250_rs485_config;
uart->port.private_data = pdata;
 
-   return fintek_8250_set_irq_mode(pdata, level_mode);
+   return 0;
 }
-- 
1.9.1

[PATCH 4/7] serial: 8250_fintek: Rearrange function

[Ji-Ze Hong (Peter Hong)]

We change the position of fintek_8250_set_irq_mode() above the
find_base_port() to eliminate the prototype define.

Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 19 ---
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index 5625203..921f742 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -56,9 +56,6 @@ struct fintek_8250 {
u8 key;
 };
 
-static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata,
-bool level_mode);
-
 static u8 sio_read_reg(struct fintek_8250 *pdata, u8 reg)
 {
outb(reg, pdata->base_port + ADDR_PORT);
@@ -179,6 +176,14 @@ static void fintek_8250_set_max_fifo(struct fintek_8250 
*pdata)
}
 }
 
+static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool is_level)
+{
+   sio_write_reg(pdata, LDN, pdata->index);
+   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_SHARE, IRQ_SHARE);
+   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_MODE_MASK,
+  is_level ? IRQ_LEVEL_LOW : IRQ_EDGE_HIGH);
+}
+
 static int find_base_port(struct fintek_8250 *pdata, u16 io_address,
  unsigned int irq)
 {
@@ -230,14 +235,6 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address,
return -ENODEV;
 }
 
-static void fintek_8250_set_irq_mode(struct fintek_8250 *pdata, bool is_level)
-{
-   sio_write_reg(pdata, LDN, pdata->index);
-   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_SHARE, IRQ_SHARE);
-   sio_write_mask_reg(pdata, FINTEK_IRQ_MODE, IRQ_MODE_MASK,
-  is_level ? IRQ_LEVEL_LOW : IRQ_EDGE_HIGH);
-}
-
 int fintek_8250_probe(struct uart_8250_port *uart)
 {
struct fintek_8250 *pdata;
-- 
1.9.1

[PATCH 5/7] serial: 8250_fintek: Add F81216 Support

[Ji-Ze Hong (Peter Hong)]

Fintek F81216 is a LPC to 4 UARTs device. It's the F81216 series but
support less functional than F81216AD/F81216H

The following list is brief descriptions of F81216 series:

F81216H (0105)
9Bit/High baud rate(not implements with mainline)
RS485, 128Bytes FIFO (implemented)

F81216AD (0216)
9Bit(not implements with mainline)
RS485(implemented)

F81216 (0208)
basically 16550A

Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 26 --
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index 921f742..a62cfdd 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -23,6 +23,7 @@
 #define CHIP_ID2  0x21
 #define CHIP_ID_F81216AD 0x1602
 #define CHIP_ID_F81216H 0x0501
+#define CHIP_ID_F81216 0x0802
 #define VENDOR_ID1 0x23
 #define VENDOR_ID1_VAL 0x19
 #define VENDOR_ID2 0x24
@@ -107,8 +108,14 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
chip = sio_read_reg(pdata, CHIP_ID1);
chip |= sio_read_reg(pdata, CHIP_ID2) << 8;
 
-   if (chip != CHIP_ID_F81216AD && chip != CHIP_ID_F81216H)
+   switch (chip) {
+   case CHIP_ID_F81216AD:
+   case CHIP_ID_F81216H:
+   case CHIP_ID_F81216:
+   break;
+   default:
return -ENODEV;
+   }
 
pdata->pid = chip;
return 0;
@@ -235,6 +242,21 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address,
return -ENODEV;
 }
 
+static void fintek_8250_set_rs485_handler(struct uart_8250_port *uart)
+{
+   struct fintek_8250 *pdata = uart->port.private_data;
+
+   switch (pdata->pid) {
+   default: /* No RS485 Auto direction functional */
+   break;
+
+   case CHIP_ID_F81216AD:
+   case CHIP_ID_F81216H:
+   uart->port.rs485_config = fintek_8250_rs485_config;
+   break;
+   }
+}
+
 int fintek_8250_probe(struct uart_8250_port *uart)
 {
struct fintek_8250 *pdata;
@@ -248,8 +270,8 @@ int fintek_8250_probe(struct uart_8250_port *uart)
return -ENOMEM;
 
memcpy(pdata, _data, sizeof(probe_data));
-   uart->port.rs485_config = fintek_8250_rs485_config;
uart->port.private_data = pdata;
+   fintek_8250_set_rs485_handler(uart);
 
return 0;
 }
-- 
1.9.1

Re: [v10,2/2] PCI: Rockchip: Add Rockchip PCIe controller support

[Shawn Lin]

Hi Guenter,

Thanks for your review, and I think it still not too late
for nitpicking as it isn't merged to next branch. :)

We have amend the code a bit, so probably we fixed some of
the minor issues against V10. But some of them are really
personal taste, if you still insist on that, I will be ok
to modify them.

I will push patch to fix them after your feedback. :)


On 2016/9/1 2:17, Guenter Roeck wrote:

On Fri, Aug 19, 2016 at 09:34:58AM +0800, Shawn Lin wrote:


[...]


+   rockchip_pcie_enable_interrupts(port);
+


There is no matching disable_interrupts call in error handling after this.
Is that ok ?



From test, probably ok since the genpd will be off and all of them wil 
be cleared.



Also, is it ok to enable interrupts this early (before the rest of the
initialization is complete) ?



The training starts, so we some client irq should be handled now, and we
already register isr.


+   err = rockchip_pcie_init_irq_domain(port);
+   if (err < 0)
+   goto err_vpcie;
+
+   err = of_pci_get_host_bridge_resources(dev->of_node, 0, 0xff,
+  , _base);
+   if (err)
+   goto err_vpcie;
+
+   err = devm_request_pci_bus_resources(dev, );
+   if (err)
+   goto err_vpcie;
+
+   /* Get the I/O and memory ranges from DT */
+   resource_list_for_each_entry(win, ) {
+   switch (resource_type(win->res)) {
+   case IORESOURCE_IO:
+   io = win->res;
+   io->name = "I/O";
+   io_size = resource_size(io);
+   io_bus_addr = io->start - win->offset;
+   err = pci_remap_iospace(io, io_base);
+   if (err) {
+   dev_warn(port->dev, "error %d: failed to map 
resource %pR\n",
+err, io);
+   continue;
+   }
+   break;
+   case IORESOURCE_MEM:
+   mem = win->res;
+   mem->name = "MEM";
+   mem_size = resource_size(mem);
+   mem_bus_addr = mem->start - win->offset;
+   break;
+   case IORESOURCE_BUS:
+   busn = win->res;
+   break;
+   default:
+   continue;
+   }
+   }
+
+   if (mem_size)


While strictly speaking not needed, I would recommend { }
here to improve readability.


+   for (reg_no = 0; reg_no < (mem_size >> 20); reg_no++) {


This doesn't support mem sizes smaller than 1 << 20 (and clips the size
if it is not aligned to 1M). Is this intentional ?


yes, we don't support.




+   err = rockchip_pcie_prog_ob_atu(port, reg_no + 1,
+   AXI_WRAPPER_MEM_WRITE,
+   20 - 1,
+   mem_bus_addr +
+   (reg_no << 20),
+   0);
+   if (err) {
+   dev_err(dev, "program RC mem outbound ATU 
failed\n");
+   goto err_vpcie;
+   }
+   }





+   err = rockchip_pcie_prog_ob_atu(port,
+   reg_no + 1 + offset,
+   AXI_WRAPPER_IO_WRITE,
+   20 - 1,
+   io_bus_addr +
+   (reg_no << 20),
+   0);
+   if (err) {
+   dev_err(dev, "program RC io outbound ATU 
failed\n");
+   goto err_vpcie;
+   }
+   }



+
+   pci_bus_add_devices(bus);
+
+   dev_warn(dev, "only 32-bit config accesses supported; smaller writes may 
corrupt adjacent RW1C fields\n");
+


A warning which is always displayed seems to be a bit useless. If this is a
concern, maybe the driver should provide its own config space access functions
and map smaller accesses into 32 bit accesses. But isn't that already done ?



No, that is just for normal code path. When users comfigure it via some
user-space tool, it is sane to make them know this limitation. So we was 
suggested to do this.



+   return err;
+
+err_vpcie:
+   if (!IS_ERR(port->vpcie3v3))
+   regulator_disable(port->vpcie3v3);
+   if (!IS_ERR(port->vpcie1v8))
+   regulator_disable(port->vpcie1v8);
+   if (!IS_ERR(port->vpcie0v9))
+   

[PATCH 3/7] serial: 8250_fintek: Set maximum FIFO of F81216H

[Ji-Ze Hong (Peter Hong)]

The Fintek F81216H had maximum 128Bytes FIFO, but some BIOS configurated
as normal 16Bytes FIFO. This patch will set 128Bytes FIFO and trigger
level multiplier as 4x when F81216H detected.

Default 16550A trigger level is 8Bytes. When this patch applied, the
trigger level will change to 8Byte x 4 = 32Byte. It can be reduce the RX
incoming interrupts.

Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 29 ++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index cd8903f..5625203 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -21,8 +21,8 @@
 #define EXIT_KEY 0xAA
 #define CHIP_ID1  0x20
 #define CHIP_ID2  0x21
-#define CHIP_ID_0 0x1602
-#define CHIP_ID_1 0x0501
+#define CHIP_ID_F81216AD 0x1602
+#define CHIP_ID_F81216H 0x0501
 #define VENDOR_ID1 0x23
 #define VENDOR_ID1_VAL 0x19
 #define VENDOR_ID2 0x24
@@ -43,7 +43,14 @@
 #define RXW4C_IRA BIT(3)
 #define TXW4C_IRA BIT(2)
 
+#define FIFO_CTRL  0xF6
+#define FIFO_MODE_MASK (BIT(1) | BIT(0))
+#define FIFO_MODE_128  (BIT(1) | BIT(0))
+#define RXFTHR_MODE_MASK   (BIT(5) | BIT(4))
+#define RXFTHR_MODE_4X BIT(5)
+
 struct fintek_8250 {
+   u16 pid;
u16 base_port;
u8 index;
u8 key;
@@ -103,9 +110,10 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
chip = sio_read_reg(pdata, CHIP_ID1);
chip |= sio_read_reg(pdata, CHIP_ID2) << 8;
 
-   if (chip != CHIP_ID_0 && chip != CHIP_ID_1)
+   if (chip != CHIP_ID_F81216AD && chip != CHIP_ID_F81216H)
return -ENODEV;
 
+   pdata->pid = chip;
return 0;
 }
 
@@ -157,6 +165,20 @@ static int fintek_8250_rs485_config(struct uart_port *port,
return 0;
 }
 
+static void fintek_8250_set_max_fifo(struct fintek_8250 *pdata)
+{
+   switch (pdata->pid) {
+   default: /* Default 16Bytes FIFO */
+   return;
+
+   case CHIP_ID_F81216H: /* 128Bytes FIFO */
+   sio_write_mask_reg(pdata, FIFO_CTRL,
+  FIFO_MODE_MASK | RXFTHR_MODE_MASK,
+  FIFO_MODE_128 | RXFTHR_MODE_4X);
+   break;
+   }
+}
+
 static int find_base_port(struct fintek_8250 *pdata, u16 io_address,
  unsigned int irq)
 {
@@ -195,6 +217,7 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address,
irqd_is_level_type(irq_data);
 
fintek_8250_set_irq_mode(pdata, level_mode);
+   fintek_8250_set_max_fifo(pdata);
fintek_8250_exit_key(addr[i]);
 
return 0;
-- 
1.9.1

[PATCH 0/7] serial: 8250_fintek: Fix the IRQ mode and code refactoring

[Ji-Ze Hong (Peter Hong)]

The following patches will fix the Fintek LPC to UARTs IRQ mode mismatch
issue, code refactoring and this series patches should follow by patch
rename IRQ_MODE macro on
'commit 87a713c8ffca ("8250/fintek: rename IRQ_MODE macro")'.
with tty-linus branch.

Some BIOS only use _OSI("Linux") to distinguish between Linux & Windows.
Apply Level/Low to UART trigger mode if Windows, Edge/High otherwise.
But since 2.6.23 the mainline kernel no longer returns true for
_OSI(“Linux”). The BIOS ASL should avoid to use _OSI() to check system
type and use ACPI MADT override IRQ mode instead.

We'll try to refactoring the source code more readable with SuperIO
read/write register functions.

Ricardo Ribalda Delgado suggest fixing a potential NULL pointer
dereference with applied patch
fix the mismatched IRQ mode on
'commit 4da22f1418cb ("serial: 8250_fintek: fix the mismatched IRQ mode")'.
had been implemented with 8250_fintek: Set IRQ Mode when port probed.

Ji-Ze Hong (Peter Hong) (7):
  serial: 8250_fintek: Refactoring read/write method
  serial: 8250_fintek: Set IRQ Mode when port probed
  serial: 8250_fintek: Set maximum FIFO of F81216H
  serial: 8250_fintek: Rearrange function
  serial: 8250_fintek: Add F81216 Support
  serial: 8250_fintek: Add F81866 Support
  serial: 8250_fintek: Add F81865 Support

 drivers/tty/serial/8250/8250_fintek.c | 233 +-
 1 file changed, 175 insertions(+), 58 deletions(-)

-- 
1.9.1

Re: [v10,2/2] PCI: Rockchip: Add Rockchip PCIe controller support

[Shawn Lin]

Hi Guenter,

Thanks for your review, and I think it still not too late
for nitpicking as it isn't merged to next branch. :)

We have amend the code a bit, so probably we fixed some of
the minor issues against V10. But some of them are really
personal taste, if you still insist on that, I will be ok
to modify them.

I will push patch to fix them after your feedback. :)


On 2016/9/1 2:17, Guenter Roeck wrote:

On Fri, Aug 19, 2016 at 09:34:58AM +0800, Shawn Lin wrote:


[...]


+   rockchip_pcie_enable_interrupts(port);
+


There is no matching disable_interrupts call in error handling after this.
Is that ok ?



From test, probably ok since the genpd will be off and all of them wil 
be cleared.



Also, is it ok to enable interrupts this early (before the rest of the
initialization is complete) ?



The training starts, so we some client irq should be handled now, and we
already register isr.


+   err = rockchip_pcie_init_irq_domain(port);
+   if (err < 0)
+   goto err_vpcie;
+
+   err = of_pci_get_host_bridge_resources(dev->of_node, 0, 0xff,
+  , _base);
+   if (err)
+   goto err_vpcie;
+
+   err = devm_request_pci_bus_resources(dev, );
+   if (err)
+   goto err_vpcie;
+
+   /* Get the I/O and memory ranges from DT */
+   resource_list_for_each_entry(win, ) {
+   switch (resource_type(win->res)) {
+   case IORESOURCE_IO:
+   io = win->res;
+   io->name = "I/O";
+   io_size = resource_size(io);
+   io_bus_addr = io->start - win->offset;
+   err = pci_remap_iospace(io, io_base);
+   if (err) {
+   dev_warn(port->dev, "error %d: failed to map 
resource %pR\n",
+err, io);
+   continue;
+   }
+   break;
+   case IORESOURCE_MEM:
+   mem = win->res;
+   mem->name = "MEM";
+   mem_size = resource_size(mem);
+   mem_bus_addr = mem->start - win->offset;
+   break;
+   case IORESOURCE_BUS:
+   busn = win->res;
+   break;
+   default:
+   continue;
+   }
+   }
+
+   if (mem_size)


While strictly speaking not needed, I would recommend { }
here to improve readability.


+   for (reg_no = 0; reg_no < (mem_size >> 20); reg_no++) {


This doesn't support mem sizes smaller than 1 << 20 (and clips the size
if it is not aligned to 1M). Is this intentional ?


yes, we don't support.




+   err = rockchip_pcie_prog_ob_atu(port, reg_no + 1,
+   AXI_WRAPPER_MEM_WRITE,
+   20 - 1,
+   mem_bus_addr +
+   (reg_no << 20),
+   0);
+   if (err) {
+   dev_err(dev, "program RC mem outbound ATU 
failed\n");
+   goto err_vpcie;
+   }
+   }





+   err = rockchip_pcie_prog_ob_atu(port,
+   reg_no + 1 + offset,
+   AXI_WRAPPER_IO_WRITE,
+   20 - 1,
+   io_bus_addr +
+   (reg_no << 20),
+   0);
+   if (err) {
+   dev_err(dev, "program RC io outbound ATU 
failed\n");
+   goto err_vpcie;
+   }
+   }



+
+   pci_bus_add_devices(bus);
+
+   dev_warn(dev, "only 32-bit config accesses supported; smaller writes may 
corrupt adjacent RW1C fields\n");
+


A warning which is always displayed seems to be a bit useless. If this is a
concern, maybe the driver should provide its own config space access functions
and map smaller accesses into 32 bit accesses. But isn't that already done ?



No, that is just for normal code path. When users comfigure it via some
user-space tool, it is sane to make them know this limitation. So we was 
suggested to do this.



+   return err;
+
+err_vpcie:
+   if (!IS_ERR(port->vpcie3v3))
+   regulator_disable(port->vpcie3v3);
+   if (!IS_ERR(port->vpcie1v8))
+   regulator_disable(port->vpcie1v8);
+   if (!IS_ERR(port->vpcie0v9))
+   

[PATCH 3/7] serial: 8250_fintek: Set maximum FIFO of F81216H

[Ji-Ze Hong (Peter Hong)]

The Fintek F81216H had maximum 128Bytes FIFO, but some BIOS configurated
as normal 16Bytes FIFO. This patch will set 128Bytes FIFO and trigger
level multiplier as 4x when F81216H detected.

Default 16550A trigger level is 8Bytes. When this patch applied, the
trigger level will change to 8Byte x 4 = 32Byte. It can be reduce the RX
incoming interrupts.

Signed-off-by: Ji-Ze Hong (Peter Hong) 
---
 drivers/tty/serial/8250/8250_fintek.c | 29 ++---
 1 file changed, 26 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_fintek.c 
b/drivers/tty/serial/8250/8250_fintek.c
index cd8903f..5625203 100644
--- a/drivers/tty/serial/8250/8250_fintek.c
+++ b/drivers/tty/serial/8250/8250_fintek.c
@@ -21,8 +21,8 @@
 #define EXIT_KEY 0xAA
 #define CHIP_ID1  0x20
 #define CHIP_ID2  0x21
-#define CHIP_ID_0 0x1602
-#define CHIP_ID_1 0x0501
+#define CHIP_ID_F81216AD 0x1602
+#define CHIP_ID_F81216H 0x0501
 #define VENDOR_ID1 0x23
 #define VENDOR_ID1_VAL 0x19
 #define VENDOR_ID2 0x24
@@ -43,7 +43,14 @@
 #define RXW4C_IRA BIT(3)
 #define TXW4C_IRA BIT(2)
 
+#define FIFO_CTRL  0xF6
+#define FIFO_MODE_MASK (BIT(1) | BIT(0))
+#define FIFO_MODE_128  (BIT(1) | BIT(0))
+#define RXFTHR_MODE_MASK   (BIT(5) | BIT(4))
+#define RXFTHR_MODE_4X BIT(5)
+
 struct fintek_8250 {
+   u16 pid;
u16 base_port;
u8 index;
u8 key;
@@ -103,9 +110,10 @@ static int fintek_8250_check_id(struct fintek_8250 *pdata)
chip = sio_read_reg(pdata, CHIP_ID1);
chip |= sio_read_reg(pdata, CHIP_ID2) << 8;
 
-   if (chip != CHIP_ID_0 && chip != CHIP_ID_1)
+   if (chip != CHIP_ID_F81216AD && chip != CHIP_ID_F81216H)
return -ENODEV;
 
+   pdata->pid = chip;
return 0;
 }
 
@@ -157,6 +165,20 @@ static int fintek_8250_rs485_config(struct uart_port *port,
return 0;
 }
 
+static void fintek_8250_set_max_fifo(struct fintek_8250 *pdata)
+{
+   switch (pdata->pid) {
+   default: /* Default 16Bytes FIFO */
+   return;
+
+   case CHIP_ID_F81216H: /* 128Bytes FIFO */
+   sio_write_mask_reg(pdata, FIFO_CTRL,
+  FIFO_MODE_MASK | RXFTHR_MODE_MASK,
+  FIFO_MODE_128 | RXFTHR_MODE_4X);
+   break;
+   }
+}
+
 static int find_base_port(struct fintek_8250 *pdata, u16 io_address,
  unsigned int irq)
 {
@@ -195,6 +217,7 @@ static int find_base_port(struct fintek_8250 *pdata, u16 
io_address,
irqd_is_level_type(irq_data);
 
fintek_8250_set_irq_mode(pdata, level_mode);
+   fintek_8250_set_max_fifo(pdata);
fintek_8250_exit_key(addr[i]);
 
return 0;
-- 
1.9.1

[PATCH 0/7] serial: 8250_fintek: Fix the IRQ mode and code refactoring

[Ji-Ze Hong (Peter Hong)]

The following patches will fix the Fintek LPC to UARTs IRQ mode mismatch
issue, code refactoring and this series patches should follow by patch
rename IRQ_MODE macro on
'commit 87a713c8ffca ("8250/fintek: rename IRQ_MODE macro")'.
with tty-linus branch.

Some BIOS only use _OSI("Linux") to distinguish between Linux & Windows.
Apply Level/Low to UART trigger mode if Windows, Edge/High otherwise.
But since 2.6.23 the mainline kernel no longer returns true for
_OSI(“Linux”). The BIOS ASL should avoid to use _OSI() to check system
type and use ACPI MADT override IRQ mode instead.

We'll try to refactoring the source code more readable with SuperIO
read/write register functions.

Ricardo Ribalda Delgado suggest fixing a potential NULL pointer
dereference with applied patch
fix the mismatched IRQ mode on
'commit 4da22f1418cb ("serial: 8250_fintek: fix the mismatched IRQ mode")'.
had been implemented with 8250_fintek: Set IRQ Mode when port probed.

Ji-Ze Hong (Peter Hong) (7):
  serial: 8250_fintek: Refactoring read/write method
  serial: 8250_fintek: Set IRQ Mode when port probed
  serial: 8250_fintek: Set maximum FIFO of F81216H
  serial: 8250_fintek: Rearrange function
  serial: 8250_fintek: Add F81216 Support
  serial: 8250_fintek: Add F81866 Support
  serial: 8250_fintek: Add F81865 Support

 drivers/tty/serial/8250/8250_fintek.c | 233 +-
 1 file changed, 175 insertions(+), 58 deletions(-)

-- 
1.9.1

Re: [PATCH 2/4] cpu: expose pm_qos_resume_latency for each cpu

[Alex Shi]

>> @@ -376,6 +377,8 @@ int register_cpu(struct cpu *cpu, int num)
>>  
>>  per_cpu(cpu_sys_devices, num) = >dev;
>>  register_cpu_under_node(num, cpu_to_node(num));
>> +if (dev_pm_qos_expose_latency_limit(>dev, 0))
>> +pr_debug("CPU%d: add resume latency failed\n", num);
> 
> Why is this a debug message?  And you have a struct device, why not use
> it?
> 

Looks no one care about the dev_pm_qos_expose_latency_limit's return
value, so this debug message like gilding the lily. will remove it.

> Also, what userspace changes does this require, or affect?  Any new
> sysfs files?

User can set values on each of cpu, like limit 100ms on cpu0, that means
the cpu0 response time should be in 100ms in possible idle. It similar
with DMA_LATENCY, but that request is for all cpu. This is just for
particular cpu, like a interrupt pined CPU.

echo 100 > /sys/devices/system/cpu/cpu0/power/pm_qos_resume_latency_us
> 
> thanks,
> 
> greg k-h
> 

Re: [PATCH 2/4] cpu: expose pm_qos_resume_latency for each cpu

[Alex Shi]

>> @@ -376,6 +377,8 @@ int register_cpu(struct cpu *cpu, int num)
>>  
>>  per_cpu(cpu_sys_devices, num) = >dev;
>>  register_cpu_under_node(num, cpu_to_node(num));
>> +if (dev_pm_qos_expose_latency_limit(>dev, 0))
>> +pr_debug("CPU%d: add resume latency failed\n", num);
> 
> Why is this a debug message?  And you have a struct device, why not use
> it?
> 

Looks no one care about the dev_pm_qos_expose_latency_limit's return
value, so this debug message like gilding the lily. will remove it.

> Also, what userspace changes does this require, or affect?  Any new
> sysfs files?

User can set values on each of cpu, like limit 100ms on cpu0, that means
the cpu0 response time should be in 100ms in possible idle. It similar
with DMA_LATENCY, but that request is for all cpu. This is just for
particular cpu, like a interrupt pined CPU.

echo 100 > /sys/devices/system/cpu/cpu0/power/pm_qos_resume_latency_us
> 
> thanks,
> 
> greg k-h
> 

Re: [RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

在 2016/9/1 6:56, Rafael J. Wysocki 写道:

On Wednesday, August 31, 2016 07:48:14 PM Dongdong Liu wrote:

Add specific quirks for PCI config space accessors.This involves:
1. New initialization call hisi_pcie_acpi_init() to get RC config resource
with hardcoded range address and setup ecam mapping.
2. New entry in common quirk array.

Signed-off-by: Dongdong Liu 
Signed-off-by: Gabriele Paoloni 


Well, what exactly is the ACPI support you're adding?  Is it the ECAM part only
or is there anything more to it?



Hi Rafael, thanks for replying.

Our host bridge is non ECAM only for the RC bus config space;
for any other bus underneath the root bus we support ECAM access.

In our case we cannot use the standard MCFG object to pass the RC itself config 
space addresses.
The more discuss information can be found:
https://lkml.org/lkml/2016/2/22/1087
[...]
I have looked into this and in our case we cannot use the
standard MCFG object to pass the RC config space addresses.

The reason is that in our HW we have the config base addresses of the
root complex ports that are less than 0x10 byte distant one from
the other as we only map the first 0x1 bytes.
Now the MCFG acpi framework always fix the MCFG resource size to 0x10
for each bus; therefore if we pass our RC addresses through MCFG we end
up with a resource conflict.
To give you a practical example we are in a situation where we have:

port0: [0xb008 - 0xb008 + 0x1]
port1: [0xb009 - 0xb009 + 0x1]
port2: [0xb00A - 0xb00A + 0x1]
port3: [0xb00B - 0xb00B + 0x1]
So if we pass the base addresses through MCFG the resources
will overlap as MCFG will consider 0x10 size for each base
address of the root complex (only the RC bus uses that address)
So far I do not see many option other than using _DSD to pass
these RC config base addresses.

Thanks and Regards

Gab
[...]

and

https://patchwork.kernel.org/patch/9178791/
[...]
Furthermore, I suspect we do not even need a way to pass the
non-ECAM compliant config space resources to the OS (ie we can't
change FW anymore anyway in some platforms) so the quirks hooks
are likely to hardcode the required config space addresses for
the respective MCFG match.

..
Thanks !
Lorenzo
[...]

So we hard code with our RC itself config resource.

Thanks
Dongdong


Thanks,
Rafael


.

Re: [RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

在 2016/9/1 6:56, Rafael J. Wysocki 写道:

On Wednesday, August 31, 2016 07:48:14 PM Dongdong Liu wrote:

Add specific quirks for PCI config space accessors.This involves:
1. New initialization call hisi_pcie_acpi_init() to get RC config resource
with hardcoded range address and setup ecam mapping.
2. New entry in common quirk array.

Signed-off-by: Dongdong Liu 
Signed-off-by: Gabriele Paoloni 


Well, what exactly is the ACPI support you're adding?  Is it the ECAM part only
or is there anything more to it?



Hi Rafael, thanks for replying.

Our host bridge is non ECAM only for the RC bus config space;
for any other bus underneath the root bus we support ECAM access.

In our case we cannot use the standard MCFG object to pass the RC itself config 
space addresses.
The more discuss information can be found:
https://lkml.org/lkml/2016/2/22/1087
[...]
I have looked into this and in our case we cannot use the
standard MCFG object to pass the RC config space addresses.

The reason is that in our HW we have the config base addresses of the
root complex ports that are less than 0x10 byte distant one from
the other as we only map the first 0x1 bytes.
Now the MCFG acpi framework always fix the MCFG resource size to 0x10
for each bus; therefore if we pass our RC addresses through MCFG we end
up with a resource conflict.
To give you a practical example we are in a situation where we have:

port0: [0xb008 - 0xb008 + 0x1]
port1: [0xb009 - 0xb009 + 0x1]
port2: [0xb00A - 0xb00A + 0x1]
port3: [0xb00B - 0xb00B + 0x1]
So if we pass the base addresses through MCFG the resources
will overlap as MCFG will consider 0x10 size for each base
address of the root complex (only the RC bus uses that address)
So far I do not see many option other than using _DSD to pass
these RC config base addresses.

Thanks and Regards

Gab
[...]

and

https://patchwork.kernel.org/patch/9178791/
[...]
Furthermore, I suspect we do not even need a way to pass the
non-ECAM compliant config space resources to the OS (ie we can't
change FW anymore anyway in some platforms) so the quirks hooks
are likely to hardcode the required config space addresses for
the respective MCFG match.

..
Thanks !
Lorenzo
[...]

So we hard code with our RC itself config resource.

Thanks
Dongdong


Thanks,
Rafael


.

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Shawn Lin]

在 2016/9/1 10:29, Ziyuan Xu 写道:

Hi,


On 2016年09月01日 01:42, Doug Anderson wrote:

Hi,

On Sun, Aug 28, 2016 at 8:25 PM, Shawn Lin 
wrote:

On 2016/8/29 10:50, Elaine Zhang wrote:



On 08/27/2016 11:05 PM, Shawn Lin wrote:

On 2016/8/27 21:41, Ziyuan Xu wrote:

Control power domain for eMMC via genpd to reduce power consumption.

Signed-off-by: Elaine Zhang 
Signed-off-by: Ziyuan Xu 


It looks nice to me. But this should be merged after applying that[0]
as your patch will break bind/unbind test for sdhci-of-arasan on
rk3399
without it[0]. Moreover, Elaine should make sure that upstreamed
rockchip power domain stuff would not off pd for emmc, *otherwise*, I
should update my patch to make sure we update clkmul every time when
doing suspend 2 resume..



Forgot to say:
If use pd, Although there is no call to power odd the pd_emmc,
it will be power off when the system doing suspend 2 resume.
(Because the system call
__device_suspend_noirq->pm_genpd_suspend_noirq->rockchip_pd_power_off)


Thanks for explaining this. I checked the code a bit and actually I
don't need to updata clkmul since it was recorded, although it is still
reset to 0x10 reading from syscon. So for that, we can now pick it
up without waiting for my sdhci-of-arasan's update.

Reviewed-by: Shawn Lin 

This is fine to pick up _only_ if you don't care about suspend/resume.
If you care about suspend/resume then someone needs to first write a
patch that will re-init all "corecfg" values after power is turned on.


Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we
don't need to strore/re-init it after resume.
corecfg_clockmultiplier is only used to fetch host->clk_mul, and
host->clk_mul has been a fixed value at run-time, unless driver unbind.
The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to
check the xin_clk at probe time, we don't reference it at run-time.


correct. That is why we don't need to update it even we power off pd.


BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC
works fine.



Technically I think this should probably use "pm runtime" and not
normal suspend/resume hooks.  Any time we end up pm runtime suspended
then I think our power will go off (because of genpd?) and we need to
restore values.


I understand your consideration. BUT genpd is in charge of on/off pd if
the corresponding device node has "power-domains" property. RPM is
unnecessary for this situation, we will not use autosuspend, right?


That is irrelevant to rpm as what we need is just to control genpd here.
If we need to support rmp, we need some extra patches to do it,
including genpd off, clk-disable, etc...



@shawn, what's your opinion?



I'm not sure if this should be done in a generic way where we try to
save and restore all values in the "sdhci_arasan_soc_ctl_map" or if we
should try to be smarter...


I am prone to keep it as-is, unless we see a stronge requirement for
coming users who argue that they have some extra corecfg_* need to
recovery due to whatever reason.:)

If we see something like this:

if(of_device_is_compatible(A))
update X
else if (of_device_is_compatible(B))
update Y
.

Then we could consider trying to save & restrore all the values.





-Doug

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip











--
Best Regards
Shawn Lin

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Shawn Lin]

在 2016/9/1 10:29, Ziyuan Xu 写道:

Hi,


On 2016年09月01日 01:42, Doug Anderson wrote:

Hi,

On Sun, Aug 28, 2016 at 8:25 PM, Shawn Lin 
wrote:

On 2016/8/29 10:50, Elaine Zhang wrote:



On 08/27/2016 11:05 PM, Shawn Lin wrote:

On 2016/8/27 21:41, Ziyuan Xu wrote:

Control power domain for eMMC via genpd to reduce power consumption.

Signed-off-by: Elaine Zhang 
Signed-off-by: Ziyuan Xu 


It looks nice to me. But this should be merged after applying that[0]
as your patch will break bind/unbind test for sdhci-of-arasan on
rk3399
without it[0]. Moreover, Elaine should make sure that upstreamed
rockchip power domain stuff would not off pd for emmc, *otherwise*, I
should update my patch to make sure we update clkmul every time when
doing suspend 2 resume..



Forgot to say:
If use pd, Although there is no call to power odd the pd_emmc,
it will be power off when the system doing suspend 2 resume.
(Because the system call
__device_suspend_noirq->pm_genpd_suspend_noirq->rockchip_pd_power_off)


Thanks for explaining this. I checked the code a bit and actually I
don't need to updata clkmul since it was recorded, although it is still
reset to 0x10 reading from syscon. So for that, we can now pick it
up without waiting for my sdhci-of-arasan's update.

Reviewed-by: Shawn Lin 

This is fine to pick up _only_ if you don't care about suspend/resume.
If you care about suspend/resume then someone needs to first write a
patch that will re-init all "corecfg" values after power is turned on.


Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we
don't need to strore/re-init it after resume.
corecfg_clockmultiplier is only used to fetch host->clk_mul, and
host->clk_mul has been a fixed value at run-time, unless driver unbind.
The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to
check the xin_clk at probe time, we don't reference it at run-time.


correct. That is why we don't need to update it even we power off pd.


BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC
works fine.



Technically I think this should probably use "pm runtime" and not
normal suspend/resume hooks.  Any time we end up pm runtime suspended
then I think our power will go off (because of genpd?) and we need to
restore values.


I understand your consideration. BUT genpd is in charge of on/off pd if
the corresponding device node has "power-domains" property. RPM is
unnecessary for this situation, we will not use autosuspend, right?


That is irrelevant to rpm as what we need is just to control genpd here.
If we need to support rmp, we need some extra patches to do it,
including genpd off, clk-disable, etc...



@shawn, what's your opinion?



I'm not sure if this should be done in a generic way where we try to
save and restore all values in the "sdhci_arasan_soc_ctl_map" or if we
should try to be smarter...


I am prone to keep it as-is, unless we see a stronge requirement for
coming users who argue that they have some extra corecfg_* need to
recovery due to whatever reason.:)

If we see something like this:

if(of_device_is_compatible(A))
update X
else if (of_device_is_compatible(B))
update Y
.

Then we could consider trying to save & restrore all the values.





-Doug

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip











--
Best Regards
Shawn Lin

Re: [PATCH 00/13] Add support for perf_arch_regs

[Madhavan Srinivasan]

On Tuesday 30 August 2016 09:31 PM, Nilay Vaish wrote:

On 28 August 2016 at 16:00, Madhavan Srinivasan
 wrote:

Patchset to extend PERF_SAMPLE_REGS_INTR to include
platform specific PMU registers.

Patchset applies cleanly on tip:perf/core branch

It's a perennial request from hardware folks to be able to
see the raw values of the pmu registers. Partly it's so that
they can verify perf is doing what they want, and some
of it is that they're interested in some of the more obscure
info that isn't plumbed out through other perf interfaces.

Over the years internally we have used various hack to get
the requested data out but this is an attempt to use a
somewhat standard mechanism (using PERF_SAMPLE_REGS_INTR).

This would also be helpful for those of us working on the perf
hardware backends, to be able to verify that we're programming
things correctly, without resorting to debug printks etc.

Mechanism proposed:

1)perf_regs structure is extended with a perf_arch_regs structure
which each arch/ can populate with their specific platform
registers to sample on each perf interrupt and an arch_regs_mask
variable, which is for perf tool to know about the perf_arch_regs
that are supported.

2)perf/core func perf_sample_regs_intr() extended to update
the perf_arch_regs structure and the perf_arch_reg_mask. Set of new
support functions added perf_get_arch_regs_mask() and
perf_get_arch_reg() to aid the updates from arch/ side.

3) perf/core funcs perf_prepare_sample() and perf_output_sample()
are extended to support the update for the perf_arch_regs_mask and
perf_arch_regs in the sample

4)perf/core func perf_output_sample_regs() extended to dump
the arch_regs to the output sample.

5)Finally, perf tool side is updated to include a new element
"arch_regs_mask" in the "struct regs_dump", event sample funcs
and print functions are updated to support perf_arch_regs.


I read the patch series and I have one suggestion to make.  I think we
should not use 'arch regs' to refer to these pmu registers.  I think

Reason is that they are arch specific pmu regs. But I guess we can go with
pmu_regs also. And having a "pregs" as option to list in -I? will be fine?
(patch 13 in the patch series)


Maddy


architectural registers typically refer to the ones that hold the
state of the process.  Can we replace arch_regs by pmu_regs, or some
other choice?

Thanks
Nilay

Re: [PATCH 00/13] Add support for perf_arch_regs

[Madhavan Srinivasan]

On Tuesday 30 August 2016 09:31 PM, Nilay Vaish wrote:

On 28 August 2016 at 16:00, Madhavan Srinivasan
 wrote:

Patchset to extend PERF_SAMPLE_REGS_INTR to include
platform specific PMU registers.

Patchset applies cleanly on tip:perf/core branch

It's a perennial request from hardware folks to be able to
see the raw values of the pmu registers. Partly it's so that
they can verify perf is doing what they want, and some
of it is that they're interested in some of the more obscure
info that isn't plumbed out through other perf interfaces.

Over the years internally we have used various hack to get
the requested data out but this is an attempt to use a
somewhat standard mechanism (using PERF_SAMPLE_REGS_INTR).

This would also be helpful for those of us working on the perf
hardware backends, to be able to verify that we're programming
things correctly, without resorting to debug printks etc.

Mechanism proposed:

1)perf_regs structure is extended with a perf_arch_regs structure
which each arch/ can populate with their specific platform
registers to sample on each perf interrupt and an arch_regs_mask
variable, which is for perf tool to know about the perf_arch_regs
that are supported.

2)perf/core func perf_sample_regs_intr() extended to update
the perf_arch_regs structure and the perf_arch_reg_mask. Set of new
support functions added perf_get_arch_regs_mask() and
perf_get_arch_reg() to aid the updates from arch/ side.

3) perf/core funcs perf_prepare_sample() and perf_output_sample()
are extended to support the update for the perf_arch_regs_mask and
perf_arch_regs in the sample

4)perf/core func perf_output_sample_regs() extended to dump
the arch_regs to the output sample.

5)Finally, perf tool side is updated to include a new element
"arch_regs_mask" in the "struct regs_dump", event sample funcs
and print functions are updated to support perf_arch_regs.


I read the patch series and I have one suggestion to make.  I think we
should not use 'arch regs' to refer to these pmu registers.  I think

Reason is that they are arch specific pmu regs. But I guess we can go with
pmu_regs also. And having a "pregs" as option to list in -I? will be fine?
(patch 13 in the patch series)


Maddy


architectural registers typically refer to the ones that hold the
state of the process.  Can we replace arch_regs by pmu_regs, or some
other choice?

Thanks
Nilay

○Hello

[hi]

Hello
This is very surprising,
Bike, brand, guitar, camera, TV, telephone  50% discount and free shipping
www .slooone .com

○Hello

[hi]

Hello
This is very surprising,
Bike, brand, guitar, camera, TV, telephone  50% discount and free shipping
www .slooone .com

Re: [PATCH v2 2/7] dts: sun8i-h3: add pinmux definitions for i2c0/i2c1

[Chen-Yu Tsai]

On Thu, Sep 1, 2016 at 3:30 AM,   wrote:
> From: Jorik Jonker 
>
> This adds proper pinmux definitions for i2c0 and i2c1. Although H3 has a third
> i2c controller, these are not exposed on my boards. If someone actually has a
> H3 board with an exposed i2c2, they could add the third.
>
> Signed-off-by: Jorik Jonker 
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index 7740748..0637b95 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -327,6 +327,20 @@
> interrupt-controller;
> #interrupt-cells = <3>;
>
> +   i2c0_pins_a: i2c0@0 {
> +   allwinner,pins = "PA11", "PA12";
> +   allwinner,function = "i2c0";
> +   allwinner,drive = ;
> +   allwinner,pull = ;
> +   };
> +
> +   i2c1_pins_a: i2c1@0 {

These pinmuxes are the only ones possible for each peripheral.
Please drop the _a suffix and the @0 address for both of them.

ChenYu

> +   allwinner,pins = "PA18", "PA19";
> +   allwinner,function = "i2c1";
> +   allwinner,drive = ;
> +   allwinner,pull = ;
> +   };
> +
> mmc0_pins_a: mmc0@0 {
> allwinner,pins = "PF0", "PF1", "PF2", "PF3",
>  "PF4", "PF5";
> --
> 2.7.4
>

Re: [PATCH v2 2/7] dts: sun8i-h3: add pinmux definitions for i2c0/i2c1

[Chen-Yu Tsai]

On Thu, Sep 1, 2016 at 3:30 AM,   wrote:
> From: Jorik Jonker 
>
> This adds proper pinmux definitions for i2c0 and i2c1. Although H3 has a third
> i2c controller, these are not exposed on my boards. If someone actually has a
> H3 board with an exposed i2c2, they could add the third.
>
> Signed-off-by: Jorik Jonker 
> ---
>  arch/arm/boot/dts/sun8i-h3.dtsi | 14 ++
>  1 file changed, 14 insertions(+)
>
> diff --git a/arch/arm/boot/dts/sun8i-h3.dtsi b/arch/arm/boot/dts/sun8i-h3.dtsi
> index 7740748..0637b95 100644
> --- a/arch/arm/boot/dts/sun8i-h3.dtsi
> +++ b/arch/arm/boot/dts/sun8i-h3.dtsi
> @@ -327,6 +327,20 @@
> interrupt-controller;
> #interrupt-cells = <3>;
>
> +   i2c0_pins_a: i2c0@0 {
> +   allwinner,pins = "PA11", "PA12";
> +   allwinner,function = "i2c0";
> +   allwinner,drive = ;
> +   allwinner,pull = ;
> +   };
> +
> +   i2c1_pins_a: i2c1@0 {

These pinmuxes are the only ones possible for each peripheral.
Please drop the _a suffix and the @0 address for both of them.

ChenYu

> +   allwinner,pins = "PA18", "PA19";
> +   allwinner,function = "i2c1";
> +   allwinner,drive = ;
> +   allwinner,pull = ;
> +   };
> +
> mmc0_pins_a: mmc0@0 {
> allwinner,pins = "PF0", "PF1", "PF2", "PF3",
>  "PF4", "PF5";
> --
> 2.7.4
>

[PATCH] ftrace: Handle TRACE_BPUTS in print_graph_comment

[Namhyung Kim]

It missed to handle TRACE_BPUTS so messages recorded by trace_bputs()
will be shown with symbol info unnecessarily.

You can see it with the trace_printk sample code:

  # cd /sys/kernel/tracing/
  # echo sys_sync > set_graph_function
  # echo 1 > options/sym-offset
  # echo function_graph > current_tracer

Note that the sys_sync filter was there to prevent recording other
functions and the sym-offset option was needed since the first message
was called from a module init function so kallsyms doesn't have the
symbol and omitted in the output.

  # cd ~/build/kernel
  # insmod samples/trace_printk/trace-printk.ko

  # cd -
  # head trace

Before:

  # tracer: function_graph
  #
  # CPU  DURATION  FUNCTION CALLS
  # | |   | |   |   |   |
   1)   |  /* 0xa0002000: This is a static string that will 
use trace_bputs */
   1)   |  /* This is a dynamic string that will use trace_puts */
   1)   |  /* trace_printk_irq_work+0x5/0x7b [trace_printk]: (irq) 
This is a static string that will use trace_bputs */
   1)   |  /* (irq) This is a dynamic string that will use 
trace_puts */
   1)   |  /* (irq) This is a static string that will use 
trace_bprintk() */
   1)   |  /* (irq) This is a dynamic string that will use 
trace_printk */

After:

  # tracer: function_graph
  #
  # CPU  DURATION  FUNCTION CALLS
  # | |   | |   |   |   |
   1)   |  /* This is a static string that will use trace_bputs */
   1)   |  /* This is a dynamic string that will use trace_puts */
   1)   |  /* (irq) This is a static string that will use 
trace_bputs */
   1)   |  /* (irq) This is a dynamic string that will use 
trace_puts */
   1)   |  /* (irq) This is a static string that will use 
trace_bprintk() */
   1)   |  /* (irq) This is a dynamic string that will use 
trace_printk */

Signed-off-by: Namhyung Kim 
---
 kernel/trace/trace_functions_graph.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/kernel/trace/trace_functions_graph.c 
b/kernel/trace/trace_functions_graph.c
index 9c7ffa4df5a8..4e480e870474 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -1182,6 +1182,11 @@ print_graph_comment(struct trace_seq *s, struct 
trace_entry *ent,
trace_seq_puts(s, "/* ");
 
switch (iter->ent->type) {
+   case TRACE_BPUTS:
+   ret = trace_print_bputs_msg_only(iter);
+   if (ret != TRACE_TYPE_HANDLED)
+   return ret;
+   break;
case TRACE_BPRINT:
ret = trace_print_bprintk_msg_only(iter);
if (ret != TRACE_TYPE_HANDLED)
-- 
2.9.3

[PATCH] ftrace: Handle TRACE_BPUTS in print_graph_comment

[Namhyung Kim]

It missed to handle TRACE_BPUTS so messages recorded by trace_bputs()
will be shown with symbol info unnecessarily.

You can see it with the trace_printk sample code:

  # cd /sys/kernel/tracing/
  # echo sys_sync > set_graph_function
  # echo 1 > options/sym-offset
  # echo function_graph > current_tracer

Note that the sys_sync filter was there to prevent recording other
functions and the sym-offset option was needed since the first message
was called from a module init function so kallsyms doesn't have the
symbol and omitted in the output.

  # cd ~/build/kernel
  # insmod samples/trace_printk/trace-printk.ko

  # cd -
  # head trace

Before:

  # tracer: function_graph
  #
  # CPU  DURATION  FUNCTION CALLS
  # | |   | |   |   |   |
   1)   |  /* 0xa0002000: This is a static string that will 
use trace_bputs */
   1)   |  /* This is a dynamic string that will use trace_puts */
   1)   |  /* trace_printk_irq_work+0x5/0x7b [trace_printk]: (irq) 
This is a static string that will use trace_bputs */
   1)   |  /* (irq) This is a dynamic string that will use 
trace_puts */
   1)   |  /* (irq) This is a static string that will use 
trace_bprintk() */
   1)   |  /* (irq) This is a dynamic string that will use 
trace_printk */

After:

  # tracer: function_graph
  #
  # CPU  DURATION  FUNCTION CALLS
  # | |   | |   |   |   |
   1)   |  /* This is a static string that will use trace_bputs */
   1)   |  /* This is a dynamic string that will use trace_puts */
   1)   |  /* (irq) This is a static string that will use 
trace_bputs */
   1)   |  /* (irq) This is a dynamic string that will use 
trace_puts */
   1)   |  /* (irq) This is a static string that will use 
trace_bprintk() */
   1)   |  /* (irq) This is a dynamic string that will use 
trace_printk */

Signed-off-by: Namhyung Kim 
---
 kernel/trace/trace_functions_graph.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/kernel/trace/trace_functions_graph.c 
b/kernel/trace/trace_functions_graph.c
index 9c7ffa4df5a8..4e480e870474 100644
--- a/kernel/trace/trace_functions_graph.c
+++ b/kernel/trace/trace_functions_graph.c
@@ -1182,6 +1182,11 @@ print_graph_comment(struct trace_seq *s, struct 
trace_entry *ent,
trace_seq_puts(s, "/* ");
 
switch (iter->ent->type) {
+   case TRACE_BPUTS:
+   ret = trace_print_bputs_msg_only(iter);
+   if (ret != TRACE_TYPE_HANDLED)
+   return ret;
+   break;
case TRACE_BPRINT:
ret = trace_print_bprintk_msg_only(iter);
if (ret != TRACE_TYPE_HANDLED)
-- 
2.9.3

Re: [PATCH v2 3/7] dts: sun8i-h3: add i2c0/i2c1 SoC peripherals

[Chen-Yu Tsai]

On Thu, Sep 1, 2016 at 3:30 AM,   wrote:
> From: Jorik Jonker 
>
> This enables the i2c0/i2c1 peripherals of the SoC. There is actually a third
> controller, but I do not have a board on hands on which i2c2 is exposed in 
> such
> a way that I can verify that it works.

If they are listed in the manual, and the interrupts, clocks, resets, pins
all exist, that is good enough for me.

>
> Signed-off-by: Jorik Jonker 

Acked-by: Chen-Yu Tsai 

Re: [PATCH v2 3/7] dts: sun8i-h3: add i2c0/i2c1 SoC peripherals

[Chen-Yu Tsai]

On Thu, Sep 1, 2016 at 3:30 AM,   wrote:
> From: Jorik Jonker 
>
> This enables the i2c0/i2c1 peripherals of the SoC. There is actually a third
> controller, but I do not have a board on hands on which i2c2 is exposed in 
> such
> a way that I can verify that it works.

If they are listed in the manual, and the interrupts, clocks, resets, pins
all exist, that is good enough for me.

>
> Signed-off-by: Jorik Jonker 

Acked-by: Chen-Yu Tsai 

Re: [PATCH] arm64: Improve kprobes test for atomic sequence

[Masami Hiramatsu]

Hi Dave,

On Wed, 31 Aug 2016 16:52:22 -0400
David Long  wrote:

> From: "David A. Long" 
> 
> Kprobes searches backwards a finite number of instructions to determine if
> there is an attempt to probe a load/store exclusive sequence. It stops when
> it hits the maximum number of instructions or a load or store exclusive.

Hmm, so on aarch64, we can not put a kprobe between load exclusive and
store exclusive, because kprobe always breaks the atomicity, am I correct?
If so, what happen if any branch in the sequence? e.g.

  load-ex
  (do something)
l1:
  store-ex
...
  load-ex
  (do something)
  branch l1;

> However this means it can run up past the beginning of the function and
> start looking at literal constants. This has been shown to cause a false
> positive and blocks insertion of the probe. To fix this add a test to see
> if the typical:
> 
>   "stp x29, x30, [sp, #n]!"
> 
> instruction beginning a function gets hit. This also improves efficiency by
> not testing code that is not part of the function. There is some
> possibility that a function will not begin with this instruction, in which
> case the fixed code will behave no worse than before.

If the function boundary is the problem, why you wouldn't use kallsyms 
information
as I did in can_optimize()@arch/x86/kernel/kprobes/opt.c ?

/* Lookup symbol including addr */
if (!kallsyms_lookup_size_offset(paddr, , ))
return 0;

With this call, symbol start address is (paddr - offset) and end address
is (paddr - offset + size).

Thank you,

> 
> There could also be the case that the stp instruction is found further in
> the body of the function, which could theoretically allow probing of an
> atomic squence. The likelihood of this seems low, and this would not be the
> only aspect of kprobes where the user needs to be careful to avoid
> problems.
> 
> Signed-off-by: David A. Long 
> ---
>  arch/arm64/kernel/probes/decode-insn.c | 25 ++---
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/kernel/probes/decode-insn.c 
> b/arch/arm64/kernel/probes/decode-insn.c
> index 37e47a9..248e820 100644
> --- a/arch/arm64/kernel/probes/decode-insn.c
> +++ b/arch/arm64/kernel/probes/decode-insn.c
> @@ -122,16 +122,28 @@ arm_probe_decode_insn(kprobe_opcode_t insn, struct 
> arch_specific_insn *asi)
>  static bool __kprobes
>  is_probed_address_atomic(kprobe_opcode_t *scan_start, kprobe_opcode_t 
> *scan_end)
>  {
> + const u32 stp_x29_x30_sp_pre = 0xa9807bfd;
> + const u32 stp_ignore_index_mask = 0xffc07fff;
> + u32 instruction = le32_to_cpu(*scan_start);
> +
>   while (scan_start > scan_end) {
>   /*
> -  * atomic region starts from exclusive load and ends with
> -  * exclusive store.
> +  * Atomic region starts from exclusive load and ends with
> +  * exclusive store. If we hit a "stp x29, x30, [sp, #n]!"
> +  * assume it is the beginning of the function and end the
> +  * search. This helps avoid false positives from literal
> +  * constants that look like a load-exclusive, in addition
> +  * to being more efficient.
>*/
> - if (aarch64_insn_is_store_ex(le32_to_cpu(*scan_start)))
> + if ((instruction & stp_ignore_index_mask) == stp_x29_x30_sp_pre)
>   return false;
> - else if (aarch64_insn_is_load_ex(le32_to_cpu(*scan_start)))
> - return true;
> +
>   scan_start--;
> + instruction = le32_to_cpu(*scan_start);
> + if (aarch64_insn_is_store_ex(instruction))
> + return false;
> + else if (aarch64_insn_is_load_ex(instruction))
> + return true;
>   }
>  
>   return false;
> @@ -142,7 +154,6 @@ arm_kprobe_decode_insn(kprobe_opcode_t *addr, struct 
> arch_specific_insn *asi)
>  {
>   enum kprobe_insn decoded;
>   kprobe_opcode_t insn = le32_to_cpu(*addr);
> - kprobe_opcode_t *scan_start = addr - 1;
>   kprobe_opcode_t *scan_end = addr - MAX_ATOMIC_CONTEXT_SIZE;
>  #if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
>   struct module *mod;
> @@ -167,7 +178,7 @@ arm_kprobe_decode_insn(kprobe_opcode_t *addr, struct 
> arch_specific_insn *asi)
>   decoded = arm_probe_decode_insn(insn, asi);
>  
>   if (decoded == INSN_REJECTED ||
> - is_probed_address_atomic(scan_start, scan_end))
> + is_probed_address_atomic(addr, scan_end))
>   return INSN_REJECTED;
>  
>   return decoded;
> -- 
> 2.5.0
> 


-- 
Masami Hiramatsu 

Re: [PATCH] arm64: Improve kprobes test for atomic sequence

[Masami Hiramatsu]

Hi Dave,

On Wed, 31 Aug 2016 16:52:22 -0400
David Long  wrote:

> From: "David A. Long" 
> 
> Kprobes searches backwards a finite number of instructions to determine if
> there is an attempt to probe a load/store exclusive sequence. It stops when
> it hits the maximum number of instructions or a load or store exclusive.

Hmm, so on aarch64, we can not put a kprobe between load exclusive and
store exclusive, because kprobe always breaks the atomicity, am I correct?
If so, what happen if any branch in the sequence? e.g.

  load-ex
  (do something)
l1:
  store-ex
...
  load-ex
  (do something)
  branch l1;

> However this means it can run up past the beginning of the function and
> start looking at literal constants. This has been shown to cause a false
> positive and blocks insertion of the probe. To fix this add a test to see
> if the typical:
> 
>   "stp x29, x30, [sp, #n]!"
> 
> instruction beginning a function gets hit. This also improves efficiency by
> not testing code that is not part of the function. There is some
> possibility that a function will not begin with this instruction, in which
> case the fixed code will behave no worse than before.

If the function boundary is the problem, why you wouldn't use kallsyms 
information
as I did in can_optimize()@arch/x86/kernel/kprobes/opt.c ?

/* Lookup symbol including addr */
if (!kallsyms_lookup_size_offset(paddr, , ))
return 0;

With this call, symbol start address is (paddr - offset) and end address
is (paddr - offset + size).

Thank you,

> 
> There could also be the case that the stp instruction is found further in
> the body of the function, which could theoretically allow probing of an
> atomic squence. The likelihood of this seems low, and this would not be the
> only aspect of kprobes where the user needs to be careful to avoid
> problems.
> 
> Signed-off-by: David A. Long 
> ---
>  arch/arm64/kernel/probes/decode-insn.c | 25 ++---
>  1 file changed, 18 insertions(+), 7 deletions(-)
> 
> diff --git a/arch/arm64/kernel/probes/decode-insn.c 
> b/arch/arm64/kernel/probes/decode-insn.c
> index 37e47a9..248e820 100644
> --- a/arch/arm64/kernel/probes/decode-insn.c
> +++ b/arch/arm64/kernel/probes/decode-insn.c
> @@ -122,16 +122,28 @@ arm_probe_decode_insn(kprobe_opcode_t insn, struct 
> arch_specific_insn *asi)
>  static bool __kprobes
>  is_probed_address_atomic(kprobe_opcode_t *scan_start, kprobe_opcode_t 
> *scan_end)
>  {
> + const u32 stp_x29_x30_sp_pre = 0xa9807bfd;
> + const u32 stp_ignore_index_mask = 0xffc07fff;
> + u32 instruction = le32_to_cpu(*scan_start);
> +
>   while (scan_start > scan_end) {
>   /*
> -  * atomic region starts from exclusive load and ends with
> -  * exclusive store.
> +  * Atomic region starts from exclusive load and ends with
> +  * exclusive store. If we hit a "stp x29, x30, [sp, #n]!"
> +  * assume it is the beginning of the function and end the
> +  * search. This helps avoid false positives from literal
> +  * constants that look like a load-exclusive, in addition
> +  * to being more efficient.
>*/
> - if (aarch64_insn_is_store_ex(le32_to_cpu(*scan_start)))
> + if ((instruction & stp_ignore_index_mask) == stp_x29_x30_sp_pre)
>   return false;
> - else if (aarch64_insn_is_load_ex(le32_to_cpu(*scan_start)))
> - return true;
> +
>   scan_start--;
> + instruction = le32_to_cpu(*scan_start);
> + if (aarch64_insn_is_store_ex(instruction))
> + return false;
> + else if (aarch64_insn_is_load_ex(instruction))
> + return true;
>   }
>  
>   return false;
> @@ -142,7 +154,6 @@ arm_kprobe_decode_insn(kprobe_opcode_t *addr, struct 
> arch_specific_insn *asi)
>  {
>   enum kprobe_insn decoded;
>   kprobe_opcode_t insn = le32_to_cpu(*addr);
> - kprobe_opcode_t *scan_start = addr - 1;
>   kprobe_opcode_t *scan_end = addr - MAX_ATOMIC_CONTEXT_SIZE;
>  #if defined(CONFIG_MODULES) && defined(MODULES_VADDR)
>   struct module *mod;
> @@ -167,7 +178,7 @@ arm_kprobe_decode_insn(kprobe_opcode_t *addr, struct 
> arch_specific_insn *asi)
>   decoded = arm_probe_decode_insn(insn, asi);
>  
>   if (decoded == INSN_REJECTED ||
> - is_probed_address_atomic(scan_start, scan_end))
> + is_probed_address_atomic(addr, scan_end))
>   return INSN_REJECTED;
>  
>   return decoded;
> -- 
> 2.5.0
> 


-- 
Masami Hiramatsu 

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

Hi,


On 2016年09月01日 01:42, Doug Anderson wrote:

Hi,

On Sun, Aug 28, 2016 at 8:25 PM, Shawn Lin  wrote:

On 2016/8/29 10:50, Elaine Zhang wrote:



On 08/27/2016 11:05 PM, Shawn Lin wrote:

On 2016/8/27 21:41, Ziyuan Xu wrote:

Control power domain for eMMC via genpd to reduce power consumption.

Signed-off-by: Elaine Zhang 
Signed-off-by: Ziyuan Xu 


It looks nice to me. But this should be merged after applying that[0]
as your patch will break bind/unbind test for sdhci-of-arasan on rk3399
without it[0]. Moreover, Elaine should make sure that upstreamed
rockchip power domain stuff would not off pd for emmc, *otherwise*, I
should update my patch to make sure we update clkmul every time when
doing suspend 2 resume..



Forgot to say:
If use pd, Although there is no call to power odd the pd_emmc,
it will be power off when the system doing suspend 2 resume.
(Because the system call
__device_suspend_noirq->pm_genpd_suspend_noirq->rockchip_pd_power_off)


Thanks for explaining this. I checked the code a bit and actually I
don't need to updata clkmul since it was recorded, although it is still
reset to 0x10 reading from syscon. So for that, we can now pick it
up without waiting for my sdhci-of-arasan's update.

Reviewed-by: Shawn Lin 

This is fine to pick up _only_ if you don't care about suspend/resume.
If you care about suspend/resume then someone needs to first write a
patch that will re-init all "corecfg" values after power is turned on.


Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we 
don't need to strore/re-init it after resume.
corecfg_clockmultiplier is only used to fetch host->clk_mul, and 
host->clk_mul has been a fixed value at run-time, unless driver unbind.
The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to 
check the xin_clk at probe time, we don't reference it at run-time.
BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC 
works fine.




Technically I think this should probably use "pm runtime" and not
normal suspend/resume hooks.  Any time we end up pm runtime suspended
then I think our power will go off (because of genpd?) and we need to
restore values.


I understand your consideration. BUT genpd is in charge of on/off pd if 
the corresponding device node has "power-domains" property. RPM is 
unnecessary for this situation, we will not use autosuspend, right?


@shawn, what's your opinion?



I'm not sure if this should be done in a generic way where we try to
save and restore all values in the "sdhci_arasan_soc_ctl_map" or if we
should try to be smarter...


-Doug

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

Re: [PATCH 2/2] arm64: dts: rockchip: add eMMC's power domain support for rk3399

[Ziyuan Xu]

Hi,


On 2016年09月01日 01:42, Doug Anderson wrote:

Hi,

On Sun, Aug 28, 2016 at 8:25 PM, Shawn Lin  wrote:

On 2016/8/29 10:50, Elaine Zhang wrote:



On 08/27/2016 11:05 PM, Shawn Lin wrote:

On 2016/8/27 21:41, Ziyuan Xu wrote:

Control power domain for eMMC via genpd to reduce power consumption.

Signed-off-by: Elaine Zhang 
Signed-off-by: Ziyuan Xu 


It looks nice to me. But this should be merged after applying that[0]
as your patch will break bind/unbind test for sdhci-of-arasan on rk3399
without it[0]. Moreover, Elaine should make sure that upstreamed
rockchip power domain stuff would not off pd for emmc, *otherwise*, I
should update my patch to make sure we update clkmul every time when
doing suspend 2 resume..



Forgot to say:
If use pd, Although there is no call to power odd the pd_emmc,
it will be power off when the system doing suspend 2 resume.
(Because the system call
__device_suspend_noirq->pm_genpd_suspend_noirq->rockchip_pd_power_off)


Thanks for explaining this. I checked the code a bit and actually I
don't need to updata clkmul since it was recorded, although it is still
reset to 0x10 reading from syscon. So for that, we can now pick it
up without waiting for my sdhci-of-arasan's update.

Reviewed-by: Shawn Lin 

This is fine to pick up _only_ if you don't care about suspend/resume.
If you care about suspend/resume then someone needs to first write a
patch that will re-init all "corecfg" values after power is turned on.


Do you mean corecfg_clockmultiplier and corecfg_baseclkfreq, if yes, we 
don't need to strore/re-init it after resume.
corecfg_clockmultiplier is only used to fetch host->clk_mul, and 
host->clk_mul has been a fixed value at run-time, unless driver unbind.
The same as corecfg_clockmultiplier, corecfg_baseclkfreq is used to 
check the xin_clk at probe time, we don't reference it at run-time.
BTW, I have tested suspend/resume on rk3399 prior to this sumbit, eMMC 
works fine.




Technically I think this should probably use "pm runtime" and not
normal suspend/resume hooks.  Any time we end up pm runtime suspended
then I think our power will go off (because of genpd?) and we need to
restore values.


I understand your consideration. BUT genpd is in charge of on/off pd if 
the corresponding device node has "power-domains" property. RPM is 
unnecessary for this situation, we will not use autosuspend, right?


@shawn, what's your opinion?



I'm not sure if this should be done in a generic way where we try to
save and restore all values in the "sdhci_arasan_soc_ctl_map" or if we
should try to be smarter...


-Doug

___
Linux-rockchip mailing list
linux-rockc...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-rockchip

[PATCH v2] serial: 8250_dw: Use an unified new dev variable in probe

[Kefeng Wang]

Use an unified new dev variable instead of >dev and p->dev
in probe function.

Reviewed-by: Heikki Krogerus 
Signed-off-by: Kefeng Wang 
---

Hi Greg, updated, based on tty-testing branch :)

v1->v2:
1) Add Heikki's reviewed-by
2) Rebase on tty-testing branch of 
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git

 drivers/tty/serial/8250/8250_dw.c | 45 ---
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c 
b/drivers/tty/serial/8250/8250_dw.c
index b022f5a..47e6d08 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -360,18 +360,19 @@ static int dw8250_probe(struct platform_device *pdev)
struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
int irq = platform_get_irq(pdev, 0);
struct uart_port *p = 
+   struct device *dev = >dev;
struct dw8250_data *data;
int err;
u32 val;
 
if (!regs) {
-   dev_err(>dev, "no registers defined\n");
+   dev_err(dev, "no registers defined\n");
return -EINVAL;
}
 
if (irq < 0) {
if (irq != -EPROBE_DEFER)
-   dev_err(>dev, "cannot get irq\n");
+   dev_err(dev, "cannot get irq\n");
return irq;
}
 
@@ -382,16 +383,16 @@ static int dw8250_probe(struct platform_device *pdev)
p->pm   = dw8250_do_pm;
p->type = PORT_8250;
p->flags= UPF_SHARE_IRQ | UPF_FIXED_PORT;
-   p->dev  = >dev;
+   p->dev  = dev;
p->iotype   = UPIO_MEM;
p->serial_in= dw8250_serial_in;
p->serial_out   = dw8250_serial_out;
 
-   p->membase = devm_ioremap(>dev, regs->start, resource_size(regs));
+   p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
if (!p->membase)
return -ENOMEM;
 
-   data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL);
+   data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
 
@@ -399,57 +400,57 @@ static int dw8250_probe(struct platform_device *pdev)
data->usr_reg = DW_UART_USR;
p->private_data = data;
 
-   data->uart_16550_compatible = device_property_read_bool(p->dev,
+   data->uart_16550_compatible = device_property_read_bool(dev,
"snps,uart-16550-compatible");
 
-   err = device_property_read_u32(p->dev, "reg-shift", );
+   err = device_property_read_u32(dev, "reg-shift", );
if (!err)
p->regshift = val;
 
-   err = device_property_read_u32(p->dev, "reg-io-width", );
+   err = device_property_read_u32(dev, "reg-io-width", );
if (!err && val == 4) {
p->iotype = UPIO_MEM32;
p->serial_in = dw8250_serial_in32;
p->serial_out = dw8250_serial_out32;
}
 
-   if (device_property_read_bool(p->dev, "dcd-override")) {
+   if (device_property_read_bool(dev, "dcd-override")) {
/* Always report DCD as active */
data->msr_mask_on |= UART_MSR_DCD;
data->msr_mask_off |= UART_MSR_DDCD;
}
 
-   if (device_property_read_bool(p->dev, "dsr-override")) {
+   if (device_property_read_bool(dev, "dsr-override")) {
/* Always report DSR as active */
data->msr_mask_on |= UART_MSR_DSR;
data->msr_mask_off |= UART_MSR_DDSR;
}
 
-   if (device_property_read_bool(p->dev, "cts-override")) {
+   if (device_property_read_bool(dev, "cts-override")) {
/* Always report CTS as active */
data->msr_mask_on |= UART_MSR_CTS;
data->msr_mask_off |= UART_MSR_DCTS;
}
 
-   if (device_property_read_bool(p->dev, "ri-override")) {
+   if (device_property_read_bool(dev, "ri-override")) {
/* Always report Ring indicator as inactive */
data->msr_mask_off |= UART_MSR_RI;
data->msr_mask_off |= UART_MSR_TERI;
}
 
/* Always ask for fixed clock rate from a property. */
-   device_property_read_u32(p->dev, "clock-frequency", >uartclk);
+   device_property_read_u32(dev, "clock-frequency", >uartclk);
 
/* If there is separate baudclk, get the rate from it. */
-   data->clk = devm_clk_get(>dev, "baudclk");
+   data->clk = devm_clk_get(dev, "baudclk");
if (IS_ERR(data->clk) && PTR_ERR(data->clk) != -EPROBE_DEFER)
-   data->clk = devm_clk_get(>dev, NULL);
+   data->clk = devm_clk_get(dev, NULL);
if (IS_ERR(data->clk) && PTR_ERR(data->clk) == -EPROBE_DEFER)
return -EPROBE_DEFER;
if (!IS_ERR_OR_NULL(data->clk)) {

[PATCH v2] serial: 8250_dw: Use an unified new dev variable in probe

[Kefeng Wang]

Use an unified new dev variable instead of >dev and p->dev
in probe function.

Reviewed-by: Heikki Krogerus 
Signed-off-by: Kefeng Wang 
---

Hi Greg, updated, based on tty-testing branch :)

v1->v2:
1) Add Heikki's reviewed-by
2) Rebase on tty-testing branch of 
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git

 drivers/tty/serial/8250/8250_dw.c | 45 ---
 1 file changed, 23 insertions(+), 22 deletions(-)

diff --git a/drivers/tty/serial/8250/8250_dw.c 
b/drivers/tty/serial/8250/8250_dw.c
index b022f5a..47e6d08 100644
--- a/drivers/tty/serial/8250/8250_dw.c
+++ b/drivers/tty/serial/8250/8250_dw.c
@@ -360,18 +360,19 @@ static int dw8250_probe(struct platform_device *pdev)
struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
int irq = platform_get_irq(pdev, 0);
struct uart_port *p = 
+   struct device *dev = >dev;
struct dw8250_data *data;
int err;
u32 val;
 
if (!regs) {
-   dev_err(>dev, "no registers defined\n");
+   dev_err(dev, "no registers defined\n");
return -EINVAL;
}
 
if (irq < 0) {
if (irq != -EPROBE_DEFER)
-   dev_err(>dev, "cannot get irq\n");
+   dev_err(dev, "cannot get irq\n");
return irq;
}
 
@@ -382,16 +383,16 @@ static int dw8250_probe(struct platform_device *pdev)
p->pm   = dw8250_do_pm;
p->type = PORT_8250;
p->flags= UPF_SHARE_IRQ | UPF_FIXED_PORT;
-   p->dev  = >dev;
+   p->dev  = dev;
p->iotype   = UPIO_MEM;
p->serial_in= dw8250_serial_in;
p->serial_out   = dw8250_serial_out;
 
-   p->membase = devm_ioremap(>dev, regs->start, resource_size(regs));
+   p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
if (!p->membase)
return -ENOMEM;
 
-   data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL);
+   data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
if (!data)
return -ENOMEM;
 
@@ -399,57 +400,57 @@ static int dw8250_probe(struct platform_device *pdev)
data->usr_reg = DW_UART_USR;
p->private_data = data;
 
-   data->uart_16550_compatible = device_property_read_bool(p->dev,
+   data->uart_16550_compatible = device_property_read_bool(dev,
"snps,uart-16550-compatible");
 
-   err = device_property_read_u32(p->dev, "reg-shift", );
+   err = device_property_read_u32(dev, "reg-shift", );
if (!err)
p->regshift = val;
 
-   err = device_property_read_u32(p->dev, "reg-io-width", );
+   err = device_property_read_u32(dev, "reg-io-width", );
if (!err && val == 4) {
p->iotype = UPIO_MEM32;
p->serial_in = dw8250_serial_in32;
p->serial_out = dw8250_serial_out32;
}
 
-   if (device_property_read_bool(p->dev, "dcd-override")) {
+   if (device_property_read_bool(dev, "dcd-override")) {
/* Always report DCD as active */
data->msr_mask_on |= UART_MSR_DCD;
data->msr_mask_off |= UART_MSR_DDCD;
}
 
-   if (device_property_read_bool(p->dev, "dsr-override")) {
+   if (device_property_read_bool(dev, "dsr-override")) {
/* Always report DSR as active */
data->msr_mask_on |= UART_MSR_DSR;
data->msr_mask_off |= UART_MSR_DDSR;
}
 
-   if (device_property_read_bool(p->dev, "cts-override")) {
+   if (device_property_read_bool(dev, "cts-override")) {
/* Always report CTS as active */
data->msr_mask_on |= UART_MSR_CTS;
data->msr_mask_off |= UART_MSR_DCTS;
}
 
-   if (device_property_read_bool(p->dev, "ri-override")) {
+   if (device_property_read_bool(dev, "ri-override")) {
/* Always report Ring indicator as inactive */
data->msr_mask_off |= UART_MSR_RI;
data->msr_mask_off |= UART_MSR_TERI;
}
 
/* Always ask for fixed clock rate from a property. */
-   device_property_read_u32(p->dev, "clock-frequency", >uartclk);
+   device_property_read_u32(dev, "clock-frequency", >uartclk);
 
/* If there is separate baudclk, get the rate from it. */
-   data->clk = devm_clk_get(>dev, "baudclk");
+   data->clk = devm_clk_get(dev, "baudclk");
if (IS_ERR(data->clk) && PTR_ERR(data->clk) != -EPROBE_DEFER)
-   data->clk = devm_clk_get(>dev, NULL);
+   data->clk = devm_clk_get(dev, NULL);
if (IS_ERR(data->clk) && PTR_ERR(data->clk) == -EPROBE_DEFER)
return -EPROBE_DEFER;
if (!IS_ERR_OR_NULL(data->clk)) {
err = clk_prepare_enable(data->clk);
if (err)
- 

Re: [PATCH] serial: 8250_dw: Use an unified new dev variable in probe

[Kefeng Wang]

Sorry, please ignore, send wrong patch.

On 2016/9/1 9:34, Kefeng Wang wrote:
> Use an unified new dev variable instead of >dev and p->dev
> in probe function.
> 
> Signed-off-by: Kefeng Wang 
> ---
>  drivers/tty/serial/8250/8250_dw.c | 45 
> ---
>  1 file changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c 
> b/drivers/tty/serial/8250/8250_dw.c
> index 3478c2c..225d797 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -363,18 +363,19 @@ static int dw8250_probe(struct platform_device *pdev)
>   struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   int irq = platform_get_irq(pdev, 0);
>   struct uart_port *p = 
> + struct device *dev = >dev;
>   struct dw8250_data *data;
>   int err;
>   u32 val;
>  
>   if (!regs) {
> - dev_err(>dev, "no registers defined\n");
> + dev_err(dev, "no registers defined\n");
>   return -EINVAL;
>   }
>  
>   if (irq < 0) {
>   if (irq != -EPROBE_DEFER)
> - dev_err(>dev, "cannot get irq\n");
> + dev_err(dev, "cannot get irq\n");
>   return irq;
>   }
>  
> @@ -385,17 +386,17 @@ static int dw8250_probe(struct platform_device *pdev)
>   p->pm   = dw8250_do_pm;
>   p->type = PORT_8250;
>   p->flags= UPF_SHARE_IRQ | UPF_FIXED_PORT;
> - p->dev  = >dev;
> + p->dev  = dev;
>   p->iotype   = UPIO_MEM;
>   p->serial_in= dw8250_serial_in;
>   p->serial_out   = dw8250_serial_out;
>   p->set_termios  = dw8250_set_termios;
>  
> - p->membase = devm_ioremap(>dev, regs->start, resource_size(regs));
> + p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
>   if (!p->membase)
>   return -ENOMEM;
>  
> - data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL);
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>   if (!data)
>   return -ENOMEM;
>  
> @@ -403,57 +404,57 @@ static int dw8250_probe(struct platform_device *pdev)
>   data->usr_reg = DW_UART_USR;
>   p->private_data = data;
>  
> - data->uart_16550_compatible = device_property_read_bool(p->dev,
> + data->uart_16550_compatible = device_property_read_bool(dev,
>   "snps,uart-16550-compatible");
>  
> - err = device_property_read_u32(p->dev, "reg-shift", );
> + err = device_property_read_u32(dev, "reg-shift", );
>   if (!err)
>   p->regshift = val;
>  
> - err = device_property_read_u32(p->dev, "reg-io-width", );
> + err = device_property_read_u32(dev, "reg-io-width", );
>   if (!err && val == 4) {
>   p->iotype = UPIO_MEM32;
>   p->serial_in = dw8250_serial_in32;
>   p->serial_out = dw8250_serial_out32;
>   }
>  
> - if (device_property_read_bool(p->dev, "dcd-override")) {
> + if (device_property_read_bool(dev, "dcd-override")) {
>   /* Always report DCD as active */
>   data->msr_mask_on |= UART_MSR_DCD;
>   data->msr_mask_off |= UART_MSR_DDCD;
>   }
>  
> - if (device_property_read_bool(p->dev, "dsr-override")) {
> + if (device_property_read_bool(dev, "dsr-override")) {
>   /* Always report DSR as active */
>   data->msr_mask_on |= UART_MSR_DSR;
>   data->msr_mask_off |= UART_MSR_DDSR;
>   }
>  
> - if (device_property_read_bool(p->dev, "cts-override")) {
> + if (device_property_read_bool(dev, "cts-override")) {
>   /* Always report CTS as active */
>   data->msr_mask_on |= UART_MSR_CTS;
>   data->msr_mask_off |= UART_MSR_DCTS;
>   }
>  
> - if (device_property_read_bool(p->dev, "ri-override")) {
> + if (device_property_read_bool(dev, "ri-override")) {
>   /* Always report Ring indicator as inactive */
>   data->msr_mask_off |= UART_MSR_RI;
>   data->msr_mask_off |= UART_MSR_TERI;
>   }
>  
>   /* Always ask for fixed clock rate from a property. */
> - device_property_read_u32(p->dev, "clock-frequency", >uartclk);
> + device_property_read_u32(dev, "clock-frequency", >uartclk);
>  
>   /* If there is separate baudclk, get the rate from it. */
> - data->clk = devm_clk_get(>dev, "baudclk");
> + data->clk = devm_clk_get(dev, "baudclk");
>   if (IS_ERR(data->clk) && PTR_ERR(data->clk) != -EPROBE_DEFER)
> - data->clk = devm_clk_get(>dev, NULL);
> + data->clk = devm_clk_get(dev, NULL);
>   if (IS_ERR(data->clk) && PTR_ERR(data->clk) == -EPROBE_DEFER)
>   return -EPROBE_DEFER;
>   if (!IS_ERR_OR_NULL(data->clk)) {
>   err = clk_prepare_enable(data->clk);
>   if (err)

Re: [PATCH] serial: 8250_dw: Use an unified new dev variable in probe

[Kefeng Wang]

Sorry, please ignore, send wrong patch.

On 2016/9/1 9:34, Kefeng Wang wrote:
> Use an unified new dev variable instead of >dev and p->dev
> in probe function.
> 
> Signed-off-by: Kefeng Wang 
> ---
>  drivers/tty/serial/8250/8250_dw.c | 45 
> ---
>  1 file changed, 23 insertions(+), 22 deletions(-)
> 
> diff --git a/drivers/tty/serial/8250/8250_dw.c 
> b/drivers/tty/serial/8250/8250_dw.c
> index 3478c2c..225d797 100644
> --- a/drivers/tty/serial/8250/8250_dw.c
> +++ b/drivers/tty/serial/8250/8250_dw.c
> @@ -363,18 +363,19 @@ static int dw8250_probe(struct platform_device *pdev)
>   struct resource *regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>   int irq = platform_get_irq(pdev, 0);
>   struct uart_port *p = 
> + struct device *dev = >dev;
>   struct dw8250_data *data;
>   int err;
>   u32 val;
>  
>   if (!regs) {
> - dev_err(>dev, "no registers defined\n");
> + dev_err(dev, "no registers defined\n");
>   return -EINVAL;
>   }
>  
>   if (irq < 0) {
>   if (irq != -EPROBE_DEFER)
> - dev_err(>dev, "cannot get irq\n");
> + dev_err(dev, "cannot get irq\n");
>   return irq;
>   }
>  
> @@ -385,17 +386,17 @@ static int dw8250_probe(struct platform_device *pdev)
>   p->pm   = dw8250_do_pm;
>   p->type = PORT_8250;
>   p->flags= UPF_SHARE_IRQ | UPF_FIXED_PORT;
> - p->dev  = >dev;
> + p->dev  = dev;
>   p->iotype   = UPIO_MEM;
>   p->serial_in= dw8250_serial_in;
>   p->serial_out   = dw8250_serial_out;
>   p->set_termios  = dw8250_set_termios;
>  
> - p->membase = devm_ioremap(>dev, regs->start, resource_size(regs));
> + p->membase = devm_ioremap(dev, regs->start, resource_size(regs));
>   if (!p->membase)
>   return -ENOMEM;
>  
> - data = devm_kzalloc(>dev, sizeof(*data), GFP_KERNEL);
> + data = devm_kzalloc(dev, sizeof(*data), GFP_KERNEL);
>   if (!data)
>   return -ENOMEM;
>  
> @@ -403,57 +404,57 @@ static int dw8250_probe(struct platform_device *pdev)
>   data->usr_reg = DW_UART_USR;
>   p->private_data = data;
>  
> - data->uart_16550_compatible = device_property_read_bool(p->dev,
> + data->uart_16550_compatible = device_property_read_bool(dev,
>   "snps,uart-16550-compatible");
>  
> - err = device_property_read_u32(p->dev, "reg-shift", );
> + err = device_property_read_u32(dev, "reg-shift", );
>   if (!err)
>   p->regshift = val;
>  
> - err = device_property_read_u32(p->dev, "reg-io-width", );
> + err = device_property_read_u32(dev, "reg-io-width", );
>   if (!err && val == 4) {
>   p->iotype = UPIO_MEM32;
>   p->serial_in = dw8250_serial_in32;
>   p->serial_out = dw8250_serial_out32;
>   }
>  
> - if (device_property_read_bool(p->dev, "dcd-override")) {
> + if (device_property_read_bool(dev, "dcd-override")) {
>   /* Always report DCD as active */
>   data->msr_mask_on |= UART_MSR_DCD;
>   data->msr_mask_off |= UART_MSR_DDCD;
>   }
>  
> - if (device_property_read_bool(p->dev, "dsr-override")) {
> + if (device_property_read_bool(dev, "dsr-override")) {
>   /* Always report DSR as active */
>   data->msr_mask_on |= UART_MSR_DSR;
>   data->msr_mask_off |= UART_MSR_DDSR;
>   }
>  
> - if (device_property_read_bool(p->dev, "cts-override")) {
> + if (device_property_read_bool(dev, "cts-override")) {
>   /* Always report CTS as active */
>   data->msr_mask_on |= UART_MSR_CTS;
>   data->msr_mask_off |= UART_MSR_DCTS;
>   }
>  
> - if (device_property_read_bool(p->dev, "ri-override")) {
> + if (device_property_read_bool(dev, "ri-override")) {
>   /* Always report Ring indicator as inactive */
>   data->msr_mask_off |= UART_MSR_RI;
>   data->msr_mask_off |= UART_MSR_TERI;
>   }
>  
>   /* Always ask for fixed clock rate from a property. */
> - device_property_read_u32(p->dev, "clock-frequency", >uartclk);
> + device_property_read_u32(dev, "clock-frequency", >uartclk);
>  
>   /* If there is separate baudclk, get the rate from it. */
> - data->clk = devm_clk_get(>dev, "baudclk");
> + data->clk = devm_clk_get(dev, "baudclk");
>   if (IS_ERR(data->clk) && PTR_ERR(data->clk) != -EPROBE_DEFER)
> - data->clk = devm_clk_get(>dev, NULL);
> + data->clk = devm_clk_get(dev, NULL);
>   if (IS_ERR(data->clk) && PTR_ERR(data->clk) == -EPROBE_DEFER)
>   return -EPROBE_DEFER;
>   if (!IS_ERR_OR_NULL(data->clk)) {
>   err = clk_prepare_enable(data->clk);
>   if (err)
> - 

Re: [PATCH 2/2] drivers/perf: arm_pmu: Fix NULL pointer dereference during probe

[Kevin Hilman]

Will Deacon  writes:

> On Sat, Aug 27, 2016 at 04:19:50PM +, Stefan Wahren wrote:
>> Patch 7f1d642fbb5c ("drivers/perf: arm-pmu: Fix handling of SPI lacking
>> interrupt-affinity property") unintended also fixes perf_event support
>> for bcm2835 which doesn't have PMU interrupts. Unfortunately this change
>> introduce a NULL pointer dereference on bcm2835, because irq_is_percpu
>> always expected to be called with a valid IRQ. So fix this regression
>> by validating the IRQ before.
>> 
>> Signed-off-by: Stefan Wahren 
>> Fixes: 7f1d642fbb5c ("drivers/perf: arm-pmu: Fix handling of SPI lacking 
>> \"interrupt-affinity\" property")
>> ---
>>  drivers/perf/arm_pmu.c |2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Thanks, these two look good to me. I'll queue them up as fixes and hopefully
> they'll land in -rc5.

FWIW, I tested this on bcm2835-rpi and verified it fixes the boot
problem in mainline.

Tested-by: Kevin Hilman 

Kevin

Re: [PATCH 2/2] drivers/perf: arm_pmu: Fix NULL pointer dereference during probe

[Kevin Hilman]

Will Deacon  writes:

> On Sat, Aug 27, 2016 at 04:19:50PM +, Stefan Wahren wrote:
>> Patch 7f1d642fbb5c ("drivers/perf: arm-pmu: Fix handling of SPI lacking
>> interrupt-affinity property") unintended also fixes perf_event support
>> for bcm2835 which doesn't have PMU interrupts. Unfortunately this change
>> introduce a NULL pointer dereference on bcm2835, because irq_is_percpu
>> always expected to be called with a valid IRQ. So fix this regression
>> by validating the IRQ before.
>> 
>> Signed-off-by: Stefan Wahren 
>> Fixes: 7f1d642fbb5c ("drivers/perf: arm-pmu: Fix handling of SPI lacking 
>> \"interrupt-affinity\" property")
>> ---
>>  drivers/perf/arm_pmu.c |2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> Thanks, these two look good to me. I'll queue them up as fixes and hopefully
> they'll land in -rc5.

FWIW, I tested this on bcm2835-rpi and verified it fixes the boot
problem in mainline.

Tested-by: Kevin Hilman 

Kevin

Re: [RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

在 2016/8/31 19:48, Arnd Bergmann 写道:

On Wednesday, August 31, 2016 7:48:14 PM CEST Dongdong Liu wrote:

+static struct hisi_rc_res rc_res[] = {
+   {
+   HIP05,
+   {
+   DEFINE_RES_MEM(0xb007, SZ_4K),
+   DEFINE_RES_MEM(0xb008, SZ_4K),
+   DEFINE_RES_MEM(0xb009, SZ_4K),
+   DEFINE_RES_MEM(0xb00a, SZ_4K)
+   }
+   },
+   {
+   HIP06,
+   {
+   DEFINE_RES_MEM(0xa009, SZ_4K),
+   DEFINE_RES_MEM(0xa020, SZ_4K),
+   DEFINE_RES_MEM(0xa00a, SZ_4K),
+   DEFINE_RES_MEM(0xa00b, SZ_4K)
+   }
+   },
+   {
+   HIP07,
+   {
+   DEFINE_RES_MEM(0xa009, SZ_4K),
+   DEFINE_RES_MEM(0xa020, SZ_4K),
+   DEFINE_RES_MEM(0xa00a, SZ_4K),
+   DEFINE_RES_MEM(0xa00b, SZ_4K),
+   DEFINE_RES_MEM(0x8a009UL, SZ_4K),
+   DEFINE_RES_MEM(0x8a020UL, SZ_4K),
+   DEFINE_RES_MEM(0x8a00aUL, SZ_4K),
+   DEFINE_RES_MEM(0x8a00bUL, SZ_4K),
+   DEFINE_RES_MEM(0x600a009UL, SZ_4K),
+   DEFINE_RES_MEM(0x600a020UL, SZ_4K),
+   DEFINE_RES_MEM(0x600a00aUL, SZ_4K),
+   DEFINE_RES_MEM(0x600a00bUL, SZ_4K),
+   DEFINE_RES_MEM(0x700a009UL, SZ_4K),
+   DEFINE_RES_MEM(0x700a020UL, SZ_4K),
+   DEFINE_RES_MEM(0x700a00aUL, SZ_4K),
+   DEFINE_RES_MEM(0x700a00bUL, SZ_4K)
+   }
+   },


I don't know much about ACPI, but I'm pretty sure this is not
the normal way to find MMIO resources. Why not read them from
the ACPI tables?



Hi Arnd

Our host bridge is non ECAM only for the RC bus config space;
for any other bus underneath the root bus we support ECAM access.

We have not found a comfortable ACPI way to describle RC itself config (not 
ECAM) resource .


Thanks
Dongdong


Arnd

.

Re: [RFC PATCH V2 3/3] PCI/ACPI: hisi: Add ACPI support for HiSilicon SoCs Host Controllers

[Dongdong Liu]

在 2016/8/31 19:48, Arnd Bergmann 写道:

On Wednesday, August 31, 2016 7:48:14 PM CEST Dongdong Liu wrote:

+static struct hisi_rc_res rc_res[] = {
+   {
+   HIP05,
+   {
+   DEFINE_RES_MEM(0xb007, SZ_4K),
+   DEFINE_RES_MEM(0xb008, SZ_4K),
+   DEFINE_RES_MEM(0xb009, SZ_4K),
+   DEFINE_RES_MEM(0xb00a, SZ_4K)
+   }
+   },
+   {
+   HIP06,
+   {
+   DEFINE_RES_MEM(0xa009, SZ_4K),
+   DEFINE_RES_MEM(0xa020, SZ_4K),
+   DEFINE_RES_MEM(0xa00a, SZ_4K),
+   DEFINE_RES_MEM(0xa00b, SZ_4K)
+   }
+   },
+   {
+   HIP07,
+   {
+   DEFINE_RES_MEM(0xa009, SZ_4K),
+   DEFINE_RES_MEM(0xa020, SZ_4K),
+   DEFINE_RES_MEM(0xa00a, SZ_4K),
+   DEFINE_RES_MEM(0xa00b, SZ_4K),
+   DEFINE_RES_MEM(0x8a009UL, SZ_4K),
+   DEFINE_RES_MEM(0x8a020UL, SZ_4K),
+   DEFINE_RES_MEM(0x8a00aUL, SZ_4K),
+   DEFINE_RES_MEM(0x8a00bUL, SZ_4K),
+   DEFINE_RES_MEM(0x600a009UL, SZ_4K),
+   DEFINE_RES_MEM(0x600a020UL, SZ_4K),
+   DEFINE_RES_MEM(0x600a00aUL, SZ_4K),
+   DEFINE_RES_MEM(0x600a00bUL, SZ_4K),
+   DEFINE_RES_MEM(0x700a009UL, SZ_4K),
+   DEFINE_RES_MEM(0x700a020UL, SZ_4K),
+   DEFINE_RES_MEM(0x700a00aUL, SZ_4K),
+   DEFINE_RES_MEM(0x700a00bUL, SZ_4K)
+   }
+   },


I don't know much about ACPI, but I'm pretty sure this is not
the normal way to find MMIO resources. Why not read them from
the ACPI tables?



Hi Arnd

Our host bridge is non ECAM only for the RC bus config space;
for any other bus underneath the root bus we support ECAM access.

We have not found a comfortable ACPI way to describle RC itself config (not 
ECAM) resource .


Thanks
Dongdong


Arnd

.

Re: [PATCH] i2c: rk3x: Restore clock settings at resume time

[David.Wu]

Hi Doug,

Last replied email HTML HEAD error, resend it.

在 2016/8/30 5:22, Douglas Anderson 写道:

Depending on a number of factors including:
- Which exact Rockchip SoC we're working with
- How deep we suspend
- Which i2c port we're on

We might lose the state of the i2c registers at suspend time.
Specifically we've found that on rk3399 the i2c ports that are not in
the PMU power domain lose their state with the current suspend depth
configured by ARM Tursted Firmware.

Note that there are very few actual i2c registers that aren't configured
per transfer anyway so all we actually need to re-configure are the
clock config registers.  We'll just add a call to rk3x_i2c_adapt_div()
at resume time and be done with it.

NOTE: On rk3399 on ports whose power was lost, I put printouts in at
resume time.  I saw things like:
  before: con=0x00010300, div=0x00060006
  after:  con=0x00010200, div=0x00180025



I do the suspend/resume stress test on my rk3399 & rk3288 board more 
than 24 hours,


the patch works well, it looks nice to me.

Reviewed-by: David Wu 
Tested-by: David Wu 


Signed-off-by: Douglas Anderson 
---
Tested on chromeos-4.4 kernel with backports.

drivers/i2c/busses/i2c-rk3x.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 3b87afe82f63..0df5ad6bfa31 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -,6 +,15 @@ static int rk3x_i2c_xfer(struct i2c_adapter *adap,
return ret < 0 ? ret : num;
 }

+static __maybe_unused int rk3x_i2c_resume(struct device *dev)
+{
+   struct rk3x_i2c *i2c = dev_get_drvdata(dev);
+
+   rk3x_i2c_adapt_div(i2c, clk_get_rate(i2c->clk));
+
+   return 0;
+}
+
 static u32 rk3x_i2c_func(struct i2c_adapter *adap)
 {
return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
@@ -1332,12 +1341,15 @@ static int rk3x_i2c_remove(struct platform_device *pdev)
return 0;
 }

+static const SIMPLE_DEV_PM_OPS(rk3x_i2c_pm_ops, NULL, rk3x_i2c_resume);
+
 static struct platform_driver rk3x_i2c_driver = {
.probe   = rk3x_i2c_probe,
.remove  = rk3x_i2c_remove,
.driver  = {
.name  = "rk3x-i2c",
.of_match_table = rk3x_i2c_match,
+   .pm = _i2c_pm_ops,
},
 };

Re: [PATCH] i2c: rk3x: Restore clock settings at resume time

[David.Wu]

Hi Doug,

Last replied email HTML HEAD error, resend it.

在 2016/8/30 5:22, Douglas Anderson 写道:

Depending on a number of factors including:
- Which exact Rockchip SoC we're working with
- How deep we suspend
- Which i2c port we're on

We might lose the state of the i2c registers at suspend time.
Specifically we've found that on rk3399 the i2c ports that are not in
the PMU power domain lose their state with the current suspend depth
configured by ARM Tursted Firmware.

Note that there are very few actual i2c registers that aren't configured
per transfer anyway so all we actually need to re-configure are the
clock config registers.  We'll just add a call to rk3x_i2c_adapt_div()
at resume time and be done with it.

NOTE: On rk3399 on ports whose power was lost, I put printouts in at
resume time.  I saw things like:
  before: con=0x00010300, div=0x00060006
  after:  con=0x00010200, div=0x00180025



I do the suspend/resume stress test on my rk3399 & rk3288 board more 
than 24 hours,


the patch works well, it looks nice to me.

Reviewed-by: David Wu 
Tested-by: David Wu 


Signed-off-by: Douglas Anderson 
---
Tested on chromeos-4.4 kernel with backports.

drivers/i2c/busses/i2c-rk3x.c | 12 
 1 file changed, 12 insertions(+)

diff --git a/drivers/i2c/busses/i2c-rk3x.c b/drivers/i2c/busses/i2c-rk3x.c
index 3b87afe82f63..0df5ad6bfa31 100644
--- a/drivers/i2c/busses/i2c-rk3x.c
+++ b/drivers/i2c/busses/i2c-rk3x.c
@@ -,6 +,15 @@ static int rk3x_i2c_xfer(struct i2c_adapter *adap,
return ret < 0 ? ret : num;
 }

+static __maybe_unused int rk3x_i2c_resume(struct device *dev)
+{
+   struct rk3x_i2c *i2c = dev_get_drvdata(dev);
+
+   rk3x_i2c_adapt_div(i2c, clk_get_rate(i2c->clk));
+
+   return 0;
+}
+
 static u32 rk3x_i2c_func(struct i2c_adapter *adap)
 {
return I2C_FUNC_I2C | I2C_FUNC_SMBUS_EMUL | I2C_FUNC_PROTOCOL_MANGLING;
@@ -1332,12 +1341,15 @@ static int rk3x_i2c_remove(struct platform_device *pdev)
return 0;
 }

+static const SIMPLE_DEV_PM_OPS(rk3x_i2c_pm_ops, NULL, rk3x_i2c_resume);
+
 static struct platform_driver rk3x_i2c_driver = {
.probe   = rk3x_i2c_probe,
.remove  = rk3x_i2c_remove,
.driver  = {
.name  = "rk3x-i2c",
.of_match_table = rk3x_i2c_match,
+   .pm = _i2c_pm_ops,
},
 };

Re: [PATCH v2 00/11] staging: octeon: multi rx group (queue) support

[Ed Swierk]

On 8/31/16 13:57, Aaro Koskinen wrote:
> This series implements multiple RX group support that should improve
> the networking performance on multi-core OCTEONs. Basically we register
> IRQ and NAPI for each group, and ask the HW to select the group for
> the incoming packets based on hash.
> 
> Tested on EdgeRouter Lite with a simple forwarding test using two flows
> and 16 RX groups distributed between two cores - the routing throughput
> is roughly doubled.
> 
> Also tested with EBH5600 (8 cores) and EBB6800 (16 cores) by sending
> and receiving traffic in both directions using SGMII interfaces.

With this series on 4.4.19, rx works with receive_group_order > 0.
Setting receive_group_order=4, I do see 16 Ethernet interrupts. I tried
fiddling with various smp_affinity values (e.g. setting them all to
, or assigning a different one to each interrupt, or giving a
few to some and a few to others), as well as different values for
rps_cpus. 10-thread parallel iperf performance varies between 0.5 and 1.5
Gbit/sec total depending on the particular settings.

With the SDK kernel I get over 8 Gbit/sec. It seems to be achieving that
using just one interrupt (not even a separate one for tx, as far as I can
tell) pegged to CPU 0 (the default smp_affinity). I must be missing some
other major configuration tweak, perhaps specific to 10G.

Can you run a test on the EBB6800 with the interfaces in 10G mode?

--Ed

Re: [PATCH v2 00/11] staging: octeon: multi rx group (queue) support

[Ed Swierk]

On 8/31/16 13:57, Aaro Koskinen wrote:
> This series implements multiple RX group support that should improve
> the networking performance on multi-core OCTEONs. Basically we register
> IRQ and NAPI for each group, and ask the HW to select the group for
> the incoming packets based on hash.
> 
> Tested on EdgeRouter Lite with a simple forwarding test using two flows
> and 16 RX groups distributed between two cores - the routing throughput
> is roughly doubled.
> 
> Also tested with EBH5600 (8 cores) and EBB6800 (16 cores) by sending
> and receiving traffic in both directions using SGMII interfaces.

With this series on 4.4.19, rx works with receive_group_order > 0.
Setting receive_group_order=4, I do see 16 Ethernet interrupts. I tried
fiddling with various smp_affinity values (e.g. setting them all to
, or assigning a different one to each interrupt, or giving a
few to some and a few to others), as well as different values for
rps_cpus. 10-thread parallel iperf performance varies between 0.5 and 1.5
Gbit/sec total depending on the particular settings.

With the SDK kernel I get over 8 Gbit/sec. It seems to be achieving that
using just one interrupt (not even a separate one for tx, as far as I can
tell) pegged to CPU 0 (the default smp_affinity). I must be missing some
other major configuration tweak, perhaps specific to 10G.

Can you run a test on the EBB6800 with the interfaces in 10G mode?

--Ed

Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Dongdong Liu]

在 2016/8/31 19:45, Arnd Bergmann 写道:

On Wednesday, August 31, 2016 7:48:12 PM CEST Dongdong Liu wrote:

+
+/* HipXX PCIe host only supports 32-bit config access */
+int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
+ u32 *val)
+{
+   u32 reg;
+   u32 reg_val;
+   void *walker = _val;
+
+   walker += (where & 0x3);
+   reg = where & ~0x3;
+   reg_val = readl(reg_base + reg);
+
+   if (size == 1)
+   *val = *(u8 __force *) walker;
+   else if (size == 2)
+   *val = *(u16 __force *) walker;


What is the __force for?


Hi Arnd, thanks for replying.

__force is used to, well, force a conversion, like casting from or to a bitwise 
type, else the Sparse checker will throw a warning.




+   else if (size == 4)
+   *val = reg_val;
+   else
+   return PCIBIOS_BAD_REGISTER_NUMBER;
+
+   return PCIBIOS_SUCCESSFUL;


It looks like you are reimplementing 
pci_generic_config_read32/pci_generic_config_write32
read here, better use them directly.



For our host bridge, access RC and EP config space are not the same way.
Our host bridge is non ECAM only for the RC bus config space;
for any other bus underneath the root bus we support ECAM access.

hisi_pcie_common_cfg_read is used to read RC config space, only supports 32-bit 
config access.
hisi_pcie_common_cfg_read/hisi_pcie_common_cfg_write may change as below will 
be better.

/* HipXX PCIe host only supports 32-bit config access */
int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
  u32 *val)
{
void __iomem *addr;

addr = reg_base + (where & ~0x3);
*val = readl(addr);

if (size <= 2)
*val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);

return PCIBIOS_SUCCESSFUL;
}

/* HipXX PCIe host only supports 32-bit config access */
int hisi_pcie_common_cfg_write(void __iomem *reg_base, int where, int  size,
u32 val)
{
void __iomem *addr;
u32 mask, tmp;

addr = reg_base + (where & ~0x3);
if (size == 4) {
writel(val, addr);
return PCIBIOS_SUCCESSFUL;
} else {
mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
}

tmp = readl(addr) & mask;
tmp |= val << ((where & 0x3) * 8);
writel(tmp, addr);

return PCIBIOS_SUCCESSFUL;
}


Thanks
Dongdong


Arnd

.

Re: [RFC PATCH V2 1/3] PCI: hisi: re-architect Hip05/Hip06 controllers driver to preapare for ACPI

[Dongdong Liu]

在 2016/8/31 19:45, Arnd Bergmann 写道:

On Wednesday, August 31, 2016 7:48:12 PM CEST Dongdong Liu wrote:

+
+/* HipXX PCIe host only supports 32-bit config access */
+int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
+ u32 *val)
+{
+   u32 reg;
+   u32 reg_val;
+   void *walker = _val;
+
+   walker += (where & 0x3);
+   reg = where & ~0x3;
+   reg_val = readl(reg_base + reg);
+
+   if (size == 1)
+   *val = *(u8 __force *) walker;
+   else if (size == 2)
+   *val = *(u16 __force *) walker;


What is the __force for?


Hi Arnd, thanks for replying.

__force is used to, well, force a conversion, like casting from or to a bitwise 
type, else the Sparse checker will throw a warning.




+   else if (size == 4)
+   *val = reg_val;
+   else
+   return PCIBIOS_BAD_REGISTER_NUMBER;
+
+   return PCIBIOS_SUCCESSFUL;


It looks like you are reimplementing 
pci_generic_config_read32/pci_generic_config_write32
read here, better use them directly.



For our host bridge, access RC and EP config space are not the same way.
Our host bridge is non ECAM only for the RC bus config space;
for any other bus underneath the root bus we support ECAM access.

hisi_pcie_common_cfg_read is used to read RC config space, only supports 32-bit 
config access.
hisi_pcie_common_cfg_read/hisi_pcie_common_cfg_write may change as below will 
be better.

/* HipXX PCIe host only supports 32-bit config access */
int hisi_pcie_common_cfg_read(void __iomem *reg_base, int where, int size,
  u32 *val)
{
void __iomem *addr;

addr = reg_base + (where & ~0x3);
*val = readl(addr);

if (size <= 2)
*val = (*val >> (8 * (where & 3))) & ((1 << (size * 8)) - 1);

return PCIBIOS_SUCCESSFUL;
}

/* HipXX PCIe host only supports 32-bit config access */
int hisi_pcie_common_cfg_write(void __iomem *reg_base, int where, int  size,
u32 val)
{
void __iomem *addr;
u32 mask, tmp;

addr = reg_base + (where & ~0x3);
if (size == 4) {
writel(val, addr);
return PCIBIOS_SUCCESSFUL;
} else {
mask = ~(((1 << (size * 8)) - 1) << ((where & 0x3) * 8));
}

tmp = readl(addr) & mask;
tmp |= val << ((where & 0x3) * 8);
writel(tmp, addr);

return PCIBIOS_SUCCESSFUL;
}


Thanks
Dongdong


Arnd

.

Re: [PATCH 2/2] staging: lustre: hide unused variable

[James Simmons]

> After a code cleanup, we get a harmless warning about a variable
> that is unused when CONFIG_FS_POSIX_ACL is disabled:
> 
> drivers/staging/lustre/lustre/llite/xattr.c: In function 
> 'll_xattr_get_common':
> drivers/staging/lustre/lustre/llite/xattr.c:312:24: error: unused variable 
> 'lli' [-Werror=unused-variable]
> 
> This puts the variable declaration into the same #ifdef.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: 1e1f9ff406fd ("staging: lustre: llite: break ll_getxattr_common into 2 
> functions")

Reviewed-by: James Simmons 

> ---
>  drivers/staging/lustre/lustre/llite/xattr.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/xattr.c 
> b/drivers/staging/lustre/lustre/llite/xattr.c
> index f252c26ec30f..7b8d4699a71a 100644
> --- a/drivers/staging/lustre/lustre/llite/xattr.c
> +++ b/drivers/staging/lustre/lustre/llite/xattr.c
> @@ -309,7 +309,9 @@ static int ll_xattr_get_common(const struct xattr_handler 
> *handler,
>  {
>   char fullname[strlen(handler->prefix) + strlen(name) + 1];
>   struct ll_sb_info *sbi = ll_i2sbi(inode);
> +#ifdef CONFIG_FS_POSIX_ACL
>   struct ll_inode_info *lli = ll_i2info(inode);
> +#endif
>   int rc;
>  
>   CDEBUG(D_VFSTRACE, "VFS Op:inode="DFID"(%p)\n",
> -- 
> 2.9.0
> 
> 

Re: [PATCH 2/2] staging: lustre: hide unused variable

[James Simmons]

> After a code cleanup, we get a harmless warning about a variable
> that is unused when CONFIG_FS_POSIX_ACL is disabled:
> 
> drivers/staging/lustre/lustre/llite/xattr.c: In function 
> 'll_xattr_get_common':
> drivers/staging/lustre/lustre/llite/xattr.c:312:24: error: unused variable 
> 'lli' [-Werror=unused-variable]
> 
> This puts the variable declaration into the same #ifdef.
> 
> Signed-off-by: Arnd Bergmann 
> Fixes: 1e1f9ff406fd ("staging: lustre: llite: break ll_getxattr_common into 2 
> functions")

Reviewed-by: James Simmons 

> ---
>  drivers/staging/lustre/lustre/llite/xattr.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/staging/lustre/lustre/llite/xattr.c 
> b/drivers/staging/lustre/lustre/llite/xattr.c
> index f252c26ec30f..7b8d4699a71a 100644
> --- a/drivers/staging/lustre/lustre/llite/xattr.c
> +++ b/drivers/staging/lustre/lustre/llite/xattr.c
> @@ -309,7 +309,9 @@ static int ll_xattr_get_common(const struct xattr_handler 
> *handler,
>  {
>   char fullname[strlen(handler->prefix) + strlen(name) + 1];
>   struct ll_sb_info *sbi = ll_i2sbi(inode);
> +#ifdef CONFIG_FS_POSIX_ACL
>   struct ll_inode_info *lli = ll_i2info(inode);
> +#endif
>   int rc;
>  
>   CDEBUG(D_VFSTRACE, "VFS Op:inode="DFID"(%p)\n",
> -- 
> 2.9.0
> 
>