Re: [PATCH V2 62/63] clocksource/drivers/oxnas-rps: Convert init function to return error
On 06/16/2016 11:27 PM, Daniel Lezcano wrote: > The init functions do not return any error. They behave as the following: > > - panic, thus leading to a kernel crash while another timer may work and >make the system boot up correctly > > or > > - print an error and let the caller unaware if the state of the system > > Change that by converting the init functions to return an error conforming > to the CLOCKSOURCE_OF_RET prototype. > > Proper error handling (rollback, errno value) will be changed later case > by case, thus this change just return back an error or success in the init > function. > > Signed-off-by: Daniel Lezcano > --- > drivers/clocksource/timer-oxnas-rps.c | 33 - > 1 file changed, 20 insertions(+), 13 deletions(-) Acked-by: Neil Armstrong Thanks, Neil
Re: [kernel-hardening] [PATCH 2/2] security,perf: Allow further restriction of perf_event_open
On Thu, Jun 16, 2016 at 03:27:55PM -0700, Kees Cook wrote: > Hi guys, > > This patch wasn't originally CCed to you (I'm fixing that now). Would > you consider taking this into the perf tree? No. > It's been in active use > in both Debian and Android for a while now. Very nice of you all to finally inform us I suppose :/ > >>> When kernel.perf_event_open is set to 3 (or greater), disallow all > >>> access to performance events by users without CAP_SYS_ADMIN. > >>> Add a Kconfig symbol CONFIG_SECURITY_PERF_EVENTS_RESTRICT that > >>> makes this value the default. > >>> > >>> This is based on a similar feature in grsecurity > >>> (CONFIG_GRKERNSEC_PERF_HARDEN). This version doesn't include making > >>> the variable read-only. It also allows enabling further restriction > >>> at run-time regardless of whether the default is changed. This Changelog is completely devoid of information. _WHY_ are you doing this? Also, hate the CONFIG.
Re: [PATCH V2 35/63] clocksource/drivers/time-lpc32xx: Convert init function to return error
Hello Daniel,
$Subject ~= s/lpc32xx/efm32/
On Thu, Jun 16, 2016 at 11:26:54PM +0200, Daniel Lezcano wrote:
> The init functions do not return any error. They behave as the following:
>
> - panic, thus leading to a kernel crash while another timer may work and
>make the system boot up correctly
>
> or
>
> - print an error and let the caller unaware if the state of the system
These are both wrong for efm32. It doesn't panic and doesn't print an
error message (obviously the "let the caller unaware" part is true).
> Change that by converting the init functions to return an error conforming
> to the CLOCKSOURCE_OF_RET prototype.
>
> Proper error handling (rollback, errno value) will be changed later case
> by case, thus this change just return back an error or success in the init
> function.
Apart from the comment below the error handling is already fine in this
driver.
> Signed-off-by: Daniel Lezcano
> ---
> drivers/clocksource/time-efm32.c | 20 +---
> 1 file changed, 13 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/clocksource/time-efm32.c
> b/drivers/clocksource/time-efm32.c
> index b06e4c2..b71ffc6 100644
> --- a/drivers/clocksource/time-efm32.c
> +++ b/drivers/clocksource/time-efm32.c
> @@ -233,7 +233,11 @@ static int __init efm32_clockevent_init(struct
> device_node *np)
> DIV_ROUND_CLOSEST(rate, 1024),
> 0xf, 0x);
>
> - setup_irq(irq, &efm32_clock_event_irq);
> + ret = setup_irq(irq, &efm32_clock_event_irq);
> + if (ret) {
> + pr_err("Failed setup irq");
> + goto err_get_irq;
I would prefer to introduce another label "err_setup_irq" at the same
place as err_get_irq and use it here.
> + }
>
> return 0;
>
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/ |
Re: [PATCH 3/6] [media] s5p-jpeg: set capablity bus_info as required by VIDIOC_QUERYCAP
Hi Javier, On 06/16/2016 11:40 PM, Javier Martinez Canillas wrote: The driver doesn't set the struct v4l2_capability cap_info field so the v4l2-compliance tool reports the following errors for VIDIOC_QUERYCAP: Required ioctls: VIDIOC_QUERYCAP returned 0 (Success) fail: v4l2-compliance.cpp(304): string empty fail: v4l2-compliance.cpp(528): check_ustring(vcap.bus_info, sizeof(vcap.bus_info)) test VIDIOC_QUERYCAP: FAIL This patch fixes by setting the field in VIDIOC_QUERYCAP ioctl handler: Required ioctls: VIDIOC_QUERYCAP returned 0 (Success) test VIDIOC_QUERYCAP: OK Signed-off-by: Javier Martinez Canillas --- drivers/media/platform/s5p-jpeg/jpeg-core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c b/drivers/media/platform/s5p-jpeg/jpeg-core.c index 17bc94092864..e3ff3d4bd72e 100644 --- a/drivers/media/platform/s5p-jpeg/jpeg-core.c +++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c @@ -1256,7 +1256,8 @@ static int s5p_jpeg_querycap(struct file *file, void *priv, strlcpy(cap->card, S5P_JPEG_M2M_NAME " decoder", sizeof(cap->card)); } - cap->bus_info[0] = 0; + snprintf(cap->bus_info, sizeof(cap->bus_info), "platform:%s", +dev_name(ctx->jpeg->dev)); cap->device_caps = V4L2_CAP_STREAMING | V4L2_CAP_VIDEO_M2M; cap->capabilities = cap->device_caps | V4L2_CAP_DEVICE_CAPS; return 0; Acked-by: Jacek Anaszewski -- Best regards, Jacek Anaszewski
Re: [PATCH 4/6] [media] s5p-jpeg: only fill driver's name in capabilities driver field
Hi Javier,
On 06/16/2016 11:40 PM, Javier Martinez Canillas wrote:
The driver fills in both the struct v4l2_capability driver and card fields
the same values, that is the driver's name plus the information if the dev
is a decoder or an encoder.
But the driver field has a fixed length of 16 bytes so the filled data is
truncated:
Driver Info (not using libv4l2):
Driver name : s5p-jpeg decode
Card type : s5p-jpeg decoder
Bus info : platform:11f5.jpeg
Driver version: 4.7.0
Also, this field should only contain the driver's name so use just that.
The information if the device is a decoder or an encoder is in the card
type field anyways.
Signed-off-by: Javier Martinez Canillas
---
drivers/media/platform/s5p-jpeg/jpeg-core.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/media/platform/s5p-jpeg/jpeg-core.c
b/drivers/media/platform/s5p-jpeg/jpeg-core.c
index e3ff3d4bd72e..f9fb52a53e79 100644
--- a/drivers/media/platform/s5p-jpeg/jpeg-core.c
+++ b/drivers/media/platform/s5p-jpeg/jpeg-core.c
@@ -1246,12 +1246,12 @@ static int s5p_jpeg_querycap(struct file *file, void
*priv,
struct s5p_jpeg_ctx *ctx = fh_to_ctx(priv);
if (ctx->mode == S5P_JPEG_ENCODE) {
- strlcpy(cap->driver, S5P_JPEG_M2M_NAME " encoder",
+ strlcpy(cap->driver, S5P_JPEG_M2M_NAME,
sizeof(cap->driver));
strlcpy(cap->card, S5P_JPEG_M2M_NAME " encoder",
sizeof(cap->card));
} else {
- strlcpy(cap->driver, S5P_JPEG_M2M_NAME " decoder",
+ strlcpy(cap->driver, S5P_JPEG_M2M_NAME,
sizeof(cap->driver));
strlcpy(cap->card, S5P_JPEG_M2M_NAME " decoder",
sizeof(cap->card));
Acked-by: Jacek Anaszewski
--
Best regards,
Jacek Anaszewski
Re: [PATCH v6 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
Hi Guenter,
On 2016/6/17 12:59, Guenter Roeck wrote:
On 06/16/2016 07:09 PM, Frank Wang wrote:
The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
than rk3288 and before, and most of phy-related registers are also
different from the past, so a new phy driver is required necessarily.
Signed-off-by: Frank Wang
Suggested-by: Guenter Roeck
Suggested-by: Doug Anderson
Reviewed-by: Heiko Stuebner
Tested-by: Heiko Stuebner
---
[ ... ]
+
+static int rockchip_usb2phy_resume(struct phy *phy)
+{
+struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
+struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
+int ret;
+
+dev_dbg(&rport->phy->dev, "port resume\n");
+
+ret = clk_prepare_enable(rphy->clk480m);
+if (ret)
+return ret;
+
If suspend can be called multiple times, resume can be called
multiple times as well. Doesn't this cause a clock imbalance
if you call clk_prepare_enable() multiple times on resume,
but clk_disable_unprepare() only once on suspend ?
Well, what you said is reasonable, How does something like below?
@@ -307,6 +307,9 @@ static int rockchip_usb2phy_resume(struct phy *phy)
dev_dbg(&rport->phy->dev, "port resume\n");
+ if (!rport->suspended)
+ return 0;
+
ret = clk_prepare_enable(rphy->clk480m);
if (ret)
return ret;
@@ -327,12 +330,16 @@ static int rockchip_usb2phy_suspend(struct phy *phy)
dev_dbg(&rport->phy->dev, "port suspend\n");
+ if (rport->suspended)
+ return 0;
+
ret = property_enable(rphy, &rport->port_cfg->phy_sus, true);
if (ret)
return ret;
rport->suspended = true;
clk_disable_unprepare(rphy->clk480m);
+
return 0;
}
@@ -485,6 +492,7 @@ static int rockchip_usb2phy_host_port_init(struct
rockchip_usb2phy *rphy,
rport->port_id = USB2PHY_PORT_HOST;
rport->port_cfg = &rphy->phy_cfg->port_cfgs[USB2PHY_PORT_HOST];
+ rport->suspended = true;
mutex_init(&rport->mutex);
INIT_DELAYED_WORK(&rport->sm_work, rockchip_usb2phy_sm_work);
+ret = property_enable(rphy, &rport->port_cfg->phy_sus, false);
+if (ret)
+return ret;
+
+rport->suspended = false;
+return 0;
+}
+
+static int rockchip_usb2phy_suspend(struct phy *phy)
+{
+struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
+struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
+int ret;
+
+dev_dbg(&rport->phy->dev, "port suspend\n");
+
+if (rport->suspended)
+goto exit;
+
I know I am nitpicking, but
return 0;
would be fine here, be more consistent with the rest of the code,
Yeah, please see above changes.
BR.
Frank
+ret = property_enable(rphy, &rport->port_cfg->phy_sus, true);
+if (ret)
+return ret;
+
+rport->suspended = true;
+clk_disable_unprepare(rphy->clk480m);
+
+exit:
+return 0;
and this label is really unnecessary.
+}
+
[ ... ]
Re: [PATCH 2/2] clk: Add fractional scale clock support
On Fri, Jun 17, 2016 at 1:40 AM, Hoan Tran wrote:
> This patch adds fractional scale clock support.
> Fractional scale clock is implemented for a single register field.
> Output rate = parent_rate * scale / denominator
> For example, for 1 / 8 fractional scale, denominator will be 8 and scale
> will be computed and programmed accordingly.
>
> Signed-off-by: Hoan Tran
> Signed-off-by: Loc Ho
> --- /dev/null
> +++ b/drivers/clk/clk-fractional-scale.c
> +static long clk_fs_round_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long *parent_rate)
> +{
> + struct clk_fractional_scale *fd = to_clk_sf(hw);
> + u64 ret, scale;
> +
> + if (!rate || rate >= *parent_rate)
> + return *parent_rate;
> +
> + /* freq = parent_rate * scaler / denom */
> + ret = rate * fd->denom;
Can this overflow? No, because fd->denom is u64.
But if fd->denom is changed to u32, it needs a cast to u64 here.
> + scale = ret / *parent_rate;
As detected by the kbuild test robot, this should use do_div().
> +
> + ret = (u64) *parent_rate * scale;
> + do_div(ret, fd->denom);
> +
> + return ret;
> +}
> +
> +static int clk_fs_set_rate(struct clk_hw *hw, unsigned long rate,
> + unsigned long parent_rate)
> +{
> + struct clk_fractional_scale *fd = to_clk_sf(hw);
> + unsigned long flags = 0;
> + u64 scale, ret;
> + u32 val;
> +
> + /*
> +* Compute the scaler:
> +*
> +* freq = parent_rate * scaler / denom, or
> +* scaler = freq * denom / parent_rate
> +*/
> + ret = rate * fd->denom;
Can this overflow? No, because fd->denom is u64.
But if fd->denom is changed to u32, it needs a cast to u64 here.
> + scale = ret / (u64)parent_rate;
Don't cast the divider to u64, as this will turn a 64-by-32 division into a
64-by-64 division.
As detected by the kbuild test robot, this should use do_div().
> --- a/include/linux/clk-provider.h
> +++ b/include/linux/clk-provider.h
> +struct clk_fractional_scale {
> + struct clk_hw hw;
> + void __iomem*reg;
> + u8 shift;
> + u32 mask;
> + u64 denom;
u64 sounds overkill to me, but it looks like that was done to avoid overflow in
the multiplications?
> + u32 flags;
> + spinlock_t *lock;
> +};
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- ge...@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
Re: [PATCH v4 net] ipvs: fix bind to link-local mcast IPv6 address in backup
Hello,
On Thu, 16 Jun 2016, Quentin Armitage wrote:
> When using HEAD from
> https://git.kernel.org/cgit/utils/kernel/ipvsadm/ipvsadm.git/,
> the command:
> ipvsadm --start-daemon backup --mcast-interface eth0.60 \
> --mcast-group ff02::1:81
> fails with the error message:
> Argument list too long
>
> whereas both:
> ipvsadm --start-daemon master --mcast-interface eth0.60 \
> --mcast-group ff02::1:81
> and:
> ipvsadm --start-daemon backup --mcast-interface eth0.60 \
> --mcast-group 224.0.0.81
> are successful.
>
> The error message "Argument list too long" isn't helpful. The error occurs
> because an IPv6 address is given in backup mode.
>
> The error is in make_receive_sock() in net/netfilter/ipvs/ip_vs_sync.c,
> since it fails to set the interface on the address or the socket before
> calling inet6_bind() (via sock->ops->bind), where the test
> 'if (!sk->sk_bound_dev_if)' failed.
>
> Setting sock->sk->sk_bound_dev_if on the socket before calling
> inet6_bind() resolves the issue.
>
> Fixes: d33288172e72 ("ipvs: add more mcast parameters for the sync daemon")
> Signed-off-by: Quentin Armitage
Looks good to me, thanks!
Acked-by: Julian Anastasov
Simon, please apply to ipvs tree. Patch compiles
also on stable 4.4.13, 4.5.7 and 4.6.2, so no need for
special versions. The ack is also for the other 3 patches
from v4 (for ipvs-next) but they depend on this patch.
> ---
> net/netfilter/ipvs/ip_vs_sync.c |6 --
> 1 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/net/netfilter/ipvs/ip_vs_sync.c b/net/netfilter/ipvs/ip_vs_sync.c
> index 803001a..1b07578 100644
> --- a/net/netfilter/ipvs/ip_vs_sync.c
> +++ b/net/netfilter/ipvs/ip_vs_sync.c
> @@ -1545,7 +1545,8 @@ error:
> /*
> * Set up receiving multicast socket over UDP
> */
> -static struct socket *make_receive_sock(struct netns_ipvs *ipvs, int id)
> +static struct socket *make_receive_sock(struct netns_ipvs *ipvs, int id,
> + int ifindex)
> {
> /* multicast addr */
> union ipvs_sockaddr mcast_addr;
> @@ -1566,6 +1567,7 @@ static struct socket *make_receive_sock(struct
> netns_ipvs *ipvs, int id)
> set_sock_size(sock->sk, 0, result);
>
> get_mcast_sockaddr(&mcast_addr, &salen, &ipvs->bcfg, id);
> + sock->sk->sk_bound_dev_if = ifindex;
> result = sock->ops->bind(sock, (struct sockaddr *)&mcast_addr, salen);
> if (result < 0) {
> pr_err("Error binding to the multicast addr\n");
> @@ -1868,7 +1870,7 @@ int start_sync_thread(struct netns_ipvs *ipvs, struct
> ipvs_sync_daemon_cfg *c,
> if (state == IP_VS_STATE_MASTER)
> sock = make_send_sock(ipvs, id);
> else
> - sock = make_receive_sock(ipvs, id);
> + sock = make_receive_sock(ipvs, id, dev->ifindex);
> if (IS_ERR(sock)) {
> result = PTR_ERR(sock);
> goto outtinfo;
> --
> 1.7.7.6
Regards
--
Julian Anastasov
Re: [PATCH v1 3/3] mm: per-process reclaim
Hi Hannes, On Thu, Jun 16, 2016 at 10:41:02AM -0400, Johannes Weiner wrote: > On Wed, Jun 15, 2016 at 09:40:27AM +0900, Minchan Kim wrote: > > A question is it seems cgroup2 doesn't have per-cgroup swappiness. > > Why? > > > > I think we need it in one-cgroup-per-app model. > > Can you explain why you think that? > > As we have talked about this recently in the LRU balancing thread, > swappiness is the cost factor between file IO and swapping, so the > only situation I can imagine you'd need a memcg swappiness setting is > when you have different cgroups use different storage devices that do > not have comparable speeds. > > So I'm not sure I understand the relationship to an app-group model. Sorry for lacking the inforamtion. I should have written more clear. In fact, what we need is *per-memcg-swap-device*. What I want is to avoid kill background application although memory is overflow because cold launcing of app takes a very long time compared to resume(ie, just switching). I also want to keep a mount of free pages in the memory so that new application startup cannot be stuck by reclaim activities. To get free memory, I want to reclaim less important app rather than killing. In this time, we can support two swap devices. A one is zram, other is slow storage but much bigger than zram size. Then, we can use storage swap to reclaim pages for not-important app while we can use zram swap for for important app(e.g., forground app, system services, daemon and so on). IOW, we want to support mutiple swap device with one-cgroup-per-app and the storage speed is totally different.
Re: [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm.
On 06/17/2016 08:28 AM, Chris Lapa wrote:
> On 17/06/2016 4:26 PM, Krzysztof Kozlowski wrote:
>> On 06/17/2016 07:00 AM, Chris Lapa wrote:
>>> From: Chris Lapa
>>>
>>> The max8903_charger.h file indicated that dcm and dok were not optional
>>> when dc_valid is set.
>>>
>>> It makes sense to have dok as a compulsory pin when dc_valid is given.
>>> However dcm can be optionally wired to a fixed level especially when the
>>> circuit is configured for dc power exclusively.
>>>
>>> The previous implementation already allowed for this somewhat,
>>> however no
>>> error was given if dok wasn't given whilst dc_valid was.
>>>
>>> The new implementation enforces dok presence when dc_valid is given.
>>> Whilst
>>> allowing dcm to be optional.
>>>
>>> Signed-off-by: Chris Lapa
>>> ---
>>> drivers/power/max8903_charger.c | 23 ++-
>>> include/linux/power/max8903_charger.h | 6 +++---
>>> 2 files changed, 13 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/drivers/power/max8903_charger.c
>>> b/drivers/power/max8903_charger.c
>>> index 0a5b0e1..dbd911c4 100644
>>> --- a/drivers/power/max8903_charger.c
>>> +++ b/drivers/power/max8903_charger.c
>>> @@ -211,27 +211,24 @@ static int max8903_probe(struct platform_device
>>> *pdev)
>>> }
>>>
>>> if (pdata->dc_valid) {
>>> -if (pdata->dok && gpio_is_valid(pdata->dok) &&
>>> -pdata->dcm && gpio_is_valid(pdata->dcm)) {
>>> +if (pdata->dok && gpio_is_valid(pdata->dok)) {
>>> gpio = pdata->dok; /* PULL_UPed Interrupt */
>>> ta_in = gpio_get_value(gpio) ? 0 : 1;
>>> +} else {
>>> +dev_err(dev, "When DC is wired, DOK should"
>>> +" be wired as well.\n");
>>
>> Just found one nit. Don't split the strings.
>> dev_err(dev,
>> "When DC is wired, DOK should be wired as well.\n");
> I saw that one as well when I ran checkpatch, however I thought if I
> changed it then I would get a warning about the line being > 80 chars.
> So wasn't sure which direction to go.
Checkpatch shouldn't complain on strings so if you move the string to
next line, it should be ok.
Best regards,
Krzysztof
Re: [PATCH v3 7/7] max8903: adds support for initiation via device tree.
On 06/17/2016 07:00 AM, Chris Lapa wrote:
> From: Chris Lapa
>
> Adds support for device tree to setup a max8903 battery charger. DC and USB
> validity are determined by looking the presence of the dok and uok gpios.
>
> Signed-off-by: Chris Lapa
> ---
> drivers/power/max8903_charger.c | 217
> +++-
> 1 file changed, 145 insertions(+), 72 deletions(-)
>
> diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c
> index 5ddc667..3c59213 100644
> --- a/drivers/power/max8903_charger.c
> +++ b/drivers/power/max8903_charger.c
> @@ -23,6 +23,9 @@
> #include
> #include
> #include
> +#include
> +#include
> +#include
> #include
> #include
> #include
> @@ -75,6 +78,7 @@ static int max8903_get_property(struct power_supply *psy,
> default:
> return -EINVAL;
> }
> +
> return 0;
> }
>
> @@ -179,48 +183,116 @@ static irqreturn_t max8903_fault(int irq, void *_data)
> return IRQ_HANDLED;
> }
>
> +static struct max8903_pdata *max8903_parse_dt_data(
> + struct device *dev)
> +{
> + struct device_node *of_node = dev->of_node;
Common naming convention for device_node is just 'np':
struct device_node *np = dev->of_node;
It is shorter and already spread in the kernel.
> + struct max8903_pdata *pdata = NULL;
> +
> + if (!of_node)
> + return NULL;
> +
> + pdata = devm_kzalloc(dev, sizeof(*pdata),
> + GFP_KERNEL);
No need to break the line.
> + if (!pdata)
> + return NULL;
> +
> + pdata->dc_valid = false;
> + pdata->usb_valid = false;
> +
> + pdata->cen = of_get_named_gpio(of_node, "cen-gpios", 0);
> + if (!gpio_is_valid(pdata->cen))
> + pdata->cen = -EINVAL;
> +
> + pdata->chg = of_get_named_gpio(of_node, "chg-gpios", 0);
> + if (!gpio_is_valid(pdata->chg))
> + pdata->chg = -EINVAL;
> +
> + pdata->flt = of_get_named_gpio(of_node, "flt-gpios", 0);
> + if (!gpio_is_valid(pdata->flt))
> + pdata->flt = -EINVAL;
> +
> + pdata->usus = of_get_named_gpio(of_node, "usus-gpios", 0);
> + if (!gpio_is_valid(pdata->usus))
> + pdata->usus = -EINVAL;
> +
> + pdata->dcm = of_get_named_gpio(of_node, "dcm-gpios", 0);
> + if (!gpio_is_valid(pdata->dcm))
> + pdata->dcm = -EINVAL;
> +
> + pdata->dok = of_get_named_gpio(of_node, "dok-gpios", 0);
> + if (!gpio_is_valid(pdata->dok))
> + pdata->dok = -EINVAL;
> + else
> + pdata->dc_valid = true;
> +
> + pdata->uok = of_get_named_gpio(of_node, "uok-gpios", 0);
> + if (!gpio_is_valid(pdata->uok))
> + pdata->uok = -EINVAL;
> + else
> + pdata->usb_valid = true;
> +
> + return pdata;
> +}
> +
> static int max8903_probe(struct platform_device *pdev)
> {
> - struct max8903_data *data;
> + struct max8903_data *charger;
> struct device *dev = &pdev->dev;
> - struct max8903_pdata *pdata = pdev->dev.platform_data;
> struct power_supply_config psy_cfg = {};
> int ret = 0;
> int gpio;
> int ta_in = 0;
> int usb_in = 0;
>
> - if (pdata == NULL) {
> + charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL);
> + if (!charger)
> + return -ENOMEM;
> +
> + charger->pdata = pdev->dev.platform_data;
> + if (IS_ENABLED(CONFIG_OF) && !charger->pdata && dev->of_node)
> + charger->pdata = max8903_parse_dt_data(dev);
> +
> + if (!charger->pdata) {
> dev_err(dev, "No platform data.\n");
> return -EINVAL;
> }
>
> - data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL);
> - if (!data)
> - return -ENOMEM;
> + charger->dev = dev;
>
> - data->pdata = pdev->dev.platform_data;
> - data->dev = dev;
> - platform_set_drvdata(pdev, data);
> + charger->fault = false;
> + charger->ta_in = ta_in;
> + charger->usb_in = usb_in;
>
> - if (pdata->dc_valid == false && pdata->usb_valid == false) {
> + charger->psy_desc.name = "max8903_charger";
> + charger->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS :
> + ((usb_in) ? POWER_SUPPLY_TYPE_USB :
> + POWER_SUPPLY_TYPE_BATTERY);
> + charger->psy_desc.get_property = max8903_get_property;
> + charger->psy_desc.properties = max8903_charger_props;
> + charger->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props);
> +
> + platform_set_drvdata(pdev, charger);
> +
> + if (charger->pdata->dc_valid == false && charger->pdata->usb_valid ==
> false) {
> dev_err(dev, "No valid power sources.\n");
> return -EINVAL;
> }
>
> - if (pdata->dc_valid) {
> - if (gpio_is_valid(pdata->dok)) {
> +
> + if (charger->pdata->dc_valid) {
> + if (gpio_is_valid(charger->pdata->d
Re: [PATCH] tools/perf: Fix the mask in regs_dump__printf
On Fri, Jun 17, 2016 at 10:52:38AM +0530, Madhavan Srinivasan wrote:
> When decoding the perf_regs mask in regs_dump__printf(),
> we loop through the mask using find_first_bit and find_next_bit functions.
> And mask is of type "u64". But "u64" is send as a "unsigned long *" to
> lib functions along with sizeof().
>
> While the exisitng code works fine in most of the case, when using a
> 32bit perf on a 64bit kernel (Big Endian), we end up reading the wrong word
> in the u64 mask. Patch to fix the mask in regs_dump__printf().
>
> Suggested-by: Yury Norov
> Cc: Yury Norov
> Cc: Peter Zijlstra
> Cc: Ingo Molnar
> Cc: Arnaldo Carvalho de Melo
> Cc: Alexander Shishkin
> Cc: Jiri Olsa
> Cc: Adrian Hunter
> Cc: Kan Liang
> Cc: Wang Nan
> Cc: Michael Ellerman
> Signed-off-by: Madhavan Srinivasan
> ---
> tools/perf/util/session.c | 7 ++-
> 1 file changed, 6 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
> index 5214974e841a..2eaa42a4832a 100644
> --- a/tools/perf/util/session.c
> +++ b/tools/perf/util/session.c
> @@ -940,8 +940,13 @@ static void branch_stack__printf(struct perf_sample
> *sample)
> static void regs_dump__printf(u64 mask, u64 *regs)
> {
> unsigned rid, i = 0;
> + unsigned long _mask[sizeof(mask)/sizeof(unsigned long)];
>
> - for_each_set_bit(rid, (unsigned long *) &mask, sizeof(mask) * 8) {
> + _mask[0] = mask & ULONG_MAX;
> + if (sizeof(mask) > sizeof(unsigned long))
> + _mask[1] = mask >> 32;
> +
I think we should do this earlier when reading the mask,
not at the moment we just print it, like we do for other
types.. maybe we could do this as an extra bit for
perf_event__swap_ops[PERF_RECORD_SAMPLE] function
also there's print_sample_iregs in builtin-script.c that's
most likely affected as well
thanks,
jirka
> + for_each_set_bit(rid, _mask, sizeof(mask) * 8) {
> u64 val = regs[i++];
>
> printf(" %-5s 0x%" PRIx64 "\n",
> --
> 1.9.1
>
Re: [PATCH v3 6/7] max8903: remove unnecessary 'out of memory' error message.
On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa > > Remove the 'out of memory' error message as it is printed by the core. > > Signed-off-by: Chris Lapa > --- > drivers/power/max8903_charger.c | 4 +--- > 1 file changed, 1 insertion(+), 3 deletions(-) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v3 5/7] max8903: removes non zero validity checks on gpios.
On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa > > Prior to this commit a zero gpio was treated as invalid. Whereas > gpio_is_valid() will treat a zero gpio as valid. > > This commit removes the confusion and explicity uses gpio_is_valid() s/explicity/explicitly/ > throughout. Which in turn results in several of the error messages becoming > redundant and thus removed. > > Signed-off-by: Chris Lapa > --- > drivers/power/max8903_charger.c | 125 > ++-- > 1 file changed, 55 insertions(+), 70 deletions(-) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH V2 25/63] clocksource/drivers/mxs_timer: Convert init function to return error
Hello,
On Thu, Jun 16, 2016 at 11:26:44PM +0200, Daniel Lezcano wrote:
> The init functions do not return any error. They behave as the following:
>
> - panic, thus leading to a kernel crash while another timer may work and
>make the system boot up correctly
>
> or
>
> - print an error and let the caller unaware if the state of the system
>
> Change that by converting the init functions to return an error conforming
> to the CLOCKSOURCE_OF_RET prototype.
>
> Proper error handling (rollback, errno value) will be changed later case
> by case, thus this change just return back an error or success in the init
> function.
>
> Signed-off-by: Daniel Lezcano
> ---
> drivers/clocksource/mps2-timer.c | 10 ++
> drivers/clocksource/mxs_timer.c | 2 +-
Did you forget to split here? The changelog only mentions mxs.
> 2 files changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/clocksource/mps2-timer.c
> b/drivers/clocksource/mps2-timer.c
> index 3d33a5e..c303fa9 100644
> --- a/drivers/clocksource/mps2-timer.c
> +++ b/drivers/clocksource/mps2-timer.c
> @@ -250,7 +250,7 @@ out:
> return ret;
> }
>
> -static void __init mps2_timer_init(struct device_node *np)
> +static int __init mps2_timer_init(struct device_node *np)
> {
> static int has_clocksource, has_clockevent;
> int ret;
> @@ -259,7 +259,7 @@ static void __init mps2_timer_init(struct device_node *np)
> ret = mps2_clocksource_init(np);
> if (!ret) {
> has_clocksource = 1;
> - return;
> + return 0;
> }
> }
>
> @@ -267,9 +267,11 @@ static void __init mps2_timer_init(struct device_node
> *np)
> ret = mps2_clockevent_init(np);
> if (!ret) {
> has_clockevent = 1;
> - return;
> + return 0;
> }
> }
> +
> + return 0;
> }
>
> -CLOCKSOURCE_OF_DECLARE(mps2_timer, "arm,mps2-timer", mps2_timer_init);
> +CLOCKSOURCE_OF_DECLARE_RET(mps2_timer, "arm,mps2-timer", mps2_timer_init);
> diff --git a/drivers/clocksource/mxs_timer.c b/drivers/clocksource/mxs_timer.c
> index f5ce296..16cf53b 100644
> --- a/drivers/clocksource/mxs_timer.c
> +++ b/drivers/clocksource/mxs_timer.c
> @@ -226,7 +226,7 @@ static int __init mxs_clocksource_init(struct clk
> *timer_clk)
> return 0;
> }
>
> -static void __init mxs_timer_init(struct device_node *np)
> +static int __init mxs_timer_init(struct device_node *np)
> {
> struct clk *timer_clk;
> int irq;
I didn't try, but I expect that the "return;" a few lines below results
in at least a warning and the compiler also warns about a missing return
at the end of the function.
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | http://www.pengutronix.de/ |
[PATCH v2 02/10] driver core: Functional dependencies tracking support
From: "Rafael J. Wysocki"
Currently, there is a problem with handling cases where functional
dependencies between devices are involved.
What I mean by a "functional dependency" is when the driver of device
B needs both device A and its driver to be present and functional to
be able to work. This implies that the driver of A needs to be
working for B to be probed successfully and it cannot be unbound from
the device before the B's driver. This also has certain consequences
for power management of these devices (suspend/resume and runtime PM
ordering).
Add support for representing those functional dependencies between
devices to allow the driver core to track them and act on them in
certain cases where they matter.
The argument for doing that in the driver core is that there are
quite a few distinct use cases related to that, they are relatively
hard to get right in a driver (if one wants to address all of them
properly) and it only gets worse if multiplied by the number of
drivers potentially needing to do it. Morever, at least one case
(asynchronous system suspend/resume) cannot be handled in a single
driver at all, because it requires the driver of A to wait for B to
suspend (during system suspend) and the driver of B to wait for
A to resume (during system resume).
To that end, represent links between devices (or more precisely
between device+driver combos) as a struct devlink object containing
pointers to the devices in question, a list node for each of them,
status information, flags, a lock and an RCU head for synchronization.
Also add two new list heads, supplier_links and consumer_links, to
struct device to represent the lists of links to the devices that
depend on the given one (consumers) and to the devices depended on
by it (suppliers), respectively.
The entire data structure consisting of all of the lists of link
objects for all devices is protected by SRCU (for list walking)
and a by mutex (for link object addition/removal). In addition
to that, each link object has an internal status field whose
value reflects what's happening to the devices pointed to by
the link. That status field is protected by an internal spinlock.
New links are added by calling device_link_add() which may happen
either before the consumer device is probed or when probing it, in
which case the caller needs to ensure that the driver of the
supplier device is present and functional and the DEVICE_LINK_PROBE_TIME
flag should be passed to device_link_add() to reflect that.
Link objects are deleted either explicitly, by calling
device_link_del() on the link object in question, or automatically,
when the consumer device is unbound from its driver or when one
of the target devices is deleted, depending on the link type.
There are two types of link objects, persistent and non-persistent.
The difference between them is that the persistent links stay around
until one of the target devices is deleted, which the non-persistent
ones are deleted when the consumer driver is unbound from its device
(ie. they are assumed to be valid only as long as the consumer device
has a driver bound to it). The DEVICE_LINK_PERSISTENT flag has to
be passed to device_link_add() so as to create a persistent link.
One of the actions carried out by device_link_add() is to reorder
the lists used for device shutdown and system suspend/resume to
put the consumer device along with all of its children and all of
its consumers (and so on, recursively) to the ends of those list
in order to ensure the right ordering between the all of the supplier
and consumer devices.
Signed-off-by: Rafael J. Wysocki
Signed-off-by: Marek Szyprowski
---
drivers/base/base.h| 11 ++
drivers/base/core.c| 386 +
drivers/base/dd.c | 42 +-
include/linux/device.h | 36 +
4 files changed, 470 insertions(+), 5 deletions(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index e05db388bd1c..cccb1d211541 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -107,6 +107,9 @@ extern void bus_remove_device(struct device *dev);
extern int bus_add_driver(struct device_driver *drv);
extern void bus_remove_driver(struct device_driver *drv);
+extern void device_release_driver_internal(struct device *dev,
+ struct device_driver *drv,
+ struct device *parent);
extern void driver_detach(struct device_driver *drv);
extern int driver_probe_device(struct device_driver *drv, struct device *dev);
@@ -152,3 +155,11 @@ extern int devtmpfs_init(void);
#else
static inline int devtmpfs_init(void) { return 0; }
#endif
+
+/* Device links */
+extern int device_links_check_suppliers(struct device *dev);
+extern void device_links_driver_bound(struct device *dev);
+extern void device_links_driver_gone(struct device *dev);
+extern void device_links_no_driver(struct device *dev);
+extern bool device_links_busy(struct dev
Re: [PATCH v3 4/7] max8903: adds requesting of gpios.
On 06/17/2016 07:00 AM, Chris Lapa wrote:
> From: Chris Lapa
>
> This change ensures all gpios are available for the driver to use.
>
> Signed-off-by: Chris Lapa
> ---
> drivers/power/max8903_charger.c | 79
> -
> 1 file changed, 70 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c
> index dbd911c4..c068efe 100644
> --- a/drivers/power/max8903_charger.c
> +++ b/drivers/power/max8903_charger.c
> @@ -212,6 +212,16 @@ static int max8903_probe(struct platform_device *pdev)
>
> if (pdata->dc_valid) {
> if (pdata->dok && gpio_is_valid(pdata->dok)) {
> + ret = devm_gpio_request(dev,
> + pdata->dok,
No need to split these two arguments. They fit in 80 chars.
> + data->psy_desc.name);
> + if (ret) {
> + dev_err(dev,
> + "Failed GPIO request for dok:
> %d err %d\n",
> + pdata->dok, ret);
Indentation here is wrong. These should be aligned to first argument (dev).
> + return -EINVAL;
Return 'ret'.
All these three comments apply also to code below.
Anyway the probe function is getting bigger and more difficult to read.
Consider factoring out some code to a separate function. For example all
the GPIO handling stuff could be factored.
Best regards,
Krzysztof
> + }
> +
> gpio = pdata->dok; /* PULL_UPed Interrupt */
> ta_in = gpio_get_value(gpio) ? 0 : 1;
> } else {
> @@ -223,6 +233,16 @@ static int max8903_probe(struct platform_device *pdev)
>
> if (pdata->dcm) {
> if (gpio_is_valid(pdata->dcm)) {
> + ret = devm_gpio_request(dev,
> + pdata->dcm,
> + data->psy_desc.name);
> + if (ret) {
> + dev_err(dev,
> + "Failed GPIO request for dcm:
> %d err %d\n",
> + pdata->dcm, ret);
> + return -EINVAL;
> + }
> +
> gpio = pdata->dcm; /* Output */
> gpio_set_value(gpio, ta_in);
> } else {
> @@ -233,6 +253,16 @@ static int max8903_probe(struct platform_device *pdev)
>
> if (pdata->usb_valid) {
> if (pdata->uok && gpio_is_valid(pdata->uok)) {
> + ret = devm_gpio_request(dev,
> + pdata->uok,
> + data->psy_desc.name);
> + if (ret) {
> + dev_err(dev,
> + "Failed GPIO request for uok:
> %d err %d\n",
> + pdata->uok, ret);
> + return -EINVAL;
> + }
> +
> gpio = pdata->uok;
> usb_in = gpio_get_value(gpio) ? 0 : 1;
> } else {
> @@ -244,6 +274,16 @@ static int max8903_probe(struct platform_device *pdev)
>
> if (pdata->cen) {
> if (gpio_is_valid(pdata->cen)) {
> + ret = devm_gpio_request(dev,
> + pdata->cen,
> + data->psy_desc.name);
> + if (ret) {
> + dev_err(dev,
> + "Failed GPIO request for cen:
> %d err %d\n",
> + pdata->cen, ret);
> + return -EINVAL;
> + }
> +
> gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1);
> } else {
> dev_err(dev, "Invalid pin: cen.\n");
> @@ -252,23 +292,44 @@ static int max8903_probe(struct platform_device *pdev)
> }
>
> if (pdata->chg) {
> - if (!gpio_is_valid(pdata->chg)) {
> - dev_err(dev, "Invalid pin: chg.\n");
> - return -EINVAL;
> + if (gpio_is_valid(pdata->chg)) {
> + ret = devm_gpio_request(dev,
> + pdata->chg,
> + data->psy_desc.name);
> + if (ret) {
> + dev_err(dev,
> + "Failed GPIO request for chg:
> %d err %d\n",
> + pdata->chg, ret);
> + return -EINVAL;
> +
[PATCH v2 08/10] PM core: Fix restoring devices with links during system PM transition
When devices are being runtime resumed during the system PM transition to
suspend state, the link suppliers might be already resumed and
have runtime pm disabled. This is normal case. This patch adds special
support for such case. Simple call to pm_runtime_get_syncreturns error
when device has runtime PM disabled, what results in incorrect runtime PM
state during system wide PM transition.
Signed-off-by: Marek Szyprowski
---
drivers/base/power/runtime.c | 22 +-
1 file changed, 21 insertions(+), 1 deletion(-)
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index 0ea00d442e0f..d00b079fad88 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -261,6 +261,26 @@ static int rpm_check_suspend_allowed(struct device *dev)
}
/**
+ * __pm_runtime_force_resume - force resume of the device
+ * @dev: Device to resume
+ *
+ * This function works like __pm_runtime_resume(dev, RPM_GET_PUT), but
+ * also handles devices with runtime PM disabled. This allows to properly
+ * control all devices during preparation for system PM transition.
+ */
+static int __pm_runtime_force_resume(struct device *dev)
+{
+ if (!dev->power.disable_depth)
+ return pm_runtime_get_sync(dev);
+
+ if (dev->power.runtime_status != RPM_ACTIVE ||
+ atomic_inc_not_zero(&dev->power.usage_count) == 0)
+ return -EINVAL;
+
+ return 0;
+}
+
+/**
* __rpm_callback - Run a given runtime PM callback for a given device.
* @cb: Runtime PM callback to run.
* @dev: Device to run the callback for.
@@ -291,7 +311,7 @@ static int __rpm_callback(int (*cb)(struct device *),
struct device *dev)
if ((link->flags & DEVICE_LINK_PM_RUNTIME)
&& link->status !=
DEVICE_LINK_SUPPLIER_UNBIND
&& !link->rpm_active) {
- retval =
pm_runtime_get_sync(link->supplier);
+ retval =
__pm_runtime_force_resume(link->supplier);
if (retval < 0) {
pm_runtime_put_noidle(link->supplier);
goto fail;
--
1.9.1
Re: [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm.
On 17/06/2016 4:26 PM, Krzysztof Kozlowski wrote:
On 06/17/2016 07:00 AM, Chris Lapa wrote:
From: Chris Lapa
The max8903_charger.h file indicated that dcm and dok were not optional
when dc_valid is set.
It makes sense to have dok as a compulsory pin when dc_valid is given.
However dcm can be optionally wired to a fixed level especially when the
circuit is configured for dc power exclusively.
The previous implementation already allowed for this somewhat, however no
error was given if dok wasn't given whilst dc_valid was.
The new implementation enforces dok presence when dc_valid is given. Whilst
allowing dcm to be optional.
Signed-off-by: Chris Lapa
---
drivers/power/max8903_charger.c | 23 ++-
include/linux/power/max8903_charger.h | 6 +++---
2 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c
index 0a5b0e1..dbd911c4 100644
--- a/drivers/power/max8903_charger.c
+++ b/drivers/power/max8903_charger.c
@@ -211,27 +211,24 @@ static int max8903_probe(struct platform_device *pdev)
}
if (pdata->dc_valid) {
- if (pdata->dok && gpio_is_valid(pdata->dok) &&
- pdata->dcm && gpio_is_valid(pdata->dcm)) {
+ if (pdata->dok && gpio_is_valid(pdata->dok)) {
gpio = pdata->dok; /* PULL_UPed Interrupt */
ta_in = gpio_get_value(gpio) ? 0 : 1;
+ } else {
+ dev_err(dev, "When DC is wired, DOK should"
+ " be wired as well.\n");
Just found one nit. Don't split the strings.
dev_err(dev,
"When DC is wired, DOK should be wired as well.\n");
I saw that one as well when I ran checkpatch, however I thought if I
changed it then I would get a warning about the line being > 80 chars.
So wasn't sure which direction to go.
Thanks
Chris
Best regards,
Krzysztof
[PATCH v2 01/10] driver core: Add a wrapper around __device_release_driver()
From: "Rafael J. Wysocki"
Add an internal wrapper around __device_release_driver() that will
acquire device locks and do the necessary checks before calling it.
The next patch will make use of it.
Signed-off-by: Rafael J. Wysocki
Signed-off-by: Marek Szyprowski
---
drivers/base/dd.c | 30 ++
1 file changed, 18 insertions(+), 12 deletions(-)
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 16688f50729c..d9e76e9205c7 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -796,6 +796,22 @@ static void __device_release_driver(struct device *dev)
}
}
+static void device_release_driver_internal(struct device *dev,
+ struct device_driver *drv,
+ struct device *parent)
+{
+ if (parent)
+ device_lock(parent);
+
+ device_lock(dev);
+ if (!drv || drv == dev->driver)
+ __device_release_driver(dev);
+
+ device_unlock(dev);
+ if (parent)
+ device_unlock(parent);
+}
+
/**
* device_release_driver - manually detach device from driver.
* @dev: device.
@@ -810,9 +826,7 @@ void device_release_driver(struct device *dev)
* within their ->remove callback for the same device, they
* will deadlock right here.
*/
- device_lock(dev);
- __device_release_driver(dev);
- device_unlock(dev);
+ device_release_driver_internal(dev, NULL, NULL);
}
EXPORT_SYMBOL_GPL(device_release_driver);
@@ -837,15 +851,7 @@ void driver_detach(struct device_driver *drv)
dev = dev_prv->device;
get_device(dev);
spin_unlock(&drv->p->klist_devices.k_lock);
-
- if (dev->parent)/* Needed for USB */
- device_lock(dev->parent);
- device_lock(dev);
- if (dev->driver == drv)
- __device_release_driver(dev);
- device_unlock(dev);
- if (dev->parent)
- device_unlock(dev->parent);
+ device_release_driver_internal(dev, drv, dev->parent);
put_device(dev);
}
}
--
1.9.1
[PATCH v2 07/10] driver core: Add support for links to already probed drivers
Set proper link state if link is created between already probed supplier device and to be probed consumer device. Signed-off-by: Marek Szyprowski --- drivers/base/core.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 4e778539b750..d9c5c5542a6b 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -125,7 +125,9 @@ struct devlink *device_link_add(struct device *consumer, link->flags = flags; link->status = (flags & DEVICE_LINK_PROBE_TIME) ? - DEVICE_LINK_CONSUMER_PROBE : DEVICE_LINK_DORMANT; + DEVICE_LINK_CONSUMER_PROBE : + (supplier->driver ? DEVICE_LINK_AVAILABLE : +DEVICE_LINK_DORMANT); spin_lock_init(&link->lock); /* -- 1.9.1
[PATCH v2 06/10] driver core: Avoid endless recursion if device has more than one link
This patch fixes endless recursion, which happends when device has more than one link. Signed-off-by: Marek Szyprowski --- drivers/base/core.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index 215cd44de761..4e778539b750 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -57,7 +57,8 @@ static int device_reorder_to_tail(struct device *dev, void *not_used) device_pm_move_last(dev); device_for_each_child(dev, NULL, device_reorder_to_tail); list_for_each_entry(link, &dev->consumer_links, c_node) - device_reorder_to_tail(link->consumer, NULL); + if (link->consumer != dev) + device_reorder_to_tail(link->consumer, NULL); return 0; } -- 1.9.1
[PATCH v2 04/10] PM core: Make runtime PM of devices use device links
From: "Rafael J. Wysocki"
Modify the runtime PM framework to use device links to ensure that
supplier devices will not be suspended if any of their consumer
devices are active.
The idea is to reference count suppliers on the consumer's resume
and drop references to them on its suspend. The information on
whether or not the supplier has been reference counted by the
consumer's (runtime) resume is stored in a new field (rpm_active)
in the link object for each link.
It may be necessary to clean up those references when the
supplier is unbinding and that's why the links whose status is
DEVICE_LINK_SUPPLIER_UNBIND are skipped by the runtime suspend
and resume code.
The above means that if the consumer device is probed in the
runtime-active state, the supplier has to be resumed and reference
counted by device_link_add() so the code works as expected on its
(runtime) suspend. There is a new flag, DEVICE_LINK_RPM_ACTIVE,
to tell device_link_add() about that (in which case the caller
is responsible for making sure that the consumer really will
be runtime-active when runtime PM is enabled for it).
The other new link flag, DEVICE_LINK_PM_RUNTIME, tells the core
whether or not the link should be used for runtime PM at all.
Signed-off-by: Rafael J. Wysocki
Signed-off-by: Marek Szyprowski
---
drivers/base/core.c | 15 +++
drivers/base/dd.c| 1 +
drivers/base/power/runtime.c | 93 +---
include/linux/device.h | 5 +++
include/linux/pm_runtime.h | 2 +
5 files changed, 111 insertions(+), 5 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 51a479ed68b5..1fe7cf232e1e 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -88,6 +88,11 @@ struct devlink *device_link_add(struct device *consumer,
if (!consumer || !supplier || !flags)
return NULL;
+#define RPM_ACTIVE_FLAGS (DEVICE_LINK_PM_RUNTIME | DEVICE_LINK_PROBE_TIME)
+ if ((flags & DEVICE_LINK_RPM_ACTIVE)
+ && (flags & RPM_ACTIVE_FLAGS) != RPM_ACTIVE_FLAGS)
+ return NULL;
+
mutex_lock(&device_links_lock);
list_for_each_entry(link, &supplier->supplier_links, s_node)
@@ -98,6 +103,16 @@ struct devlink *device_link_add(struct device *consumer,
if (!link)
goto out;
+ if (flags & DEVICE_LINK_RPM_ACTIVE) {
+ if (pm_runtime_get_sync(supplier) < 0) {
+ pm_runtime_put_noidle(supplier);
+ kfree(link);
+ goto out;
+ }
+ link->rpm_active = true;
+ } else {
+ link->rpm_active = false;
+ }
get_device(supplier);
link->supplier = supplier;
INIT_LIST_HEAD(&link->s_node);
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 7c0abeba89e9..2efec98ddb12 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -791,6 +791,7 @@ static void __device_release_driver(struct device *dev,
struct device *parent)
}
pm_runtime_get_sync(dev);
+ pm_runtime_clean_up_links(dev);
driver_sysfs_remove(dev);
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index b74690418504..ba21c123b16e 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -12,6 +12,8 @@
#include
#include
#include
+
+#include "../base.h"
#include "power.h"
typedef int (*pm_callback_t)(struct device *);
@@ -266,19 +268,69 @@ static int rpm_check_suspend_allowed(struct device *dev)
static int __rpm_callback(int (*cb)(struct device *), struct device *dev)
__releases(&dev->power.lock) __acquires(&dev->power.lock)
{
- int retval;
+ struct devlink *link;
+ int retval, idx;
- if (dev->power.irq_safe)
+ if (dev->power.irq_safe) {
spin_unlock(&dev->power.lock);
- else
+ } else {
spin_unlock_irq(&dev->power.lock);
+ /*
+* Resume suppliers if necessary.
+*
+* The device's runtime PM status cannot change until this
+* routine returns, so it is safe to read the status outside of
+* the lock.
+*/
+ if (dev->power.runtime_status == RPM_RESUMING) {
+ idx = device_links_read_lock();
+
+ list_for_each_entry_rcu(link, &dev->consumer_links,
c_node)
+ if ((link->flags & DEVICE_LINK_PM_RUNTIME)
+ && link->status !=
DEVICE_LINK_SUPPLIER_UNBIND
+ && !link->rpm_active) {
+ retval =
pm_runtime_get_sync(link->supplier);
+ if (retval < 0) {
+
pm_runtime_put_noidle(link->supplier);
+
[PATCH v2 09/10] iommu/exynos: Remove excessive, useless debug
Remove excessive, useless debug about skipping TLB invalidation, which
is a normal situation when more aggressive power management is enabled.
Signed-off-by: Marek Szyprowski
---
drivers/iommu/exynos-iommu.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 633e6d023c0d..9d1a14f88891 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -574,9 +574,6 @@ static void sysmmu_tlb_invalidate_entry(struct
sysmmu_drvdata *data,
sysmmu_unblock(data);
}
clk_disable(data->clk_master);
- } else {
- dev_dbg(data->master,
- "disabled. Skipping TLB invalidation @ %#x\n", iova);
}
spin_unlock_irqrestore(&data->lock, flags);
}
--
1.9.1
[PATCH v2 05/10] PM core: Optimize the use of device links for runtime PM
From: "Rafael J. Wysocki"
If the device has no links to suppliers that should be used for
runtime PM (links with DEVICE_LINK_PM_RUNTIME set), there is no
reason to walk the list of suppliers for that device during
runtime suspend and resume.
Add a simple mechanism to detect that case and possibly avoid the
extra unnecessary overhead.
Signed-off-by: Rafael J. Wysocki
Signed-off-by: Marek Szyprowski
---
drivers/base/core.c | 6 ++
drivers/base/power/runtime.c | 23 ---
include/linux/pm.h | 1 +
include/linux/pm_runtime.h | 4
4 files changed, 31 insertions(+), 3 deletions(-)
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 1fe7cf232e1e..215cd44de761 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -119,6 +119,9 @@ struct devlink *device_link_add(struct device *consumer,
get_device(consumer);
link->consumer = consumer;
INIT_LIST_HEAD(&link->c_node);
+ if (flags & DEVICE_LINK_PM_RUNTIME)
+ pm_runtime_new_link(consumer);
+
link->flags = flags;
link->status = (flags & DEVICE_LINK_PROBE_TIME) ?
DEVICE_LINK_CONSUMER_PROBE : DEVICE_LINK_DORMANT;
@@ -161,6 +164,9 @@ static void devlink_del(struct devlink *link)
dev_info(link->consumer, "Dropping the link to %s\n",
dev_name(link->supplier));
+ if (link->flags & DEVICE_LINK_PM_RUNTIME)
+ pm_runtime_drop_link(link->consumer);
+
list_del_rcu(&link->s_node);
list_del_rcu(&link->c_node);
call_srcu(&device_links_srcu, &link->rcu_head, __devlink_free_srcu);
diff --git a/drivers/base/power/runtime.c b/drivers/base/power/runtime.c
index ba21c123b16e..0ea00d442e0f 100644
--- a/drivers/base/power/runtime.c
+++ b/drivers/base/power/runtime.c
@@ -270,6 +270,7 @@ static int __rpm_callback(int (*cb)(struct device *),
struct device *dev)
{
struct devlink *link;
int retval, idx;
+ bool use_links = dev->power.links_count > 0;
if (dev->power.irq_safe) {
spin_unlock(&dev->power.lock);
@@ -283,7 +284,7 @@ static int __rpm_callback(int (*cb)(struct device *),
struct device *dev)
* routine returns, so it is safe to read the status outside of
* the lock.
*/
- if (dev->power.runtime_status == RPM_RESUMING) {
+ if (use_links && dev->power.runtime_status == RPM_RESUMING) {
idx = device_links_read_lock();
list_for_each_entry_rcu(link, &dev->consumer_links,
c_node)
@@ -314,8 +315,9 @@ static int __rpm_callback(int (*cb)(struct device *),
struct device *dev)
*
* Do that if resume fails too.
*/
- if ((dev->power.runtime_status == RPM_SUSPENDING && !retval)
- || (dev->power.runtime_status == RPM_RESUMING && retval)) {
+ if (use_links
+ && ((dev->power.runtime_status == RPM_SUSPENDING && !retval)
+ || (dev->power.runtime_status == RPM_RESUMING && retval))) {
idx = device_links_read_lock();
fail:
@@ -1525,6 +1527,21 @@ void pm_runtime_clean_up_links(struct device *dev)
device_links_read_unlock(idx);
}
+void pm_runtime_new_link(struct device *dev)
+{
+ spin_lock_irq(&dev->power.lock);
+ dev->power.links_count++;
+ spin_unlock_irq(&dev->power.lock);
+}
+
+void pm_runtime_drop_link(struct device *dev)
+{
+ spin_lock_irq(&dev->power.lock);
+ WARN_ON(dev->power.links_count == 0);
+ dev->power.links_count--;
+ spin_unlock_irq(&dev->power.lock);
+}
+
/**
* pm_runtime_force_suspend - Force a device into suspend state if needed.
* @dev: Device to suspend.
diff --git a/include/linux/pm.h b/include/linux/pm.h
index 06eb353182ab..49c811378110 100644
--- a/include/linux/pm.h
+++ b/include/linux/pm.h
@@ -596,6 +596,7 @@ struct dev_pm_info {
unsigned intuse_autosuspend:1;
unsigned inttimer_autosuspends:1;
unsigned intmemalloc_noio:1;
+ unsigned intlinks_count;
enum rpm_requestrequest;
enum rpm_status runtime_status;
int runtime_error;
diff --git a/include/linux/pm_runtime.h b/include/linux/pm_runtime.h
index c21a21c064e3..bef6203aaf14 100644
--- a/include/linux/pm_runtime.h
+++ b/include/linux/pm_runtime.h
@@ -56,6 +56,8 @@ extern void pm_runtime_update_max_time_suspended(struct
device *dev,
s64 delta_ns);
extern void pm_runtime_set_memalloc_noio(struct device *dev, bool enable);
extern void pm_runtime_clean_up_links(struct device *dev);
+extern void pm_runtime_new_link(struct device *dev);
+extern void pm_runtime_drop_link(struct device *dev);
static inline void pm_suspend_ignore_children(struct
[PATCH v2 03/10] PM core: Make async suspend/resume of devices use device links
From: "Rafael J. Wysocki"
Make the device suspend/resume part of the core system
suspend/resume code use device links to ensure that supplier
and consumer devices will be suspended and resumed in the right
order in case of async suspend/resume.
The idea, roughly, is to use dpm_wait() to wait for all consumers
before a supplier device suspend and to wait for all suppliers
before a consumer device resume.
Signed-off-by: Rafael J. Wysocki
Signed-off-by: Marek Szyprowski
---
drivers/base/base.h | 2 ++
drivers/base/core.c | 4 +--
drivers/base/power/main.c | 68 ++-
3 files changed, 66 insertions(+), 8 deletions(-)
diff --git a/drivers/base/base.h b/drivers/base/base.h
index cccb1d211541..123b986eb061 100644
--- a/drivers/base/base.h
+++ b/drivers/base/base.h
@@ -163,3 +163,5 @@ extern void device_links_driver_gone(struct device *dev);
extern void device_links_no_driver(struct device *dev);
extern bool device_links_busy(struct device *dev);
extern void device_links_unbind_consumers(struct device *dev);
+extern int device_links_read_lock(void);
+extern void device_links_read_unlock(int idx);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 416341df3268..51a479ed68b5 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -163,12 +163,12 @@ void device_link_del(struct devlink *link)
}
EXPORT_SYMBOL_GPL(device_link_del);
-static int device_links_read_lock(void)
+int device_links_read_lock(void)
{
return srcu_read_lock(&device_links_srcu);
}
-static void device_links_read_unlock(int idx)
+void device_links_read_unlock(int idx)
{
return srcu_read_unlock(&device_links_srcu, idx);
}
diff --git a/drivers/base/power/main.c b/drivers/base/power/main.c
index e44944f4be77..9470ccd87f11 100644
--- a/drivers/base/power/main.c
+++ b/drivers/base/power/main.c
@@ -244,6 +244,62 @@ static void dpm_wait_for_children(struct device *dev, bool
async)
device_for_each_child(dev, &async, dpm_wait_fn);
}
+static void dpm_wait_for_suppliers(struct device *dev, bool async)
+{
+ struct devlink *link;
+ int idx;
+
+ idx = device_links_read_lock();
+
+ /*
+* If the supplier goes away right after we've checked the link to it,
+* we'll wait for its completion to change the state, but that's fine,
+* because the only things that will block as a result are the SRCU
+* callbacks freeing the link objects for the links in the list we're
+* walking.
+*/
+ list_for_each_entry_rcu(link, &dev->consumer_links, c_node)
+ if (link->status != DEVICE_LINK_DORMANT)
+ dpm_wait(link->supplier, async);
+
+ device_links_read_unlock(idx);
+}
+
+static void dpm_wait_for_superior(struct device *dev, bool async)
+{
+ dpm_wait(dev->parent, async);
+ dpm_wait_for_suppliers(dev, async);
+}
+
+static void dpm_wait_for_consumers(struct device *dev, bool async)
+{
+ struct devlink *link;
+ int idx;
+
+ idx = device_links_read_lock();
+
+ /*
+* The status of a device link can only be changed from "dormant" by a
+* probe, but that cannot happen during system suspend/resume. In
+* theory it can change to "dormant" at that time, but then it is
+* reasonable to wait for the target device anyway (eg. if it goes
+* away, it's better to wait for it to go away completely and then
+* continue instead of trying to continue in parallel with its
+* unregistration).
+*/
+ list_for_each_entry_rcu(link, &dev->supplier_links, s_node)
+ if (link->status != DEVICE_LINK_DORMANT)
+ dpm_wait(link->consumer, async);
+
+ device_links_read_unlock(idx);
+}
+
+static void dpm_wait_for_subordinate(struct device *dev, bool async)
+{
+ dpm_wait_for_children(dev, async);
+ dpm_wait_for_consumers(dev, async);
+}
+
/**
* pm_op - Return the PM operation appropriate for given PM event.
* @ops: PM operations to choose from.
@@ -488,7 +544,7 @@ static int device_resume_noirq(struct device *dev,
pm_message_t state, bool asyn
if (!dev->power.is_noirq_suspended)
goto Out;
- dpm_wait(dev->parent, async);
+ dpm_wait_for_superior(dev, async);
if (dev->pm_domain) {
info = "noirq power domain ";
@@ -618,7 +674,7 @@ static int device_resume_early(struct device *dev,
pm_message_t state, bool asyn
if (!dev->power.is_late_suspended)
goto Out;
- dpm_wait(dev->parent, async);
+ dpm_wait_for_superior(dev, async);
if (dev->pm_domain) {
info = "early power domain ";
@@ -750,7 +806,7 @@ static int device_resume(struct device *dev, pm_message_t
state, bool async)
goto Complete;
}
- dpm_wait(dev->parent, async);
+ dpm_wait_for_superior(dev, async);
d
Re: [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm.
On 06/17/2016 07:00 AM, Chris Lapa wrote:
> From: Chris Lapa
>
> The max8903_charger.h file indicated that dcm and dok were not optional
> when dc_valid is set.
>
> It makes sense to have dok as a compulsory pin when dc_valid is given.
> However dcm can be optionally wired to a fixed level especially when the
> circuit is configured for dc power exclusively.
>
> The previous implementation already allowed for this somewhat, however no
> error was given if dok wasn't given whilst dc_valid was.
>
> The new implementation enforces dok presence when dc_valid is given. Whilst
> allowing dcm to be optional.
>
> Signed-off-by: Chris Lapa
> ---
> drivers/power/max8903_charger.c | 23 ++-
> include/linux/power/max8903_charger.h | 6 +++---
> 2 files changed, 13 insertions(+), 16 deletions(-)
>
> diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c
> index 0a5b0e1..dbd911c4 100644
> --- a/drivers/power/max8903_charger.c
> +++ b/drivers/power/max8903_charger.c
> @@ -211,27 +211,24 @@ static int max8903_probe(struct platform_device *pdev)
> }
>
> if (pdata->dc_valid) {
> - if (pdata->dok && gpio_is_valid(pdata->dok) &&
> - pdata->dcm && gpio_is_valid(pdata->dcm)) {
> + if (pdata->dok && gpio_is_valid(pdata->dok)) {
> gpio = pdata->dok; /* PULL_UPed Interrupt */
> ta_in = gpio_get_value(gpio) ? 0 : 1;
> + } else {
> + dev_err(dev, "When DC is wired, DOK should"
> + " be wired as well.\n");
Just found one nit. Don't split the strings.
dev_err(dev,
"When DC is wired, DOK should be wired as well.\n");
Best regards,
Krzysztof
[PATCH v2 10/10] iommu/exynos: Add proper runtime pm support
This patch uses recently introduced device links to track the runtime pm
state of the master's device. This way each SYSMMU controller is runtime
active its master's device is active and can save/restore its state
instead of being enabled all the time. This way SYSMMU controllers no
longer prevents respective power domains to be turned off when master's
device is not used.
Signed-off-by: Marek Szyprowski
---
drivers/iommu/exynos-iommu.c | 218 ---
1 file changed, 99 insertions(+), 119 deletions(-)
diff --git a/drivers/iommu/exynos-iommu.c b/drivers/iommu/exynos-iommu.c
index 9d1a14f88891..80b7f1e7268c 100644
--- a/drivers/iommu/exynos-iommu.c
+++ b/drivers/iommu/exynos-iommu.c
@@ -233,8 +233,8 @@ struct sysmmu_drvdata {
struct clk *aclk; /* SYSMMU's aclk clock */
struct clk *pclk; /* SYSMMU's pclk clock */
struct clk *clk_master; /* master's device clock */
- int activations;/* number of calls to sysmmu_enable */
spinlock_t lock;/* lock for modyfying state */
+ int active; /* current status */
struct exynos_iommu_domain *domain; /* domain we belong to */
struct list_head domain_node; /* node for domain clients list */
struct list_head owner_node;/* node for owner controllers list */
@@ -247,25 +247,6 @@ static struct exynos_iommu_domain *to_exynos_domain(struct
iommu_domain *dom)
return container_of(dom, struct exynos_iommu_domain, domain);
}
-static bool set_sysmmu_active(struct sysmmu_drvdata *data)
-{
- /* return true if the System MMU was not active previously
- and it needs to be initialized */
- return ++data->activations == 1;
-}
-
-static bool set_sysmmu_inactive(struct sysmmu_drvdata *data)
-{
- /* return true if the System MMU is needed to be disabled */
- BUG_ON(data->activations < 1);
- return --data->activations == 0;
-}
-
-static bool is_sysmmu_active(struct sysmmu_drvdata *data)
-{
- return data->activations > 0;
-}
-
static void sysmmu_unblock(struct sysmmu_drvdata *data)
{
writel(CTRL_ENABLE, data->sfrbase + REG_MMU_CTRL);
@@ -384,7 +365,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
unsigned short reg_status, reg_clear;
int ret = -ENOSYS;
- WARN_ON(!is_sysmmu_active(data));
+ WARN_ON(!data->active);
if (MMU_MAJ_VER(data->version) < 5) {
reg_status = REG_INT_STATUS;
@@ -430,7 +411,7 @@ static irqreturn_t exynos_sysmmu_irq(int irq, void *dev_id)
return IRQ_HANDLED;
}
-static void __sysmmu_disable_nocount(struct sysmmu_drvdata *data)
+static void __sysmmu_disable(struct sysmmu_drvdata *data)
{
clk_enable(data->clk_master);
@@ -440,32 +421,6 @@ static void __sysmmu_disable_nocount(struct sysmmu_drvdata
*data)
__sysmmu_disable_clocks(data);
}
-static bool __sysmmu_disable(struct sysmmu_drvdata *data)
-{
- bool disabled;
- unsigned long flags;
-
- spin_lock_irqsave(&data->lock, flags);
-
- disabled = set_sysmmu_inactive(data);
-
- if (disabled) {
- data->pgtable = 0;
- data->domain = NULL;
-
- __sysmmu_disable_nocount(data);
-
- dev_dbg(data->sysmmu, "Disabled\n");
- } else {
- dev_dbg(data->sysmmu, "%d times left to disable\n",
- data->activations);
- }
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return disabled;
-}
-
static void __sysmmu_init_config(struct sysmmu_drvdata *data)
{
unsigned int cfg;
@@ -480,7 +435,7 @@ static void __sysmmu_init_config(struct sysmmu_drvdata
*data)
writel(cfg, data->sfrbase + REG_MMU_CFG);
}
-static void __sysmmu_enable_nocount(struct sysmmu_drvdata *data)
+static void __sysmmu_enable(struct sysmmu_drvdata *data)
{
__sysmmu_enable_clocks(data);
@@ -501,34 +456,6 @@ static void __sysmmu_enable_nocount(struct sysmmu_drvdata
*data)
clk_disable(data->clk_master);
}
-static int __sysmmu_enable(struct sysmmu_drvdata *data, phys_addr_t pgtable,
- struct exynos_iommu_domain *domain)
-{
- int ret = 0;
- unsigned long flags;
-
- spin_lock_irqsave(&data->lock, flags);
- if (set_sysmmu_active(data)) {
- data->pgtable = pgtable;
- data->domain = domain;
-
- __sysmmu_enable_nocount(data);
-
- dev_dbg(data->sysmmu, "Enabled\n");
- } else {
- ret = (pgtable == data->pgtable) ? 1 : -EBUSY;
-
- dev_dbg(data->sysmmu, "already enabled\n");
- }
-
- if (WARN_ON(ret < 0))
- set_sysmmu_inactive(data); /* decrement count */
-
- spin_unlock_irqrestore(&data->lock, flags);
-
- return ret;
-}
-
static void sysmmu_tlb_invalidate_fl
[PATCH v2 00/10] Exynos IOMMU: proper runtime PM support (use device dependencies)
Hello, This patch series finally implements proper runtime PM support in Exynos IOMMU driver. This has been achieved by using device links, which lets SYSMMU controller's runtime PM to follow master's device runtime PM (the device which actually performs DMA transaction). The main idea behind this solution is an observation that any DMA activity from master device can be done only when master device is active, thus when master device is suspended SYSMMU controller device can also be suspended. This patchset solves the situation that power domains are always enabled, because all SYSMMU controllers (which belongs to those domains) are permanently active (because existing driver was simplified and kept SYSMMU device active all the time after initialization). Patches 1-5 are resend of the "[RFC][PATCH 0/5] Functional dependencies between devices" patchset: http://thread.gmane.org/gmane.linux.power-management.general/67424/focus=2126379 I've included them here, because it is hard to find them all on mailing list archives. Patches 6-8 are fixes to device dependencies/links code, which were required to use this solution for Exynos IOMMU driver. I'm not PM/runtime PM code expert, so please double check if my changes are really correct. This patchset requires my previous changes to Exynos IOMMU driver submitted in the "Exynos IOMMU: improve clock management" thread: http://www.spinics.net/lists/arm-kernel/msg505695.html Best regards Marek Szyprowski Samsung R&D Institute Poland Changelog: v2: - replaced PM notifiers with generic device dependencies/links developped by Rafael J. Wysocki v1: http://www.spinics.net/lists/arm-kernel/msg509600.html - initial version Patch summary: Marek Szyprowski (5): driver core: Avoid endless recursion if device has more than one link driver core: Add support for links to already probed drivers PM core: Fix restoring devices with links during system PM transition iommu/exynos: Remove excessive, useless debug iommu/exynos: Add proper runtime pm support Rafael J. Wysocki (5): driver core: Add a wrapper around __device_release_driver() driver core: Functional dependencies tracking support PM core: Make async suspend/resume of devices use device links PM core: Make runtime PM of devices use device links PM core: Optimize the use of device links for runtime PM drivers/base/base.h | 13 ++ drivers/base/core.c | 410 +++ drivers/base/dd.c| 65 +-- drivers/base/power/main.c| 68 ++- drivers/base/power/runtime.c | 130 +- drivers/iommu/exynos-iommu.c | 221 +++ include/linux/device.h | 41 + include/linux/pm.h | 1 + include/linux/pm_runtime.h | 6 + 9 files changed, 809 insertions(+), 146 deletions(-) -- 1.9.1
Re: [PATCH v3] Doc/memory-barriers: Add Korean translation
On Wed, Jun 15, 2016 at 03:47:34PM +0900, SeongJae Park wrote: > 2016-06-09 5:45 GMT+09:00 SeongJae Park : > > 2016-06-09 2:24 GMT+09:00 Paul E. McKenney : > >> On Wed, Jun 08, 2016 at 05:58:41PM +0900, SeongJae Park wrote: > >>> This commit adds Korean version of memory-barriers.txt document. The > >>> header is refered to HOWTO Korean version. > >>> > >>> The translator, SeongJae Park, is interested in parallel programming and > >>> translating[1] a book[2] about the topic. > >>> > >>> The translation has started from Feb, 2016 and using a github public > >>> repository[3] to maintain the work. The work is following[4] updates to > >>> the original document as well. > >>> > >>> Because the translator has knowledge about the topic and already > >>> following up the upstream changes, one would sure that this translation > >>> will keep reasonable quality and freshness. > >>> > >>> [1] > >>> https://git.kernel.org/cgit/linux/kernel/git/paulmck/perfbook.git/commit/FAQ.txt?id=edbfcdee0460 > >>> [2] > >>> https://www.kernel.org/pub/linux/kernel/people/paulmck/perfbook/perfbook.html > >>> [3] https://github.com/sjp38/linux.doc_trans_membarrier > >>> [4] > >>> https://github.com/sjp38/linux.doc_trans_membarrier/commit/06bd12d390f164fd253b861fc3aa8006d6d19ed9 > >>> > >>> Signed-off-by: SeongJae Park > >>> Acked-by: David Howells > >>> Signed-off-by: Parl E. McKenney > >>> Acked-by: Minchan Kim > >> > >> I cannot judge this, so I must defer to Minchan. That said, the diffstat > >> since your version of April 18 is as follows: > >> > >> memory-barriers.txt | 303 > >> ++-- > >> 1 file changed, 175 insertions(+), 128 deletions(-) > >> > >> I cannot tell which mainline version these patches correspond to, but the > >> English version has this diffstat since v4.5: > >> > >> memory-barriers.txt | 293 > >> +--- > >> 1 file changed, 234 insertions(+), 59 deletions(-) > >> > >> So your level of change thus far seems plausible. > > > > I agree that Minchan deserves to judge that since he is the Korean > > translations > > maintainer. > > Minchan, may I ask your opinion about this patch? Sorry for late response. I think it's really worth. That's why I support it. However, until now, I didn't review it in detail. Sorry about that and stuck with urgent works so I asked colleague byungchul who scheduler guy, he has an interest so He will review this patch soon. Hope it helps you. Thanks.
Re: [PATCH 1/3] PM / Runtime: Add notifiers for device runtime PM events
Hi Rafael, On 2016-06-08 19:18, Rafael J. Wysocki wrote: On Wed, Jun 8, 2016 at 12:25 PM, Marek Szyprowski wrote: From: Krzysztof Kozlowski Allow drivers registering for certain runtime PM events of other devices. Some drivers in power domain are more or less coupled. When one driver is suspending (thus leading to power domain being also turned off) the other might have to perform some necessary steps. For example Exynos IOMMU has to save its context. Based on previous work of Sylwester Nawrocki. Signed-off-by: Krzysztof Kozlowski Signed-off-by: Marek Szyprowski No, this is not the right way to address this and using notifiers for that is just wrong (because of the potential ordering issues). Also, the problem is not limited to runtime PM, but also to system suspend/resume and initialization/shutdown. I posted a series of device dependencies patches a few months ago that might help to address this problem, but there was almost no interest in it at that time. I spent some time digging for your patches. Sadly no list archives had them all and I finally found them only in the linux-pm patchwork. This may explain why there was almost no interest in them. After some debugging I've managed to get it working for my case. I will include your patches in my v2 patchset together with the fixes needed to get it working. Best regards -- Marek Szyprowski, PhD Samsung R&D Institute Poland
Re: [PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm.
On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa > > The max8903_charger.h file indicated that dcm and dok were not optional > when dc_valid is set. > > It makes sense to have dok as a compulsory pin when dc_valid is given. > However dcm can be optionally wired to a fixed level especially when the > circuit is configured for dc power exclusively. > > The previous implementation already allowed for this somewhat, however no > error was given if dok wasn't given whilst dc_valid was. > > The new implementation enforces dok presence when dc_valid is given. Whilst > allowing dcm to be optional. > > Signed-off-by: Chris Lapa > --- > drivers/power/max8903_charger.c | 23 ++- > include/linux/power/max8903_charger.h | 6 +++--- > 2 files changed, 13 insertions(+), 16 deletions(-) Code looks correct: Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [patch] sched/fair: Use instantaneous load in wakeup paths
Here are some schbench runs on an 8x8 box to show that longish run/sleep period corner I mentioned. vogelweide:~/:[1]# for i in `seq 5`; do schbench -m 8 -t 1 -a -r 10 2>&1 | grep 'threads 8'; done cputime 3 threads 8 p99 68 cputime 3 threads 8 p99 46 cputime 3 threads 8 p99 46 cputime 3 threads 8 p99 45 cputime 3 threads 8 p99 49 vogelweide:~/:[0]# echo NO_WAKE_INSTANTANEOUS_LOAD > /sys/kernel/debug/sched_features vogelweide:~/:[0]# for i in `seq 5`; do schbench -m 8 -t 1 -a -r 10 2>&1 | grep 'threads 8'; done cputime 3 threads 8 p99 9968 cputime 3 threads 8 p99 10224 vogelweide:~/:[0]# Using instantaneous load, we fill the box every time, without, we stack every time. This was with Peter's select_idle_sibling() rewrite applied as well, but you can see that it does matter. That doesn't mean I think my patch should immediately fly upstream 'course, who knows, there may be a less messy way to deal with it, or, as already stated, maybe it just doesn't matter enough to the real world to even bother with. -Mike
Re: [RESEND PATCH v11 1/4] gadget: Introduce the usb charger framework
Hi Felipe, On 13 June 2016 at 23:13, Jun Li wrote: > Hi > >> -Original Message- >> From: linux-usb-ow...@vger.kernel.org [mailto:linux-usb- >> ow...@vger.kernel.org] On Behalf Of Baolin Wang >> Sent: Monday, June 13, 2016 4:47 PM >> To: ba...@kernel.org; gre...@linuxfoundation.org; s...@kernel.org; >> dbarysh...@gmail.com; dw...@infradead.org >> Cc: r...@kernel.org; m.szyprow...@samsung.com; ruslan.bilo...@gmail.com; >> peter.c...@freescale.com; st...@rowland.harvard.edu; r.bald...@samsung.com; >> yoshihiro.shimoda...@renesas.com; lee.jo...@linaro.org; broo...@kernel.org; >> ckee...@opensource.wolfsonmicro.com; patc...@opensource.wolfsonmicro.com; >> baolin.w...@linaro.org; linux...@vger.kernel.org; linux- >> u...@vger.kernel.org; device-mainlin...@lists.linuxfoundation.org; linux- >> ker...@vger.kernel.org >> Subject: [RESEND PATCH v11 1/4] gadget: Introduce the usb charger >> framework >> >> This patch introduces the usb charger driver based on usb gadget that >> makes an enhancement to a power driver. It works well in practice but that >> requires a system with suitable hardware. >> >> The basic conception of the usb charger is that, when one usb charger is >> added or removed by reporting from the usb gadget state change or the >> extcon device state change, the usb charger will report to power user to >> set the current limitation. >> >> The usb charger will register notifiees on the usb gadget or the extcon >> device to get notified the usb charger state. It also supplies the >> notification mechanism to userspace When the usb charger state is changed. >> >> Power user will register a notifiee on the usb charger to get notified by >> status changes from the usb charger. It will report to power user to set >> the current limitation when detecting the usb charger is added or removed >> from extcon device state or usb gadget state. >> >> This patch doesn't yet integrate with the gadget code, so some functions >> which rely on the 'gadget' are not completed, that will be implemented in >> the following patches. >> >> Signed-off-by: Baolin Wang > > Reviewed-by: Li Jun > Tested-by: Li Jun Do you have any comments about this patchset, if not I will resend it with testing by Li Jun (thanks for Jun reviewing and testing). Thanks. > >> --- >> drivers/usb/gadget/Kconfig |7 + >> drivers/usb/gadget/udc/Makefile |1 + >> drivers/usb/gadget/udc/charger.c | 770 >> ++ >> include/linux/usb/charger.h | 191 ++ >> include/uapi/linux/usb/charger.h | 31 ++ >> 5 files changed, 1000 insertions(+) >> create mode 100644 drivers/usb/gadget/udc/charger.c create mode 100644 >> include/linux/usb/charger.h create mode 100644 >> include/uapi/linux/usb/charger.h -- Baolin.wang Best Regards
[PATCH RESEND V2] ARM64: config: Enable MAX77620 pinctrl/gpio/rtc for Tegra210 P2180
NVIDIA's Tegra210 Jetson platform (P2180) uses the MAX77620 as PMIC. Enable GPIO, Pincontrol and RTC driver for this device. Signed-off-by: Laxman Dewangan --- Changes from V1: - MFD and regulator is already enabled, enabling remaining driver. Resending to linux-tegra group also. arch/arm64/configs/defconfig | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig index 9af9ba5..90a6705 100644 --- a/arch/arm64/configs/defconfig +++ b/arch/arm64/configs/defconfig @@ -186,6 +186,7 @@ CONFIG_SPI_QUP=y CONFIG_SPI_SPIDEV=m CONFIG_SPMI=y CONFIG_PINCTRL_SINGLE=y +CONFIG_PINCTRL_MAX77620=y CONFIG_PINCTRL_MSM8916=y CONFIG_PINCTRL_QCOM_SPMI_PMIC=y CONFIG_GPIO_SYSFS=y @@ -194,6 +195,7 @@ CONFIG_GPIO_PL061=y CONFIG_GPIO_RCAR=y CONFIG_GPIO_XGENE=y CONFIG_GPIO_PCA953X=m +CONFIG_GPIO_MAX77620=y CONFIG_POWER_RESET_MSM=y CONFIG_POWER_RESET_XGENE=y CONFIG_POWER_RESET_SYSCON=y @@ -274,6 +276,7 @@ CONFIG_LEDS_TRIGGERS=y CONFIG_LEDS_TRIGGER_HEARTBEAT=y CONFIG_LEDS_TRIGGER_CPU=y CONFIG_RTC_CLASS=y +CONFIG_RTC_DRV_MAX77686=y CONFIG_RTC_DRV_S5M=y CONFIG_RTC_DRV_DS3232=y CONFIG_RTC_DRV_EFI=y -- 2.1.4
Re: [PATCH v3 1/7] max8903: adds documentation for device tree bindings.
On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa > > Signed-off-by: Chris Lapa > --- > .../devicetree/bindings/power/max8903-charger.txt | 25 > ++ > 1 file changed, 25 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/power/max8903-charger.txt Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH v3 2/7] max8903: store pointer to pdata instead of copying it.
On 06/17/2016 07:00 AM, Chris Lapa wrote: > From: Chris Lapa > > Stores pointer to pdata because it easily allows pdata to reference > either platform data or in the future device tree data. > > Signed-off-by: Chris Lapa > --- > drivers/power/max8903_charger.c | 20 +--- > 1 file changed, 13 insertions(+), 7 deletions(-) Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
[RESEND v7 1/6] mfd: max8997: Use regmap to access registers
From: Robert Baldyga
This patch modifies max8997 driver and each associated function driver,
to use regmap instead of operating directly on i2c bus. It will allow to
simplify IRQ handling using regmap-irq.
Signed-off-by: Robert Baldyga
Reviewed-by: Krzysztof Kozlowski
Acked-by: Chanwoo Choi
Acked-by: Bryan Wu
Acked-by: Lee Jones
Acked-by: Sebastian Reichel
Acked-by: Dmitry Torokhov
Acked-by: Alexandre Belloni
[k.kozlowski: Collect acks, rebase on v4.6-rc6]
Signed-off-by: Krzysztof Kozlowski
Acked-by: Jacek Anaszewski
---
drivers/extcon/extcon-max8997.c | 31
drivers/input/misc/max8997_haptic.c | 34
drivers/leds/leds-max8997.c | 13 ++--
drivers/mfd/Kconfig | 1 +
drivers/mfd/max8997-irq.c | 64 ++-
drivers/mfd/max8997.c | 141 +++---
drivers/power/max8997_charger.c | 33
drivers/regulator/max8997-regulator.c | 87 ++---
drivers/rtc/rtc-max8997.c | 56 --
include/linux/mfd/max8997-private.h | 17 ++--
10 files changed, 228 insertions(+), 249 deletions(-)
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 9a89320d09a8..fc812257bfc7 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#defineDEV_NAME"max8997-muic"
#defineDELAY_MS_DEFAULT2 /* unit:
millisecond */
@@ -116,7 +117,7 @@ enum max8997_muic_charger_type {
struct max8997_muic_info {
struct device *dev;
- struct i2c_client *muic;
+ struct max8997_dev *max8997;
struct extcon_dev *edev;
int prev_cable_type;
int prev_chg_type;
@@ -174,10 +175,10 @@ static int max8997_muic_set_debounce_time(struct
max8997_muic_info *info,
case ADC_DEBOUNCE_TIME_10MS:
case ADC_DEBOUNCE_TIME_25MS:
case ADC_DEBOUNCE_TIME_38_62MS:
- ret = max8997_update_reg(info->muic,
+ ret = regmap_update_bits(info->max8997->regmap_muic,
MAX8997_MUIC_REG_CONTROL3,
- time << CONTROL3_ADCDBSET_SHIFT,
- CONTROL3_ADCDBSET_MASK);
+ CONTROL3_ADCDBSET_MASK,
+ time << CONTROL3_ADCDBSET_SHIFT);
if (ret) {
dev_err(info->dev, "failed to set ADC debounce time\n");
return ret;
@@ -212,8 +213,8 @@ static int max8997_muic_set_path(struct max8997_muic_info
*info,
else
ctrl1 = CONTROL1_SW_OPEN;
- ret = max8997_update_reg(info->muic,
- MAX8997_MUIC_REG_CONTROL1, ctrl1, COMP_SW_MASK);
+ ret = regmap_update_bits(info->max8997->regmap_muic,
+ MAX8997_MUIC_REG_CONTROL1, COMP_SW_MASK, ctrl1);
if (ret < 0) {
dev_err(info->dev, "failed to update MUIC register\n");
return ret;
@@ -224,9 +225,9 @@ static int max8997_muic_set_path(struct max8997_muic_info
*info,
else
ctrl2 |= CONTROL2_LOWPWR_MASK; /* LowPwr=1, CPEn=0 */
- ret = max8997_update_reg(info->muic,
- MAX8997_MUIC_REG_CONTROL2, ctrl2,
- CONTROL2_LOWPWR_MASK | CONTROL2_CPEN_MASK);
+ ret = regmap_update_bits(info->max8997->regmap_muic,
+ MAX8997_MUIC_REG_CONTROL2,
+ CONTROL2_LOWPWR_MASK | CONTROL2_CPEN_MASK, ctrl2);
if (ret < 0) {
dev_err(info->dev, "failed to update MUIC register\n");
return ret;
@@ -530,8 +531,8 @@ static void max8997_muic_irq_work(struct work_struct *work)
if (info->irq == muic_irqs[i].virq)
irq_type = muic_irqs[i].irq;
- ret = max8997_bulk_read(info->muic, MAX8997_MUIC_REG_STATUS1,
- 2, info->status);
+ ret = regmap_bulk_read(info->max8997->regmap_muic,
+ MAX8997_MUIC_REG_STATUS1, info->status, 2);
if (ret) {
dev_err(info->dev, "failed to read muic register\n");
mutex_unlock(&info->mutex);
@@ -590,8 +591,8 @@ static int max8997_muic_detect_dev(struct max8997_muic_info
*info)
mutex_lock(&info->mutex);
/* Read STATUSx register to detect accessory */
- ret = max8997_bulk_read(info->muic,
- MAX8997_MUIC_REG_STATUS1, 2, info->status);
+ ret = regmap_bulk_read(info->max8997->regmap_muic,
+ MAX8997_MUIC_REG_STATUS1, info->status, 2);
if (ret) {
dev_err(info->dev, "failed to read MUIC register\n");
mutex_unlock(&info->mutex);
@@ -650,7 +651,7
[RESEND v7 3/6] mfd: max8997: Change irq names to upper case
From: Robert Baldyga
This patch changes naming convention of MUIC interrupts form CamelCase
to upper case. It makes names more readable and consistent with another
interrupt names in max8997 driver.
Signed-off-by: Robert Baldyga
Reviewed-by: Krzysztof Kozlowski
Acked-by: Lee Jones
Acked-by: Chanwoo Choi
Acked-by: Alexandre Belloni
[k.kozlowski: Collect acks, rebase on v4.6-rc6]
Signed-off-by: Krzysztof Kozlowski
---
drivers/extcon/extcon-max8997.c | 32
include/linux/mfd/max8997-private.h | 16
2 files changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 9d27cc23038f..68754ac2c8ea 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -46,15 +46,15 @@ struct max8997_muic_irq {
};
static struct max8997_muic_irq muic_irqs[] = {
- { MAX8997_MUICIRQ_ADCError, "muic-ADCERROR" },
- { MAX8997_MUICIRQ_ADCLow, "muic-ADCLOW" },
- { MAX8997_MUICIRQ_ADC, "muic-ADC" },
- { MAX8997_MUICIRQ_VBVolt, "muic-VBVOLT" },
- { MAX8997_MUICIRQ_DBChg,"muic-DBCHG" },
- { MAX8997_MUICIRQ_DCDTmr, "muic-DCDTMR" },
- { MAX8997_MUICIRQ_ChgDetRun,"muic-CHGDETRUN" },
- { MAX8997_MUICIRQ_ChgTyp, "muic-CHGTYP" },
- { MAX8997_MUICIRQ_OVP, "muic-OVP" },
+ { MAX8997_MUICIRQ_ADCERROR, "MUIC-ADCERROR" },
+ { MAX8997_MUICIRQ_ADCLOW, "MUIC-ADCLOW" },
+ { MAX8997_MUICIRQ_ADC, "MUIC-ADC" },
+ { MAX8997_MUICIRQ_VBVOLT, "MUIC-VBVOLT" },
+ { MAX8997_MUICIRQ_DBCHG,"MUIC-DBCHG" },
+ { MAX8997_MUICIRQ_DCDTMR, "MUIC-DCDTMR" },
+ { MAX8997_MUICIRQ_CHGDETRUN,"MUIC-CHGDETRUN" },
+ { MAX8997_MUICIRQ_CHGTYP, "MUIC-CHGTYP" },
+ { MAX8997_MUICIRQ_OVP, "MUIC-OVP" },
};
/* Define supported cable type */
@@ -540,17 +540,17 @@ static void max8997_muic_irq_work(struct work_struct
*work)
}
switch (irq_type) {
- case MAX8997_MUICIRQ_ADCError:
- case MAX8997_MUICIRQ_ADCLow:
+ case MAX8997_MUICIRQ_ADCERROR:
+ case MAX8997_MUICIRQ_ADCLOW:
case MAX8997_MUICIRQ_ADC:
/* Handle all of cable except for charger cable */
ret = max8997_muic_adc_handler(info);
break;
- case MAX8997_MUICIRQ_VBVolt:
- case MAX8997_MUICIRQ_DBChg:
- case MAX8997_MUICIRQ_DCDTmr:
- case MAX8997_MUICIRQ_ChgDetRun:
- case MAX8997_MUICIRQ_ChgTyp:
+ case MAX8997_MUICIRQ_VBVOLT:
+ case MAX8997_MUICIRQ_DBCHG:
+ case MAX8997_MUICIRQ_DCDTMR:
+ case MAX8997_MUICIRQ_CHGDETRUN:
+ case MAX8997_MUICIRQ_CHGTYP:
/* Handle charger cable */
ret = max8997_muic_chg_handler(info);
break;
diff --git a/include/linux/mfd/max8997-private.h
b/include/linux/mfd/max8997-private.h
index f42ea222988f..2817fa69383e 100644
--- a/include/linux/mfd/max8997-private.h
+++ b/include/linux/mfd/max8997-private.h
@@ -411,14 +411,14 @@ enum max8997_irq {
enum max8997_irq_muic {
MAX8997_MUICIRQ_ADC,
- MAX8997_MUICIRQ_ADCLow,
- MAX8997_MUICIRQ_ADCError,
-
- MAX8997_MUICIRQ_ChgTyp,
- MAX8997_MUICIRQ_ChgDetRun,
- MAX8997_MUICIRQ_DCDTmr,
- MAX8997_MUICIRQ_DBChg,
- MAX8997_MUICIRQ_VBVolt,
+ MAX8997_MUICIRQ_ADCLOW,
+ MAX8997_MUICIRQ_ADCERROR,
+
+ MAX8997_MUICIRQ_CHGTYP,
+ MAX8997_MUICIRQ_CHGDETRUN,
+ MAX8997_MUICIRQ_DCDTMR,
+ MAX8997_MUICIRQ_DBCHG,
+ MAX8997_MUICIRQ_VBVOLT,
MAX8997_MUICIRQ_OVP,
--
1.9.1
Re: [PATCH V2 4/4] clk: max77686: Add support for MAX77620 clocks
On 06/17/2016 07:53 AM, Laxman Dewangan wrote: > > On Friday 17 June 2016 11:33 AM, Krzysztof Kozlowski wrote: >> On 06/16/2016 03:23 PM, Laxman Dewangan wrote: >>> config COMMON_CLK_MAX77686 >>> -tristate "Clock driver for Maxim 77686/77802 MFD" >>> -depends on MFD_MAX77686 >>> +tristate "Clock driver for Maxim 77686/77802/MAX77620 MFD" >> You missed Javier's comment here. Just >> "Maxim 77686/77802/77620" > > Sigh.. did not see this comment when going one by one.. really missed.. > I will collect all Reviewed/ack/tested and will post V3 with fixing this. No problem. I think the device names could be in numerical order, so: Maxim 77620/77686/77802 Best regards, Krzysztof
[RESEND v7 4/6] extcon: max8997: Fix handling error code of regmap_irq_get_virq()
The regmap_irq_get_virq() can return negative ERRNO, so its result
should not be stored in unsigned int because error would be ignored.
Signed-off-by: Krzysztof Kozlowski
Acked-by: Chanwoo Choi
---
drivers/extcon/extcon-max8997.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index 68754ac2c8ea..0afa3e8b5dc3 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -660,11 +660,11 @@ static int max8997_muic_probe(struct platform_device
*pdev)
for (i = 0; i < ARRAY_SIZE(muic_irqs); i++) {
struct max8997_muic_irq *muic_irq = &muic_irqs[i];
- unsigned int virq = 0;
+ int virq = 0;
virq = regmap_irq_get_virq(max8997->irq_data_muic,
muic_irq->irq);
- if (!virq) {
+ if (virq <= 0) {
ret = -EINVAL;
goto err_irq;
}
--
1.9.1
[svcrpc] 12e5cf2d75: BUG: unable to handle kernel NULL pointer dereference at 0000000000000014
FYI, we noticed the following commit:
https://github.com/0day-ci/linux
Chuck-Lever/NFS-RDMA-client-patches-proposed-for-v4-8/20160615-114209
commit 12e5cf2d7547fc43a3004cdf52e562abfc95eb55 ("svcrpc: remove unnecessary
svc_exit_thread serv check")
in testcase: fsmark
with following parameters:
disk=1HDD/filesize=16MB/fs2=nfsv4/fs=ext4/iterations=1x/nr_directories=16d/nr_files_per_directory=256fpd/nr_threads=32t/sync_method=NoSync/test_size=60G
on test machine: vm-vp-1G: 2 threads qemu-system-x86_64 -enable-kvm -cpu
Nehalem with 1G memory
caused below changes:
+--+++
| | 5a9570253a | 12e5cf2d75 |
+--+++
| boot_successes | 8 | 4 |
| boot_failures| 0 | 4 |
| BUG:unable_to_handle_kernel | 0 | 4 |
| Oops | 0 | 4 |
| RIP:svc_destroy | 0 | 4 |
| Kernel_panic-not_syncing:Fatal_exception | 0 | 4 |
| backtrace:nfsd | 0 | 4 |
+--+++
[ 106.491736] random: nonblocking pool is initialized
Terminated
[ 3635.898268] LKP: rebooting
[ 3636.338752] BUG: unable to handle kernel NULL pointer dereference at
0014
[ 3636.340257] IP: [] svc_destroy+0xa/0x8e
[ 3636.341284] PGD 0
[ 3636.341715] Oops: [#1] SMP
[ 3636.342301] Modules linked in: acpi_cpufreq
[ 3636.343143] CPU: 0 PID: 2686 Comm: nfsd Not tainted 4.7.0-rc3-6-g12e5cf2
#1
[ 3636.37] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
Debian-1.8.2-1 04/01/2014
[ 3636.351717] task: 88003e8f2580 ti: 88003e6ac000 task.ti:
88003e6ac000
[ 3636.353040] RIP: 0010:[] []
svc_destroy+0xa/0x8e
[ 3636.354525] RSP: :88003e6afe68 EFLAGS: 00010246
[ 3636.355434] RAX: 2710 RBX: 880019714000 RCX: 0001
[ 3636.356660] RDX: 8225df80 RSI: 0010 RDI:
[ 3636.357864] RBP: 88003e6afe70 R08: 0001 R09: 81a25511
[ 3636.359725] R10: 88003e6afe38 R11: 0124 R12: 88003ed2f500
[ 3636.361563] R13: 88003ed2f504 R14: R15:
[ 3636.363402] FS: () GS:88003820()
knlGS:
[ 3636.366025] CS: 0010 DS: ES: CR0: 80050033
[ 3636.367651] CR2: 0014 CR3: 35a55000 CR4: 06f0
[ 3636.369493] Stack:
[ 3636.370482] 880019714000 88003e6afea0 81a25769
880019714000
[ 3636.372930] 82355c00 88001c674c00 88003e8f2580
88003e6afec8
[ 3636.375241] 8137b367 8800227a1040 880019714000
8137b228
[ 3636.377532] Call Trace:
[ 3636.378458] [] svc_exit_thread+0x72/0x7b
[ 3636.392474] [] nfsd+0x13f/0x15c
[ 3636.393743] [] ? nfsd_destroy+0x6a/0x6a
[ 3636.395098] [] kthread+0xdd/0xe5
[ 3636.396387] [] ret_from_fork+0x1f/0x40
[ 3636.397725] [] ? kthread_create_on_node+0x18a/0x18a
[ 3636.399234] Code: e8 58 e0 00 00 48 8b 83 98 00 00 00 48 8b 00 48 85 c0 74
08 4c 89 e6 48 89 df ff d0 5b 41 5c 5d c3 66 66 66 66 90 55 48 89 e5 53 <8b> 47
14 48 89 fb 85 c0 74 10 ff c8 85 c0 89 47 14 74 18 e8 ba
[ 3636.418176] RIP [] svc_destroy+0xa/0x8e
[ 3636.419584] RSP
[ 3636.420643] CR2: 0014
[ 3636.422097] ---[ end trace c906da9fce8f6a66 ]---
[ 3636.423409] Kernel panic - not syncing: Fatal exception
FYI, raw QEMU command line is:
qemu-system-x86_64 -enable-kvm -cpu Nehalem -kernel
/pkg/linux/x86_64-lkp/gcc-4.9/12e5cf2d7547fc43a3004cdf52e562abfc95eb55/vmlinuz-4.7.0-rc3-6-g12e5cf2
-append 'root=/dev/ram0 user=lkp
job=/lkp/scheduled/vm-vp-1G-6/bisect_fsmark-1x-32t-1HDD-ext4-nfsv4-16MB-60G-NoSync-16d-256fpd-debian-x86_64-2015-02-07.cgz-x86_64-lkp-12e5cf2d7547fc43a3004cdf52e562abfc95eb55-20160616-124433-16zojbo-0.yaml~
ARCH=x86_64 kconfig=x86_64-lkp branch=linux-devel/devel-spot-201606151525
commit=12e5cf2d7547fc43a3004cdf52e562abfc95eb55
BOOT_IMAGE=/pkg/linux/x86_64-lkp/gcc-4.9/12e5cf2d7547fc43a3004cdf52e562abfc95eb55/vmlinuz-4.7.0-rc3-6-g12e5cf2
max_uptime=3600
RESULT_ROOT=/result/fsmark/1x-32t-1HDD-ext4-nfsv4-16MB-60G-NoSync-16d-256fpd/vm-vp-1G/debian-x86_64-2015-02-07.cgz/x86_64-lkp/gcc-4.9/12e5cf2d7547fc43a3004cdf52e562abfc95eb55/0
LKP_SERVER=inn earlyprintk=ttyS0,115200 systemd.log_level=err debug apic=debug
sysrq_always_enabled rcupdate.rcu_cpu_stall_timeout=100 panic=-1
softlockup_panic=1 nmi_watchdog=panic oops=panic load_ramdisk=2
prompt_ramdisk=0 console=ttyS0,115200 console=tty0 vga=normal rw
ip=vm-vp-1G-6::dhcp' -initrd /fs/sdh1/initrd-vm-vp-1G-6 -m 1024 -smp 2
-device e1000,netdev
[RESEND v7 5/6] rtc: max8997: Check for ERRNO of regmap_irq_get_virq()
The regmap_irq_get_virq() can return zero or negative ERRNO, so it is
insufficient to check only for non-zero value.
Signed-off-by: Krzysztof Kozlowski
Acked-by: Alexandre Belloni
---
drivers/rtc/rtc-max8997.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/rtc/rtc-max8997.c b/drivers/rtc/rtc-max8997.c
index 4998aee0f07f..1a06e17ebe9b 100644
--- a/drivers/rtc/rtc-max8997.c
+++ b/drivers/rtc/rtc-max8997.c
@@ -497,7 +497,7 @@ static int max8997_rtc_probe(struct platform_device *pdev)
}
virq = regmap_irq_get_virq(max8997->irq_data, MAX8997_PMICIRQ_RTCA1);
- if (!virq) {
+ if (virq <= 0) {
dev_err(&pdev->dev, "Failed to create mapping alarm IRQ\n");
ret = -ENXIO;
goto err_out;
--
1.9.1
[RESEND v7 6/6] extcon: max8997: Fix inconsistent indenting
Fix smatch warning: max8997_muic_probe() warn: inconsistent indenting Signed-off-by: Krzysztof Kozlowski Acked-by: Chanwoo Choi --- drivers/extcon/extcon-max8997.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c index 0afa3e8b5dc3..cf8de5be2405 100644 --- a/drivers/extcon/extcon-max8997.c +++ b/drivers/extcon/extcon-max8997.c @@ -737,7 +737,7 @@ static int max8997_muic_probe(struct platform_device *pdev) } /* Set initial path for UART */ -max8997_muic_set_path(info, info->path_uart, true); + max8997_muic_set_path(info, info->path_uart, true); /* Set ADC debounce time */ max8997_muic_set_debounce_time(info, ADC_DEBOUNCE_TIME_25MS); -- 1.9.1
[RESEND v7 0/6] regulator: mfd: max8997: Add regmap
Dear Mark, I'm pushing old Robert's work and I miss only ack from you. Could you take a look at patch 1/6? Changes since v6 1. Add some acks. 2. Remove patches already applied. Best regards, Krzysztof Krzysztof Kozlowski (3): extcon: max8997: Fix handling error code of regmap_irq_get_virq() rtc: max8997: Check for ERRNO of regmap_irq_get_virq() extcon: max8997: Fix inconsistent indenting Robert Baldyga (3): mfd: max8997: Use regmap to access registers mfd: max8997: handle IRQs using regmap mfd: max8997: Change irq names to upper case drivers/extcon/extcon-max8997.c | 72 +++ drivers/input/misc/max8997_haptic.c | 34 +-- drivers/leds/leds-max8997.c | 13 +- drivers/mfd/Kconfig | 3 +- drivers/mfd/Makefile | 2 +- drivers/mfd/max8997-irq.c | 383 -- drivers/mfd/max8997.c | 245 +++--- drivers/power/max8997_charger.c | 33 +-- drivers/regulator/max8997-regulator.c | 87 drivers/rtc/rtc-max8997.c | 60 +++--- include/linux/mfd/max8997-private.h | 82 ++-- 11 files changed, 385 insertions(+), 629 deletions(-) delete mode 100644 drivers/mfd/max8997-irq.c -- 1.9.1
[RESEND v7 2/6] mfd: max8997: handle IRQs using regmap
From: Robert Baldyga
This patch modifies mfd driver to use regmap for handling interrupts.
It allows to simplify irq handling process. This modifications needed
to make small changes in function drivers, which use interrupts.
Signed-off-by: Robert Baldyga
Reviewed-by: Krzysztof Kozlowski
Acked-by: Chanwoo Choi
Acked-by: Lee Jones
Acked-by: Alexandre Belloni
[k.kozlowski: Collect acks, rebase on v4.6-rc6]
Signed-off-by: Krzysztof Kozlowski
---
drivers/extcon/extcon-max8997.c | 3 +-
drivers/mfd/Kconfig | 2 +-
drivers/mfd/Makefile| 2 +-
drivers/mfd/max8997-irq.c | 369
drivers/mfd/max8997.c | 112 ++-
drivers/rtc/rtc-max8997.c | 2 +-
include/linux/mfd/max8997-private.h | 63 +-
7 files changed, 165 insertions(+), 388 deletions(-)
delete mode 100644 drivers/mfd/max8997-irq.c
diff --git a/drivers/extcon/extcon-max8997.c b/drivers/extcon/extcon-max8997.c
index fc812257bfc7..9d27cc23038f 100644
--- a/drivers/extcon/extcon-max8997.c
+++ b/drivers/extcon/extcon-max8997.c
@@ -662,7 +662,8 @@ static int max8997_muic_probe(struct platform_device *pdev)
struct max8997_muic_irq *muic_irq = &muic_irqs[i];
unsigned int virq = 0;
- virq = irq_create_mapping(max8997->irq_domain, muic_irq->irq);
+ virq = regmap_irq_get_virq(max8997->irq_data_muic,
+ muic_irq->irq);
if (!virq) {
ret = -EINVAL;
goto err_irq;
diff --git a/drivers/mfd/Kconfig b/drivers/mfd/Kconfig
index d7c6491ce3b9..b1bc339a91f9 100644
--- a/drivers/mfd/Kconfig
+++ b/drivers/mfd/Kconfig
@@ -622,7 +622,7 @@ config MFD_MAX8997
depends on I2C=y
select MFD_CORE
select REGMAP_I2C
- select IRQ_DOMAIN
+ select REGMAP_IRQ
help
Say yes here to add support for Maxim Semiconductor MAX8997/8966.
This is a Power Management IC with RTC, Flash, Fuel Gauge, Haptic,
diff --git a/drivers/mfd/Makefile b/drivers/mfd/Makefile
index 42a66e19e191..cd2cd2b2c316 100644
--- a/drivers/mfd/Makefile
+++ b/drivers/mfd/Makefile
@@ -135,7 +135,7 @@ obj-$(CONFIG_MFD_MAX77843) += max77843.o
obj-$(CONFIG_MFD_MAX8907) += max8907.o
max8925-objs := max8925-core.o max8925-i2c.o
obj-$(CONFIG_MFD_MAX8925) += max8925.o
-obj-$(CONFIG_MFD_MAX8997) += max8997.o max8997-irq.o
+obj-$(CONFIG_MFD_MAX8997) += max8997.o
obj-$(CONFIG_MFD_MAX8998) += max8998.o max8998-irq.o
pcf50633-objs := pcf50633-core.o pcf50633-irq.o
diff --git a/drivers/mfd/max8997-irq.c b/drivers/mfd/max8997-irq.c
deleted file mode 100644
index 6ab5f955c510..
--- a/drivers/mfd/max8997-irq.c
+++ /dev/null
@@ -1,369 +0,0 @@
-/*
- * max8997-irq.c - Interrupt controller support for MAX8997
- *
- * Copyright (C) 2011 Samsung Electronics Co.Ltd
- * MyungJoo Ham
- *
- * This program is free software; you can redistribute it and/or modify
- * it under the terms of the GNU General Public License as published by
- * the Free Software Foundation; either version 2 of the License, or
- * (at your option) any later version.
- *
- * This program is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
- * GNU General Public License for more details.
- *
- * You should have received a copy of the GNU General Public License
- * along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
- *
- * This driver is based on max8998-irq.c
- */
-
-#include
-#include
-#include
-#include
-#include
-#include
-
-static const u8 max8997_mask_reg[] = {
- [PMIC_INT1] = MAX8997_REG_INT1MSK,
- [PMIC_INT2] = MAX8997_REG_INT2MSK,
- [PMIC_INT3] = MAX8997_REG_INT3MSK,
- [PMIC_INT4] = MAX8997_REG_INT4MSK,
- [FUEL_GAUGE] = MAX8997_REG_INVALID,
- [MUIC_INT1] = MAX8997_MUIC_REG_INTMASK1,
- [MUIC_INT2] = MAX8997_MUIC_REG_INTMASK2,
- [MUIC_INT3] = MAX8997_MUIC_REG_INTMASK3,
- [GPIO_LOW] = MAX8997_REG_INVALID,
- [GPIO_HI] = MAX8997_REG_INVALID,
- [FLASH_STATUS] = MAX8997_REG_INVALID,
-};
-
-struct max8997_irq_data {
- int mask;
- enum max8997_irq_source group;
-};
-
-#define DECLARE_IRQ(idx, _group, _mask)\
- [(idx)] = { .group = (_group), .mask = (_mask) }
-static const struct max8997_irq_data max8997_irqs[] = {
- DECLARE_IRQ(MAX8997_PMICIRQ_PWRONR, PMIC_INT1, 1 << 0),
- DECLARE_IRQ(MAX8997_PMICIRQ_PWRONF, PMIC_INT1, 1 << 1),
- DECLARE_IRQ(MAX8997_PMICIRQ_PWRON1SEC, PMIC_INT1, 1 << 3),
- DECLARE_IRQ(MAX8997_PMICIRQ_JIGONR, PMIC_INT1, 1 << 4),
- DECLARE_IRQ(MAX8997_PMICIRQ_JIGONF, PMIC_INT
Re: [PATCH] media: fix media devnode ioctl/syscall and unregister race
Hi Shuah,
On Fri, Jun 10, 2016 at 11:37:23AM -0600, Shuah Khan wrote:
> Media devnode open/ioctl could be in progress when media device unregister
> is initiated. System calls and ioctls check media device registered status
> at the beginning, however, there is a window where unregister could be in
> progress without changing the media devnode status to unregistered.
>
> process 1 process 2
> fd = open(/dev/media0)
> media_devnode_is_registered()
> (returns true here)
>
> media_device_unregister()
> (unregister is in progress
> and devnode isn't
> unregistered yet)
> ...
> ioctl(fd, ...)
> __media_ioctl()
> media_devnode_is_registered()
> (returns true here)
> ...
> media_devnode_unregister()
> ...
> (driver releases the media device
> memory)
>
> media_device_ioctl()
> (By this point
> devnode->media_dev does not
> point to allocated memory.
> use-after free in in mutex_lock_nested)
>
> BUG: KASAN: use-after-free in mutex_lock_nested+0x79c/0x800 at addr
> 8801ebe914f0
>
> Fix it by clearing register bit when unregister starts to avoid the race.
Does this patch solve the problem? You'd have to take the mutex for the
duration of the IOCTL which I don't see the patch doing.
Instead of serialising operations using mutexes, I believe a proper fix for
this is to take a reference to the data structures required.
>
> process 1 process 2
> fd = open(/dev/media0)
> media_devnode_is_registered()
> (could return true here)
>
> media_device_unregister()
> (clear the register bit,
>then start unregister.)
> ...
> ioctl(fd, ...)
> __media_ioctl()
> media_devnode_is_registered()
> (return false here, ioctl
>returns I/O error, and
>will not access media
>device memory)
> ...
> media_devnode_unregister()
> ...
> (driver releases the media device
>memory)
>
> Signed-off-by: Shuah Khan
> Suggested-by: Sakari Ailus
> Reported-by: Mauro Carvalho Chehab
> ---
>
> Test Procedure and Results:
>
> https://drive.google.com/file/d/0B0NIL0BQg-AlN1VxT2oyTXBPRHc/view?usp=sharing
>
> drivers/media/media-device.c | 15 ---
> drivers/media/media-devnode.c | 8 +++-
> include/media/media-devnode.h | 16 ++--
> 3 files changed, 29 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/media/media-device.c b/drivers/media/media-device.c
> index 33a9952..1795abe 100644
> --- a/drivers/media/media-device.c
> +++ b/drivers/media/media-device.c
> @@ -732,6 +732,7 @@ int __must_check __media_device_register(struct
> media_device *mdev,
> if (ret < 0) {
> /* devnode free is handled in media_devnode_*() */
> mdev->devnode = NULL;
> + media_devnode_unregister_prepare(devnode);
> media_devnode_unregister(devnode);
> return ret;
> }
> @@ -788,6 +789,9 @@ void media_device_unregister(struct media_device *mdev)
> return;
> }
>
> + /* Clear the devnode register bit to avoid races with media dev open */
> + media_devnode_unregister_prepare(mdev->devnode);
> +
> /* Remove all entities from the media device */
> list_for_each_entry_safe(entity, next, &mdev->entities, graph_obj.list)
> __media_device_unregister_entity(entity);
> @@ -808,13 +812,10 @@ void media_device_unregister(struct media_device *mdev)
>
> dev_dbg(mdev->dev, "Media device unregistered\n");
>
> - /* Check if mdev devnode was registered */
> - if (media_devnode_is_registered(mdev->devnode)) {
> - device_remove_file(&mdev->devnode->dev, &dev_attr_model);
> - media_devnode_unregister(mdev->devnode);
> - /* devnode free is handled in media_devnode_*() */
> - mdev->devnode = NULL;
> - }
> + device_remove_file(&mdev->devnode->dev, &dev_attr_model);
> + media_devnode_unregister(mdev->devnode);
> + /* devnode free is handled in media_devnode_*() */
> + mdev->devnode = NULL;
> }
> EXPORT_SYMBOL_GPL(media_device_unregister);
>
> diff --git a/drivers/media/media-devnode.c b/drivers/media/media-devnode.c
> index 5b605ff..f2772ba
Re: [PATCH V2 4/4] clk: max77686: Add support for MAX77620 clocks
On Friday 17 June 2016 11:33 AM, Krzysztof Kozlowski wrote: On 06/16/2016 03:23 PM, Laxman Dewangan wrote: config COMMON_CLK_MAX77686 - tristate "Clock driver for Maxim 77686/77802 MFD" - depends on MFD_MAX77686 + tristate "Clock driver for Maxim 77686/77802/MAX77620 MFD" You missed Javier's comment here. Just "Maxim 77686/77802/77620" Sigh.. did not see this comment when going one by one.. really missed.. I will collect all Reviewed/ack/tested and will post V3 with fixing this.
Re: [PATCH V2 4/4] clk: max77686: Add support for MAX77620 clocks
On 06/16/2016 03:23 PM, Laxman Dewangan wrote: > Maxim Max77620 has one 32KHz clock output and the clock HW > IP used on this PMIC is same as what it is there in the MAX77686. > > Add clock driver support for MAX77620 on the MAX77686 driver. > > Signed-off-by: Laxman Dewangan > CC: Krzysztof Kozlowski > CC: Javier Martinez Canillas > Signed-off-by: Laxman Dewangan > Tested-by: Krzysztof Kozlowski > > --- > Changes from V1: > - Updated based on changes done in first patch. > --- > drivers/clk/Kconfig| 7 --- > drivers/clk/clk-max77686.c | 18 ++ > 2 files changed, 22 insertions(+), 3 deletions(-) > > diff --git a/drivers/clk/Kconfig b/drivers/clk/Kconfig > index 6afad74..d75f4c5 100644 > --- a/drivers/clk/Kconfig > +++ b/drivers/clk/Kconfig > @@ -32,10 +32,11 @@ config COMMON_CLK_WM831X > source "drivers/clk/versatile/Kconfig" > > config COMMON_CLK_MAX77686 > - tristate "Clock driver for Maxim 77686/77802 MFD" > - depends on MFD_MAX77686 > + tristate "Clock driver for Maxim 77686/77802/MAX77620 MFD" You missed Javier's comment here. Just "Maxim 77686/77802/77620" > + depends on MFD_MAX77686 || MFD_MAX77620 > ---help--- > - This driver supports Maxim 77686/77802 crystal oscillator clock. > + This driver supports Maxim 77686/77802/MAX77620 crystal oscillator Ditto Beside that: Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH V2 1/4] clk: max77686: Combine Maxim max77686 and max77802 driver
On 06/16/2016 03:23 PM, Laxman Dewangan wrote: > The clock IP used on the Maxim PMICs max77686 and max77802 are > same. The configuration of clock register is also same except > the number of clocks. > > Part of common code utilisation, there is 3 files for these chips > clock driver, one for common and two files for driver registration. > > Combine both drivers into single file and move common code into > same common file reduces the 2 files and make max77686 and max77802 > clock driver in single fine. This driver does not depends on the > parent driver structure. The regmap handle is acquired through > regmap APIs for the register access. > > This combination of driver helps on adding clock driver for different > Maxim PMICs which has similar clock IP like MAX77620 and MAX20024. > > Signed-off-by: Laxman Dewangan > CC: Krzysztof Kozlowski > CC: Javier Martinez Canillas > Reviewed-by: Javier Martinez Canillas > Tested-by: Javier Martinez Canillas > Tested-by: Krzysztof Kozlowski > > --- > Changes from V1: > - Collected RB and TB from Javier. > - Change the max_gen to max77686. > - Rename structure and avoid duplication for reg and mask. > - Taken care of comment from Krzysztof on coding. > --- > drivers/clk/Kconfig| 15 +-- > drivers/clk/Makefile | 2 - > drivers/clk/clk-max-gen.c | 194 --- > drivers/clk/clk-max-gen.h | 32 -- > drivers/clk/clk-max77686.c | 244 > ++--- > drivers/clk/clk-max77802.c | 96 -- > 6 files changed, 234 insertions(+), 349 deletions(-) > delete mode 100644 drivers/clk/clk-max-gen.c > delete mode 100644 drivers/clk/clk-max-gen.h > delete mode 100644 drivers/clk/clk-max77802.c Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH V2 3/4] clk: max77686: Add DT binding details for PMIC MAX77620
On 06/16/2016 03:23 PM, Laxman Dewangan wrote: > Maxim has used the same clock IP on multiple PMICs like MAX77686, > MAX77802, MAX77620. Only differences are the number of clocks > from these PMICs like MAX77686 has 3 clocks output, MAX776802 have > two clock output and MAX77620 has one clock output. > > Add clock binding details and DT example for the MAX77620. > > Signed-off-by: Laxman Dewangan > CC: Krzysztof Kozlowski > CC: Javier Martinez Canillas > > --- > Changes from V1: > - Use proper comment for the > - Added dt binding preprocessor file as maxim,max77620.h for max77620 > inline with the other chip. > - Refer the dt binding header for the supported clock index. > --- > .../devicetree/bindings/clock/maxim,max77686.txt | 52 > +- > include/dt-bindings/clock/maxim,max77620.h | 21 + > 2 files changed, 63 insertions(+), 10 deletions(-) > create mode 100644 include/dt-bindings/clock/maxim,max77620.h Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH V2 2/4] clk: Combine DT binding doc for max77686 and max77802
On 06/16/2016 03:23 PM, Laxman Dewangan wrote: > The clock IP used on the Maxim PMICs max77686 and max77802 are > same. The configuration of clock register is also same except > the number of clocks. > > Define the common DT binding file for the clocks of Maxim PMICs > MAX77686 and MAX77802. For this, remove the separate DT binding > document file for maxim,max77802 and move all information to > maxim,max77686 DT binding document. > > Signed-off-by: Laxman Dewangan > CC: Krzysztof Kozlowski > CC: Javier Martinez Canillas > > --- > Changes from V1: > - Rewrite description to remvoe ref to driver. > - Taken care of comment from Krzysztof on the supported clocks and > DT bindings. > --- > .../devicetree/bindings/clock/maxim,max77686.txt | 82 > -- > .../devicetree/bindings/clock/maxim,max77802.txt | 44 > 2 files changed, 59 insertions(+), 67 deletions(-) > delete mode 100644 Documentation/devicetree/bindings/clock/maxim,max77802.txt Reviewed-by: Krzysztof Kozlowski Best regards, Krzysztof
Re: [PATCH 2/2] security,perf: Allow further restriction of perf_event_open
Ben Hutchings writes: > When kernel.perf_event_open is set to 3 (or greater), disallow all > access to performance events by users without CAP_SYS_ADMIN. > Add a Kconfig symbol CONFIG_SECURITY_PERF_EVENTS_RESTRICT that > makes this value the default. So this patch does two things, can it then be made into two patches? > > This is based on a similar feature in grsecurity > (CONFIG_GRKERNSEC_PERF_HARDEN). This version doesn't include making > the variable read-only. It also allows enabling further restriction > at run-time regardless of whether the default is changed. This paragraph doesn't seem to belong in the commit message. What this commit message is missing entirely is the rationale behind this change other than "grsecurity does the same". Can you please elaborate? > Signed-off-by: Ben Hutchings > --- > I made a similar change to Debian's kernel packages in August, > including the more restrictive default, and no-one has complained yet. As a debian user, is this a good place to complain? Because it does get it the way. Thanks, -- Alex
Re: [LKP] [lkp] [mm] 5c0a85fad9: unixbench.score -6.3% regression
On Thu, Jun 16, 2016 at 03:27:44PM -0700, Huang, Ying wrote:
> Minchan Kim writes:
>
> > On Thu, Jun 16, 2016 at 07:52:26AM +0800, Huang, Ying wrote:
> >> "Kirill A. Shutemov" writes:
> >>
> >> > On Tue, Jun 14, 2016 at 05:57:28PM +0900, Minchan Kim wrote:
> >> >> On Wed, Jun 08, 2016 at 11:58:11AM +0300, Kirill A. Shutemov wrote:
> >> >> > On Wed, Jun 08, 2016 at 04:41:37PM +0800, Huang, Ying wrote:
> >> >> > > "Huang, Ying" writes:
> >> >> > >
> >> >> > > > "Kirill A. Shutemov" writes:
> >> >> > > >
> >> >> > > >> On Mon, Jun 06, 2016 at 10:27:24AM +0800, kernel test robot
> >> >> > > >> wrote:
> >> >> > > >>>
> >> >> > > >>> FYI, we noticed a -6.3% regression of unixbench.score due to
> >> >> > > >>> commit:
> >> >> > > >>>
> >> >> > > >>> commit 5c0a85fad949212b3e059692deecdeed74ae7ec7 ("mm: make
> >> >> > > >>> faultaround produce old ptes")
> >> >> > > >>> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
> >> >> > > >>> master
> >> >> > > >>>
> >> >> > > >>> in testcase: unixbench
> >> >> > > >>> on test machine: lituya: 16 threads Haswell High-end Desktop
> >> >> > > >>> (i7-5960X 3.0G) with 16G memory
> >> >> > > >>> with following parameters:
> >> >> > > >>> cpufreq_governor=performance/nr_task=1/test=shell8
> >> >> > > >>>
> >> >> > > >>>
> >> >> > > >>> Details are as below:
> >> >> > > >>> -->
> >> >> > > >>>
> >> >> > > >>>
> >> >> > > >>> =
> >> >> > > >>> compiler/cpufreq_governor/kconfig/nr_task/rootfs/tbox_group/test/testcase:
> >> >> > > >>>
> >> >> > > >>> gcc-4.9/performance/x86_64-rhel/1/debian-x86_64-2015-02-07.cgz/lituya/shell8/unixbench
> >> >> > > >>>
> >> >> > > >>> commit:
> >> >> > > >>> 4b50bcc7eda4d3cc9e3f2a0aa60e590fedf728c5
> >> >> > > >>> 5c0a85fad949212b3e059692deecdeed74ae7ec7
> >> >> > > >>>
> >> >> > > >>> 4b50bcc7eda4d3cc 5c0a85fad949212b3e059692de
> >> >> > > >>> --
> >> >> > > >>>fail:runs %reproductionfail:runs
> >> >> > > >>>| | |
> >> >> > > >>> 3:4 -75%:4
> >> >> > > >>> kmsg.DHCP/BOOTP:Reply_not_for_us,op[#]xid[#]
> >> >> > > >>> %stddev %change %stddev
> >> >> > > >>> \ |\
> >> >> > > >>> 14321 . 0% -6.3% 13425 . 0% unixbench.score
> >> >> > > >>>1996897 . 0% -6.1%1874635 . 0%
> >> >> > > >>> unixbench.time.involuntary_context_switches
> >> >> > > >>> 1.721e+08 . 0% -6.2% 1.613e+08 . 0%
> >> >> > > >>> unixbench.time.minor_page_faults
> >> >> > > >>> 758.65 . 0% -3.0% 735.86 . 0%
> >> >> > > >>> unixbench.time.system_time
> >> >> > > >>> 387.66 . 0% +5.4% 408.49 . 0%
> >> >> > > >>> unixbench.time.user_time
> >> >> > > >>>5950278 . 0% -6.2%5583456 . 0%
> >> >> > > >>> unixbench.time.voluntary_context_switches
> >> >> > > >>
> >> >> > > >> That's weird.
> >> >> > > >>
> >> >> > > >> I don't understand why the change would reduce number or minor
> >> >> > > >> faults.
> >> >> > > >> It should stay the same on x86-64. Rise of user_time is puzzling
> >> >> > > >> too.
> >> >> > > >
> >> >> > > > unixbench runs in fixed time mode. That is, the total time to run
> >> >> > > > unixbench is fixed, but the work done varies. So the
> >> >> > > > minor_page_faults
> >> >> > > > change may reflect only the work done.
> >> >> > > >
> >> >> > > >> Hm. Is reproducible? Across reboot?
> >> >> > > >
> >> >> > >
> >> >> > > And FYI, there is no swap setup for test, all root file system
> >> >> > > including
> >> >> > > benchmark files are in tmpfs, so no real page reclaim will be
> >> >> > > triggered. But it appears that active file cache reduced after the
> >> >> > > commit.
> >> >> > >
> >> >> > > 111331 . 1% -13.3% 96503 . 0% meminfo.Active
> >> >> > > 27603 . 1% -43.9% 15486 . 0% meminfo.Active(file)
> >> >> > >
> >> >> > > I think this is the expected behavior of the commit?
> >> >> >
> >> >> > Yes, it's expected.
> >> >> >
> >> >> > After the change faularound would produce old pte. It means there's
> >> >> > more
> >> >> > chance for these pages to be on inactive lru, unless somebody actually
> >> >> > touch them and flip accessed bit.
> >> >>
> >> >> Hmm, tmpfs pages should be in anonymous LRU list and VM shouldn't scan
> >> >> anonymous LRU list on swapless system so I really wonder why active file
> >> >> LRU is shrunk.
> >> >
> >> > Hm. Good point. I don't why we have anything on file lru if there's no
> >> > filesystems except tmpfs.
> >> >
> >> > Ying, how do you get stuff to the tmpfs?
> >>
> >> We put root file system and benchmark into a set of compressed cpio
> >> archive, then concatenate them into on
Re: net: stmmac: dwmac-rk: fixes for Wake-on-Lan on RK3288
On 6/16/2016 4:51 PM, Vincent Palatin wrote: Hi Giuseppe, On Thu, Jun 16, 2016 at 6:37 AM, Giuseppe CAVALLARO wrote: Hi Vincent On 6/15/2016 7:04 PM, Vincent Palatin wrote: On Sun, Jun 12, 2016 at 11:46 PM, Giuseppe CAVALLARO wrote: On 6/11/2016 3:00 AM, Vincent Palatin wrote: In order to support Wake-On-Lan when using the RK3288 integrated MAC (with an external RGMII PHY), we need to avoid shutting down the regulator of the external PHY when the MAC is suspended as it's currently done in the MAC platform code. As a first step, create independant callbacks for suspend/resume rather than re-using exit/init callbacks. So the dwmac platform driver can behave differently on suspend where it might skip shutting the PHY and at module unloading. Then update the dwmac-rk driver to switch off the PHY regulator only if we are not planning to wake up from the LAN. Finally add the PMT interrupt to the MAC device tree configuration, so we can wake up the core from it when the PHY has received the magic packet. IMO these could be sent for net-next and also other glue logic files should be reworked in order to use the new API for coherence. Given they will have the same set of functions for exit/init and suspend/resume, you mean duplicating the callbacks like this : --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sti.c @@ -359,6 +359,8 @@ static int sti_dwmac_probe(struct platform_device *pdev) plat_dat->bsp_priv = dwmac; plat_dat->init = sti_dwmac_init; plat_dat->exit = sti_dwmac_exit; + plat_dat->suspend = sti_dwmac_exit; + plat_dat->resume = sti_dwmac_init; plat_dat->fix_mac_speed = data->fix_retime_src; ret = sti_dwmac_init(pdev, plat_dat->bsp_priv); Is this anyhow useful ? I think this is mandatory otherwise you are not guaranteeing the PM stuff working on the rest of the glue-logics (not only sti); because init/exit calls won't be called anymore. As mentioned in the PATCH 1/3 description: "If the driver does not provide the suspend or resume callback, we fall back to the old behavior trying to use exit or init. [...]" ie. --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c @@ -411,7 +411,9 @@ static int stmmac_pltfr_suspend(struct device *dev) struct platform_device *pdev = to_platform_device(dev); ret = stmmac_suspend(dev); - if (priv->plat->exit) + if (priv->plat->suspend) + priv->plat->suspend(pdev, priv->plat->bsp_priv); + else if (priv->plat->exit) priv->plat->exit(pdev, priv->plat->bsp_priv); return ret; @@ -430,7 +432,9 @@ static int stmmac_pltfr_resume(struct device *dev) struct stmmac_priv *priv = netdev_priv(ndev); struct platform_device *pdev = to_platform_device(dev); - if (priv->plat->init) + if (priv->plat->resume) + priv->plat->resume(pdev, priv->plat->bsp_priv); + else if (priv->plat->init) priv->plat->init(pdev, priv->plat->bsp_priv); return stmmac_resume(dev); So I was under the impression that everything should continue working as before for drivers only providing init/exit, by falling back on calling ->exit() if there is no suspend() callback initialized for the PM calls. You think that won't work ? it's ok for me. Peppe So I kindly ask you to propagate the fix and send the V3. The implementation above is ok for me. peppe
Re: [BUG] act_ife: sleeping functions called in atomic context
On Thu, Jun 16, 2016 at 7:14 PM, Cong Wang wrote:
>
> I think we can just remove that tcf_lock, I am testing a patch now.
Please try the attached patch, I will do more tests tomorrow.
Thanks!
diff --git a/net/sched/act_ife.c b/net/sched/act_ife.c
index 658046d..859fb02 100644
--- a/net/sched/act_ife.c
+++ b/net/sched/act_ife.c
@@ -240,8 +240,7 @@ static int ife_validate_metatype(struct tcf_meta_ops *ops,
void *val, int len)
}
/* called when adding new meta information
- * under ife->tcf_lock
-*/
+ */
static int load_metaops_and_vet(struct tcf_ife_info *ife, u32 metaid,
void *val, int len)
{
@@ -251,11 +250,9 @@ static int load_metaops_and_vet(struct tcf_ife_info *ife,
u32 metaid,
if (!ops) {
ret = -ENOENT;
#ifdef CONFIG_MODULES
- spin_unlock_bh(&ife->tcf_lock);
rtnl_unlock();
request_module("ifemeta%u", metaid);
rtnl_lock();
- spin_lock_bh(&ife->tcf_lock);
ops = find_ife_oplist(metaid);
#endif
}
@@ -272,8 +269,8 @@ static int load_metaops_and_vet(struct tcf_ife_info *ife,
u32 metaid,
}
/* called when adding new meta information
- * under ife->tcf_lock
-*/
+ * under RTNL lock
+ */
static int add_metainfo(struct tcf_ife_info *ife, u32 metaid, void *metaval,
int len)
{
@@ -284,7 +281,7 @@ static int add_metainfo(struct tcf_ife_info *ife, u32
metaid, void *metaval,
if (!ops)
return -ENOENT;
- mi = kzalloc(sizeof(*mi), GFP_KERNEL);
+ mi = kzalloc(sizeof(*mi), GFP_ATOMIC);
if (!mi) {
/*put back what find_ife_oplist took */
module_put(ops->owner);
@@ -302,8 +299,7 @@ static int add_metainfo(struct tcf_ife_info *ife, u32
metaid, void *metaval,
}
}
- list_add_tail(&mi->metalist, &ife->metalist);
-
+ list_add_tail_rcu(&mi->metalist, &ife->metalist);
return ret;
}
@@ -313,11 +309,13 @@ static int use_all_metadata(struct tcf_ife_info *ife)
int rc = 0;
int installed = 0;
+ read_lock(&ife_mod_lock);
list_for_each_entry(o, &ifeoplist, list) {
rc = add_metainfo(ife, o->metaid, NULL, 0);
if (rc == 0)
installed += 1;
}
+ read_unlock(&ife_mod_lock);
if (installed)
return 0;
@@ -340,10 +338,12 @@ static int dump_metalist(struct sk_buff *skb, struct
tcf_ife_info *ife)
if (!nest)
goto out_nlmsg_trim;
- list_for_each_entry(e, &ife->metalist, metalist) {
+ rcu_read_lock();
+ list_for_each_entry_rcu(e, &ife->metalist, metalist) {
if (!e->ops->get(skb, e))
total_encoded += 1;
}
+ rcu_read_unlock();
if (!total_encoded)
goto out_nlmsg_trim;
@@ -357,15 +357,14 @@ out_nlmsg_trim:
return -1;
}
-/* under ife->tcf_lock */
-static void _tcf_ife_cleanup(struct tc_action *a, int bind)
+static void tcf_ife_cleanup(struct tc_action *a, int bind)
{
struct tcf_ife_info *ife = a->priv;
struct tcf_meta_info *e, *n;
list_for_each_entry_safe(e, n, &ife->metalist, metalist) {
module_put(e->ops->owner);
- list_del(&e->metalist);
+ list_del_rcu(&e->metalist);
if (e->metaval) {
if (e->ops->release)
e->ops->release(e);
@@ -376,16 +375,6 @@ static void _tcf_ife_cleanup(struct tc_action *a, int bind)
}
}
-static void tcf_ife_cleanup(struct tc_action *a, int bind)
-{
- struct tcf_ife_info *ife = a->priv;
-
- spin_lock_bh(&ife->tcf_lock);
- _tcf_ife_cleanup(a, bind);
- spin_unlock_bh(&ife->tcf_lock);
-}
-
-/* under ife->tcf_lock */
static int populate_metalist(struct tcf_ife_info *ife, struct nlattr **tb)
{
int len = 0;
@@ -474,7 +463,6 @@ static int tcf_ife_init(struct net *net, struct nlattr *nla,
saddr = nla_data(tb[TCA_IFE_SMAC]);
}
- spin_lock_bh(&ife->tcf_lock);
ife->tcf_action = parm->action;
if (parm->flags & IFE_ENCODE) {
@@ -502,9 +490,8 @@ metadata_parse_err:
if (exists)
tcf_hash_release(a, bind);
if (ret == ACT_P_CREATED)
- _tcf_ife_cleanup(a, bind);
+ tcf_ife_cleanup(a, bind);
- spin_unlock_bh(&ife->tcf_lock);
return err;
}
@@ -521,15 +508,12 @@ metadata_parse_err:
err = use_all_metadata(ife);
if (err) {
if (ret == ACT_P_CREATED)
- _tcf_ife_cleanup(a, bind);
+ tcf_ife_cleanup(a, bind);
-
Re: [PATCH net-next v5 0/7] vmxnet3: upgrade to version 3
From: Shrikrishna Khare Date: Thu, 16 Jun 2016 10:51:52 -0700 > vmxnet3 emulation has recently added several new features which includes > support for new commands the driver can issue to emulation, change in > descriptor fields etc. This patch series extends the vmxnet3 driver to > leverage these new features. > > Compatibility is maintained using existing vmxnet3 versioning mechanism as > follows: > - new features added to vmxnet3 emulation are associated with new vmxnet3 >version viz. vmxnet3 version 3. > - emulation advertises all the versions it supports to the driver. > - during initialization, vmxnet3 driver picks the highest version number > supported by both the emulation and the driver and configures emulation > to run at that version. > > In particular, following changes are introduced: ... Series applied, thanks.
Re: [PATCH v3] i2c: designware: Use transfer timeout from ioctl I2C_TIMEOUT
On 06/17/2016 04:46 AM, Weifeng Voon wrote:
This allows applications to set the transfer timeout in 10ms increments via
ioctl I2C_TIMEOUT.
Signed-off-by: Weifeng Voon
---
changelog v2:
* No code change, just change to a more suitable title
changelog v3:
* Move changelog out of commit log
drivers/i2c/busses/i2c-designware-core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/i2c/busses/i2c-designware-core.c
b/drivers/i2c/busses/i2c-designware-core.c
index 99b54be..c6922b8 100644
--- a/drivers/i2c/busses/i2c-designware-core.c
+++ b/drivers/i2c/busses/i2c-designware-core.c
@@ -663,7 +663,7 @@ i2c_dw_xfer(struct i2c_adapter *adap, struct i2c_msg
msgs[], int num)
i2c_dw_xfer_init(dev);
/* wait for tx to complete */
- if (!wait_for_completion_timeout(&dev->cmd_complete, HZ)) {
+ if (!wait_for_completion_timeout(&dev->cmd_complete, adap->timeout)) {
dev_err(dev->dev, "controller timed out\n");
/* i2c_dw_init implicitly disables the adapter */
i2c_dw_init(dev);
Acked-by: Jarkko Nikula
Re: [PATCH v2] cgroup: disable irqs while holding css_set_lock
Hello, On Thu, Jun 16, 2016 at 09:12:32PM -0300, Daniel Bristot de Oliveira wrote: > The use of the irq spin_(un)lock_irq() assumes that the code is always > called with IRQs enabled. But that is not always true in this case, as > we call cgroup_free() in the hard IRQ context, and unconditionally > enable IRQ in this context is a problem. So we need to use irqsave/restore. > > Discussing with rostedt, we figured that this needs to be IRQ safe > (using irqsave/restore) in the PREEMPT RT too, so I need to code a v3 of > this patch using raw_spin_*() functions to avoid this problem in the -rt > kernel as well. > > Do you see any problems on this? Use of raw_spin is fine but I don't see how, say, rebind_subsystems() or cgroup_setup_root() can ever be called with irq disabled given that they assume sleepable context. Please use _irq and _irqsave appropriately depending on the circumstances. Thanks. -- tejun
Re: [PATCH 4.7 FIX] brcmfmac: fix lockup when removing P2P interface after event timeout
On 17 June 2016 at 07:13, Kalle Valo wrote: > Rafał Miłecki writes: > >> On 16 June 2016 at 17:10, Kalle Valo wrote: >>> Rafał Miłecki writes: >>> Removing P2P interface is handled by sending a proper request to the firmware. On success firmware triggers an event and driver's handler removes a matching interface. However on event timeout we remove interface directly from the cfg80211 callback. Current code doesn't handle this case correctly as it always assumes rtnl to be unlocked. Fix it by adding an extra rtnl_locked parameter to functions and calling unregister_netdevice when needed. Signed-off-by: Rafał Miłecki >>> >>> Failed to apply, please rebase: >>> >>> Applying: brcmfmac: fix lockup when removing P2P interface after event >>> timeout >>> Using index info to reconstruct a base tree... >>> Falling back to patching base and 3-way merge... >>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c >>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h >>> CONFLICT (content): Merge conflict in >>> drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h >>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c >>> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >>> Failed to merge in the changes. >>> Patch failed at 0001 brcmfmac: fix lockup when removing P2P interface after >>> event timeout >> >> What tree did you try it on? >> >> I just went into a dir where I have cloned: >> git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git >> >> My HEAD commit is: >> 034fdd4 Merge ath-current from ath.git >> >> And I can apply this patch cleanly doing: >> curl https://patchwork.kernel.org/patch/9138925/mbox/ | git am > > I was trying to apply this to wireless-drivers-next. I didn't get a > confirmation from Arend and I didn't consider the fix important enough > for 4.7. But of course I can reconsider if needed. I think I agree it won't hurt to get it into -next. Noone earlier reported this bug and it seems to be there for a long time. Also applying it to -next will allow avoiding merge conflicts and immediate development work on -next. I'll resend this patch rebased on -next soon. -- Rafał
Re: [PATCH] MADVISE_FREE, THP: Fix madvise_free_huge_pmd return value after splitting
Hi,
On Thu, Jun 16, 2016 at 08:03:54PM -0700, Huang, Ying wrote:
> From: Huang Ying
>
> madvise_free_huge_pmd should return 0 if the fallback PTE operations are
> required. In madvise_free_huge_pmd, if part pages of THP are discarded,
> the THP will be split and fallback PTE operations should be used if
> splitting succeeds. But the original code will make fallback PTE
> operations skipped, after splitting succeeds. Fix that via make
> madvise_free_huge_pmd return 0 after splitting successfully, so that the
> fallback PTE operations will be done.
You're right. Thanks!
>
> Know issues: if my understanding were correct, return 1 from
> madvise_free_huge_pmd means the following processing for the PMD should
> be skipped, while return 0 means the following processing is still
> needed. So the function should return 0 only if the THP is split
> successfully or the PMD is not trans huge. But the pmd_trans_unstable
> after madvise_free_huge_pmd guarantee the following processing will be
> skipped for huge PMD. So current code can run properly. But if my
> understanding were correct, we can clean up return code of
> madvise_free_huge_pmd accordingly.
I like your clean up. Just a minor comment below.
>
> Signed-off-by: "Huang, Ying"
> ---
> mm/huge_memory.c | 7 +--
> 1 file changed, 1 insertion(+), 6 deletions(-)
>
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 2ad52d5..64dc95d 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
First of all, let's change ret from int to bool.
And then, add description in the function entry.
/*
* Return true if we do MADV_FREE successfully on entire pmd page.
* Otherwise, return false.
*/
And do not set to 1 if it is huge_zero_pmd but just goto out to
return false.
Thanks!
> @@ -1655,14 +1655,9 @@ int madvise_free_huge_pmd(struct mmu_gather *tlb,
> struct vm_area_struct *vma,
> if (next - addr != HPAGE_PMD_SIZE) {
> get_page(page);
> spin_unlock(ptl);
> - if (split_huge_page(page)) {
> - put_page(page);
> - unlock_page(page);
> - goto out_unlocked;
> - }
> + split_huge_page(page);
> put_page(page);
> unlock_page(page);
> - ret = 1;
> goto out_unlocked;
> }
>
> --
> 2.8.1
>
> --
> To unsubscribe, send a message with 'unsubscribe linux-mm' in
> the body to majord...@kvack.org. For more info on Linux MM,
> see: http://www.linux-mm.org/ .
> Don't email: mailto:"d...@kvack.org";> em...@kvack.org
[PATCH 2/2] idle_intel: Add Denverton
From: Jacob Pan
Denverton is an Intel Atom based micro server which shares the same
Goldmont architecture as Broxton. The available C-states on
Denverton is a subset of Broxton with only C1, C1e, and C6.
Signed-off-by: Jacob Pan
Signed-off-by: Len Brown
---
drivers/idle/intel_idle.c | 35 +++
1 file changed, 35 insertions(+)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index f93788f..46fed5a 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -825,6 +825,35 @@ static struct cpuidle_state bxt_cstates[] = {
.enter = NULL }
};
+static struct cpuidle_state dnv_cstates[] = {
+ {
+ .name = "C1-DNV",
+ .desc = "MWAIT 0x00",
+ .flags = MWAIT2flg(0x00),
+ .exit_latency = 2,
+ .target_residency = 2,
+ .enter = &intel_idle,
+ .enter_freeze = intel_idle_freeze, },
+ {
+ .name = "C1E-DNV",
+ .desc = "MWAIT 0x01",
+ .flags = MWAIT2flg(0x01),
+ .exit_latency = 10,
+ .target_residency = 20,
+ .enter = &intel_idle,
+ .enter_freeze = intel_idle_freeze, },
+ {
+ .name = "C6-DNV",
+ .desc = "MWAIT 0x20",
+ .flags = MWAIT2flg(0x20) | CPUIDLE_FLAG_TLB_FLUSHED,
+ .exit_latency = 50,
+ .target_residency = 500,
+ .enter = &intel_idle,
+ .enter_freeze = intel_idle_freeze, },
+ {
+ .enter = NULL }
+};
+
/**
* intel_idle
* @dev: cpuidle_device
@@ -1014,6 +1043,11 @@ static const struct idle_cpu idle_cpu_bxt = {
.disable_promotion_to_c1e = true,
};
+static const struct idle_cpu idle_cpu_dnv = {
+ .state_table = dnv_cstates,
+ .disable_promotion_to_c1e = true,
+};
+
#define ICPU(model, cpu) \
{ X86_VENDOR_INTEL, 6, model, X86_FEATURE_MWAIT, (unsigned long)&cpu }
@@ -1050,6 +1084,7 @@ static const struct x86_cpu_id intel_idle_ids[]
__initconst = {
ICPU(0x55, idle_cpu_skx),
ICPU(0x57, idle_cpu_knl),
ICPU(0x5c, idle_cpu_bxt),
+ ICPU(0x5f, idle_cpu_dnv),
{}
};
--
2.9.0
[PATCH 1/2] drivers/idle: make intel_idle.c driver more explicitly non-modular
From: Paul Gortmaker
The Kconfig for this driver is currently declared with:
config INTEL_IDLE
bool "Cpuidle Driver for Intel Processors"
...meaning that it currently is not being built as a module by anyone.
This was done in commit 6ce9cd8669fa1195fdc21643370e34523c7ac988
("intel_idle: disable module support") since "...the module capability
is cauing more trouble than it is worth."
This was done over 5y ago, and Daniel adds that:
...the modular support has been removed from almost all the cpuidle
drivers and the cpuidle framework is no longer assuming driver could
be unloaded.
Removing the modular dead code in the driver makes sense as this
what have been done in the others drivers.
So lets remove the modular code that is essentially orphaned, so that
when reading the driver there is no doubt it is builtin-only.
Since module_init translates to device_initcall in the non-modular
case, the init ordering remains unchanged with this commit. At a
later date we might want to consider whether subsys_init or another
init category seems more appropriate than device_init.
We replace module.h with moduleparam.h since the file does declare
some module parameters, and leaving them as such is currently the
easiest way to remain compatible with existing boot arg use cases.
Note that MODULE_DEVICE_TABLE is a no-op for non-modular code.
Also note that we can't remove intel_idle_cpuidle_devices_uninit() as
that is still used for unwind purposes if the init fails.
We also delete the MODULE_LICENSE tag etc. since all that information
is already contained at the top of the file in the comments.
Signed-off-by: Paul Gortmaker
Signed-off-by: Len Brown
---
drivers/idle/intel_idle.c | 41 -
1 file changed, 8 insertions(+), 33 deletions(-)
diff --git a/drivers/idle/intel_idle.c b/drivers/idle/intel_idle.c
index c966492..f93788f 100644
--- a/drivers/idle/intel_idle.c
+++ b/drivers/idle/intel_idle.c
@@ -46,8 +46,6 @@
* to avoid complications with the lapic timer workaround.
* Have not seen issues with suspend, but may need same workaround here.
*
- * There is currently no kernel-based automatic probing/loading mechanism
- * if the driver is built as a module.
*/
/* un-comment DEBUG to enable pr_debug() statements */
@@ -60,7 +58,7 @@
#include
#include
#include
-#include
+#include
#include
#include
#include
@@ -1054,7 +1052,6 @@ static const struct x86_cpu_id intel_idle_ids[]
__initconst = {
ICPU(0x5c, idle_cpu_bxt),
{}
};
-MODULE_DEVICE_TABLE(x86cpu, intel_idle_ids);
/*
* intel_idle_probe()
@@ -1415,34 +1412,12 @@ static int __init intel_idle_init(void)
return 0;
}
+device_initcall(intel_idle_init);
-static void __exit intel_idle_exit(void)
-{
- struct cpuidle_device *dev;
- int i;
-
- cpu_notifier_register_begin();
-
- if (lapic_timer_reliable_states != LAPIC_TIMER_ALWAYS_RELIABLE)
- on_each_cpu(__setup_broadcast_timer, (void *)false, 1);
- __unregister_cpu_notifier(&cpu_hotplug_notifier);
-
- for_each_possible_cpu(i) {
- dev = per_cpu_ptr(intel_idle_cpuidle_devices, i);
- cpuidle_unregister_device(dev);
- }
-
- cpu_notifier_register_done();
-
- cpuidle_unregister_driver(&intel_idle_driver);
- free_percpu(intel_idle_cpuidle_devices);
-}
-
-module_init(intel_idle_init);
-module_exit(intel_idle_exit);
-
+/*
+ * We are not really modular, but we used to support that. Meaning we also
+ * support "intel_idle.max_cstate=..." at boot and also a read-only export of
+ * it at /sys/module/intel_idle/parameters/max_cstate -- so using module_param
+ * is the easiest way (currently) to continue doing that.
+ */
module_param(max_cstate, int, 0444);
-
-MODULE_AUTHOR("Len Brown ");
-MODULE_DESCRIPTION("Cpuidle driver for Intel Hardware v" INTEL_IDLE_VERSION);
-MODULE_LICENSE("GPL");
--
2.9.0
[PATCH 0/2] intel_idle updates
Rafael, Since you've already pulled Dave's intel-family.h intel_idle changes into your tree, please also apply these intel_idle patches to that branch. [PATCH 1/2] drivers/idle: make intel_idle.c driver more explicitly [PATCH 2/2] idle_intel: Add Denverton And if you prefer to use git... The following changes since commit 5edb56491d4812c42175980759da53388e5d86f5: Linux 4.7-rc3 (2016-06-12 07:20:35 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/lenb/linux.git cpuidle for you to fetch changes up to 264f6eef366c0669cf9c8b1850977e13be85160a: idle_intel: Add Denverton (2016-06-16 21:05:24 -0400) Jacob Pan (1): idle_intel: Add Denverton Paul Gortmaker (1): drivers/idle: make intel_idle.c driver more explicitly non-modular drivers/idle/intel_idle.c | 76 +++ 1 file changed, 43 insertions(+), 33 deletions(-) thanks! -Len
[PATCH 02/10] x86 tsc_msr: Identify Intel-specific code
From: Len Brown try_msr_calibrate_tsc() is currently Intel-specific, and should not execute on any other vendor's parts. Signed-off-by: Len Brown --- arch/x86/kernel/tsc_msr.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c index 6aa0f4d..4ec5e56 100644 --- a/arch/x86/kernel/tsc_msr.c +++ b/arch/x86/kernel/tsc_msr.c @@ -86,6 +86,9 @@ unsigned long try_msr_calibrate_tsc(void) unsigned long res; int cpu_index; + if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL) + return 0; + cpu_index = match_cpu(boot_cpu_data.x86, boot_cpu_data.x86_model); if (cpu_index < 0) return 0; -- 2.9.0
[PATCH 05/10] x86 tsc_msr: Correct Silvermont reference clock values
From: Len Brown
Atom processors use a 19.2 MHz crystal oscillator.
Early processors generate 100 MHz via 19.2 MHz * 26 / 5 = 99.84 MHz.
Later preocessor generate 100 MHz via 19.2 MHz * 125 / 24 = 100 MHz.
Update the Silvermont-based tables accordingly,
matching the Software Developers Manual.
Also, correct a 166 MHz entry that should have been 116 MHz,
and add a missing 80 MHz entry.
Reported-by: Stephane Gasparini
Signed-off-by: Len Brown
---
arch/x86/kernel/tsc_msr.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c
index 4110f72..20487e2 100644
--- a/arch/x86/kernel/tsc_msr.c
+++ b/arch/x86/kernel/tsc_msr.c
@@ -35,11 +35,11 @@ static struct freq_desc freq_desc_tables[] = {
/* CLV+ */
{ 6, 0x35, 0, { 0, 133200, 0, 0, 0, 99840, 0, 83200 } },
/* TNG - Intel Atom processor Z3400 series */
- { 6, 0x4a, 1, { 0, 99840, 133200, 0, 0, 0, 0, 0 } },
+ { 6, 0x4a, 1, { 0, 10, 133300, 0, 0, 0, 0, 0 } },
/* VLV2 - Intel Atom processor E3000, Z3600, Z3700 series */
- { 6, 0x37, 1, { 83200, 99840, 133200, 166400, 0, 0, 0, 0 } },
+ { 6, 0x37, 1, { 83300, 10, 133300, 116700, 8, 0, 0, 0 } },
/* ANN - Intel Atom processor Z3500 series */
- { 6, 0x5a, 1, { 83200, 99840, 133200, 99840, 0, 0, 0, 0 } },
+ { 6, 0x5a, 1, { 83300, 10, 133300, 10, 0, 0, 0, 0 } },
};
static int match_cpu(u8 family, u8 model)
--
2.9.0
[PATCH 06/10] x86 tsc_msr: Add Airmont reference clock values
From: Len Brown
per the Intel 64 and IA-32 Architecture Software Developer's Manual...
Add the reference clock for Intel Atom Processors
Based on the Airmont Microarchitecture.
Reported-by: Stephane Gasparini
Signed-off-by: Len Brown
---
arch/x86/kernel/tsc_msr.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c
index 20487e2..65b3d8cb 100644
--- a/arch/x86/kernel/tsc_msr.c
+++ b/arch/x86/kernel/tsc_msr.c
@@ -13,7 +13,7 @@
#include
#include
-#define MAX_NUM_FREQS 8
+#define MAX_NUM_FREQS 9
/*
* If MSR_PERF_STAT[31] is set, the maximum resolved bus ratio can be
@@ -40,6 +40,9 @@ static struct freq_desc freq_desc_tables[] = {
{ 6, 0x37, 1, { 83300, 10, 133300, 116700, 8, 0, 0, 0 } },
/* ANN - Intel Atom processor Z3500 series */
{ 6, 0x5a, 1, { 83300, 10, 133300, 10, 0, 0, 0, 0 } },
+ /* AMT - Intel Atom processor X7-Z8000 and X5-Z8000 series */
+ { 6, 0x4c, 1, { 83300, 10, 133300, 116700,
+ 8, 93300, 9, 88900, 87500 } },
};
static int match_cpu(u8 family, u8 model)
--
2.9.0
[PATCH 01/10] Revert "x86/tsc: Add missing Cherrytrail frequency to the table"
From: Len Brown
This reverts commit e2724e9d969294879936daf7833d4adda26c8efc.
as it is incomplete, and is replaced by a more complete
pach later in this series.
Signed-off-by: Len Brown
---
arch/x86/kernel/tsc_msr.c | 3 ---
1 file changed, 3 deletions(-)
diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c
index 9911a06..6aa0f4d 100644
--- a/arch/x86/kernel/tsc_msr.c
+++ b/arch/x86/kernel/tsc_msr.c
@@ -23,7 +23,6 @@
#include
/* CPU reference clock frequency: in KHz */
-#define FREQ_808
#define FREQ_8383200
#define FREQ_100 99840
#define FREQ_133 133200
@@ -57,8 +56,6 @@ static struct freq_desc freq_desc_tables[] = {
{ 6, 0x37, 1, { FREQ_83, FREQ_100, FREQ_133, FREQ_166, 0, 0, 0, 0 } },
/* ANN */
{ 6, 0x5a, 1, { FREQ_83, FREQ_100, FREQ_133, FREQ_100, 0, 0, 0, 0 } },
- /* AIRMONT */
- { 6, 0x4c, 1, { FREQ_83, FREQ_100, FREQ_133, FREQ_166, FREQ_80, 0, 0, 0
} },
};
static int match_cpu(u8 family, u8 model)
--
2.9.0
[PATCH 08/10] x86 tsc_msr: Remove irqoff around MSR-based TSC enumeration
From: Len Brown
Remove the irqoff/irqon around MSR-based TSC enumeration,
as it is not necessary.
Also rename: try_msr_calibrate_tsc() to cpu_khz_from_msr(),
as that better describes what the routine does.
Signed-off-by: Len Brown
---
arch/x86/include/asm/tsc.h | 3 +--
arch/x86/kernel/tsc.c | 5 +
arch/x86/kernel/tsc_msr.c | 2 +-
3 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index 7428697..db1f779 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -52,7 +52,6 @@ extern int notsc_setup(char *);
extern void tsc_save_sched_clock_state(void);
extern void tsc_restore_sched_clock_state(void);
-/* MSR based TSC calibration for Intel Atom SoC platforms */
-unsigned long try_msr_calibrate_tsc(void);
+unsigned long cpu_khz_from_msr(void);
#endif /* _ASM_X86_TSC_H */
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 38ba6de..35a3976 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -674,10 +674,7 @@ unsigned long native_calibrate_tsc(void)
unsigned long flags, latch, ms, fast_calibrate;
int hpet = is_hpet_enabled(), i, loopmin;
- /* Calibrate TSC using MSR for Intel Atom SoCs */
- local_irq_save(flags);
- fast_calibrate = try_msr_calibrate_tsc();
- local_irq_restore(flags);
+ fast_calibrate = cpu_khz_from_msr();
if (fast_calibrate)
return fast_calibrate;
diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c
index 9d56ebd..e0c2b30 100644
--- a/arch/x86/kernel/tsc_msr.c
+++ b/arch/x86/kernel/tsc_msr.c
@@ -68,7 +68,7 @@ static int match_cpu(u8 family, u8 model)
* Set global "lapic_timer_frequency" to bus_clock_cycles/jiffy
* Return processor base frequency in KHz, or 0 on failure.
*/
-unsigned long try_msr_calibrate_tsc(void)
+unsigned long cpu_khz_from_msr(void)
{
u32 lo, hi, ratio, freq_id, freq;
unsigned long res;
--
2.9.0
[PATCH 07/10] x86 tsc_msr: Extend to include Intel Core Architecture
From: Len Brown
tsc_msr is used to quickly and reliably
enumerate the CPU/TSC frequencies at boot time
For the Intel Atom Architecture.
Extend tsc_msr to include recent Intel Core Architecture.
As this code discovers BCLK, it also sets lapic_timer_frequency,
which allows LAPIC timer calibration to be skipped,
though it is already skipped on systems with a TSC deadline timer.
Signed-off-by: Len Brown
---
arch/x86/kernel/tsc_msr.c | 49 +++
1 file changed, 41 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c
index 65b3d8cb..9d56ebd 100644
--- a/arch/x86/kernel/tsc_msr.c
+++ b/arch/x86/kernel/tsc_msr.c
@@ -77,23 +77,56 @@ unsigned long try_msr_calibrate_tsc(void)
if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
return 0;
+ /*
+* 100 MHz BCLK Core Architecture -- before SKL.
+* De-rate 100Mhz by about 0.25% to account
+* for the average effect of spread-spectrum clocking.
+*/
+ switch (boot_cpu_data.x86_model) {
+
+ case 0x2A: /* SNB */
+ case 0x3A: /* IVB */
+ freq = 99773;
+ goto get_ratio;
+ case 0x2D: /* SNB Xeon */
+ case 0x3E: /* IVB Xeon */
+ freq = 99760;
+ goto get_ratio;
+ case 0x3C: /* HSW */
+ case 0x3F: /* HSW */
+ case 0x45: /* HSW */
+ case 0x46: /* HSW */
+ case 0x3D: /* BDW */
+ case 0x47: /* BDW */
+ case 0x4F: /* BDX */
+ case 0x56: /* BDX-DE */
+ freq = 99769;
+ goto get_ratio;
+ }
+
+ /*
+* Atom Architecture
+*/
cpu_index = match_cpu(boot_cpu_data.x86, boot_cpu_data.x86_model);
if (cpu_index < 0)
return 0;
- if (freq_desc_tables[cpu_index].msr_plat) {
- rdmsr(MSR_PLATFORM_INFO, lo, hi);
- ratio = (lo >> 8) & 0xff;
- } else {
- rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
- ratio = (hi >> 8) & 0x1f;
- }
-
/* Get FSB FREQ ID */
rdmsr(MSR_FSB_FREQ, lo, hi);
freq_id = lo & 0x7;
freq = id_to_freq(cpu_index, freq_id);
+ if (!freq_desc_tables[cpu_index].msr_plat) {
+ rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
+ ratio = (hi >> 8) & 0x1f;
+ goto done;
+ }
+
+get_ratio:
+ rdmsr(MSR_PLATFORM_INFO, lo, hi);
+ ratio = (lo >> 8) & 0xff;
+
+done:
/* TSC frequency = maximum resolved freq * maximum resolved bus ratio */
res = freq * ratio;
--
2.9.0
[PATCH 03/10] x86 tsc_msr: Remove debugging messages
From: Len Brown
Debugging messages are not necessary after all of the
possible hardware failures that never occur.
Instead, this code can simply return 0.
This code also doesn't need to print in the success case.
tsc_init() already prints the TSC frequency,
and apic=debug is available if anybody really is
interested in printing the LAPIC frequency.
Signed-off-by: Len Brown
---
arch/x86/kernel/tsc_msr.c | 19 +++
1 file changed, 3 insertions(+), 16 deletions(-)
diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c
index 4ec5e56..f7ba44b 100644
--- a/arch/x86/kernel/tsc_msr.c
+++ b/arch/x86/kernel/tsc_msr.c
@@ -76,9 +76,10 @@ static int match_cpu(u8 family, u8 model)
(freq_desc_tables[cpu_index].freqs[freq_id])
/*
- * Do MSR calibration only for known/supported CPUs.
+ * MSR-based CPU/TSC frequency discovery for certain CPUs.
*
- * Returns the calibration value or 0 if MSR calibration failed.
+ * Set global "lapic_timer_frequency" to bus_clock_cycles/jiffy
+ * Return processor base frequency in KHz, or 0 on failure.
*/
unsigned long try_msr_calibrate_tsc(void)
{
@@ -100,31 +101,17 @@ unsigned long try_msr_calibrate_tsc(void)
rdmsr(MSR_IA32_PERF_STATUS, lo, hi);
ratio = (hi >> 8) & 0x1f;
}
- pr_info("Maximum core-clock to bus-clock ratio: 0x%x\n", ratio);
-
- if (!ratio)
- goto fail;
/* Get FSB FREQ ID */
rdmsr(MSR_FSB_FREQ, lo, hi);
freq_id = lo & 0x7;
freq = id_to_freq(cpu_index, freq_id);
- pr_info("Resolved frequency ID: %u, frequency: %u KHz\n",
- freq_id, freq);
- if (!freq)
- goto fail;
/* TSC frequency = maximum resolved freq * maximum resolved bus ratio */
res = freq * ratio;
- pr_info("TSC runs at %lu KHz\n", res);
#ifdef CONFIG_X86_LOCAL_APIC
lapic_timer_frequency = (freq * 1000) / HZ;
- pr_info("lapic_timer_frequency = %d\n", lapic_timer_frequency);
#endif
return res;
-
-fail:
- pr_warn("Fast TSC calibration using MSR failed\n");
- return 0;
}
--
2.9.0
[PATCH 09/10] x86 tsc: enumerate SKL cpu_khz and tsc_khz via CPUID
From: Len Brown
Skylake CPU base-frequency and TSC frequency may differ
by up to 2%.
Enumerate CPU and TSC frequencies separately, allowing
cpu_khz and tsc_khz to differ.
The existing CPU frequency calibration mechanism is unchanged.
However, CPUID extensions are preferred, when available.
CPUID.0x16 is preferred over MSR and timer calibration
for CPU frequency discovery.
CPUID.0x15 takes precedence over CPU-frequency
for TSC frequency discovery.
Signed-off-by: Len Brown
---
arch/x86/include/asm/tsc.h | 1 +
arch/x86/include/asm/x86_init.h | 4 ++-
arch/x86/kernel/tsc.c | 75 +
arch/x86/kernel/x86_init.c | 1 +
4 files changed, 73 insertions(+), 8 deletions(-)
diff --git a/arch/x86/include/asm/tsc.h b/arch/x86/include/asm/tsc.h
index db1f779..a30591e 100644
--- a/arch/x86/include/asm/tsc.h
+++ b/arch/x86/include/asm/tsc.h
@@ -36,6 +36,7 @@ extern void mark_tsc_unstable(char *reason);
extern int unsynchronized_tsc(void);
extern int check_tsc_unstable(void);
extern int check_tsc_disabled(void);
+extern unsigned long native_calibrate_cpu(void);
extern unsigned long native_calibrate_tsc(void);
extern unsigned long long native_sched_clock_from_tsc(u64 tsc);
diff --git a/arch/x86/include/asm/x86_init.h b/arch/x86/include/asm/x86_init.h
index 4dcdf74..08a08a8 100644
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -181,7 +181,8 @@ struct x86_legacy_features {
/**
* struct x86_platform_ops - platform specific runtime functions
- * @calibrate_tsc: calibrate TSC
+ * @calibrate_cpu: calibrate CPU
+ * @calibrate_tsc: calibrate TSC, if different from CPU
* @get_wallclock: get time from HW clock like RTC etc.
* @set_wallclock: set time back to HW clock
* @is_untracked_pat_range exclude from PAT logic
@@ -200,6 +201,7 @@ struct x86_legacy_features {
* semantics.
*/
struct x86_platform_ops {
+ unsigned long (*calibrate_cpu)(void);
unsigned long (*calibrate_tsc)(void);
void (*get_wallclock)(struct timespec *ts);
int (*set_wallclock)(const struct timespec *ts);
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index 35a3976..e1496b7 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -239,7 +239,7 @@ static inline unsigned long long cycles_2_ns(unsigned long
long cyc)
return ns;
}
-static void set_cyc2ns_scale(unsigned long cpu_khz, int cpu)
+static void set_cyc2ns_scale(unsigned long khz, int cpu)
{
unsigned long long tsc_now, ns_now;
struct cyc2ns_data *data;
@@ -248,7 +248,7 @@ static void set_cyc2ns_scale(unsigned long cpu_khz, int cpu)
local_irq_save(flags);
sched_clock_idle_sleep_event();
- if (!cpu_khz)
+ if (!khz)
goto done;
data = cyc2ns_write_begin(cpu);
@@ -261,7 +261,7 @@ static void set_cyc2ns_scale(unsigned long cpu_khz, int cpu)
* time function is continuous; see the comment near struct
* cyc2ns_data.
*/
- clocks_calc_mult_shift(&data->cyc2ns_mul, &data->cyc2ns_shift, cpu_khz,
+ clocks_calc_mult_shift(&data->cyc2ns_mul, &data->cyc2ns_shift, khz,
NSEC_PER_MSEC, 0);
/*
@@ -665,15 +665,72 @@ success:
}
/**
- * native_calibrate_tsc - calibrate the tsc on boot
+ * native_calibrate_tsc
+ * Determine TSC frequency via CPUID, else return 0.
*/
unsigned long native_calibrate_tsc(void)
{
+ unsigned int eax_denominator, ebx_numerator, ecx_hz, edx;
+ unsigned int crystal_khz;
+
+ if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
+ return 0;
+
+ if (boot_cpu_data.cpuid_level < 0x15)
+ return 0;
+
+ eax_denominator = ebx_numerator = ecx_hz = edx = 0;
+
+ /* CPUID 15H TSC/Crystal ratio, plus optionally Crystal Hz */
+ cpuid(0x15, &eax_denominator, &ebx_numerator, &ecx_hz, &edx);
+
+ if (ebx_numerator == 0 || eax_denominator == 0)
+ return 0;
+
+ crystal_khz = ecx_hz / 1000;
+
+ if (crystal_khz == 0) {
+ switch (boot_cpu_data.x86_model) {
+ case 0x4E: /* SKL */
+ case 0x5E: /* SKL */
+ crystal_khz = 24000;/* 24 MHz */
+ }
+ }
+
+ return crystal_khz * ebx_numerator / eax_denominator;
+}
+
+static unsigned long cpu_khz_from_cpuid(void)
+{
+ unsigned int eax_base_mhz, ebx_max_mhz, ecx_bus_mhz, edx;
+
+ if (boot_cpu_data.x86_vendor != X86_VENDOR_INTEL)
+ return 0;
+
+ if (boot_cpu_data.cpuid_level < 0x16)
+ return 0;
+
+ eax_base_mhz = ebx_max_mhz = ecx_bus_mhz = edx = 0;
+
+ cpuid(0x16, &eax_base_mhz, &ebx_max_mhz, &ecx_bus_mhz, &edx);
+
+ return eax_base_mhz * 1000;
+}
+
+/**
+ * native_calibrate_cpu - calibrate the cpu on boo
[PATCH 0/10] x86/tsc: fast calibration updates
Originally I wrote this series to increase TSC calibration accuracy and speed, but it now it also includes changes that are mandatory for some systems to work correctly. In particular, the Airmont table entires are already being used in products, and some BXT steppings will fail without the check added by patch 10. LKP tested this patch series in early April on top of 4.6-rc1 and found a failure. That failure has been fixed upstream in 4.6 by commit 886123fb3a86 (x86/tsc: Read all ratio bits from MSR_PLATFORM_INFO) So I have re-based the series on top of 4.7-rc3 to take advantage of that fix, as well as to handle 3 merge conflicts due to intervening upstream commits. The series now starts by reverting e2724e9d9692 (x86/tsc: Add missing Cherrytrail frequency to the table) as that is replaced by the more correct and complete "x86 tsc_msr: Add Airmont reference clock values" in this series. Also, syntax changes were necessary to both "x86 tsc: enumerate SKL cpu_khz and tsc_khz via CPUID" "x86 tsc: enumerate BXT tsc_khz via CPUID" in response to cleanups recently applied upstream to tsc.c [PATCH 01/10] Revert "x86/tsc: Add missing Cherrytrail frequency to [PATCH 02/10] x86 tsc_msr: Identify Intel-specific code [PATCH 03/10] x86 tsc_msr: Remove debugging messages [PATCH 04/10] x86 tsc_msr: Update comments, expand definitions [PATCH 05/10] x86 tsc_msr: Correct Silvermont reference clock values [PATCH 06/10] x86 tsc_msr: Add Airmont reference clock values [PATCH 07/10] x86 tsc_msr: Extend to include Intel Core Architecture [PATCH 08/10] x86 tsc_msr: Remove irqoff around MSR-based TSC [PATCH 09/10] x86 tsc: enumerate SKL cpu_khz and tsc_khz via CPUID [PATCH 10/10] x86 tsc: enumerate BXT tsc_khz via CPUID
[PATCH 10/10] x86 tsc: enumerate BXT tsc_khz via CPUID
From: Len Brown
Hard code the BXT crystal clock (aka ART - Always Running Timer)
to 19.200 MHz, and use CPUID leaf 0x15 to determine the BXT TSC frequency.
Use tsc_khz to sanity check BXT cpu_khz,
which can be erroneous in some configurations.
Signed-off-by: Bin Gao
[lenb: simplified]
Signed-off-by: Len Brown
---
arch/x86/kernel/tsc.c | 16 +++-
1 file changed, 15 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/tsc.c b/arch/x86/kernel/tsc.c
index e1496b7..2a952fc 100644
--- a/arch/x86/kernel/tsc.c
+++ b/arch/x86/kernel/tsc.c
@@ -693,7 +693,11 @@ unsigned long native_calibrate_tsc(void)
switch (boot_cpu_data.x86_model) {
case 0x4E: /* SKL */
case 0x5E: /* SKL */
- crystal_khz = 24000;/* 24 MHz */
+ crystal_khz = 24000;/* 24.0 MHz */
+ break;
+ case 0x5C: /* BXT */
+ crystal_khz = 19200;/* 19.2 MHz */
+ break;
}
}
@@ -895,6 +899,8 @@ int recalibrate_cpu_khz(void)
tsc_khz = x86_platform.calibrate_tsc();
if (tsc_khz == 0)
tsc_khz = cpu_khz;
+ else if (abs(cpu_khz - tsc_khz) * 10 > tsc_khz)
+ cpu_khz = tsc_khz;
cpu_data(0).loops_per_jiffy = cpufreq_scale(cpu_data(0).loops_per_jiffy,
cpu_khz_old, cpu_khz);
@@ -1302,8 +1308,16 @@ void __init tsc_init(void)
cpu_khz = x86_platform.calibrate_cpu();
tsc_khz = x86_platform.calibrate_tsc();
+
+ /*
+* Trust non-zero tsc_khz as authorative,
+* and use it to sanity check cpu_khz,
+* which will be off if system timer is off.
+*/
if (tsc_khz == 0)
tsc_khz = cpu_khz;
+ else if (abs(cpu_khz - tsc_khz) * 10 > tsc_khz)
+ cpu_khz = tsc_khz;
if (!tsc_khz) {
mark_tsc_unstable("could not calculate TSC khz");
--
2.9.0
[PATCH 04/10] x86 tsc_msr: Update comments, expand definitions
From: Len Brown
Syntax only, no functional change.
Signed-off-by: Len Brown
---
arch/x86/kernel/tsc_msr.c | 36 ++--
1 file changed, 10 insertions(+), 26 deletions(-)
diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c
index f7ba44b..4110f72 100644
--- a/arch/x86/kernel/tsc_msr.c
+++ b/arch/x86/kernel/tsc_msr.c
@@ -1,14 +1,5 @@
/*
- * tsc_msr.c - MSR based TSC calibration on Intel Atom SoC platforms.
- *
- * TSC in Intel Atom SoC runs at a constant rate which can be figured
- * by this formula:
- * *
- * See Intel 64 and IA-32 System Programming Guid section 16.12 and 30.11.5
- * for details.
- * Especially some Intel Atom SoCs don't have PIT(i8254) or HPET, so MSR
- * based calibration is the only option.
- *
+ * tsc_msr.c - TSC frequency enumeration via MSR
*
* Copyright (C) 2013 Intel Corporation
* Author: Bin Gao
@@ -22,17 +13,10 @@
#include
#include
-/* CPU reference clock frequency: in KHz */
-#define FREQ_8383200
-#define FREQ_100 99840
-#define FREQ_133 133200
-#define FREQ_166 166400
-
#define MAX_NUM_FREQS 8
/*
- * According to Intel 64 and IA-32 System Programming Guide,
- * if MSR_PERF_STAT[31] is set, the maximum resolved bus ratio can be
+ * If MSR_PERF_STAT[31] is set, the maximum resolved bus ratio can be
* read in MSR_PLATFORM_ID[12:8], otherwise in MSR_PERF_STAT[44:40].
* Unfortunately some Intel Atom SoCs aren't quite compliant to this,
* so we need manually differentiate SoC families. This is what the
@@ -47,15 +31,15 @@ struct freq_desc {
static struct freq_desc freq_desc_tables[] = {
/* PNW */
- { 6, 0x27, 0, { 0, 0, 0, 0, 0, FREQ_100, 0, FREQ_83 } },
+ { 6, 0x27, 0, { 0, 0, 0, 0, 0, 99840, 0, 83200 } },
/* CLV+ */
- { 6, 0x35, 0, { 0, FREQ_133, 0, 0, 0, FREQ_100, 0, FREQ_83 } },
- /* TNG */
- { 6, 0x4a, 1, { 0, FREQ_100, FREQ_133, 0, 0, 0, 0, 0 } },
- /* VLV2 */
- { 6, 0x37, 1, { FREQ_83, FREQ_100, FREQ_133, FREQ_166, 0, 0, 0, 0 } },
- /* ANN */
- { 6, 0x5a, 1, { FREQ_83, FREQ_100, FREQ_133, FREQ_100, 0, 0, 0, 0 } },
+ { 6, 0x35, 0, { 0, 133200, 0, 0, 0, 99840, 0, 83200 } },
+ /* TNG - Intel Atom processor Z3400 series */
+ { 6, 0x4a, 1, { 0, 99840, 133200, 0, 0, 0, 0, 0 } },
+ /* VLV2 - Intel Atom processor E3000, Z3600, Z3700 series */
+ { 6, 0x37, 1, { 83200, 99840, 133200, 166400, 0, 0, 0, 0 } },
+ /* ANN - Intel Atom processor Z3500 series */
+ { 6, 0x5a, 1, { 83200, 99840, 133200, 99840, 0, 0, 0, 0 } },
};
static int match_cpu(u8 family, u8 model)
--
2.9.0
[PATCH] tools/perf: Fix the mask in regs_dump__printf
When decoding the perf_regs mask in regs_dump__printf(),
we loop through the mask using find_first_bit and find_next_bit functions.
And mask is of type "u64". But "u64" is send as a "unsigned long *" to
lib functions along with sizeof().
While the exisitng code works fine in most of the case, when using a
32bit perf on a 64bit kernel (Big Endian), we end up reading the wrong word
in the u64 mask. Patch to fix the mask in regs_dump__printf().
Suggested-by: Yury Norov
Cc: Yury Norov
Cc: Peter Zijlstra
Cc: Ingo Molnar
Cc: Arnaldo Carvalho de Melo
Cc: Alexander Shishkin
Cc: Jiri Olsa
Cc: Adrian Hunter
Cc: Kan Liang
Cc: Wang Nan
Cc: Michael Ellerman
Signed-off-by: Madhavan Srinivasan
---
tools/perf/util/session.c | 7 ++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/perf/util/session.c b/tools/perf/util/session.c
index 5214974e841a..2eaa42a4832a 100644
--- a/tools/perf/util/session.c
+++ b/tools/perf/util/session.c
@@ -940,8 +940,13 @@ static void branch_stack__printf(struct perf_sample
*sample)
static void regs_dump__printf(u64 mask, u64 *regs)
{
unsigned rid, i = 0;
+ unsigned long _mask[sizeof(mask)/sizeof(unsigned long)];
- for_each_set_bit(rid, (unsigned long *) &mask, sizeof(mask) * 8) {
+ _mask[0] = mask & ULONG_MAX;
+ if (sizeof(mask) > sizeof(unsigned long))
+ _mask[1] = mask >> 32;
+
+ for_each_set_bit(rid, _mask, sizeof(mask) * 8) {
u64 val = regs[i++];
printf(" %-5s 0x%" PRIx64 "\n",
--
1.9.1
Re: [RFC PATCH 2/2] KVM: x86: use __kvm_guest_exit
> > static bool vmx_has_high_real_mode_segbase(void) > > diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c > > index 7e3041ef050f..cc741b68139c 100644 > > --- a/arch/x86/kvm/x86.c > > +++ b/arch/x86/kvm/x86.c > > @@ -6706,21 +6706,13 @@ static int vcpu_enter_guest(struct kvm_vcpu *vcpu) > > > > kvm_put_guest_xcr0(vcpu); > > > > - /* Interrupt is enabled by handle_external_intr() */ > > kvm_x86_ops->handle_external_intr(vcpu); > > > > ++vcpu->stat.exits; > > > > - /* > > -* We must have an instruction between local_irq_enable() and > > -* kvm_guest_exit(), so the timer interrupt isn't delayed by > > -* the interrupt shadow. The stat.exits increment will do nicely. > > -* But we need to prevent reordering, hence this barrier(): > > -*/ > > - barrier(); > > - > > - kvm_guest_exit(); > > + __kvm_guest_exit(); > > kvm_guest_exit has no more callers and so can be removed. ARM and PPC call it. Paolo
[PATCH v2] mmc: dw_mmc: remove UBSAN warning in dw_mci_setup_bus()
This patch removes following UBSAN warnings in dw_mci_setup_bus().
UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1102:14
shift exponent 250 is too large for 32-bit type 'unsigned int'
Call trace:
[] dump_backtrace+0x0/0x380
[] show_stack+0x14/0x20
[] dump_stack+0xe0/0x120
[] ubsan_epilogue+0x18/0x68
[] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
[] dw_mci_setup_bus+0x3a0/0x438
[...]
UBSAN: Undefined behaviour in drivers/mmc/host/dw_mmc.c:1132:27
shift exponent 250 is too large for 32-bit type 'unsigned int'
Call trace:
[] dump_backtrace+0x0/0x380
[] show_stack+0x14/0x20
[] dump_stack+0xe0/0x120
[] ubsan_epilogue+0x18/0x68
[] __ubsan_handle_shift_out_of_bounds+0x18c/0x1bc
[] dw_mci_setup_bus+0x384/0x438
[...]
The warnings are caused because of shift with more than 31 on 32
bit variable, so this patch fixes to keep both clock and divider
instead of shift.
Signed-off-by: Seung-Woo Kim
---
drivers/mmc/host/dw_mmc.c |8 +---
drivers/mmc/host/dw_mmc.h |8 +---
2 files changed, 10 insertions(+), 6 deletions(-)
diff --git a/drivers/mmc/host/dw_mmc.c b/drivers/mmc/host/dw_mmc.c
index 2cc6123..d05c8cc 100644
--- a/drivers/mmc/host/dw_mmc.c
+++ b/drivers/mmc/host/dw_mmc.c
@@ -1099,7 +1099,8 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
bool force_clkinit)
div = (host->bus_hz != clock) ? DIV_ROUND_UP(div, 2) : 0;
- if ((clock << div) != slot->__clk_old || force_clkinit)
+ if (clock != slot->__clk_old || div != slot->__div_old ||
+ force_clkinit)
dev_info(&slot->mmc->class_dev,
"Bus speed (slot %d) = %dHz (slot req %dHz,
actual %dHZ div = %d)\n",
slot->id, host->bus_hz, clock,
@@ -1128,8 +1129,9 @@ static void dw_mci_setup_bus(struct dw_mci_slot *slot,
bool force_clkinit)
/* inform CIU */
mci_send_cmd(slot, sdmmc_cmd_bits, 0);
- /* keep the clock with reflecting clock dividor */
- slot->__clk_old = clock << div;
+ /* keep the clock and clock divider */
+ slot->__clk_old = clock;
+ slot->__div_old = div;
}
host->current_speed = clock;
diff --git a/drivers/mmc/host/dw_mmc.h b/drivers/mmc/host/dw_mmc.h
index 1e8d838..fdfc3f5 100644
--- a/drivers/mmc/host/dw_mmc.h
+++ b/drivers/mmc/host/dw_mmc.h
@@ -245,9 +245,10 @@ extern int dw_mci_resume(struct dw_mci *host);
* @queue_node: List node for placing this node in the @queue list of
* &struct dw_mci.
* @clock: Clock rate configured by set_ios(). Protected by host->lock.
- * @__clk_old: The last updated clock with reflecting clock divider.
- * Keeping track of this helps us to avoid spamming the console
- * with CONFIG_MMC_CLKGATE.
+ * @__clk_old: The last updated clock.
+ * @__div_old: The last updated clock divider.
+ * Keeping track of clock and clock divider helps us to avoid spamming
+ * the console with CONFIG_MMC_CLKGATE.
* @flags: Random state bits associated with the slot.
* @id: Number of this slot.
* @sdio_id: Number of this slot in the SDIO interrupt registers.
@@ -263,6 +264,7 @@ struct dw_mci_slot {
unsigned intclock;
unsigned int__clk_old;
+ unsigned int__div_old;
unsigned long flags;
#define DW_MMC_CARD_PRESENT0
--
1.7.4.1
Re: [PATCH 4.7 FIX] brcmfmac: fix lockup when removing P2P interface after event timeout
Rafał Miłecki writes: > On 16 June 2016 at 17:10, Kalle Valo wrote: >> Rafał Miłecki writes: >> >>> Removing P2P interface is handled by sending a proper request to the >>> firmware. On success firmware triggers an event and driver's handler >>> removes a matching interface. >>> >>> However on event timeout we remove interface directly from the cfg80211 >>> callback. Current code doesn't handle this case correctly as it always >>> assumes rtnl to be unlocked. >>> >>> Fix it by adding an extra rtnl_locked parameter to functions and calling >>> unregister_netdevice when needed. >>> >>> Signed-off-by: Rafał Miłecki >> >> Failed to apply, please rebase: >> >> Applying: brcmfmac: fix lockup when removing P2P interface after event >> timeout >> Using index info to reconstruct a base tree... >> Falling back to patching base and 3-way merge... >> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c >> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h >> CONFLICT (content): Merge conflict in >> drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h >> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c >> Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c >> Failed to merge in the changes. >> Patch failed at 0001 brcmfmac: fix lockup when removing P2P interface after >> event timeout > > What tree did you try it on? > > I just went into a dir where I have cloned: > git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git > > My HEAD commit is: > 034fdd4 Merge ath-current from ath.git > > And I can apply this patch cleanly doing: > curl https://patchwork.kernel.org/patch/9138925/mbox/ | git am I was trying to apply this to wireless-drivers-next. I didn't get a confirmation from Arend and I didn't consider the fix important enough for 4.7. But of course I can reconsider if needed. -- Kalle Valo
Re: [PATCH 4/8] hamradio: baycom: fix old-style declaration
From: Arnd Bergmann Date: Thu, 16 Jun 2016 15:52:11 +0200 > Modern C standards expect the '__inline__' keyword to come before the return > type in a declaration, and we get a warning for this with "make W=1": > > drivers/net/hamradio/baycom_par.c:159:1: error: '__inline__' is not at > beginning of declaration [-Werror=old-style-declaration] > > For consistency with other drivers, I'm changing '__inline__' to 'inline' > at the same time. > > Signed-off-by: Arnd Bergmann Applied.
Re: [PATCH 7/8] net: xfrm: fix old-style declaration
From: Arnd Bergmann
Date: Thu, 16 Jun 2016 15:59:25 +0200
> Modern C standards expect the '__inline__' keyword to come before the return
> type in a declaration, and we get a couple of warnings for this with "make
> W=1"
> in the xfrm{4,6}_policy.c files:
>
> net/ipv6/xfrm6_policy.c:369:1: error: 'inline' is not at beginning of
> declaration [-Werror=old-style-declaration]
> static int inline xfrm6_net_sysctl_init(struct net *net)
> net/ipv6/xfrm6_policy.c:374:1: error: 'inline' is not at beginning of
> declaration [-Werror=old-style-declaration]
> static void inline xfrm6_net_sysctl_exit(struct net *net)
> net/ipv4/xfrm4_policy.c:339:1: error: 'inline' is not at beginning of
> declaration [-Werror=old-style-declaration]
> static int inline xfrm4_net_sysctl_init(struct net *net)
> net/ipv4/xfrm4_policy.c:344:1: error: 'inline' is not at beginning of
> declaration [-Werror=old-style-declaration]
> static void inline xfrm4_net_sysctl_exit(struct net *net)
>
> Signed-off-by: Arnd Bergmann
Applied.
Re: [PATCH 5/8] isdn: eicon: fix old-style declarations
From: Arnd Bergmann Date: Thu, 16 Jun 2016 15:52:12 +0200 > Modern C standards expect the '__inline__' keyword to come before the return > type in a declaration, and we get many warnings for this with "make W=1" > because the eicon driver has this in a header file: > > eicon/divasmain.c:448:1: error: '__inline__' is not at beginning of > declaration [-Werror=old-style-declaration] > eicon/divasmain.c:453:1: error: '__inline__' is not at beginning of > declaration [-Werror=old-style-declaration] > eicon/divasmain.c:458:1: error: '__inline__' is not at beginning of > declaration [-Werror=old-style-declaration] > eicon/divasmain.c:463:1: error: '__inline__' is not at beginning of > declaration [-Werror=old-style-declaration] > eicon/divasmain.c:468:1: error: '__inline__' is not at beginning of > declaration [-Werror=old-style-declaration] > eicon/divasmain.c:473:1: error: '__inline__' is not at beginning of > declaration [-Werror=old-style-declaration] > eicon/platform.h:274:1: error: '__inline__' is not at beginning of > declaration [-Werror=old-style-declaration] > eicon/platform.h:280:1: error: '__inline__' is not at beginning of > declaration [-Werror=old-style-declaration] > > A similar warning gets printed for the diva_os_register_io_port() > declaration, because 'register' is interpreted as a keyword instead > of a variable name: > > In file included from eicon/diva_didd.c:21:0: > eicon/platform.h:206:1: error: 'register' is not at beginning of declaration > [-Werror=old-style-declaration] > > Signed-off-by: Arnd Bergmann Applied.
Re: [PATCH 6/8] net: gianfar: fix old-style declaration
From: Arnd Bergmann Date: Thu, 16 Jun 2016 15:52:13 +0200 > Modern C standards expect the '__inline__' keyword to come before the return > type in a declaration, and we get a warning for this with "make W=1": > > drivers/net/ethernet/freescale/gianfar.c:2278:1: error: 'inline' is not at > beginning of declaration [-Werror=old-style-declaration] > > Signed-off-by: Arnd Bergmann Applied.
[PATCH v3 0/7] max8903: Add device tree support and misc fixes
From: Chris Lapa This patch set adds device tree support for the MAX8903 battery charger. It also cleans up logic with dc_valid, dok and dcm pins as well as fixing up validity checking of gpios. I verified these patches work on a board I have here, which uses the DC power side (not the USB portition) of the MAX8903. Changes v2 -> v3: * Seperate requesting of gpio's into its own commit * Fixed up validity checking of GPIO's * Remove dc_valid and usb_valid from device tree * Remove some unncessary init to psy_cfg.num_supplicants and psy_cfg.supplied_to * Reorder patches so device tree implementation is final patch Changes v1 -> v2: * Seperate DT bindings documentation into its own commit * Add maxim prefix to DT compatible field * Add gpios suffix to gpio's in DT * Remove malloc failed error message Chris Lapa (7): max8903: adds documentation for device tree bindings. max8903: store pointer to pdata instead of copying it. max8903: cleans up confusing relationship between dc_valid, dok and dcm. max8903: adds requesting of gpios. max8903: removes non zero validity checks on gpios. max8903: remove unnecessary 'out of memory' error message. max8903: adds support for initiation via device tree. .../devicetree/bindings/power/max8903-charger.txt | 25 ++ drivers/power/max8903_charger.c| 278 +++-- include/linux/power/max8903_charger.h | 6 +- 3 files changed, 227 insertions(+), 82 deletions(-) create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt -- 1.9.1
[PATCH v3 1/7] max8903: adds documentation for device tree bindings.
From: Chris Lapa
Signed-off-by: Chris Lapa
---
.../devicetree/bindings/power/max8903-charger.txt | 25 ++
1 file changed, 25 insertions(+)
create mode 100644 Documentation/devicetree/bindings/power/max8903-charger.txt
diff --git a/Documentation/devicetree/bindings/power/max8903-charger.txt
b/Documentation/devicetree/bindings/power/max8903-charger.txt
new file mode 100644
index 000..aea1dd2a
--- /dev/null
+++ b/Documentation/devicetree/bindings/power/max8903-charger.txt
@@ -0,0 +1,25 @@
+Maxim Semiconductor MAX8903 Battery Charger bindings
+
+Required properties:
+- compatible: "maxim,max8903-charger" for MAX8903 Battery Charger
+- dok-gpios: Valid DC power has been detected, optional if uok-gpios is
provided
+- uok-gpios: Valid USB power has been detected, optional if dok-gpios is
provided
+
+Optional properties:
+- cen-gpios: Charge enable pin
+- chg-gpios: Charger status pin
+- flt-gpios: Fault pin
+- dcm-gpios: Current limit mode setting (DC or USB)
+- usus-gpios: USB suspend pin
+
+
+Example:
+
+ max8903-charger {
+ compatible = "maxim,max8903-charger";
+ dok-gpios = <&gpio2 3 GPIO_ACTIVE_LOW>;
+ flt-gpios = <&gpio2 2 GPIO_ACTIVE_LOW>;
+ chg-gpios = <&gpio3 15 GPIO_ACTIVE_LOW>;
+ cen-gpios = <&gpio2 5 GPIO_ACTIVE_LOW>;
+ status = "okay";
+ };
--
1.9.1
[PATCH v3 3/7] max8903: cleans up confusing relationship between dc_valid, dok and dcm.
From: Chris Lapa
The max8903_charger.h file indicated that dcm and dok were not optional
when dc_valid is set.
It makes sense to have dok as a compulsory pin when dc_valid is given.
However dcm can be optionally wired to a fixed level especially when the
circuit is configured for dc power exclusively.
The previous implementation already allowed for this somewhat, however no
error was given if dok wasn't given whilst dc_valid was.
The new implementation enforces dok presence when dc_valid is given. Whilst
allowing dcm to be optional.
Signed-off-by: Chris Lapa
---
drivers/power/max8903_charger.c | 23 ++-
include/linux/power/max8903_charger.h | 6 +++---
2 files changed, 13 insertions(+), 16 deletions(-)
diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c
index 0a5b0e1..dbd911c4 100644
--- a/drivers/power/max8903_charger.c
+++ b/drivers/power/max8903_charger.c
@@ -211,27 +211,24 @@ static int max8903_probe(struct platform_device *pdev)
}
if (pdata->dc_valid) {
- if (pdata->dok && gpio_is_valid(pdata->dok) &&
- pdata->dcm && gpio_is_valid(pdata->dcm)) {
+ if (pdata->dok && gpio_is_valid(pdata->dok)) {
gpio = pdata->dok; /* PULL_UPed Interrupt */
ta_in = gpio_get_value(gpio) ? 0 : 1;
+ } else {
+ dev_err(dev, "When DC is wired, DOK should"
+ " be wired as well.\n");
+ return -EINVAL;
+ }
+ }
+ if (pdata->dcm) {
+ if (gpio_is_valid(pdata->dcm)) {
gpio = pdata->dcm; /* Output */
gpio_set_value(gpio, ta_in);
} else {
- dev_err(dev, "When DC is wired, DOK and DCM should"
- " be wired as well.\n");
+ dev_err(dev, "Invalid pin: dcm.\n");
return -EINVAL;
}
- } else {
- if (pdata->dcm) {
- if (gpio_is_valid(pdata->dcm))
- gpio_set_value(pdata->dcm, 0);
- else {
- dev_err(dev, "Invalid pin: dcm.\n");
- return -EINVAL;
- }
- }
}
if (pdata->usb_valid) {
diff --git a/include/linux/power/max8903_charger.h
b/include/linux/power/max8903_charger.h
index 24f51db..89d3f1c 100644
--- a/include/linux/power/max8903_charger.h
+++ b/include/linux/power/max8903_charger.h
@@ -26,8 +26,8 @@
struct max8903_pdata {
/*
* GPIOs
-* cen, chg, flt, and usus are optional.
-* dok, dcm, and uok are not optional depending on the status of
+* cen, chg, flt, dcm and usus are optional.
+* dok and uok are not optional depending on the status of
* dc_valid and usb_valid.
*/
int cen;/* Charger Enable input */
@@ -41,7 +41,7 @@ struct max8903_pdata {
/*
* DC(Adapter/TA) is wired
* When dc_valid is true,
-* dok and dcm should be valid.
+* dok should be valid.
*
* At least one of dc_valid or usb_valid should be true.
*/
--
1.9.1
[PATCH v2 1/2] Crypto: Add SHA-3 hash algorithm
From: Jeff Garzik
This patch adds the implementation of SHA3 algorithm
in software and it's based on original implementation
pushed in patch https://lwn.net/Articles/518415/ with
additional changes to match the padding rules specified
in SHA-3 specification.
Signed-off-by: Jeff Garzik
Signed-off-by: Raveendra Padasalagi
---
crypto/Kconfig| 10 ++
crypto/Makefile | 1 +
crypto/sha3_generic.c | 300 ++
include/crypto/sha3.h | 29 +
4 files changed, 340 insertions(+)
create mode 100644 crypto/sha3_generic.c
create mode 100644 include/crypto/sha3.h
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 1d33beb..83ee8cb 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -750,6 +750,16 @@ config CRYPTO_SHA512_SPARC64
SHA-512 secure hash standard (DFIPS 180-2) implemented
using sparc64 crypto instructions, when available.
+config CRYPTO_SHA3
+ tristate "SHA3 digest algorithm"
+ select CRYPTO_HASH
+ help
+ SHA-3 secure hash standard (DFIPS 202). It's based on
+ cryptographic sponge function family called Keccak.
+
+ References:
+ http://keccak.noekeon.org/
+
config CRYPTO_TGR192
tristate "Tiger digest algorithms"
select CRYPTO_HASH
diff --git a/crypto/Makefile b/crypto/Makefile
index 4f4ef7e..0b82c47 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -61,6 +61,7 @@ obj-$(CONFIG_CRYPTO_RMD320) += rmd320.o
obj-$(CONFIG_CRYPTO_SHA1) += sha1_generic.o
obj-$(CONFIG_CRYPTO_SHA256) += sha256_generic.o
obj-$(CONFIG_CRYPTO_SHA512) += sha512_generic.o
+obj-$(CONFIG_CRYPTO_SHA3) += sha3_generic.o
obj-$(CONFIG_CRYPTO_WP512) += wp512.o
obj-$(CONFIG_CRYPTO_TGR192) += tgr192.o
obj-$(CONFIG_CRYPTO_GF128MUL) += gf128mul.o
diff --git a/crypto/sha3_generic.c b/crypto/sha3_generic.c
new file mode 100644
index 000..6226439
--- /dev/null
+++ b/crypto/sha3_generic.c
@@ -0,0 +1,300 @@
+/*
+ * Cryptographic API.
+ *
+ * SHA-3, as specified in
+ * http://nvlpubs.nist.gov/nistpubs/FIPS/NIST.FIPS.202.pdf
+ *
+ * SHA-3 code by Jeff Garzik
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License as published by the Free
+ * Software Foundation; either version 2 of the License, or (at your option)???
+ * any later version.
+ *
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define KECCAK_ROUNDS 24
+
+#define ROTL64(x, y) (((x) << (y)) | ((x) >> (64 - (y
+
+static const u64 keccakf_rndc[24] = {
+ 0x0001, 0x8082, 0x8000808a,
+ 0x800080008000, 0x808b, 0x8001,
+ 0x800080008081, 0x80008009, 0x008a,
+ 0x0088, 0x80008009, 0x800a,
+ 0x8000808b, 0x808b, 0x80008089,
+ 0x80008003, 0x80008002, 0x8080,
+ 0x800a, 0x8000800a, 0x800080008081,
+ 0x80008080, 0x8001, 0x800080008008
+};
+
+static const int keccakf_rotc[24] = {
+ 1, 3, 6, 10, 15, 21, 28, 36, 45, 55, 2, 14,
+ 27, 41, 56, 8, 25, 43, 62, 18, 39, 61, 20, 44
+};
+
+static const int keccakf_piln[24] = {
+ 10, 7, 11, 17, 18, 3, 5, 16, 8, 21, 24, 4,
+ 15, 23, 19, 13, 12, 2, 20, 14, 22, 9, 6, 1
+};
+
+/* update the state with given number of rounds */
+
+static void keccakf(u64 st[25])
+{
+ int i, j, round;
+ u64 t, bc[5];
+
+ for (round = 0; round < KECCAK_ROUNDS; round++) {
+
+ /* Theta */
+ for (i = 0; i < 5; i++)
+ bc[i] = st[i] ^ st[i + 5] ^ st[i + 10] ^ st[i + 15]
+ ^ st[i + 20];
+
+ for (i = 0; i < 5; i++) {
+ t = bc[(i + 4) % 5] ^ ROTL64(bc[(i + 1) % 5], 1);
+ for (j = 0; j < 25; j += 5)
+ st[j + i] ^= t;
+ }
+
+ /* Rho Pi */
+ t = st[1];
+ for (i = 0; i < 24; i++) {
+ j = keccakf_piln[i];
+ bc[0] = st[j];
+ st[j] = ROTL64(t, keccakf_rotc[i]);
+ t = bc[0];
+ }
+
+ /* Chi */
+ for (j = 0; j < 25; j += 5) {
+ for (i = 0; i < 5; i++)
+ bc[i] = st[j + i];
+ for (i = 0; i < 5; i++)
+ st[j + i] ^= (~bc[(i + 1) % 5]) &
+bc[(i + 2) % 5];
+ }
+
+ /* Iota */
+ st[0] ^= keccakf_rndc[round];
+ }
+}
+
+static void sha3_init(struct sha3_state *sctx, unsigned int digest_sz)
+{
+ memset(sctx, 0, sizeof(*sctx));
+ sctx->md_len = digest_sz;
+ sctx->rsiz = 200 - 2 *
[PATCH v2 2/2] Crypto: Add SHA-3 Test's in tcrypt
Added support for SHA-3 algorithm test's
in tcrypt module and related test vectors.
Signed-off-by: Raveendra Padasalagi
---
crypto/tcrypt.c | 53 ++-
crypto/testmgr.c | 40 ++
crypto/testmgr.h | 125 +++
3 files changed, 217 insertions(+), 1 deletion(-)
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 579dce0..4675459 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -72,7 +72,8 @@ static char *check[] = {
"cast6", "arc4", "michael_mic", "deflate", "crc32c", "tea", "xtea",
"khazad", "wp512", "wp384", "wp256", "tnepres", "xeta", "fcrypt",
"camellia", "seed", "salsa20", "rmd128", "rmd160", "rmd256", "rmd320",
- "lzo", "cts", "zlib", NULL
+ "lzo", "cts", "zlib", "sha3-224", "sha3-256", "sha3-384", "sha3-512",
+ NULL
};
struct tcrypt_result {
@@ -1284,6 +1285,22 @@ static int do_test(const char *alg, u32 type, u32 mask,
int m)
ret += tcrypt_test("crct10dif");
break;
+ case 48:
+ ret += tcrypt_test("sha3-224");
+ break;
+
+ case 49:
+ ret += tcrypt_test("sha3-256");
+ break;
+
+ case 50:
+ ret += tcrypt_test("sha3-384");
+ break;
+
+ case 51:
+ ret += tcrypt_test("sha3-512");
+ break;
+
case 100:
ret += tcrypt_test("hmac(md5)");
break;
@@ -1691,6 +1708,22 @@ static int do_test(const char *alg, u32 type, u32 mask,
int m)
test_hash_speed("poly1305", sec, poly1305_speed_template);
if (mode > 300 && mode < 400) break;
+ case 322:
+ test_hash_speed("sha3-224", sec, generic_hash_speed_template);
+ if (mode > 300 && mode < 400) break;
+
+ case 323:
+ test_hash_speed("sha3-256", sec, generic_hash_speed_template);
+ if (mode > 300 && mode < 400) break;
+
+ case 324:
+ test_hash_speed("sha3-384", sec, generic_hash_speed_template);
+ if (mode > 300 && mode < 400) break;
+
+ case 325:
+ test_hash_speed("sha3-512", sec, generic_hash_speed_template);
+ if (mode > 300 && mode < 400) break;
+
case 399:
break;
@@ -1770,6 +1803,24 @@ static int do_test(const char *alg, u32 type, u32 mask,
int m)
test_ahash_speed("rmd320", sec, generic_hash_speed_template);
if (mode > 400 && mode < 500) break;
+ case 418:
+ test_ahash_speed("sha3-224", sec, generic_hash_speed_template);
+ if (mode > 400 && mode < 500) break;
+
+ case 419:
+ test_ahash_speed("sha3-256", sec, generic_hash_speed_template);
+ if (mode > 400 && mode < 500) break;
+
+ case 420:
+ test_ahash_speed("sha3-384", sec, generic_hash_speed_template);
+ if (mode > 400 && mode < 500) break;
+
+
+ case 421:
+ test_ahash_speed("sha3-512", sec, generic_hash_speed_template);
+ if (mode > 400 && mode < 500) break;
+
+
case 499:
break;
diff --git a/crypto/testmgr.c b/crypto/testmgr.c
index c727fb0..b773a56 100644
--- a/crypto/testmgr.c
+++ b/crypto/testmgr.c
@@ -3659,6 +3659,46 @@ static const struct alg_test_desc alg_test_descs[] = {
}
}
}, {
+ .alg = "sha3-224",
+ .test = alg_test_hash,
+ .fips_allowed = 1,
+ .suite = {
+ .hash = {
+ .vecs = sha3_224_tv_template,
+ .count = SHA3_224_TEST_VECTORS
+ }
+ }
+ }, {
+ .alg = "sha3-256",
+ .test = alg_test_hash,
+ .fips_allowed = 1,
+ .suite = {
+ .hash = {
+ .vecs = sha3_256_tv_template,
+ .count = SHA3_256_TEST_VECTORS
+ }
+ }
+ }, {
+ .alg = "sha3-384",
+ .test = alg_test_hash,
+ .fips_allowed = 1,
+ .suite = {
+ .hash = {
+ .vecs = sha3_384_tv_template,
+ .count = SHA3_384_TEST_VECTORS
+ }
+ }
+ }, {
+ .alg = "sha3-512",
+ .test = alg_test_hash,
+ .fips_allowed = 1,
+ .suite = {
+ .hash = {
+ .vecs = sha3_512_tv_template,
+ .count = SHA3_512_TEST_VECTORS
+ }
+ }
+ }, {
.alg = "sha384",
.test = alg_test_hash,
[PATCH v3 7/7] max8903: adds support for initiation via device tree.
From: Chris Lapa
Adds support for device tree to setup a max8903 battery charger. DC and USB
validity are determined by looking the presence of the dok and uok gpios.
Signed-off-by: Chris Lapa
---
drivers/power/max8903_charger.c | 217 +++-
1 file changed, 145 insertions(+), 72 deletions(-)
diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c
index 5ddc667..3c59213 100644
--- a/drivers/power/max8903_charger.c
+++ b/drivers/power/max8903_charger.c
@@ -23,6 +23,9 @@
#include
#include
#include
+#include
+#include
+#include
#include
#include
#include
@@ -75,6 +78,7 @@ static int max8903_get_property(struct power_supply *psy,
default:
return -EINVAL;
}
+
return 0;
}
@@ -179,48 +183,116 @@ static irqreturn_t max8903_fault(int irq, void *_data)
return IRQ_HANDLED;
}
+static struct max8903_pdata *max8903_parse_dt_data(
+ struct device *dev)
+{
+ struct device_node *of_node = dev->of_node;
+ struct max8903_pdata *pdata = NULL;
+
+ if (!of_node)
+ return NULL;
+
+ pdata = devm_kzalloc(dev, sizeof(*pdata),
+ GFP_KERNEL);
+ if (!pdata)
+ return NULL;
+
+ pdata->dc_valid = false;
+ pdata->usb_valid = false;
+
+ pdata->cen = of_get_named_gpio(of_node, "cen-gpios", 0);
+ if (!gpio_is_valid(pdata->cen))
+ pdata->cen = -EINVAL;
+
+ pdata->chg = of_get_named_gpio(of_node, "chg-gpios", 0);
+ if (!gpio_is_valid(pdata->chg))
+ pdata->chg = -EINVAL;
+
+ pdata->flt = of_get_named_gpio(of_node, "flt-gpios", 0);
+ if (!gpio_is_valid(pdata->flt))
+ pdata->flt = -EINVAL;
+
+ pdata->usus = of_get_named_gpio(of_node, "usus-gpios", 0);
+ if (!gpio_is_valid(pdata->usus))
+ pdata->usus = -EINVAL;
+
+ pdata->dcm = of_get_named_gpio(of_node, "dcm-gpios", 0);
+ if (!gpio_is_valid(pdata->dcm))
+ pdata->dcm = -EINVAL;
+
+ pdata->dok = of_get_named_gpio(of_node, "dok-gpios", 0);
+ if (!gpio_is_valid(pdata->dok))
+ pdata->dok = -EINVAL;
+ else
+ pdata->dc_valid = true;
+
+ pdata->uok = of_get_named_gpio(of_node, "uok-gpios", 0);
+ if (!gpio_is_valid(pdata->uok))
+ pdata->uok = -EINVAL;
+ else
+ pdata->usb_valid = true;
+
+ return pdata;
+}
+
static int max8903_probe(struct platform_device *pdev)
{
- struct max8903_data *data;
+ struct max8903_data *charger;
struct device *dev = &pdev->dev;
- struct max8903_pdata *pdata = pdev->dev.platform_data;
struct power_supply_config psy_cfg = {};
int ret = 0;
int gpio;
int ta_in = 0;
int usb_in = 0;
- if (pdata == NULL) {
+ charger = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL);
+ if (!charger)
+ return -ENOMEM;
+
+ charger->pdata = pdev->dev.platform_data;
+ if (IS_ENABLED(CONFIG_OF) && !charger->pdata && dev->of_node)
+ charger->pdata = max8903_parse_dt_data(dev);
+
+ if (!charger->pdata) {
dev_err(dev, "No platform data.\n");
return -EINVAL;
}
- data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL);
- if (!data)
- return -ENOMEM;
+ charger->dev = dev;
- data->pdata = pdev->dev.platform_data;
- data->dev = dev;
- platform_set_drvdata(pdev, data);
+ charger->fault = false;
+ charger->ta_in = ta_in;
+ charger->usb_in = usb_in;
- if (pdata->dc_valid == false && pdata->usb_valid == false) {
+ charger->psy_desc.name = "max8903_charger";
+ charger->psy_desc.type = (ta_in) ? POWER_SUPPLY_TYPE_MAINS :
+ ((usb_in) ? POWER_SUPPLY_TYPE_USB :
+POWER_SUPPLY_TYPE_BATTERY);
+ charger->psy_desc.get_property = max8903_get_property;
+ charger->psy_desc.properties = max8903_charger_props;
+ charger->psy_desc.num_properties = ARRAY_SIZE(max8903_charger_props);
+
+ platform_set_drvdata(pdev, charger);
+
+ if (charger->pdata->dc_valid == false && charger->pdata->usb_valid ==
false) {
dev_err(dev, "No valid power sources.\n");
return -EINVAL;
}
- if (pdata->dc_valid) {
- if (gpio_is_valid(pdata->dok)) {
+
+ if (charger->pdata->dc_valid) {
+ if (gpio_is_valid(charger->pdata->dok)) {
ret = devm_gpio_request(dev,
- pdata->dok,
- data->psy_desc.name);
+ charger->pdata->dok,
+ charger->psy_desc.name);
[PATCH v3 5/7] max8903: removes non zero validity checks on gpios.
From: Chris Lapa
Prior to this commit a zero gpio was treated as invalid. Whereas
gpio_is_valid() will treat a zero gpio as valid.
This commit removes the confusion and explicity uses gpio_is_valid()
throughout. Which in turn results in several of the error messages becoming
redundant and thus removed.
Signed-off-by: Chris Lapa
---
drivers/power/max8903_charger.c | 125 ++--
1 file changed, 55 insertions(+), 70 deletions(-)
diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c
index c068efe..bfb81a2 100644
--- a/drivers/power/max8903_charger.c
+++ b/drivers/power/max8903_charger.c
@@ -53,7 +53,7 @@ static int max8903_get_property(struct power_supply *psy,
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
- if (data->pdata->chg) {
+ if (gpio_is_valid(data->pdata->chg)) {
if (gpio_get_value(data->pdata->chg) == 0)
val->intval = POWER_SUPPLY_STATUS_CHARGING;
else if (data->usb_in || data->ta_in)
@@ -93,11 +93,11 @@ static irqreturn_t max8903_dcin(int irq, void *_data)
data->ta_in = ta_in;
/* Set Current-Limit-Mode 1:DC 0:USB */
- if (pdata->dcm)
+ if (gpio_is_valid(pdata->dcm))
gpio_set_value(pdata->dcm, ta_in ? 1 : 0);
/* Charger Enable / Disable (cen is negated) */
- if (pdata->cen)
+ if (gpio_is_valid(pdata->cen))
gpio_set_value(pdata->cen, ta_in ? 0 :
(data->usb_in ? 0 : 1));
@@ -136,7 +136,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data)
/* Do not touch Current-Limit-Mode */
/* Charger Enable / Disable (cen is negated) */
- if (pdata->cen)
+ if (gpio_is_valid(pdata->cen))
gpio_set_value(pdata->cen, usb_in ? 0 :
(data->ta_in ? 0 : 1));
@@ -211,7 +211,7 @@ static int max8903_probe(struct platform_device *pdev)
}
if (pdata->dc_valid) {
- if (pdata->dok && gpio_is_valid(pdata->dok)) {
+ if (gpio_is_valid(pdata->dok)) {
ret = devm_gpio_request(dev,
pdata->dok,
data->psy_desc.name);
@@ -231,28 +231,23 @@ static int max8903_probe(struct platform_device *pdev)
}
}
- if (pdata->dcm) {
- if (gpio_is_valid(pdata->dcm)) {
- ret = devm_gpio_request(dev,
- pdata->dcm,
- data->psy_desc.name);
- if (ret) {
- dev_err(dev,
- "Failed GPIO request for dcm:
%d err %d\n",
- pdata->dcm, ret);
- return -EINVAL;
- }
-
- gpio = pdata->dcm; /* Output */
- gpio_set_value(gpio, ta_in);
- } else {
- dev_err(dev, "Invalid pin: dcm.\n");
+ if (gpio_is_valid(pdata->dcm)) {
+ ret = devm_gpio_request(dev,
+ pdata->dcm,
+ data->psy_desc.name);
+ if (ret) {
+ dev_err(dev,
+ "Failed GPIO request for dcm: %d err
%d\n",
+ pdata->dcm, ret);
return -EINVAL;
}
+
+ gpio = pdata->dcm; /* Output */
+ gpio_set_value(gpio, ta_in);
}
if (pdata->usb_valid) {
- if (pdata->uok && gpio_is_valid(pdata->uok)) {
+ if (gpio_is_valid(pdata->uok)) {
ret = devm_gpio_request(dev,
pdata->uok,
data->psy_desc.name);
@@ -272,64 +267,54 @@ static int max8903_probe(struct platform_device *pdev)
}
}
- if (pdata->cen) {
- if (gpio_is_valid(pdata->cen)) {
- ret = devm_gpio_request(dev,
- pdata->cen,
- data->psy_desc.name);
- if (ret) {
- dev_err(dev,
- "Failed GPIO request for cen:
%d err %d\n",
- pdata->cen, ret);
- return -EINVAL;
- }
- gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1);
- } else {
-
[PATCH v3 2/7] max8903: store pointer to pdata instead of copying it.
From: Chris Lapa
Stores pointer to pdata because it easily allows pdata to reference
either platform data or in the future device tree data.
Signed-off-by: Chris Lapa
---
drivers/power/max8903_charger.c | 20 +---
1 file changed, 13 insertions(+), 7 deletions(-)
diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c
index 17876ca..0a5b0e1 100644
--- a/drivers/power/max8903_charger.c
+++ b/drivers/power/max8903_charger.c
@@ -29,7 +29,7 @@
#include
struct max8903_data {
- struct max8903_pdata pdata;
+ struct max8903_pdata *pdata;
struct device *dev;
struct power_supply *psy;
struct power_supply_desc psy_desc;
@@ -53,8 +53,8 @@ static int max8903_get_property(struct power_supply *psy,
switch (psp) {
case POWER_SUPPLY_PROP_STATUS:
val->intval = POWER_SUPPLY_STATUS_UNKNOWN;
- if (data->pdata.chg) {
- if (gpio_get_value(data->pdata.chg) == 0)
+ if (data->pdata->chg) {
+ if (gpio_get_value(data->pdata->chg) == 0)
val->intval = POWER_SUPPLY_STATUS_CHARGING;
else if (data->usb_in || data->ta_in)
val->intval = POWER_SUPPLY_STATUS_NOT_CHARGING;
@@ -81,7 +81,7 @@ static int max8903_get_property(struct power_supply *psy,
static irqreturn_t max8903_dcin(int irq, void *_data)
{
struct max8903_data *data = _data;
- struct max8903_pdata *pdata = &data->pdata;
+ struct max8903_pdata *pdata = data->pdata;
bool ta_in;
enum power_supply_type old_type;
@@ -122,7 +122,7 @@ static irqreturn_t max8903_dcin(int irq, void *_data)
static irqreturn_t max8903_usbin(int irq, void *_data)
{
struct max8903_data *data = _data;
- struct max8903_pdata *pdata = &data->pdata;
+ struct max8903_pdata *pdata = data->pdata;
bool usb_in;
enum power_supply_type old_type;
@@ -161,7 +161,7 @@ static irqreturn_t max8903_usbin(int irq, void *_data)
static irqreturn_t max8903_fault(int irq, void *_data)
{
struct max8903_data *data = _data;
- struct max8903_pdata *pdata = &data->pdata;
+ struct max8903_pdata *pdata = data->pdata;
bool fault;
fault = gpio_get_value(pdata->flt) ? false : true;
@@ -190,12 +190,18 @@ static int max8903_probe(struct platform_device *pdev)
int ta_in = 0;
int usb_in = 0;
+ if (pdata == NULL) {
+ dev_err(dev, "No platform data.\n");
+ return -EINVAL;
+ }
+
data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL);
if (data == NULL) {
dev_err(dev, "Cannot allocate memory.\n");
return -ENOMEM;
}
- memcpy(&data->pdata, pdata, sizeof(struct max8903_pdata));
+
+ data->pdata = pdev->dev.platform_data;
data->dev = dev;
platform_set_drvdata(pdev, data);
--
1.9.1
[PATCH v3 4/7] max8903: adds requesting of gpios.
From: Chris Lapa
This change ensures all gpios are available for the driver to use.
Signed-off-by: Chris Lapa
---
drivers/power/max8903_charger.c | 79 -
1 file changed, 70 insertions(+), 9 deletions(-)
diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c
index dbd911c4..c068efe 100644
--- a/drivers/power/max8903_charger.c
+++ b/drivers/power/max8903_charger.c
@@ -212,6 +212,16 @@ static int max8903_probe(struct platform_device *pdev)
if (pdata->dc_valid) {
if (pdata->dok && gpio_is_valid(pdata->dok)) {
+ ret = devm_gpio_request(dev,
+ pdata->dok,
+ data->psy_desc.name);
+ if (ret) {
+ dev_err(dev,
+ "Failed GPIO request for dok:
%d err %d\n",
+ pdata->dok, ret);
+ return -EINVAL;
+ }
+
gpio = pdata->dok; /* PULL_UPed Interrupt */
ta_in = gpio_get_value(gpio) ? 0 : 1;
} else {
@@ -223,6 +233,16 @@ static int max8903_probe(struct platform_device *pdev)
if (pdata->dcm) {
if (gpio_is_valid(pdata->dcm)) {
+ ret = devm_gpio_request(dev,
+ pdata->dcm,
+ data->psy_desc.name);
+ if (ret) {
+ dev_err(dev,
+ "Failed GPIO request for dcm:
%d err %d\n",
+ pdata->dcm, ret);
+ return -EINVAL;
+ }
+
gpio = pdata->dcm; /* Output */
gpio_set_value(gpio, ta_in);
} else {
@@ -233,6 +253,16 @@ static int max8903_probe(struct platform_device *pdev)
if (pdata->usb_valid) {
if (pdata->uok && gpio_is_valid(pdata->uok)) {
+ ret = devm_gpio_request(dev,
+ pdata->uok,
+ data->psy_desc.name);
+ if (ret) {
+ dev_err(dev,
+ "Failed GPIO request for uok:
%d err %d\n",
+ pdata->uok, ret);
+ return -EINVAL;
+ }
+
gpio = pdata->uok;
usb_in = gpio_get_value(gpio) ? 0 : 1;
} else {
@@ -244,6 +274,16 @@ static int max8903_probe(struct platform_device *pdev)
if (pdata->cen) {
if (gpio_is_valid(pdata->cen)) {
+ ret = devm_gpio_request(dev,
+ pdata->cen,
+ data->psy_desc.name);
+ if (ret) {
+ dev_err(dev,
+ "Failed GPIO request for cen:
%d err %d\n",
+ pdata->cen, ret);
+ return -EINVAL;
+ }
+
gpio_set_value(pdata->cen, (ta_in || usb_in) ? 0 : 1);
} else {
dev_err(dev, "Invalid pin: cen.\n");
@@ -252,23 +292,44 @@ static int max8903_probe(struct platform_device *pdev)
}
if (pdata->chg) {
- if (!gpio_is_valid(pdata->chg)) {
- dev_err(dev, "Invalid pin: chg.\n");
- return -EINVAL;
+ if (gpio_is_valid(pdata->chg)) {
+ ret = devm_gpio_request(dev,
+ pdata->chg,
+ data->psy_desc.name);
+ if (ret) {
+ dev_err(dev,
+ "Failed GPIO request for chg:
%d err %d\n",
+ pdata->chg, ret);
+ return -EINVAL;
+ }
}
}
if (pdata->flt) {
- if (!gpio_is_valid(pdata->flt)) {
- dev_err(dev, "Invalid pin: flt.\n");
- return -EINVAL;
+ if (gpio_is_valid(pdata->flt)) {
+ ret = devm_gpio_request(dev,
+ pdata->flt,
+ data->psy_desc.name);
+ if (ret) {
+ dev_err(dev,
+
[PATCH v2 0/2] Add SHA-3 algorithm and test vectors.
This patchset adds the implementation of SHA-3 algorithm in software and it's based on original implementation pushed in patch https://lwn.net/Articles/518415/ with additional changes to match the padding rules specified in SHA-3 specification. This patchset also includes changes in tcrypt module to add support for SHA-3 algorithms test and related test vectors for basic testing. Broadcom Secure Processing Unit-2(SPU-2) engine supports offloading of SHA-3 operations in hardware, in order to add SHA-3 support in SPU-2 driver we needed to have the software implementation and test framework in place. The patchset is based on v4.7-rc1 tag and its tested on Broadcom NorthStar2 SoC. The patch set can be fetched from iproc-sha3-v2 branch of https://github.com/Broadcom/arm64-linux.git Changes since v1: - Renamed MODULE_ALIAS to MODULE_ALIAS_CRYPTO - Added aliases for below cra_driver_name's sha3-224-generic sha3-256-generic sha3-384-generic sha3-512-generic Jeff Garzik (1): Crypto: Add SHA-3 hash algorithm Raveendra Padasalagi (1): Crypto: Add SHA-3 Test's in tcrypt crypto/Kconfig| 10 ++ crypto/Makefile | 1 + crypto/sha3_generic.c | 300 ++ crypto/tcrypt.c | 53 - crypto/testmgr.c | 40 +++ crypto/testmgr.h | 125 + include/crypto/sha3.h | 29 + 7 files changed, 557 insertions(+), 1 deletion(-) create mode 100644 crypto/sha3_generic.c create mode 100644 include/crypto/sha3.h -- 1.9.1
[PATCH v3 6/7] max8903: remove unnecessary 'out of memory' error message.
From: Chris Lapa
Remove the 'out of memory' error message as it is printed by the core.
Signed-off-by: Chris Lapa
---
drivers/power/max8903_charger.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/power/max8903_charger.c b/drivers/power/max8903_charger.c
index bfb81a2..5ddc667 100644
--- a/drivers/power/max8903_charger.c
+++ b/drivers/power/max8903_charger.c
@@ -196,10 +196,8 @@ static int max8903_probe(struct platform_device *pdev)
}
data = devm_kzalloc(dev, sizeof(struct max8903_data), GFP_KERNEL);
- if (data == NULL) {
- dev_err(dev, "Cannot allocate memory.\n");
+ if (!data)
return -ENOMEM;
- }
data->pdata = pdev->dev.platform_data;
data->dev = dev;
--
1.9.1
Re: [PATCH v6 2/2] phy: rockchip-inno-usb2: add a new driver for Rockchip usb2phy
On 06/16/2016 07:09 PM, Frank Wang wrote:
The newer SoCs (rk3366, rk3399) take a different usb-phy IP block
than rk3288 and before, and most of phy-related registers are also
different from the past, so a new phy driver is required necessarily.
Signed-off-by: Frank Wang
Suggested-by: Guenter Roeck
Suggested-by: Doug Anderson
Reviewed-by: Heiko Stuebner
Tested-by: Heiko Stuebner
---
[ ... ]
+
+static int rockchip_usb2phy_resume(struct phy *phy)
+{
+ struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
+ struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
+ int ret;
+
+ dev_dbg(&rport->phy->dev, "port resume\n");
+
+ ret = clk_prepare_enable(rphy->clk480m);
+ if (ret)
+ return ret;
+
If suspend can be called multiple times, resume can be called
multiple times as well. Doesn't this cause a clock imbalance
if you call clk_prepare_enable() multiple times on resume,
but clk_disable_unprepare() only once on suspend ?
+ ret = property_enable(rphy, &rport->port_cfg->phy_sus, false);
+ if (ret)
+ return ret;
+
+ rport->suspended = false;
+ return 0;
+}
+
+static int rockchip_usb2phy_suspend(struct phy *phy)
+{
+ struct rockchip_usb2phy_port *rport = phy_get_drvdata(phy);
+ struct rockchip_usb2phy *rphy = dev_get_drvdata(phy->dev.parent);
+ int ret;
+
+ dev_dbg(&rport->phy->dev, "port suspend\n");
+
+ if (rport->suspended)
+ goto exit;
+
I know I am nitpicking, but
return 0;
would be fine here, be more consistent with the rest of the code,
+ ret = property_enable(rphy, &rport->port_cfg->phy_sus, true);
+ if (ret)
+ return ret;
+
+ rport->suspended = true;
+ clk_disable_unprepare(rphy->clk480m);
+
+exit:
+ return 0;
and this label is really unnecessary.
+}
+
[ ... ]
Re: [PATCH 4.7 FIX] brcmfmac: fix lockup when removing P2P interface after event timeout
On 16 June 2016 at 17:10, Kalle Valo wrote: > Rafał Miłecki writes: > >> Removing P2P interface is handled by sending a proper request to the >> firmware. On success firmware triggers an event and driver's handler >> removes a matching interface. >> >> However on event timeout we remove interface directly from the cfg80211 >> callback. Current code doesn't handle this case correctly as it always >> assumes rtnl to be unlocked. >> >> Fix it by adding an extra rtnl_locked parameter to functions and calling >> unregister_netdevice when needed. >> >> Signed-off-by: Rafał Miłecki > > Failed to apply, please rebase: > > Applying: brcmfmac: fix lockup when removing P2P interface after event timeout > Using index info to reconstruct a base tree... > Falling back to patching base and 3-way merge... > Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/p2p.c > Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h > CONFLICT (content): Merge conflict in > drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.h > Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/core.c > Auto-merging drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c > Failed to merge in the changes. > Patch failed at 0001 brcmfmac: fix lockup when removing P2P interface after > event timeout What tree did you try it on? I just went into a dir where I have cloned: git://git.kernel.org/pub/scm/linux/kernel/git/kvalo/wireless-drivers.git My HEAD commit is: 034fdd4 Merge ath-current from ath.git And I can apply this patch cleanly doing: curl https://patchwork.kernel.org/patch/9138925/mbox/ | git am -- Rafał
Re: More parallel atomic_open/d_splice_alias fun with NFS and possibly more FSes.
On Fri, Jun 17, 2016 at 12:09:19AM -0400, Oleg Drokin wrote: > So they both do d_drop(), the dentry is now unhashed, and they both > dive into nfs_lookup(). > There eventually they both call > > res = d_splice_alias(inode, dentry); > >And so the first lucky one continues on it's merry way with a hashed > dentry, >but the other less lucky one ends up calling into d_splice_alias() with >dentry that's already hashed and hits the very familiar assertion. > >I took a brief look into ceph and it looks like a very similar thing >might happen there with handle_reply() for two parallel replies calling > into >ceph_fill_trace() and then splice_alias()->d_splice_alias(), since the >unhashed check it does is not under any locks, it's unsafe, so the problem >might be more generic than just NFS too. > >So I wonder how to best fix this? Holding some sort of dentry lock across > a call >into atomic_open in VFS? We cannot just make d_splice_alias() callers call > with >inode->i_lock held because dentry might be negative. Oh, lovely... So basically the problem is that we violate the "no lookups on the same name in parallel" rule on those fallbacks from foo_atomic_open() to foo_lookup(). The thing is, a lot of ->atomic_open() instances have such fallbacks and I wonder if that's a sign that we need to lift some of that to fs/namei.c... Hell knows; alternative is to have that d_drop() followed by d_alloc_parallel() and feeding that dentry to lookup. I'll play with that a bit and see what's better; hopefully I'll have something by tomorrow.
linux-next: Tree for Jun 17
Hi all, Changes since 20160616: Non-merge commits (relative to Linus' tree): 3866 3847 files changed, 179431 insertions(+), 67790 deletions(-) I have created today's linux-next tree at git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git (patches at http://www.kernel.org/pub/linux/kernel/next/ ). If you are tracking the linux-next tree using git, you should not use "git pull" to do so as that will try to merge the new linux-next release with the old one. You should use "git fetch" and checkout or reset to the new master. You can see which trees have been included by looking in the Next/Trees file in the source. There are also quilt-import.log and merge.log files in the Next directory. Between each merge, the tree was built with a ppc64_defconfig for powerpc and an allmodconfig (with CONFIG_BUILD_DOCSRC=n) for x86_64, a multi_v7_defconfig for arm and a native build of tools/perf. After the final fixups (if any), I do an x86_64 modules_install followed by builds for x86_64 allnoconfig, powerpc allnoconfig (32 and 64 bit), ppc44x_defconfig, allyesconfig (this fails its final link) and pseries_le_defconfig and i386, sparc and sparc64 defconfig. Below is a summary of the state of the merge. I am currently merging 234 trees (counting Linus' and 34 trees of patches pending for Linus' tree). Stats about the size of the tree over time can be seen at http://neuling.org/linux-next-size.html . Status of my local build tests will be at http://kisskb.ellerman.id.au/linux-next . If maintainers want to give advice about cross compilers/configs that work, we are always open to add more builds. Thanks to Randy Dunlap for doing many randconfig builds. And to Paul Gortmaker for triage and bug fixes. -- Cheers, Stephen Rothwell $ git checkout master $ git reset --hard stable Merging origin/master (d325ea859490 Merge tag 'drm-fixes-for-v4.7-rc4' of git://people.freedesktop.org/~airlied/linux) Merging fixes/master (5edb56491d48 Linux 4.7-rc3) Merging kbuild-current/rc-fixes (b36fad65d61f kbuild: Initialize exported variables) Merging arc-current/for-curr (5edb56491d48 Linux 4.7-rc3) Merging arm-current/fixes (56530f5d2ddc ARM: 8579/1: mm: Fix definition of pmd_mknotpresent) Merging m68k-current/for-linus (9a6462763b17 m68k/mvme16x: Include generic ) Merging metag-fixes/fixes (0164a711c97b metag: Fix ioremap_wc/ioremap_cached build errors) Merging powerpc-fixes/fixes (8550e2fa34f0 powerpc/mm/hash: Use the correct PPP mask when updating HPTE) Merging powerpc-merge-mpe/fixes (bc0195aad0da Linux 4.2-rc2) Merging sparc/master (6b15d6650c53 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net) Merging net/master (a547224dceed mlx4e: Do not attempt to offload VXLAN ports that are unrecognized) Merging ipsec/master (d6af1a31cc72 vti: Add pmtu handling to vti_xmit.) Merging ipvs/master (50219538ffc0 vmxnet3: segCnt can be 1 for LRO packets) Merging wireless-drivers/master (034fdd4a17ff Merge ath-current from ath.git) Merging mac80211/master (3d5fdff46c4b wext: Fix 32 bit iwpriv compatibility issue with 64 bit Kernel) Merging sound-current/for-linus (35639a0e9839 ALSA: hda - Add PCI ID for Kabylake) Merging pci-current/for-linus (96381c04ef9b PCI: hv: Handle all pending messages in hv_pci_onchannelcallback()) Merging driver-core.current/driver-core-linus (7e1b1fc4dabd base: make module_create_drivers_dir race-free) Merging tty.current/tty-linus (5edb56491d48 Linux 4.7-rc3) Merging usb.current/usb-linus (1c4bf5ac6a16 usb: musb: sunxi: Remove bogus "Frees glue" comment) Merging usb-gadget-fixes/fixes (50c763f8c1ba usb: dwc3: Set the ClearPendIN bit on Clear Stall EP command) Merging usb-serial-fixes/usb-linus (af8c34ce6ae3 Linux 4.7-rc2) Merging usb-chipidea-fixes/ci-for-usb-stable (ea1d39a31d3b usb: common: otg-fsm: add license to usb-otg-fsm) Merging staging.current/staging-linus (a9cc4006155a staging: lustre: lnet: Don't access NULL NI on failure path) Merging char-misc.current/char-misc-linus (5014e904681d coresight: Handle build path error) Merging input-current/for-linus (540c26087bfb Input: xpad - fix rumble on Xbox One controllers with 2015 firmware) Merging crypto-current/master (19ced623db2f crypto: ux500 - memmove the right size) Merging ide/master (1993b176a822 Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/ide) Merging rr-fixes/fixes (8244062ef1e5 modules: fix longstanding /proc/kallsyms vs module insertion race.) Merging vfio-fixes/for-linus (ce7585f3c4d7 vfio/pci: Allow VPD short read) Merging kselftest-fixes/fixes (f80eb4289491 selftests/exec: Makefile is a run-time dependency, add it to the install list) Merging backlight-fixes/for-backlight-fixes (68feaca0b13e backlight: pwm: Handle EPROBE_DEFER while requesting the PWM) Merging ftrace-fixes/for-next-urgent (6224beb12e19 tracing: Have branch tracer us
Re: [PATCH] zram: use writer semaphores in device attributes store
Hello,
On (06/17/16 11:31), Geliang Tang wrote:
[..]
> Since the device attributes store provides write access. This patch uses
> down_write()/up_write() instead of down_read()/up_read() in
> mem_used_max_store() and compact_store().
we use ->init_lock not to make attrs exclusive, but to prevent
concurrent reset/etc.
> Signed-off-by: Geliang Tang
> ---
> drivers/block/zram/zram_drv.c | 10 +-
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/zram/zram_drv.c b/drivers/block/zram/zram_drv.c
> index 7454cf1..cfed743 100644
> --- a/drivers/block/zram/zram_drv.c
> +++ b/drivers/block/zram/zram_drv.c
> @@ -293,13 +293,13 @@ static ssize_t mem_used_max_store(struct device *dev,
> if (err || val != 0)
> return -EINVAL;
>
> - down_read(&zram->init_lock);
> + down_write(&zram->init_lock);
> if (init_done(zram)) {
> struct zram_meta *meta = zram->meta;
> atomic_long_set(&zram->stats.max_used_pages,
> zs_get_total_pages(meta->mem_pool));
> }
> - up_read(&zram->init_lock);
> + up_write(&zram->init_lock);
not critical. can work.
> return len;
> }
> @@ -372,15 +372,15 @@ static ssize_t compact_store(struct device *dev,
> struct zram *zram = dev_to_zram(dev);
> struct zram_meta *meta;
>
> - down_read(&zram->init_lock);
> + down_write(&zram->init_lock);
> if (!init_done(zram)) {
> - up_read(&zram->init_lock);
> + up_write(&zram->init_lock);
> return -EINVAL;
> }
>
> meta = zram->meta;
> zs_compact(meta->mem_pool);
> - up_read(&zram->init_lock);
> + up_write(&zram->init_lock);
pool compaction can take some time. *probably* seconds in the worst
case, when the device is under IO pressure and class' locks are heavily
contended and every class has a considerable number of objects to move.
so I think we don't want to block other attrs while we compact the pool,
compaction takes care of the concurrency internally.
-ss
More parallel atomic_open/d_splice_alias fun with NFS and possibly more FSes.
Hello!
So I think I finally kind of understand this other d_splice_alias problem
I've been having.
Imagine that we have a negative dentry in the cache.
Now we have two threads that are trying to open this file in parallel,
the fs is NFS, so atomic_open is defined, it's not a create, so
parent is locked in shared mode and they both enter first lookup_open
and then (because the dentry is negative) into nfs_atomic_open().
Now in nfs_atomic_open we have two threads that run alongside each other
trying to open this file and both fail (the code is with Al's earlier patch
for the other d_splice_alias problem).
inode = NFS_PROTO(dir)->open_context(dir, ctx, open_flags, &attr,
opened);
if (IS_ERR(inode)) {
err = PTR_ERR(inode);
trace_nfs_atomic_open_exit(dir, ctx, open_flags, err);
put_nfs_open_context(ctx);
d_drop(dentry);
. . .
case -ENOTDIR:
goto no_open;
. . .
no_open:
res = nfs_lookup(dir, dentry, lookup_flags);
So they both do d_drop(), the dentry is now unhashed, and they both
dive into nfs_lookup().
There eventually they both call
res = d_splice_alias(inode, dentry);
And so the first lucky one continues on it's merry way with a hashed dentry,
but the other less lucky one ends up calling into d_splice_alias() with
dentry that's already hashed and hits the very familiar assertion.
I took a brief look into ceph and it looks like a very similar thing
might happen there with handle_reply() for two parallel replies calling into
ceph_fill_trace() and then splice_alias()->d_splice_alias(), since the
unhashed check it does is not under any locks, it's unsafe, so the problem
might be more generic than just NFS too.
So I wonder how to best fix this? Holding some sort of dentry lock across a
call
into atomic_open in VFS? We cannot just make d_splice_alias() callers call
with
inode->i_lock held because dentry might be negative.
I know this is really happening because I in fact have 3 threads doing the
above
described race, one luckily passing through and two hitting the assertion:
PID: 2689619 TASK: 8800ca740540 CPU: 6 COMMAND: "ls"
#0 [8800c6da3758] machine_kexec at 81042462
#1 [8800c6da37b8] __crash_kexec at 811365ff
#2 [8800c6da3880] __crash_kexec at 811366d5
#3 [8800c6da3898] crash_kexec at 8113671b
#4 [8800c6da38b8] oops_end at 81018cb4
#5 [8800c6da38e0] die at 8101917b
#6 [8800c6da3910] do_trap at 81015b92
#7 [8800c6da3960] do_error_trap at 81015d8b
#8 [8800c6da3a20] do_invalid_op at 810166e0
#9 [8800c6da3a30] invalid_op at 8188c91e
[exception RIP: d_splice_alias+478]
RIP: 8128784e RSP: 8800c6da3ae8 RFLAGS: 00010286
RAX: 8800d8661ab8 RBX: 8800d3b6b178 RCX: 8800ca740540
RDX: 81382f09 RSI: 8800d3b6b178 RDI: 8800d8661ab8
RBP: 8800c6da3b20 R8: R9:
R10: 8800ca740540 R11: R12: 8800d6491600
R13: 0102 R14: 8800d39063d0 R15:
ORIG_RAX: CS: 0010 SS: 0018
#10 [8800c6da3b28] nfs_lookup at 8137cd1c
#11 [8800c6da3b80] nfs_atomic_open at 8137ed61
#12 [8800c6da3c30] lookup_open at 8127901a
#13 [8800c6da3d50] path_openat at 8127bf15
#14 [8800c6da3dd8] do_filp_open at 8127d9b1
#15 [8800c6da3ee0] do_sys_open at 81269f90
#16 [8800c6da3f40] sys_open at 8126a09e
#17 [8800c6da3f50] entry_SYSCALL_64_fastpath at 8188aebc
RIP: 7f92a76d4d20 RSP: 7fff640c1160 RFLAGS: 0206
RAX: ffda RBX: 0003 RCX: 7f92a76d4d20
RDX: 55c9f5be9f50 RSI: 00090800 RDI: 55c9f5bea1d0
RBP: R8: 55c9f5bea340 R9:
R10: 55c9f5bea300 R11: 0206 R12: 7f92a800d6b0
R13: 55c9f5bf3000 R14: 55c9f5bea3e0 R15:
ORIG_RAX: 0002 CS: 0033 SS: 002b
PID: 2689597 TASK: 880109b40bc0 CPU: 7 COMMAND: "ls"
#0 [8800c07178e0] die at 8101914e
#1 [8800c0717910] do_trap at 81015b92
#2 [8800c0717960] do_error_trap at 81015d8b
#3 [8800c0717a20] do_invalid_op at 810166e0
#4 [8800c0717a30] invalid_op at 8188c91e
[exception RIP: d_splice_alias+478]
RIP: 8128784e RSP: 8800c0717ae8 RFLAGS: 00010286
RAX: 8800d8661ab8 RBX: 8800d3b6b178 RCX: 880109b40bc0
RDX: 81382f09 RSI: 8800d3b6b178 RDI: 8800d8661ab8
RBP: 8800c0717b20 R8: R9:
R10: 880109b40bc0 R11: R12: ff
