* [PATCH v6 5/5] ARM: dts: imx6q-evi: support cyclone-ps-spi
From: Joshua Clayton @ 2016-12-16 23:17 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <cover.1481918884.git.stillcompiling@gmail.com>
Add support for Altera cyclone V FPGA connected to an spi port
to the evi devicetree file
Signed-off-by: Joshua Clayton <stillcompiling@gmail.com>
---
arch/arm/boot/dts/imx6q-evi.dts | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/arch/arm/boot/dts/imx6q-evi.dts b/arch/arm/boot/dts/imx6q-evi.dts
index 7c7c1a8..ec4d365 100644
--- a/arch/arm/boot/dts/imx6q-evi.dts
+++ b/arch/arm/boot/dts/imx6q-evi.dts
@@ -95,6 +95,15 @@
pinctrl-names = "default";
pinctrl-0 = <&pinctrl_ecspi1 &pinctrl_ecspi1cs>;
status = "okay";
+
+ fpga_spi: cyclonespi at 0 {
+ compatible = "altr,cyclone-ps-spi-fpga-mgr";
+ spi-max-frequency = <20000000>;
+ reg = <0>;
+ pinctrl-0 = <&pinctrl_fpgaspi>;
+ config-gpios = <&gpio4 9 GPIO_ACTIVE_LOW>;
+ status-gpios = <&gpio4 11 GPIO_ACTIVE_LOW>;
+ };
};
&ecspi3 {
@@ -322,6 +331,13 @@
>;
};
+ pinctrl_fpgaspi: fpgaspigrp {
+ fsl,pins = <
+ MX6QDL_PAD_KEY_ROW1__GPIO4_IO09 0x1b0b0
+ MX6QDL_PAD_KEY_ROW2__GPIO4_IO11 0x1b0b0
+ >;
+ };
+
pinctrl_gpminand: gpminandgrp {
fsl,pins = <
MX6QDL_PAD_NANDF_CLE__NAND_CLE 0xb0b1
--
2.9.3
^ permalink raw reply related
* [PATCH] i2c: mux: pca954x: fix i2c mux selection caching
From: Russell King - ARM Linux @ 2016-12-16 23:23 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <251c31e3-141c-8884-d56a-fa539714d1ff@axentia.se>
On Fri, Dec 16, 2016 at 10:20:35PM +0100, Peter Rosin wrote:
> On 2016-12-16 21:06, Russell King wrote:
> > smbus functions return -ve on error, 0 on success. However,
> > __i2c_transfer() have a different return signature - -ve on error, or
> > number of buffers transferred (which may be zero or greater.)
> >
> > The upshot of this is that the sense of the test is reversed when using
> > the mux on a bus supporting the master_xfer method: we cache the value
> > and never retry if we fail to transfer any buffers, but if we succeed,
> > we clear the cached value.
>
> Ouch! Thanks for catching this.
>
> > Fix this.
>
> But lets fix the corner case of __i2c_transfer returning 0 instead of
> the expected 1 as well (not sure if that's even possible, but lets close
> the possibility just in case), so I'd prefer if you could fix
> pca954x_reg_write() to return 0 iff __i2c_transfer(...) returns 1
> instead, and -EREMOTEIO on other non-negative return values. Thanks!
So you want something like this instead?
diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
index 8bc3d36d2837..9c4ac26c014e 100644
--- a/drivers/i2c/muxes/i2c-mux-pca954x.c
+++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
@@ -151,6 +151,9 @@ static int pca954x_reg_write(struct i2c_adapter *adap,
buf[0] = val;
msg.buf = buf;
ret = __i2c_transfer(adap, &msg, 1);
+
+ if (ret >= 0 && ret != 1)
+ ret = -EREMOTEIO;
} else {
union i2c_smbus_data data;
ret = adap->algo->smbus_xfer(adap, client->addr,
@@ -179,7 +182,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
/* Only select the channel if its different from the last channel */
if (data->last_chan != regval) {
ret = pca954x_reg_write(muxc->parent, client, regval);
- data->last_chan = ret ? 0 : regval;
+ data->last_chan = ret < 0 ? 0 : regval;
}
return ret;
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently@9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply related
* [PATCH v6 1/5] [media] davinci: VPIF: fix module loading, init errors
From: Kevin Hilman @ 2016-12-16 23:39 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <19dc6321-1433-e089-f753-e5f736e26073@xs4all.nl>
Hans Verkuil <hverkuil@xs4all.nl> writes:
> On 07/12/16 19:30, Kevin Hilman wrote:
>> Fix problems with automatic module loading by adding MODULE_ALIAS. Also
>> fix various load-time errors cause by incorrect or not present
>> platform_data.
>>
>> Signed-off-by: Kevin Hilman <khilman@baylibre.com>
>> ---
>> drivers/media/platform/davinci/vpif.c | 5 ++++-
>> drivers/media/platform/davinci/vpif_capture.c | 15 ++++++++++++++-
>> drivers/media/platform/davinci/vpif_display.c | 6 ++++++
>> 3 files changed, 24 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
>> index 0380cf2e5775..f50148dcba64 100644
>> --- a/drivers/media/platform/davinci/vpif.c
>> +++ b/drivers/media/platform/davinci/vpif.c
>> @@ -32,6 +32,9 @@
>> MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
>> MODULE_LICENSE("GPL");
>>
>> +#define VPIF_DRIVER_NAME "vpif"
>> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>> +
>> #define VPIF_CH0_MAX_MODES 22
>> #define VPIF_CH1_MAX_MODES 2
>> #define VPIF_CH2_MAX_MODES 15
>> @@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
>>
>> static struct platform_driver vpif_driver = {
>> .driver = {
>> - .name = "vpif",
>> + .name = VPIF_DRIVER_NAME,
>> .pm = vpif_pm_ops,
>> },
>> .remove = vpif_remove,
>> diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
>> index 5104cc0ee40e..20c4344ed118 100644
>> --- a/drivers/media/platform/davinci/vpif_capture.c
>> +++ b/drivers/media/platform/davinci/vpif_capture.c
>> @@ -45,6 +45,7 @@ module_param(debug, int, 0644);
>> MODULE_PARM_DESC(debug, "Debug level 0-1");
>>
>> #define VPIF_DRIVER_NAME "vpif_capture"
>> +MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
>>
>> /* global variables */
>> static struct vpif_device vpif_obj = { {NULL} };
>> @@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
>>
>> vpif_dbg(2, debug, "vpif_input_to_subdev\n");
>>
>> + if (!chan_cfg)
>> + return -1;
>> + if (input_index >= chan_cfg->input_count)
>> + return -1;
>> subdev_name = chan_cfg->inputs[input_index].subdev_name;
>> if (subdev_name == NULL)
>> return -1;
>> @@ -654,7 +659,7 @@ static int vpif_input_to_subdev(
>> /* loop through the sub device list to get the sub device info */
>> for (i = 0; i < vpif_cfg->subdev_count; i++) {
>> subdev_info = &vpif_cfg->subdev_info[i];
>> - if (!strcmp(subdev_info->name, subdev_name))
>> + if (subdev_info && !strcmp(subdev_info->name, subdev_name))
>
> Why this change? subdev_info can never be NULL.
A debugging leftover I guess. Will remove and resend.
Thanks for the review,
Kevin
^ permalink raw reply
* [PATCH v6.1 1/5] [media] davinci: VPIF: fix module loading, init errors
From: Kevin Hilman @ 2016-12-17 0:47 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161207183025.20684-2-khilman@baylibre.com>
Fix problems with automatic module loading by adding MODULE_ALIAS. Also
fix various load-time errors cause by incorrect or not present
platform_data.
Acked-by: Sakari Ailus <sakari.ailus@linux.intel.com>
Signed-off-by: Kevin Hilman <khilman@baylibre.com>
---
Minor tweaks since v6
- added ack from Sakari
- droped an extraneous change for NULL subdev_info
drivers/media/platform/davinci/vpif.c | 5 ++++-
drivers/media/platform/davinci/vpif_capture.c | 13 +++++++++++++
drivers/media/platform/davinci/vpif_display.c | 6 ++++++
3 files changed, 23 insertions(+), 1 deletion(-)
diff --git a/drivers/media/platform/davinci/vpif.c b/drivers/media/platform/davinci/vpif.c
index 0380cf2e5775..f50148dcba64 100644
--- a/drivers/media/platform/davinci/vpif.c
+++ b/drivers/media/platform/davinci/vpif.c
@@ -32,6 +32,9 @@
MODULE_DESCRIPTION("TI DaVinci Video Port Interface driver");
MODULE_LICENSE("GPL");
+#define VPIF_DRIVER_NAME "vpif"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
+
#define VPIF_CH0_MAX_MODES 22
#define VPIF_CH1_MAX_MODES 2
#define VPIF_CH2_MAX_MODES 15
@@ -466,7 +469,7 @@ static const struct dev_pm_ops vpif_pm = {
static struct platform_driver vpif_driver = {
.driver = {
- .name = "vpif",
+ .name = VPIF_DRIVER_NAME,
.pm = vpif_pm_ops,
},
.remove = vpif_remove,
diff --git a/drivers/media/platform/davinci/vpif_capture.c b/drivers/media/platform/davinci/vpif_capture.c
index 5104cc0ee40e..892a26f3c5b4 100644
--- a/drivers/media/platform/davinci/vpif_capture.c
+++ b/drivers/media/platform/davinci/vpif_capture.c
@@ -45,6 +45,7 @@ module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Debug level 0-1");
#define VPIF_DRIVER_NAME "vpif_capture"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
/* global variables */
static struct vpif_device vpif_obj = { {NULL} };
@@ -647,6 +648,10 @@ static int vpif_input_to_subdev(
vpif_dbg(2, debug, "vpif_input_to_subdev\n");
+ if (!chan_cfg)
+ return -1;
+ if (input_index >= chan_cfg->input_count)
+ return -1;
subdev_name = chan_cfg->inputs[input_index].subdev_name;
if (subdev_name == NULL)
return -1;
@@ -685,6 +690,9 @@ static int vpif_set_input(
if (sd_index >= 0) {
sd = vpif_obj.sd[sd_index];
subdev_info = &vpif_cfg->subdev_info[sd_index];
+ } else {
+ /* no subdevice, no input to setup */
+ return 0;
}
/* first setup input path from sub device to vpif */
@@ -1435,6 +1443,11 @@ static __init int vpif_probe(struct platform_device *pdev)
int res_idx = 0;
int i, err;
+ if (!pdev->dev.platform_data) {
+ dev_warn(&pdev->dev, "Missing platform data. Giving up.\n");
+ return -EINVAL;
+ }
+
vpif_dev = &pdev->dev;
err = initialize_vpif();
diff --git a/drivers/media/platform/davinci/vpif_display.c b/drivers/media/platform/davinci/vpif_display.c
index 75b27233ec2f..7f632b757d32 100644
--- a/drivers/media/platform/davinci/vpif_display.c
+++ b/drivers/media/platform/davinci/vpif_display.c
@@ -42,6 +42,7 @@ module_param(debug, int, 0644);
MODULE_PARM_DESC(debug, "Debug level 0-1");
#define VPIF_DRIVER_NAME "vpif_display"
+MODULE_ALIAS("platform:" VPIF_DRIVER_NAME);
/* Is set to 1 in case of SDTV formats, 2 in case of HDTV formats. */
static int ycmux_mode;
@@ -1249,6 +1250,11 @@ static __init int vpif_probe(struct platform_device *pdev)
int res_idx = 0;
int i, err;
+ if (!pdev->dev.platform_data) {
+ dev_warn(&pdev->dev, "Missing platform data. Giving up.\n");
+ return -EINVAL;
+ }
+
vpif_dev = &pdev->dev;
err = initialize_vpif();
--
2.9.3
^ permalink raw reply related
* [PATCH v6 0/5] davinci: VPIF: add DT support
From: Kevin Hilman @ 2016-12-17 0:49 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <d4b0501a-f83a-c8b1-e460-1ba50f68cca7@xs4all.nl>
Hans Verkuil <hverkuil@xs4all.nl> writes:
> On 07/12/16 19:30, Kevin Hilman wrote:
>> Prepare the groundwork for adding DT support for davinci VPIF drivers.
>> This series does some fixups/cleanups and then adds the DT binding and
>> DT compatible string matching for DT probing.
>>
>> The controversial part from previous versions around async subdev
>> parsing, and specifically hard-coding the input/output routing of
>> subdevs, has been left out of this series. That part can be done as a
>> follow-on step after agreement has been reached on the path forward.
>> With this version, platforms can still use the VPIF capture/display
>> drivers, but must provide platform_data for the subdevs and subdev
>> routing.
>>
>> Tested video capture to memory on da850-lcdk board using composite
>> input.
>
> Other than the comment for the first patch this series looks good.
>
> So once that's addressed I'll queue it up for 4.11.
I've fixed that issue, and sent an update for just that patch in reply
to the original.
Thanks for the review,
Kevin
^ permalink raw reply
* [PATCH] mtk-vcodec: use designated initializers
From: Kees Cook @ 2016-12-17 1:00 UTC (permalink / raw)
To: linux-arm-kernel
Prepare to mark sensitive kernel structures for randomization by making
sure they're using designated initializers. These were identified during
allyesconfig builds of x86, arm, and arm64, with most initializer fixes
extracted from grsecurity.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c | 8 ++++----
drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
index b76c80bdf30b..4eb3be37ba14 100644
--- a/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc/venc_h264_if.c
@@ -665,10 +665,10 @@ static int h264_enc_deinit(unsigned long handle)
}
static const struct venc_common_if venc_h264_if = {
- h264_enc_init,
- h264_enc_encode,
- h264_enc_set_param,
- h264_enc_deinit,
+ .init = h264_enc_init,
+ .encode = h264_enc_encode,
+ .set_param = h264_enc_set_param,
+ .deinit = h264_enc_deinit,
};
const struct venc_common_if *get_h264_enc_comm_if(void);
diff --git a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
index 544f57186243..a6fa145f2c54 100644
--- a/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
+++ b/drivers/media/platform/mtk-vcodec/venc/venc_vp8_if.c
@@ -470,10 +470,10 @@ static int vp8_enc_deinit(unsigned long handle)
}
static const struct venc_common_if venc_vp8_if = {
- vp8_enc_init,
- vp8_enc_encode,
- vp8_enc_set_param,
- vp8_enc_deinit,
+ .init = vp8_enc_init,
+ .encode = vp8_enc_encode,
+ .set_param = vp8_enc_set_param,
+ .deinit = vp8_enc_deinit,
};
const struct venc_common_if *get_vp8_enc_comm_if(void);
--
2.7.4
--
Kees Cook
Nexus Security
^ permalink raw reply related
* [PATCH 3/3] arm64: dts: rockchip: add clk-480m for ehci and ohci of rk3399
From: Heiko Stuebner @ 2016-12-17 1:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5853903D.8030605@rock-chips.com>
Am Freitag, 16. Dezember 2016, 14:57:01 CET schrieb Xing Zheng:
> Hi Heiko, Doug,
>
> On 2016?12?16? 02:18, Heiko Stuebner wrote:
> > Am Donnerstag, 15. Dezember 2016, 08:34:09 CET schrieb Doug Anderson:
> >> I still need to digest all of the things that were added to this
> >> thread overnight, but nothing I've seen so far indicates that you need
> >> the post-gated clock. AKA I still think you need to redo your patch
> >>
> >> to replace:
> >> clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>,
> >>
> >> <&cru SCLK_USBPHY0_480M_SRC>;
> >>
> >> with:
> >> clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>,
> >>
> >> <&u2phy0>;
> >>
> >> Can you please comment on that?
> >
> > Also, with the change, the ehci will keep the clock (and thus the phy)
> > always on. Does the phy-autosuspend even save anything now?
> >
> > In any case, could we make the clock-names entry sound nicer than
> > usbphy0_480m please? bindings/usb/atmel-usb.txt calls its UTMI clock
> > simply "usb_clk", but something like "utmi" should also work.
> > While at it you could also fix up the other clock names to something like
> > "host" and "arbiter" or so?.
> >
> >
> > Heiko
>
> The usbphy related clock tress like this:
>
>
> Actually, at drivers/phy/phy-rockchip-inno-usb2.c, we can only
> enable/disable the master gate via GRF is PHY_PLL, not UTMI_CLK.
>
> And the naming style of the "hclk_host0" keep the name "hclk_host0" on
> the clcok tree diagram:
>
>
> Therefore, could we rename the clock name like this:
> ----
> for usb_host0_ehci and usb_host0_ohci:
> clocks = <&cru HCLK_HOST0>, <&cru HCLK_HOST0_ARB>,
> <&cru SCLK_U2PHY0>;
> clock-names = "hclk_host0", "hclk_host0_arb",
> "sclk_u2phy0";
>
> for usb_host1_ehci and usb_host1_ohci:
> clocks = <&cru HCLK_HOST1>, <&cru HCLK_HOST1_ARB>,
> <&cru SCLK_U2PHY1>;
> clock-names = "hclk_host1", "hclk_host1_arb",
> "sclk_u2phy1";
> ----
>
> BTW, the "arb" is an abbreviation for arbiter.
clock-naming wise, the clock names in devicetree bindings should always
describe the clock in the context of the peripheral, not the hosts clock-tree.
So if the clock supplies the "arbiter" part, the clock-name should be called
"arbiter". Same for "utmi" and the host clock that could be named "usbhost" or
just "host" in the clock-names property.
^ permalink raw reply
* [PATCHv4 00/15] clk: ti: add support for hwmod clocks
From: Stephen Boyd @ 2016-12-17 1:46 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <4b016883-e7c9-94d8-d964-3c8dfee6ee13@ti.com>
On 12/13, Tero Kristo wrote:
> On 13/12/16 06:40, Michael Turquette wrote:
> >>>On 12/12, Michael Turquette wrote:
> >
> >Is the goal to describe this hardware topology in DT? Is that right
> >thing to do? I think it's cool to have this modeled *somehow* in Linux,
> >but I'm not sure DT is the right place to model the interconnect and
> >every device hanging off of it.
> >
> >I don't want to put words in Stephen's mouth, but I think the issue over
> >whether clockdomains are CCF clock providers or some genpd thing is
> >probably less important to him than the fact that the DT bindings are
> >super detailed to inner workings of the SoC.
>
> Ok, so your preference would be to reduce the data under DT, and the
> ideal approach would be a single prcm node. I think we still need to
> keep the prm / cm1 / cm2 as separate nodes, as they are pretty
> individual from hardware point of view, provide quite different
> features, and they reside in some cases in quite different address
> spaces also. Anyway, here's what I gather we should probably have in
> DT:
>
> - reset provider
> * example: resets = <&prm OMAP4_IVA2_RESET>;
> * only from 'prm' node
>
> - genpd provider (for the hwmods, clockdomains, powerdomains,
> voltage domains)
> * examples: power-domains = <&cm2 OMAP4_DSS_CORE_MOD>;
> power-domains = <&cm2 OMAP4_DSS_CLKDM>;
> power-domains = <&prm OMAP4_DSS_PWRDM>;
> power-domains = <&prm OMAP4_CORE_VOLTDM>;
> * from all 'prm', 'cm1' and 'cm2' nodes, though 'prm' would be the
> only one providing _CLKDM, _PWRDM, _VOLTDM genpds.
>
> - clock provider (for anything that requires clocks)
> * example: clocks = <&cm1 OMAP4_DPLL_MPU_CK>;
> * from all 'prm', 'cm1' and 'cm2' nodes
>
> This would eventually cause an ABI breakage for the clock handles,
> if we transfer the existing clocks to this format, and remove the
> existing clock handles from DT. Otherwise, I think we could just
> transition the existing hwmod data to this new format only, and add
> the clockdomain / powerdomain / voltagedomain support a bit later.
>
This sounds about right. Is the ABI break because we get rid of
clock nodes and just have a few big nodes? I thought we had
already broken DT ABI here but if we didn't then that isn't
great. Perhaps to make things keep working we can detect the old
style one node per clock design and register a bunch of providers
individually from the single driver probe? It would have to match
up the registers with a clk_hw pointer somewhere, but it should
be possible. Alternatively, we keep both designs around for some
time and have different compatibles and different driver entry
points.
--
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH 2/2] remoteproc: Remove firmware_loading_complete
From: Sarangdhar Joshi @ 2016-12-17 2:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216192839.GT3439@tuxbot>
On 12/16/2016 11:28 AM, Bjorn Andersson wrote:
> On Fri 16 Dec 00:26 PST 2016, loic pallardy wrote:
>
>>
>>
>> On 12/16/2016 01:03 AM, Sarangdhar Joshi wrote:
>>> rproc_del() waits on firmware_loading_complete in order to
>>> make sure rproc_add() completed successfully before calling
>>> rproc_shutdown(). However since rproc_add() will always be
>>> called before rproc_del(), we do not need to wait on
>>> firmware_loading_complete. Drop this completion variable
>>> altogether.
>>>
>> Hi,
>>
>> firmware_loading_complete is used to synchronize all operations on rproc
>> with parallel work launched by request_firmware_nowait.
>
> We had a deadlock scenario in this code, where a call to rproc_boot()
> would grab the rproc mutex and the request_firmware_nowait() callback
> would wait on this lock before it would signal the completion that the
> rproc_boot() was waiting for.
>
> As the request_firmware_nowait() doesn't do anything other than handle
> auto_boot and signal the completion - and there is an internal sleep
> mechanism for handling concurrent request_firmware calls - I posted a
> patch and dropped the rproc_boot() wait thing.
That's right. Should have added reference to commit
"e9b4f9efff5021 ("remoteproc: Drop wait in __rproc_boot()")"
>
>> rproc_add could be done and firmware loading still pending. In that case
>> rproc_del mustn't be called before end of the procedure.
>
> You're right.
>
> We might have an outstanding request_firmware_nowait() when we hit
> rproc_del() and we might free the underlaying rproc context.
>
> Holding a reference over the request_firmware_nowait() would solve this,
> but would cause issues if we get a rproc_add() from the same driver
> (e.g. after module unload/load) before the firmware timer has fired -
> and released the resources.
The asynchronous work request_firmware_work_func() is protected by
get_device()/put_device() on remoteproc device. So we are probably
covered for remoteproc device. However, I agree that parent device will
still be an issue.
>
> This issue could be remedied by moving the rproc_delete_debug_dir() to
> rproc_del() and aim for not having any objects exposed outside the
> remoteproc core once rproc_del() returns.
>
>>
>> If you decide to remove this synchronization you need either to modify rproc
>> boot sequence or to replace it by something else.
>>
>
> I agree.
I agree too. rproc_boot() calls for non auto_boot case anyway calls
request_firmware(). So calling __request_firmware asynchronously for non
auto_boot case seems redundant. I was planning to send a patch to call
rproc_add_virtio_devices() for auto_boot case only. I guess I'll need to
take care of only auto_boot case for the current issue then.
Regards,
Sarang
>
> Regards,
> Bjorn
>
--
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply
* [PATCH] Documentation: dt: Explicitly mark Samsung Exynos SoC bindings as unstable
From: Pankaj Dubey @ 2016-12-17 3:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481897676-13578-1-git-send-email-m.szyprowski@samsung.com>
Hi Marek,
On 16 December 2016 at 19:44, Marek Szyprowski <m.szyprowski@samsung.com> wrote:
> Samsung Exynos SoCs and boards related bindings evolved since the initial
> introduction, but initially the bindings were minimal and a bit incomplete
> (they never described all the hardware modules available in the SoCs).
> Since then some significant (not fully compatible) changes have been
> already committed a few times (like gpio replaced by pinctrl, display ddc,
> mfc reserved memory, some core clocks added to various hardware modules,
> added more required nodes).
>
> On the other side there are no boards which have device tree embedded in
> the bootloader. Device tree blob is always compiled from the kernel tree
> and updated together with the kernel image.
>
> Thus to avoid further adding a bunch of workarounds for old/missing
> bindings and allow to make cleanup of the existing code and device tree
> files, lets mark Samsung Exynos SoC platform bindings as unstable. This
> means that bindings can may change at any time and users should use the
> dtb file compiled from the same kernel source tree as the kernel image.
>
> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> ---
I agree with you. This is very much required. It's not only about new
bindings, we are facing problems in adopting existing bindings as well
(e.g scu), to make exynos support completely DT based and simplify our
code base.
I expect and foresee requirements of many more such changes in very near future.
Reviewed-by: Pankaj Dubey <pankaj.dubey@samsung.com>
Thanks,
Pankaj Dubey
^ permalink raw reply
* [PATCH v8 4/8] ARM: EXYNOS: refactor firmware specific routines
From: Pankaj Dubey @ 2016-12-17 3:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216182531.GA6440@kozik-lap>
Hi Krzysztof,
On 16 December 2016 at 23:55, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Sat, Dec 10, 2016 at 06:38:39PM +0530, Pankaj Dubey wrote:
>> To remove dependency on soc_is_exynosMMMM macros and remove multiple
>> checks for such macros lets refactor code in firmware.c file.
>> SoC specific firmware_ops are separated and registered during
>> exynos_firmware_init based on matching machine compatible.
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-exynos/firmware.c | 100 ++++++++++++++++++++++++++++++----------
>> 1 file changed, 75 insertions(+), 25 deletions(-)
>>
>> diff --git a/arch/arm/mach-exynos/firmware.c b/arch/arm/mach-exynos/firmware.c
>> index fd6da54..525fbd9 100644
>> --- a/arch/arm/mach-exynos/firmware.c
>> +++ b/arch/arm/mach-exynos/firmware.c
>> @@ -35,6 +35,25 @@ static void exynos_save_cp15(void)
>> : : "cc");
>> }
>>
>> +static int exynos3250_do_idle(unsigned long mode)
>> +{
>> + switch (mode) {
>> + case FW_DO_IDLE_AFTR:
>> + writel_relaxed(virt_to_phys(exynos_cpu_resume_ns),
>> + sysram_ns_base_addr + 0x24);
>> + writel_relaxed(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
>> + flush_cache_all();
>> + exynos_smc(SMC_CMD_SAVE, OP_TYPE_CORE,
>> + SMC_POWERSTATE_IDLE, 0);
>> + exynos_smc(SMC_CMD_SHUTDOWN, OP_TYPE_CLUSTER,
>> + SMC_POWERSTATE_IDLE, 0);
>> + break;
>> + case FW_DO_IDLE_SLEEP:
>> + exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
>> + }
>> + return 0;
>> +}
>> +
>> static int exynos_do_idle(unsigned long mode)
>> {
>> switch (mode) {
>> @@ -44,14 +63,7 @@ static int exynos_do_idle(unsigned long mode)
>> writel_relaxed(virt_to_phys(exynos_cpu_resume_ns),
>> sysram_ns_base_addr + 0x24);
>> writel_relaxed(EXYNOS_AFTR_MAGIC, sysram_ns_base_addr + 0x20);
>> - if (soc_is_exynos3250()) {
>> - flush_cache_all();
>> - exynos_smc(SMC_CMD_SAVE, OP_TYPE_CORE,
>> - SMC_POWERSTATE_IDLE, 0);
>> - exynos_smc(SMC_CMD_SHUTDOWN, OP_TYPE_CLUSTER,
>> - SMC_POWERSTATE_IDLE, 0);
>> - } else
>> - exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
>> + exynos_smc(SMC_CMD_CPU0AFTR, 0, 0, 0);
>> break;
>> case FW_DO_IDLE_SLEEP:
>> exynos_smc(SMC_CMD_SLEEP, 0, 0, 0);
>> @@ -59,28 +71,25 @@ static int exynos_do_idle(unsigned long mode)
>> return 0;
>> }
>>
>> -static int exynos_cpu_boot(int cpu)
>> +static int exynos4412_cpu_boot(int cpu)
>> {
>> /*
>> - * Exynos3250 doesn't need to send smc command for secondary CPU boot
>> - * because Exynos3250 removes WFE in secure mode.
>> - */
>> - if (soc_is_exynos3250())
>> - return 0;
>> -
>> - /*
>> * The second parameter of SMC_CMD_CPU1BOOT command means CPU id.
>> * But, Exynos4212 has only one secondary CPU so second parameter
>> * isn't used for informing secure firmware about CPU id.
>> */
>> - if (soc_is_exynos4212())
>> - cpu = 0;
>> + cpu = 0;
>
> Why are you clearing the cpu for Exynos4412? Was it tested on
> Exynos4412?
>
No I have not tested on Exynos4412.
I can see I missed this, and we are suppose clear the cpu only for Exynos4212.
I will fix this in v9 and resubmit again. Thanks for noticing this and
pointing out.
>> + exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
>> + return 0;
>> +}
>>
>> +static int exynos_cpu_boot(int cpu)
>> +{
>> exynos_smc(SMC_CMD_CPU1BOOT, cpu, 0, 0);
>
> This will be executed on Exynos4212...
>
Yes, which is wrong. This should be for Exynos4412 and previous one
(exynos4412_cpu_boot) is applicable for Exynos4212. I will fix this in v9.
>> return 0;
>> }
>>
>> -static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> +static int exynos4412_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> {
>> void __iomem *boot_reg;
>>
>> @@ -94,14 +103,24 @@ static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> * additional offset for every CPU, with Exynos4412 being the only
>> * exception.
>> */
>> - if (soc_is_exynos4412())
>> - boot_reg += 4 * cpu;
>> + boot_reg += 4 * cpu;
>> + writel_relaxed(boot_addr, boot_reg);
>> + return 0;
>> +}
>> +
>> +static int exynos_set_cpu_boot_addr(int cpu, unsigned long boot_addr)
>> +{
>> + void __iomem *boot_reg;
>>
>> + if (!sysram_ns_base_addr)
>> + return -ENODEV;
>> +
>> + boot_reg = sysram_ns_base_addr + 0x1c;
>> writel_relaxed(boot_addr, boot_reg);
>> return 0;
>> }
>>
>> -static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
>> +static int exynos4412_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
>> {
>> void __iomem *boot_reg;
>>
>> @@ -109,10 +128,19 @@ static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
>> return -ENODEV;
>>
>> boot_reg = sysram_ns_base_addr + 0x1c;
>> + boot_reg += 4 * cpu;
>> + *boot_addr = readl_relaxed(boot_reg);
>> + return 0;
>> +}
>> +
>> +static int exynos_get_cpu_boot_addr(int cpu, unsigned long *boot_addr)
>> +{
>> + void __iomem *boot_reg;
>>
>> - if (soc_is_exynos4412())
>> - boot_reg += 4 * cpu;
>> + if (!sysram_ns_base_addr)
>> + return -ENODEV;
>>
>> + boot_reg = sysram_ns_base_addr + 0x1c;
>> *boot_addr = readl_relaxed(boot_reg);
>> return 0;
>> }
>> @@ -148,6 +176,23 @@ static int exynos_resume(void)
>> return 0;
>> }
>>
>> +static const struct firmware_ops exynos3250_firmware_ops = {
>> + .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos3250_do_idle : NULL,
>> + .set_cpu_boot_addr = exynos_set_cpu_boot_addr,
>> + .get_cpu_boot_addr = exynos_get_cpu_boot_addr,
>
> You know that lack of cpu_boot() is not equivalent to previous
> 'return 0' code? Now -ENOSYS will be returned... which is not a problem
> because return values for cpu_boot are ignored... just wondering whether
> this was planned.
Yes, I feel it should return -ENOSYS, if the particular ops is not
relevant or applicable
for some SoC, rather having blank implementation and returning 0 is
should return error
code.
>
>> + .suspend = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
>> + .resume = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
>> +};
>> +
>> +static const struct firmware_ops exynos4412_firmware_ops = {
>> + .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
>> + .set_cpu_boot_addr = exynos4412_set_cpu_boot_addr,
>> + .get_cpu_boot_addr = exynos4412_get_cpu_boot_addr,
>> + .cpu_boot = exynos4412_cpu_boot,
>> + .suspend = IS_ENABLED(CONFIG_PM_SLEEP) ? exynos_suspend : NULL,
>> + .resume = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_resume : NULL,
>> +};
>> +
>> static const struct firmware_ops exynos_firmware_ops = {
>> .do_idle = IS_ENABLED(CONFIG_EXYNOS_CPU_SUSPEND) ? exynos_do_idle : NULL,
>> .set_cpu_boot_addr = exynos_set_cpu_boot_addr,
>> @@ -212,7 +257,12 @@ void __init exynos_firmware_init(void)
>>
>> pr_info("Running under secure firmware.\n");
>>
>> - register_firmware_ops(&exynos_firmware_ops);
>> + if (of_machine_is_compatible("samsung,exynos3250"))
>> + register_firmware_ops(&exynos3250_firmware_ops);
>> + else if (of_machine_is_compatible("samsung,exynos4412"))
>> + register_firmware_ops(&exynos4412_firmware_ops);
>> + else
>> + register_firmware_ops(&exynos_firmware_ops);
>
> I prefer one register_firmware_ops() call, so something like:
> const struct firmware_ops *ops;
> if (...)
> ops = &exynos3250_firmware_ops;
> else if ()
> ...
> register_firmware_ops(ops);
>
> It is a matter of taste but for me it is more common pattern, looks more
> readable and it reduces number of callers to register_firmware_ops() (so
> it is easier to find them).
>
This suggestion looks good to me as well. Will adopt this in v9.
Thanks for your review.
Pankaj Dubey
^ permalink raw reply
* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
From: Pankaj Dubey @ 2016-12-17 4:06 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216173703.GA3746@kozik-lap>
Hi Krzysztof,
On 16 December 2016 at 23:07, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Sat, Dec 10, 2016 at 06:38:36PM +0530, Pankaj Dubey wrote:
>> Exynos SoCs have Chipid, for identification of product IDs and SoC revisions.
>> This patch intends to provide initialization code for all these functionalities,
>> at the same time it provides some sysfs entries for accessing these information
>> to user-space.
>>
>> This driver uses existing binding for exynos-chipid.
>>
>> CC: Grant Likely <grant.likely@linaro.org>
>> CC: Rob Herring <robh+dt@kernel.org>
>> CC: Linus Walleij <linus.walleij@linaro.org>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> [m.szyprowski: for suggestion and code snippet of product_id_to_soc_id]
>> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
>> ---
>> drivers/soc/samsung/Kconfig | 5 ++
>> drivers/soc/samsung/Makefile | 1 +
>> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++++++++++++++++++++++++++
>> 3 files changed, 122 insertions(+)
>> create mode 100644 drivers/soc/samsung/exynos-chipid.c
>>
>> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
>> index 2455339..f9ab858 100644
>> --- a/drivers/soc/samsung/Kconfig
>> +++ b/drivers/soc/samsung/Kconfig
>> @@ -14,4 +14,9 @@ config EXYNOS_PM_DOMAINS
>> bool "Exynos PM domains" if COMPILE_TEST
>> depends on PM_GENERIC_DOMAINS || COMPILE_TEST
>>
>> +config EXYNOS_CHIPID
>> + bool "Exynos Chipid controller driver" if COMPILE_TEST
>> + depends on (ARM && ARCH_EXYNOS) || ((ARM || ARM64) && COMPILE_TEST)
>
> 1. Why this can be compile tested only on ARM architectures?
Well I just used dependency same as EXYNOS_PMU, but I can see it will
be enabled for compile test on ARM64 isn't it?
> 2. Don't you need also SOC_BUS?
CHIPID needs SoC_BUS and for the same reason it is selecting SOC_BUS
in the next line.
If we mark it as a dependency (under depends on), even then we need to
select this either
under same EXYNOS_CHIPID config or ARCH_EXYNOS config.
>
>> + select SOC_BUS
>> +
>> endif
>> diff --git a/drivers/soc/samsung/Makefile b/drivers/soc/samsung/Makefile
>> index 3619f2e..2a8a85e 100644
>> --- a/drivers/soc/samsung/Makefile
>> +++ b/drivers/soc/samsung/Makefile
>> @@ -1,3 +1,4 @@
>> obj-$(CONFIG_EXYNOS_PMU) += exynos-pmu.o exynos3250-pmu.o exynos4-pmu.o \
>> exynos5250-pmu.o exynos5420-pmu.o
>> obj-$(CONFIG_EXYNOS_PM_DOMAINS) += pm_domains.o
>> +obj-$(CONFIG_EXYNOS_CHIPID) += exynos-chipid.o
>
> Please put it before EXYNOS_PMU, keeping this sorted alphabetical helps
> avoiding conflicts of continuous edits.
>
>> diff --git a/drivers/soc/samsung/exynos-chipid.c b/drivers/soc/samsung/exynos-chipid.c
>> new file mode 100644
>> index 0000000..cf0128b
>> --- /dev/null
>> +++ b/drivers/soc/samsung/exynos-chipid.c
>> @@ -0,0 +1,116 @@
>> +/*
>> + * Copyright (c) 2016 Samsung Electronics Co., Ltd.
>> + * http://www.samsung.com/
>> + *
>> + * EXYNOS - CHIP ID support
>> + * Author: Pankaj Dubey <pankaj.dubey@samsung.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <linux/io.h>
>> +#include <linux/of.h>
>> +#include <linux/of_address.h>
>> +#include <linux/of_platform.h>
>> +#include <linux/platform_device.h>
>> +#include <linux/slab.h>
>> +#include <linux/sys_soc.h>
>> +
>> +#define EXYNOS_SUBREV_MASK (0xF << 4)
>> +#define EXYNOS_MAINREV_MASK (0xF << 0)
>> +#define EXYNOS_REV_MASK (EXYNOS_SUBREV_MASK | EXYNOS_MAINREV_MASK)
>> +
>> +static const struct exynos_soc_id {
>> + const char *name;
>> + unsigned int id;
>> + unsigned int mask;
>> +} soc_ids[] = {
>> + { "EXYNOS3250", 0xE3472000, 0xFFFFF000 },
>> + { "EXYNOS4210", 0x43200000, 0xFFFE0000 },
>> + { "EXYNOS4212", 0x43220000, 0xFFFE0000 },
>> + { "EXYNOS4412", 0xE4412000, 0xFFFE0000 },
>> + { "EXYNOS5250", 0x43520000, 0xFFFFF000 },
>> + { "EXYNOS5260", 0xE5260000, 0xFFFFF000 },
>> + { "EXYNOS5410", 0xE5410000, 0xFFFFF000 },
>> + { "EXYNOS5420", 0xE5420000, 0xFFFFF000 },
>> + { "EXYNOS5440", 0xE5440000, 0xFFFFF000 },
>> + { "EXYNOS5800", 0xE5422000, 0xFFFFF000 },
>> + { "EXYNOS7420", 0xE7420000, 0xFFFFF000 },
>> + { "EXYNOS5433", 0xE5433000, 0xFFFFF000 },
>> +};
>> +
>> +static const char * __init product_id_to_soc_id(unsigned int product_id)
>> +{
>> + int i;
>> +
>> + for (i = 0; i < ARRAY_SIZE(soc_ids); i++)
>> + if ((product_id & soc_ids[i].mask) == soc_ids[i].id)
>> + return soc_ids[i].name;
>> + return "UNKNOWN";
>> +}
>> +
>> +static const struct of_device_id of_exynos_chipid_ids[] = {
>> + {
>> + .compatible = "samsung,exynos4210-chipid",
>> + },
>> + {},
>> +};
>> +
>> +/**
>> + * exynos_chipid_early_init: Early chipid initialization
>> + */
>
> This comment is meaningless, it duplicates the name of function.
Ok, will remove this in v9.
>
>> +int __init exynos_chipid_early_init(void)
>> +{
>> + struct soc_device_attribute *soc_dev_attr;
>> + struct soc_device *soc_dev;
>> + struct device_node *root;
>> + struct device_node *np;
>> + void __iomem *exynos_chipid_base;
>> + const struct of_device_id *match;
>> + u32 product_id;
>> + u32 revision;
>> +
>> + np = of_find_matching_node_and_match(NULL,
>> + of_exynos_chipid_ids, &match);
>
> You don't use the match here, so how about either
> of_find_matching_node() or of_find_compatible_node()? The latter looks
> better (less calls inside) and actually you want just check one
> compatible field?
>
Ok, will adopt this in v9.
>> + if (!np)
>> + return -ENODEV;
>> +
>> + exynos_chipid_base = of_iomap(np, 0);
>> +
>> + if (!exynos_chipid_base)
>> + return PTR_ERR(exynos_chipid_base);
>> +
>> + product_id = readl_relaxed(exynos_chipid_base);
>
> Duplicated space before '='.
Ok, will fix this.
>
>> + revision = product_id & EXYNOS_REV_MASK;
>> + iounmap(exynos_chipid_base);
>> +
>> + soc_dev_attr = kzalloc(sizeof(*soc_dev_attr), GFP_KERNEL);
>> + if (!soc_dev_attr)
>> + return -ENODEV;
>> +
>> + soc_dev_attr->family = "Samsung Exynos";
>> +
>> + root = of_find_node_by_path("/");
>> + of_property_read_string(root, "model", &soc_dev_attr->machine);
>> + of_node_put(root);
>> +
>> + soc_dev_attr->revision = kasprintf(GFP_KERNEL, "%x", revision);
>> + soc_dev_attr->soc_id = product_id_to_soc_id(product_id);
>> +
>> +
>> + pr_info("Exynos: CPU[%s] CPU_REV[0x%x] Detected\n",
>> + product_id_to_soc_id(product_id), revision);
>> +
>> + soc_dev = soc_device_register(soc_dev_attr);
>> + if (IS_ERR(soc_dev)) {
>> + kfree(soc_dev_attr->revision);
>> + kfree_const(soc_dev_attr->soc_id);
>
> It wasn't allocated with *_const, so no need to free it.
Yes, will fix this.
>
> Best regards,
> Krzysztof
Thanks for review.
Pankaj Dubey
^ permalink raw reply
* [PATCH v8 2/8] ARM: EXYNOS: enable exynos_chipid for ARCH_EXYNOS
From: Pankaj Dubey @ 2016-12-17 4:15 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216180401.GA6459@kozik-lap>
Hi Krzysztof,
On 16 December 2016 at 23:34, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> On Sat, Dec 10, 2016 at 06:38:37PM +0530, Pankaj Dubey wrote:
>> As now we have chipid driver to initialize SoC related information
>> lets include it in build by default.
> s/lets/let's/
>
> Reviewed-by: Krzysztof Kozlowski <krzk@kernel.org>
>
Thanks for review. Will fix minor nitpick in v9 and include your
Reviewed-by tag.
Pankaj Dubey
> Best regards,
> Krzysztof
>
>>
>> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
>> ---
>> arch/arm/mach-exynos/Kconfig | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/arch/arm/mach-exynos/Kconfig b/arch/arm/mach-exynos/Kconfig
>> index 0bb63b8..29065c5 100644
>> --- a/arch/arm/mach-exynos/Kconfig
>> +++ b/arch/arm/mach-exynos/Kconfig
>> @@ -16,6 +16,7 @@ menuconfig ARCH_EXYNOS
>> select ARM_AMBA
>> select ARM_GIC
>> select COMMON_CLK_SAMSUNG
>> + select EXYNOS_CHIPID
>> select EXYNOS_THERMAL
>> select EXYNOS_PMU
>> select EXYNOS_SROM
>> --
>> 2.7.4
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-samsung-soc" in
> the body of a message to majordomo at vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* [PATCH] i2c: mux: pca954x: fix i2c mux selection caching
From: Peter Rosin @ 2016-12-17 6:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161216232355.GO14217@n2100.armlinux.org.uk>
On 2016-12-17 00:23, Russell King - ARM Linux wrote:
> On Fri, Dec 16, 2016 at 10:20:35PM +0100, Peter Rosin wrote:
>> On 2016-12-16 21:06, Russell King wrote:
>>> smbus functions return -ve on error, 0 on success. However,
>>> __i2c_transfer() have a different return signature - -ve on error, or
>>> number of buffers transferred (which may be zero or greater.)
>>>
>>> The upshot of this is that the sense of the test is reversed when using
>>> the mux on a bus supporting the master_xfer method: we cache the value
>>> and never retry if we fail to transfer any buffers, but if we succeed,
>>> we clear the cached value.
>>
>> Ouch! Thanks for catching this.
>>
>>> Fix this.
>>
>> But lets fix the corner case of __i2c_transfer returning 0 instead of
>> the expected 1 as well (not sure if that's even possible, but lets close
>> the possibility just in case), so I'd prefer if you could fix
>> pca954x_reg_write() to return 0 iff __i2c_transfer(...) returns 1
>> instead, and -EREMOTEIO on other non-negative return values. Thanks!
>
> So you want something like this instead?
Yes, but I was originally thinking that the second hunk was no
longer needed...
Either way,
Acked-by: Peter Rosin <peda@axentia.se>
Cheers,
peda
> diff --git a/drivers/i2c/muxes/i2c-mux-pca954x.c b/drivers/i2c/muxes/i2c-mux-pca954x.c
> index 8bc3d36d2837..9c4ac26c014e 100644
> --- a/drivers/i2c/muxes/i2c-mux-pca954x.c
> +++ b/drivers/i2c/muxes/i2c-mux-pca954x.c
> @@ -151,6 +151,9 @@ static int pca954x_reg_write(struct i2c_adapter *adap,
> buf[0] = val;
> msg.buf = buf;
> ret = __i2c_transfer(adap, &msg, 1);
> +
> + if (ret >= 0 && ret != 1)
> + ret = -EREMOTEIO;
> } else {
> union i2c_smbus_data data;
> ret = adap->algo->smbus_xfer(adap, client->addr,
> @@ -179,7 +182,7 @@ static int pca954x_select_chan(struct i2c_mux_core *muxc, u32 chan)
> /* Only select the channel if its different from the last channel */
> if (data->last_chan != regval) {
> ret = pca954x_reg_write(muxc->parent, client, regval);
> - data->last_chan = ret ? 0 : regval;
> + data->last_chan = ret < 0 ? 0 : regval;
> }
>
> return ret;
>
>
^ permalink raw reply
* [PATCH] ARM: dts: Add missing CPU frequencies for Exynos5422/5800
From: Anand Moon @ 2016-12-17 7:31 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1b6e8d3a-ec7a-db5d-dd0e-ef9d1480f80a@fivetechno.de>
Hi Markus,
On 16 December 2016 at 14:38, Markus Reichl <m.reichl@fivetechno.de> wrote:
> Am 16.12.2016 um 08:37 schrieb Krzysztof Kozlowski:
>> On Thu, Dec 15, 2016 at 04:52:58PM -0800, Doug Anderson wrote:
>>>> [ I added Arjun to Cc:, maybe he can help in explaining this issue
>>>> (unfortunately Inderpal's email is no longer working). ]
>>>>
>>>> Please also note that on Exynos5422/5800 SoCs the same ARM rail
>>>> voltage is used for 1.9 GHz & 2.0 GHz OPPs as for the 1.8 GHz one.
>>>> IOW if the problem exists it is already present in the mainline
>>>> kernel.
>>>
>>> Interesting. In the ChromeOS tree I see significantly higher voltages
>>> needed... Note that one might naively look at
>>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/drivers/cpufreq/exynos5420-cpufreq.c#178>.
>>>
>>> 1362500, /* L0 2100 */
>>> 1312500, /* L1 2000 */
>>>
>>> ..but, amazingly enough those voltages aren't used at all. Surprise!
>>>
>>> I believe that the above numbers are actually not used and the ASV
>>> numbers are used instead. See
>>> <https://chromium.googlesource.com/chromiumos/third_party/kernel/+/chromeos-3.8/arch/arm/mach-exynos/include/mach/asv-exynos542x.h#452>
>>>
>>> { 2100000,
>>> 1350000, 1350000, 1350000, 1350000, 1350000,
>>> 1337500, 1325000, 1312500, 1300000, 1287500,
>>> 1275000, 1262500, 1250000, 1237500 },
>>>
>>> I believe that interpretation there is: some bins of the CPU can run
>>> at 2.1 GHz just fine at 1.25 V but others need up to 1.35V.
>>
>> That is definitely the case. One could just look at vendors ASV table
>> (for 1.9 GHz):
>> { 1900000, 1300000, 1287500, 1262500, 1237500, 1225000, 1212500,
>> 1200000, 1187500, 1175000, 1162500, 1150000,
>> 1137500, 1125000, 1112500, 1112500},
>>
>> The theoretical difference is up to 1.875V! From my experiments I saw
>> BIN1 chips which should be the same... but some working on 1.2V, some on
>> 1.225V (@1.9 GHz). I didn't see any requiring higher voltages but that
>> does not mean that there aren't such...
>>
>>> ...so if you're running at 2.1 GHz at 1.25V then perhaps you're just
>>> running on a CPU from a nice bin?
>
> I've been running the proposed frequency/voltage combinations without any
> stability problems on my XU4, XU3 and even XU3-lite ( I did not delete the
> nodes on XU3-lite dts) with make -j8 kernel and ssvb-cpuburn.
> The chips are poorly cooled, especially the XU4 and quickly step down.
[snip]
As per my knowlegde Odroid XU3/4 can throttle at much high temperature.
https://github.com/hardkernel/linux/blob/odroidxu3-3.10.y/drivers/thermal/exynos_thermal.c#L1629
The device tree binding for thermal-zone is kept bit low alert
temperature values
in-order to avoid reaches critical temperature and board shutdown
when compiling the source code. We need t fix this thermal-zone
Their could be some race in thermal or the step wise governor for
exynos is not working correctly.
Better option is to print the cpufreq for cpu0 and cpu4 and respective temp
and plot a graph along timeline. It will give us clear idea on how much
time is spend on high frequency on stress testing.
#!/bin/bash
t=0
while true :
do
a=`cat /sys/devices/virtual/thermal/thermal_zone0/temp`
b=`cat /sys/devices/system/cpu/cpu0/cpufreq/scaling_cur_freq`
c=`cat /sys/devices/system/cpu/cpu0/cpufreq/cpuinfo_cur_freq`
d=`cat /sys/devices/system/cpu/cpu4/cpufreq/scaling_cur_freq`
e=`cat /sys/devices/system/cpu/cpu4/cpufreq/cpuinfo_cur_freq`
(( t += 5 ))
echo $t,$a,$b,$d,$e
sleep 1
done
Best Regards
-Anand Moon
^ permalink raw reply
* [PATCH] drivers: remoteproc: constify rproc_ops structures
From: Bhumika Goyal @ 2016-12-17 11:29 UTC (permalink / raw)
To: linux-arm-kernel
Declare rproc_ops structures as const as they are only passed as an
argument to the function rproc_alloc. This argument is of type const, so
rproc_ops structures having this property can be declared const too.
Done using Coccinelle:
@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct rproc_ops i at p = {...};
@ok1@
identifier r1.i;
position p;
@@
rproc_alloc(...,&i at p,...)
@bad@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i at p
@depends on !bad disable optional_qualifier@
identifier r1.i;
@@
+const
struct rproc_ops i;
File sizes before:
text data bss dec hex filename
1258 416 0 1674 68a remoteproc/omap_remoteproc.o
2402 240 0 2642 a52 remoteproc/st_remoteproc.o
2064 272 0 2336 920 remoteproc/st_slim_rproc.o
2160 240 0 2400 960 remoteproc/wkup_m3_rproc.o
File sizes after:
text data bss dec hex filename
1297 368 0 1665 681 remoteproc/omap_remoteproc.o
2434 192 0 2626 a42 remoteproc/st_remoteproc.o
2112 240 0 2352 930 remoteproc/st_slim_rproc.o
2200 192 0 2392 958 remoteproc/wkup_m3_rproc.o
Signed-off-by: Bhumika Goyal <bhumirks@gmail.com>
---
drivers/remoteproc/omap_remoteproc.c | 2 +-
drivers/remoteproc/st_remoteproc.c | 2 +-
drivers/remoteproc/st_slim_rproc.c | 2 +-
drivers/remoteproc/wkup_m3_rproc.c | 2 +-
4 files changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/remoteproc/omap_remoteproc.c b/drivers/remoteproc/omap_remoteproc.c
index fa63bf2..a96ce90 100644
--- a/drivers/remoteproc/omap_remoteproc.c
+++ b/drivers/remoteproc/omap_remoteproc.c
@@ -177,7 +177,7 @@ static int omap_rproc_stop(struct rproc *rproc)
return 0;
}
-static struct rproc_ops omap_rproc_ops = {
+static const struct rproc_ops omap_rproc_ops = {
.start = omap_rproc_start,
.stop = omap_rproc_stop,
.kick = omap_rproc_kick,
diff --git a/drivers/remoteproc/st_remoteproc.c b/drivers/remoteproc/st_remoteproc.c
index da4e152..f21787b 100644
--- a/drivers/remoteproc/st_remoteproc.c
+++ b/drivers/remoteproc/st_remoteproc.c
@@ -107,7 +107,7 @@ static int st_rproc_stop(struct rproc *rproc)
return sw_err ?: pwr_err;
}
-static struct rproc_ops st_rproc_ops = {
+static const struct rproc_ops st_rproc_ops = {
.start = st_rproc_start,
.stop = st_rproc_stop,
};
diff --git a/drivers/remoteproc/st_slim_rproc.c b/drivers/remoteproc/st_slim_rproc.c
index 507716c..6cfd862 100644
--- a/drivers/remoteproc/st_slim_rproc.c
+++ b/drivers/remoteproc/st_slim_rproc.c
@@ -200,7 +200,7 @@ static void *slim_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
return va;
}
-static struct rproc_ops slim_rproc_ops = {
+static const struct rproc_ops slim_rproc_ops = {
.start = slim_rproc_start,
.stop = slim_rproc_stop,
.da_to_va = slim_rproc_da_to_va,
diff --git a/drivers/remoteproc/wkup_m3_rproc.c b/drivers/remoteproc/wkup_m3_rproc.c
index 18175d0..1ada0e5 100644
--- a/drivers/remoteproc/wkup_m3_rproc.c
+++ b/drivers/remoteproc/wkup_m3_rproc.c
@@ -111,7 +111,7 @@ static void *wkup_m3_rproc_da_to_va(struct rproc *rproc, u64 da, int len)
return va;
}
-static struct rproc_ops wkup_m3_rproc_ops = {
+static const struct rproc_ops wkup_m3_rproc_ops = {
.start = wkup_m3_rproc_start,
.stop = wkup_m3_rproc_stop,
.da_to_va = wkup_m3_rproc_da_to_va,
--
1.9.1
^ permalink raw reply related
* [PATCH v8 1/8] soc: samsung: add exynos chipid driver support
From: Krzysztof Kozlowski @ 2016-12-17 12:03 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGcde9FBGcNpYbei0PsyHjt3_HVmm=x8oYkS-F7t0Rca6fpDAQ@mail.gmail.com>
On Sat, Dec 17, 2016 at 09:36:59AM +0530, Pankaj Dubey wrote:
> Hi Krzysztof,
>
> On 16 December 2016 at 23:07, Krzysztof Kozlowski <krzk@kernel.org> wrote:
> > On Sat, Dec 10, 2016 at 06:38:36PM +0530, Pankaj Dubey wrote:
> >> Exynos SoCs have Chipid, for identification of product IDs and SoC revisions.
> >> This patch intends to provide initialization code for all these functionalities,
> >> at the same time it provides some sysfs entries for accessing these information
> >> to user-space.
> >>
> >> This driver uses existing binding for exynos-chipid.
> >>
> >> CC: Grant Likely <grant.likely@linaro.org>
> >> CC: Rob Herring <robh+dt@kernel.org>
> >> CC: Linus Walleij <linus.walleij@linaro.org>
> >> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> >> [m.szyprowski: for suggestion and code snippet of product_id_to_soc_id]
> >> Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com>
> >> ---
> >> drivers/soc/samsung/Kconfig | 5 ++
> >> drivers/soc/samsung/Makefile | 1 +
> >> drivers/soc/samsung/exynos-chipid.c | 116 ++++++++++++++++++++++++++++++++++++
> >> 3 files changed, 122 insertions(+)
> >> create mode 100644 drivers/soc/samsung/exynos-chipid.c
> >>
> >> diff --git a/drivers/soc/samsung/Kconfig b/drivers/soc/samsung/Kconfig
> >> index 2455339..f9ab858 100644
> >> --- a/drivers/soc/samsung/Kconfig
> >> +++ b/drivers/soc/samsung/Kconfig
> >> @@ -14,4 +14,9 @@ config EXYNOS_PM_DOMAINS
> >> bool "Exynos PM domains" if COMPILE_TEST
> >> depends on PM_GENERIC_DOMAINS || COMPILE_TEST
> >>
> >> +config EXYNOS_CHIPID
> >> + bool "Exynos Chipid controller driver" if COMPILE_TEST
> >> + depends on (ARM && ARCH_EXYNOS) || ((ARM || ARM64) && COMPILE_TEST)
> >
> > 1. Why this can be compile tested only on ARM architectures?
>
> Well I just used dependency same as EXYNOS_PMU, but I can see it will
> be enabled for compile test on ARM64 isn't it?
I don't remember the PMU case... maybe it used clocks or something. Here
there are no arch-specific build dependencies.
>
> > 2. Don't you need also SOC_BUS?
>
> CHIPID needs SoC_BUS and for the same reason it is selecting SOC_BUS
> in the next line.
> If we mark it as a dependency (under depends on), even then we need to
> select this either
> under same EXYNOS_CHIPID config or ARCH_EXYNOS config.
Ah, I missed that line... Argh, sorry for noise.
Best regards,
Krzysztof
^ permalink raw reply
* [PATCH 1/2] ARM: hyp-stub: improve ABI
From: Catalin Marinas @ 2016-12-17 12:07 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161215185717.GM14217@n2100.armlinux.org.uk>
Hi Russell,
Just a quick reply expressing my opinion on the ABI and KVM maintenance
topics. Sorry I won't be able to follow up until the new year as I go on
holiday soon.
On Thu, Dec 15, 2016 at 06:57:18PM +0000, Russell King - ARM Linux wrote:
> On Thu, Dec 15, 2016 at 03:37:15PM +0000, Marc Zyngier wrote:
> > On 15/12/16 15:15, Russell King - ARM Linux wrote:
> > > On Thu, Dec 15, 2016 at 11:46:41AM +0000, Marc Zyngier wrote:
> > >> On 15/12/16 11:35, Russell King - ARM Linux wrote:
> > >>> On Thu, Dec 15, 2016 at 11:18:48AM +0000, Marc Zyngier wrote:
> > >>>> On 14/12/16 10:46, Russell King wrote:
> > >>>>> @@ -231,10 +244,14 @@ ENDPROC(__hyp_stub_do_trap)
> > >>>>> * initialisation entry point.
> > >>>>> */
> > >>>>> ENTRY(__hyp_get_vectors)
> > >>>>> - mov r0, #-1
> > >>>>> + mov r0, #HVC_GET_VECTORS
> > >>>>
> > >>>> This breaks the KVM implementation of __hyp_get_vectors, easily fixed
> > >>>> with the following patchlet:
> > >>>
> > >>> Right, so what Mark said is wrong:
> > >>>
> > >>> "The hyp-stub is part of the kernel image, and the API is private to
> > >>> that particular image, so we can change things -- there's no ABI to
> > >>> worry about."
> > >>
> > >> I think Mark is right. The API *is* private to the kernel, and KVM being
> > >> the only in-kernel hypervisor on ARM, this is not an ABI.
> > >
> > > Again, that's wrong.
> > >
> > > We have two hypervisors in the kernel. One is KVM, the other is the
> > > stub. Sure, the stub isn't a full implementation of a hypervisor, but
> > > it is nevertheless, for the purposes of _this_ discussion, a hypervisor
> > > of sorts.
> > >
> > > The reason that both are included is because they both appear to share
> > > a common interface (although that's totally not documented anywhere.)
> >
> > And this interface exists for the sole purpose of enabling KVM. Call it
> > a hypervisor if you wish, but its usefulness is doubtful on its own.
IMO, the interface between EL1 and EL2 _is_ ABI. However, it should not
be confused with a *stable* ABI like the one we expose to user. Since
all users of the EL2 ABI live inside the kernel (either on the EL1 or
EL2 side), we are free to change it as long as everything is updated
simultaneously. I don't see this any different from other in-kernel ABI
like that exposed to loadable modules (for the latter, we have the
advantage of a partly self-documenting ABI as part of the C language).
I would welcome some documentation for the EL2 ABI as well, especially
since head.S and the KVM counterpart is maintained by different people.
> What's also coming clear is that there's very few people who understand
> all the interactions here, and the whole thing seems to be an undocumented
> mess. Something needs to change there - people need to stop shovelling
> half baked crap into the kernel. KVM have the privilege of being able to
> push ARM stuff directly, I now see that was a very big mistake.
>
> So, I want KVM further changes to come through my tree once this merge
> window is over -
I'm afraid I strongly disagree. There are a few reasons neither you nor
me gatekeep the KVM port: (a) the ARM KVM maintainers know a lot more
about KVM than either of us, (b) the KVM code under arch/arm is reused
by arm64 and (c) you or I would certainly become a bottleneck. There is
a lot more to KVM support on ARM than the hyp stub and realistically you
won't have the time to review all the stuff that comes your way.
Just because there is an interface between two or more pieces of code,
it is not a strong enough argument for them to have the same gatekeeper
(I wouldn't say maintainer since IMO this implies a lot more). That
said, you can always advise other maintainers on how to do things right,
ask for proper documentation (as you did), review how things are defined
and implemented, especially when they touch code _you_ maintain.
> Let's start with some documentation on the KVM hypervisor interfaces on
> 32-bit ARM. Documentation/virtual/kvm/hypercalls.txt already contains
> some for x86, s390 and PPC, so that would be a good place to document
> the ARM side. Arguably, that should have already been done.
I'm all for documenting the interface.
(even though Will and I co-maintain arch/arm64, I deliberately kept his
name out of the above as I haven't had the chance to ask for his
opinion on this matter, which may as well differ)
I have to go and pack now. Merry Christmas!
--
Catalin
^ permalink raw reply
* [PATCH v1 & v6 1/2] PM/devfreq: add suspend frequency support
From: Chanwoo Choi @ 2016-12-17 14:50 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <5836B8D1.9050707@samsung.com>
Hi Lin,
2016-11-24 18:54 GMT+09:00 Chanwoo Choi <cw00.choi@samsung.com>:
> Hi Lin,
>
> On 2016? 11? 24? 18:28, Chanwoo Choi wrote:
>> Hi Lin,
>>
>> On 2016? 11? 24? 17:34, hl wrote:
>>> Hi Chanwoo Choi,
>>>
>>>
>>> On 2016?11?24? 16:16, Chanwoo Choi wrote:
>>>> Hi Lin,
>>>>
>>>> On 2016? 11? 24? 16:34, hl wrote:
>>>>> Hi Chanwoo Choi,
>>>>>
>>>>> I think the dev_pm_opp_get_suspend_opp() have implement most of
>>>>> the funtion, all we need is just define the node in dts, like following:
>>>>>
>>>>> &dmc_opp_table {
>>>>> opp06 {
>>>>> opp-suspend;
>>>>> };
>>>>> };
>>>> Two approaches use the 'opp-suspend' property.
>>>>
>>>> I think that the method to support suspend-opp have to
>>>> guarantee following conditions:
>>>> - Support the all of devfreq's governors.
>>> As MyungJoo Ham suggestion, i will set the suspend frequency in devfreq_suspend_device(),
>>> which will ingore governor.
>>
>> Other approach already support the all of governors.
>> Before calling the mail, I discussed with Myungjoo Ham.
>> Myungjoo prefer to use the devfreq_suspend/devfreq_resume().
>
> It is not correct expression. We need to wait the reply from Myungjoo
> to clarify this.
>
>>
>> To Myungjoo,
>> Please add your opinion how to support the suspend frequency.
>
>>
>>>> - Devfreq framework have the responsibility to change the
>>>> frequency/voltage for suspend-opp. If we uses the
>>>> new devfreq_suspend(), each devfreq device don't care
>>>> how to support the suspend-opp. Just the developer of each
>>>> devfreq device need to add 'opp-suspend' propet to OPP entry in DT file.
>>> Why should support change the voltage in devfreq framework, i think it shuold be handle in
>>> specific driver, i think the devfreq only handle it can get the right frequency, then pass it to
>>
>> No, the frequency should be handled by governor or framework.
>> The each devfreq device has no any responsibility of next frequency/voltage.
>> The governor and core of devfreq can decide the next frequency/voltage.
>> You can refer to the cpufreq subsystem.
>>
>>> specific driver, i think the voltage should handle in the devfreq->profile->target();
>>
>> The call of devfreq->profile->target() have to be handled by devfreq framework.
>> If user want to set the suspend frequency, user can add the 'suspend-opp' property.
>> It think this way is easy.
>>
>> But,
>> If the each devfreq device want to decide the next frequency/voltage only for
>> suspend state. We can check the cpufreq subsystem.
>>
>> If specific devfreq device want to handle the suspend frequency,
>> each devfreq will add the own suspend/resume functions as following:
>>
>> struct devfreq_dev_profile {
>> int (*suspend)(struct devfreq *dev); // new function pointer
>> int (*resume)(struct devfreq *dev); // new function pointer
>> } a_profile;
>>
>> a_profile = devfreq_generic_suspend;
>>
>> The devfreq framework will provide the devfreq_generic_suspend() funticon.
>> int devfreq_generic_suspend(struce devfreq *dev) {
>> ...
>> devfreq->profile->target(..., devfreq->suspend_freq);
>> ...
>> }
>>
>> or
>>
>> a_profile = a_devfreq_suspend; // specific function of each devfreq device
>>
>> The devfreq_suspend() will call 'devfreq->profile->suspend()' function
>> instead of devfreq->profile->target();
>>
>> The devfreq call the 'devfreq->profile->suspend()'
>> to support the suspend frequency.
>>
>> Regards,
>> Chanwoo Choi
>
> The key difference between two approaches:
>
> Your approach:
> - The each developer should add the 'opp-suspend' property to the dts file.
> - The each devfreq should call the devfreq_suspend_device()
> to support the suspend frequency.
>
> If each devfreq doesn't call the devfreq_suspend_device(), devfreq framework
> can support the suspend frequency.
>
> Other approach:
> - The each developer only should add the 'opp-suspend' property to the dts file
> without the additional behavior.
>
> In the cpufreq subsystem,
> When support the suspend frequency of cpufreq, we just add 'opp-suspend' property
> without the additional behavior.
I'm missing the use-case when using the devfreq_suspend_device()
before entering the suspend mode. We should consider the case when
devfreq device
calls the devfreq_suspend_device() directly. Because devfreq_suspend_device()
is exported function, each devfreq device call this function on the fly
without entering the suspend mode.
I correct my opinion. Your approach is necessary. I'm sorry to confuse you.
So, I make the following patch. This patch set the suspend frequency
in devfreq_suspend_device() after stoping the governor.
It consider the all governors of devfreq.
What do you think?
If you are ok, I'll send this patch with your author.
int devfreq_suspend_device(struct devfreq *devfreq)
{
+ int ret = 0;
+
if (!devfreq)
return -EINVAL;
if (!devfreq->governor)
return 0;
- return devfreq->governor->event_handler(devfreq,
+ ret = devfreq->governor->event_handler(devfreq,
DEVFREQ_GOV_SUSPEND, NULL);
+ if (ret < 0) {
+ dev_err(devfreq->dev.parent, "failed to suspend governor\n");
+ return ret;
+ }
+
+ if (devfreq->suspend_freq) {
+ ret = devfreq->profile->target(devfreq->dev.parent,
+ &devfreq->suspend_freq, 0);
+ if (ret < 0) {
+ dev_err(devfreq->dev.parent,
+ "failed to set suspend-freq\n");
+ return ret;
+ }
+ dev_dbg(devfreq->dev.parent, "Setting suspend-freq: %lu\n",
+ devfreq->suspend_freq);
+ }
+
+ return 0;
}
EXPORT_SYMBOL(devfreq_suspend_device);
--
Best Regards,
Chanwoo Choi
^ permalink raw reply
* [PATCH v8 5/8] ARM: EXYNOS: refactor power management specific routines
From: Krzysztof Kozlowski @ 2016-12-17 14:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481375323-29724-6-git-send-email-pankaj.dubey@samsung.com>
On Sat, Dec 10, 2016 at 06:38:40PM +0530, Pankaj Dubey wrote:
> To remove dependency on soc_is_exynosMMMM macros and remove
> multiple checks for such macros lets refactor code in pm.c
>
> This patch matches SoC specific information via private data
> of soc_device_attribute and initialize it at one place and then
> uses it all other places
>
> Signed-off-by: Pankaj Dubey <pankaj.dubey@samsung.com>
> ---
> arch/arm/mach-exynos/pm.c | 185 ++++++++++++++++++++++++++++++++++++++--------
> 1 file changed, 155 insertions(+), 30 deletions(-)
>
Too many things in one patch and all the renaming make reviewing more
difficult. Could you split some of self-contained changes to separate
patches? At least last soc_is_exynos3250->of_machine_is_compatible is
nicely separatable. I also wonder whether it made sense to split it
more into:
1. Introduce exynos_s2r_data() and one instance - generic, with the same
code (and with soc_is_...()).
2. Introduce new exynos_s2r_data instances and get remove soc_is_...()?
Best regards,
Krzysztof
> diff --git a/arch/arm/mach-exynos/pm.c b/arch/arm/mach-exynos/pm.c
> index c0b46c3..d43bea8 100644
> --- a/arch/arm/mach-exynos/pm.c
> +++ b/arch/arm/mach-exynos/pm.c
> @@ -20,6 +20,7 @@
> #include <linux/of.h>
> #include <linux/soc/samsung/exynos-regs-pmu.h>
> #include <linux/soc/samsung/exynos-pmu.h>
> +#include <linux/sys_soc.h>
>
> #include <asm/firmware.h>
> #include <asm/smp_scu.h>
> @@ -28,22 +29,49 @@
>
> #include "common.h"
>
> +struct exynos_s2r_data {
> + void __iomem* (*boot_vector_addr)(void);
> + void __iomem* (*boot_vector_flag)(void);
> + void (*set_wakeupmask)(void);
> + void (*enter_aftr)(void);
> +};
> +
> +static const struct exynos_s2r_data *s2r_data;
> +
> +static void __iomem *exynos4210_rev11_boot_vector_addr(void)
> +{
> + return pmu_base_addr + S5P_INFORM7;
> +}
> +
> +static void __iomem *exynos4210_rev10_boot_vector_addr(void)
> +{
> + return sysram_base_addr + 0x24;
> +}
> +
> static inline void __iomem *exynos_boot_vector_addr(void)
> {
> - if (samsung_rev() == EXYNOS4210_REV_1_1)
> - return pmu_base_addr + S5P_INFORM7;
> - else if (samsung_rev() == EXYNOS4210_REV_1_0)
> - return sysram_base_addr + 0x24;
> - return pmu_base_addr + S5P_INFORM0;
> + if (s2r_data && s2r_data->boot_vector_addr)
> + return s2r_data->boot_vector_addr();
> + else
> + return pmu_base_addr + S5P_INFORM0;
> +}
> +
> +static void __iomem *exynos4210_rev11_boot_vector_flag(void)
> +{
> + return pmu_base_addr + S5P_INFORM6;
> +}
> +
> +static void __iomem *exynos4210_rev10_boot_vector_flag(void)
> +{
> + return sysram_base_addr + 0x20;
> }
>
> static inline void __iomem *exynos_boot_vector_flag(void)
> {
> - if (samsung_rev() == EXYNOS4210_REV_1_1)
> - return pmu_base_addr + S5P_INFORM6;
> - else if (samsung_rev() == EXYNOS4210_REV_1_0)
> - return sysram_base_addr + 0x20;
> - return pmu_base_addr + S5P_INFORM1;
> + if (s2r_data && s2r_data->boot_vector_flag)
> + return s2r_data->boot_vector_flag();
> + else
> + return pmu_base_addr + S5P_INFORM1;
> }
>
> #define S5P_CHECK_AFTR 0xFCBA0D10
> @@ -120,12 +148,19 @@ int exynos_pm_central_resume(void)
> return 0;
> }
>
> +static void exynos3250_set_wakeupmask(void)
> +{
> + pmu_raw_writel(0x40003ffe, S5P_WAKEUP_MASK);
> + pmu_raw_writel(0x0, S5P_WAKEUP_MASK2);
> +}
> +
> /* Ext-GIC nIRQ/nFIQ is the only wakeup source in AFTR */
> -static void exynos_set_wakeupmask(long mask)
> +static void exynos_set_wakeupmask(void)
> {
> - pmu_raw_writel(mask, S5P_WAKEUP_MASK);
> - if (soc_is_exynos3250())
> - pmu_raw_writel(0x0, S5P_WAKEUP_MASK2);
> + if (s2r_data && s2r_data->set_wakeupmask)
> + s2r_data->set_wakeupmask();
> + else
> + pmu_raw_writel(0x0000ff3e, S5P_WAKEUP_MASK);
> }
>
> static void exynos_cpu_set_boot_vector(long flags)
> @@ -139,7 +174,7 @@ static int exynos_aftr_finisher(unsigned long flags)
> {
> int ret;
>
> - exynos_set_wakeupmask(soc_is_exynos3250() ? 0x40003ffe : 0x0000ff3e);
> + exynos_set_wakeupmask();
> /* Set value of power down register for aftr mode */
> exynos_sys_powerdown_conf(SYS_AFTR);
>
> @@ -154,23 +189,30 @@ static int exynos_aftr_finisher(unsigned long flags)
> return 1;
> }
>
> -void exynos_enter_aftr(void)
> +static void exynos3250_enter_aftr(void)
> {
> unsigned int cpuid = smp_processor_id();
>
> cpu_pm_enter();
>
> - if (soc_is_exynos3250())
> - exynos_set_boot_flag(cpuid, C2_STATE);
> + exynos_set_boot_flag(cpuid, C2_STATE);
> + exynos_pm_central_suspend();
> + cpu_suspend(0, exynos_aftr_finisher);
> + exynos_pm_central_resume();
> + exynos_clear_boot_flag(cpuid, C2_STATE);
> +
> + cpu_pm_exit();
> +}
> +
> +void exynos4x12_enter_aftr(void)
> +{
> + cpu_pm_enter();
>
> exynos_pm_central_suspend();
>
> - if (of_machine_is_compatible("samsung,exynos4212") ||
> - of_machine_is_compatible("samsung,exynos4412")) {
> - /* Setting SEQ_OPTION register */
> - pmu_raw_writel(S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0,
> - S5P_CENTRAL_SEQ_OPTION);
> - }
> + /* Setting SEQ_OPTION register */
> + pmu_raw_writel(S5P_USE_STANDBY_WFI0 | S5P_USE_STANDBY_WFE0,
> + S5P_CENTRAL_SEQ_OPTION);
>
> cpu_suspend(0, exynos_aftr_finisher);
>
> @@ -182,12 +224,95 @@ void exynos_enter_aftr(void)
>
> exynos_pm_central_resume();
>
> - if (soc_is_exynos3250())
> - exynos_clear_boot_flag(cpuid, C2_STATE);
> + cpu_pm_exit();
> +}
> +
> +void exynos_common_enter_aftr(void)
> +{
> + cpu_pm_enter();
> +
> + exynos_pm_central_suspend();
> + cpu_suspend(0, exynos_aftr_finisher);
> +
> + if (read_cpuid_part() == ARM_CPU_PART_CORTEX_A9) {
> + exynos_scu_enable();
> + if (call_firmware_op(resume) == -ENOSYS)
> + exynos_cpu_restore_register();
> + }
> +
> + exynos_pm_central_resume();
>
> cpu_pm_exit();
> }
>
> +void exynos_enter_aftr(void)
> +{
> + if (s2r_data && s2r_data->enter_aftr)
> + s2r_data->enter_aftr();
> +}
> +
> +static const struct exynos_s2r_data exynos_common_s2r_data = {
> + .enter_aftr = exynos_common_enter_aftr,
> +};
> +
> +static const struct exynos_s2r_data exynos3250_s2r_data = {
> + .set_wakeupmask = exynos3250_set_wakeupmask,
> + .enter_aftr = exynos3250_enter_aftr,
> +};
> +
> +static const struct exynos_s2r_data exynos4210_rev11_s2r_data = {
> + .boot_vector_addr = exynos4210_rev11_boot_vector_addr,
> + .boot_vector_flag = exynos4210_rev11_boot_vector_flag,
> + .enter_aftr = exynos_common_enter_aftr,
> +};
> +
> +static const struct exynos_s2r_data exynos4210_rev10_s2r_data = {
> + .boot_vector_addr = exynos4210_rev10_boot_vector_addr,
> + .boot_vector_flag = exynos4210_rev10_boot_vector_flag,
> + .enter_aftr = exynos_common_enter_aftr,
> +};
> +
> +static const struct exynos_s2r_data exynos4x12_s2r_data = {
> + .enter_aftr = exynos4x12_enter_aftr,
> +};
> +
> +static const struct soc_device_attribute exynos_soc_revision[] = {
> + { .soc_id = "EXYNOS3250", .data = &exynos3250_s2r_data },
> + {
> + .soc_id = "EXYNOS4210", .revision = "11",
> + .data = &exynos4210_rev11_s2r_data
> + },
> + {
> + .soc_id = "EXYNOS4210", .revision = "10",
> + .data = &exynos4210_rev10_s2r_data
> + },
> + { .soc_id = "EXYNOS4212", .data = &exynos4x12_s2r_data },
> + { .soc_id = "EXYNOS4412", .data = &exynos4x12_s2r_data },
> + { .soc_id = "EXYNOS4415", .data = &exynos_common_s2r_data },
> + { .soc_id = "EXYNOS5250", .data = &exynos_common_s2r_data },
> + { .soc_id = "EXYNOS5260", .data = &exynos_common_s2r_data },
> + { .soc_id = "EXYNOS5410", .data = &exynos_common_s2r_data },
> + { .soc_id = "EXYNOS5420", .data = &exynos_common_s2r_data },
> + { .soc_id = "EXYNOS5440", .data = &exynos_common_s2r_data },
> + { .soc_id = "EXYNOS5800", .data = &exynos_common_s2r_data },
> +};
> +
> +int __init exynos_s2r_init(void)
> +{
> + const struct soc_device_attribute *match;
> +
> + match = soc_device_match(exynos_soc_revision);
> +
> + if (match)
> + s2r_data = (const struct exynos_s2r_data *) match->data;
> +
> + if (!s2r_data)
> + return -ENODEV;
> +
> + return 0;
> +}
> +arch_initcall(exynos_s2r_init);
> +
> #if defined(CONFIG_SMP) && defined(CONFIG_ARM_EXYNOS_CPUIDLE)
> static atomic_t cpu1_wakeup = ATOMIC_INIT(0);
>
> @@ -253,7 +378,7 @@ static int exynos_cpu0_enter_aftr(void)
> while (exynos_cpu_power_state(1) != S5P_CORE_LOCAL_PWR_EN)
> cpu_relax();
>
> - if (soc_is_exynos3250()) {
> + if (of_machine_is_compatible("samsung,exynos3250")) {
> while (!pmu_raw_readl(S5P_PMU_SPARE2) &&
> !atomic_read(&cpu1_wakeup))
> cpu_relax();
> @@ -275,7 +400,7 @@ static int exynos_cpu0_enter_aftr(void)
>
> call_firmware_op(cpu_boot, 1);
>
> - if (soc_is_exynos3250())
> + if (of_machine_is_compatible("samsung,exynos3250"))
> dsb_sev();
> else
> arch_send_wakeup_ipi_mask(cpumask_of(1));
> @@ -287,7 +412,7 @@ static int exynos_cpu0_enter_aftr(void)
>
> static int exynos_wfi_finisher(unsigned long flags)
> {
> - if (soc_is_exynos3250())
> + if (of_machine_is_compatible("samsung,exynos3250"))
> flush_cache_all();
> cpu_do_idle();
>
> @@ -309,7 +434,7 @@ static int exynos_cpu1_powerdown(void)
> */
> exynos_cpu_power_down(1);
>
> - if (soc_is_exynos3250())
> + if (of_machine_is_compatible("samsung,exynos3250"))
> pmu_raw_writel(0, S5P_PMU_SPARE2);
>
> ret = cpu_suspend(0, exynos_wfi_finisher);
> --
> 2.7.4
>
^ permalink raw reply
* [PATCH v1 & v6 1/2] PM/devfreq: add suspend frequency support
From: Tobias Jakobi @ 2016-12-17 15:13 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAGTfZH34MvFGGuPgt=qUM6FMmi+Z80akY5ZH1k6gXdzg19fh9Q@mail.gmail.com>
Hey guys,
Chanwoo Choi wrote:
> Hi Lin,
>
> 2016-11-24 18:54 GMT+09:00 Chanwoo Choi <cw00.choi@samsung.com>:
>> Hi Lin,
>>
>> On 2016? 11? 24? 18:28, Chanwoo Choi wrote:
>>> Hi Lin,
>>>
>>> On 2016? 11? 24? 17:34, hl wrote:
>>>> Hi Chanwoo Choi,
>>>>
>>>>
>>>> On 2016?11?24? 16:16, Chanwoo Choi wrote:
>>>>> Hi Lin,
>>>>>
>>>>> On 2016? 11? 24? 16:34, hl wrote:
>>>>>> Hi Chanwoo Choi,
>>>>>>
>>>>>> I think the dev_pm_opp_get_suspend_opp() have implement most of
>>>>>> the funtion, all we need is just define the node in dts, like following:
>>>>>>
>>>>>> &dmc_opp_table {
>>>>>> opp06 {
>>>>>> opp-suspend;
>>>>>> };
>>>>>> };
>>>>> Two approaches use the 'opp-suspend' property.
>>>>>
>>>>> I think that the method to support suspend-opp have to
>>>>> guarantee following conditions:
>>>>> - Support the all of devfreq's governors.
>>>> As MyungJoo Ham suggestion, i will set the suspend frequency in devfreq_suspend_device(),
>>>> which will ingore governor.
>>>
>>> Other approach already support the all of governors.
>>> Before calling the mail, I discussed with Myungjoo Ham.
>>> Myungjoo prefer to use the devfreq_suspend/devfreq_resume().
>>
>> It is not correct expression. We need to wait the reply from Myungjoo
>> to clarify this.
>>
>>>
>>> To Myungjoo,
>>> Please add your opinion how to support the suspend frequency.
>>
>>>
>>>>> - Devfreq framework have the responsibility to change the
>>>>> frequency/voltage for suspend-opp. If we uses the
>>>>> new devfreq_suspend(), each devfreq device don't care
>>>>> how to support the suspend-opp. Just the developer of each
>>>>> devfreq device need to add 'opp-suspend' propet to OPP entry in DT file.
>>>> Why should support change the voltage in devfreq framework, i think it shuold be handle in
>>>> specific driver, i think the devfreq only handle it can get the right frequency, then pass it to
>>>
>>> No, the frequency should be handled by governor or framework.
>>> The each devfreq device has no any responsibility of next frequency/voltage.
>>> The governor and core of devfreq can decide the next frequency/voltage.
>>> You can refer to the cpufreq subsystem.
>>>
>>>> specific driver, i think the voltage should handle in the devfreq->profile->target();
>>>
>>> The call of devfreq->profile->target() have to be handled by devfreq framework.
>>> If user want to set the suspend frequency, user can add the 'suspend-opp' property.
>>> It think this way is easy.
>>>
>>> But,
>>> If the each devfreq device want to decide the next frequency/voltage only for
>>> suspend state. We can check the cpufreq subsystem.
>>>
>>> If specific devfreq device want to handle the suspend frequency,
>>> each devfreq will add the own suspend/resume functions as following:
>>>
>>> struct devfreq_dev_profile {
>>> int (*suspend)(struct devfreq *dev); // new function pointer
>>> int (*resume)(struct devfreq *dev); // new function pointer
>>> } a_profile;
>>>
>>> a_profile = devfreq_generic_suspend;
>>>
>>> The devfreq framework will provide the devfreq_generic_suspend() funticon.
>>> int devfreq_generic_suspend(struce devfreq *dev) {
>>> ...
>>> devfreq->profile->target(..., devfreq->suspend_freq);
>>> ...
>>> }
>>>
>>> or
>>>
>>> a_profile = a_devfreq_suspend; // specific function of each devfreq device
>>>
>>> The devfreq_suspend() will call 'devfreq->profile->suspend()' function
>>> instead of devfreq->profile->target();
>>>
>>> The devfreq call the 'devfreq->profile->suspend()'
>>> to support the suspend frequency.
>>>
>>> Regards,
>>> Chanwoo Choi
>>
>> The key difference between two approaches:
>>
>> Your approach:
>> - The each developer should add the 'opp-suspend' property to the dts file.
>> - The each devfreq should call the devfreq_suspend_device()
>> to support the suspend frequency.
>>
>> If each devfreq doesn't call the devfreq_suspend_device(), devfreq framework
>> can support the suspend frequency.
>>
>> Other approach:
>> - The each developer only should add the 'opp-suspend' property to the dts file
>> without the additional behavior.
>>
>> In the cpufreq subsystem,
>> When support the suspend frequency of cpufreq, we just add 'opp-suspend' property
>> without the additional behavior.
>
> I'm missing the use-case when using the devfreq_suspend_device()
> before entering the suspend mode. We should consider the case when
> devfreq device
> calls the devfreq_suspend_device() directly. Because devfreq_suspend_device()
> is exported function, each devfreq device call this function on the fly
> without entering the suspend mode.
>
> I correct my opinion. Your approach is necessary. I'm sorry to confuse you.
> So, I make the following patch. This patch set the suspend frequency
> in devfreq_suspend_device() after stoping the governor.
> It consider the all governors of devfreq.
>
> What do you think?
> If you are ok, I'll send this patch with your author.
The problem I see here is that we need to keep track of the suspended
state when suspending the (entire) devfreq subsystem. When doing that,
we don't know if any device driver has previously called
devfreq_suspend_device() and might end up calling it twice.
Same thing on devfreq subsystem resume.
I've prepared a new RFC of my series (going to send it shortly), but I'm
not so happy with the current design. I think it would be much cleaner
to keep some suspend_refcount in struct devfreq so that I can call
devfreq_suspend_device() multiple times, while keeping a sane internal
state.
Something like devfreq_device_runtime_{put,get}() perhaps?
- Tobias
>
> int devfreq_suspend_device(struct devfreq *devfreq)
> {
> + int ret = 0;
> +
> if (!devfreq)
> return -EINVAL;
>
> if (!devfreq->governor)
> return 0;
>
> - return devfreq->governor->event_handler(devfreq,
> + ret = devfreq->governor->event_handler(devfreq,
> DEVFREQ_GOV_SUSPEND, NULL);
> + if (ret < 0) {
> + dev_err(devfreq->dev.parent, "failed to suspend governor\n");
> + return ret;
> + }
> +
> + if (devfreq->suspend_freq) {
> + ret = devfreq->profile->target(devfreq->dev.parent,
> + &devfreq->suspend_freq, 0);
> + if (ret < 0) {
> + dev_err(devfreq->dev.parent,
> + "failed to set suspend-freq\n");
> + return ret;
> + }
> + dev_dbg(devfreq->dev.parent, "Setting suspend-freq: %lu\n",
> + devfreq->suspend_freq);
> + }
> +
> + return 0;
> }
> EXPORT_SYMBOL(devfreq_suspend_device);
>
^ permalink raw reply
* [PATCH] net: mvpp2: fix dma unmapping of TX buffers for fragments
From: David Miller @ 2016-12-17 15:20 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1481647995-7213-1-git-send-email-thomas.petazzoni@free-electrons.com>
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date: Tue, 13 Dec 2016 17:53:15 +0100
> diff --git a/drivers/net/ethernet/marvell/mvpp2.c b/drivers/net/ethernet/marvell/mvpp2.c
> index 1026c45..d168b13 100644
> --- a/drivers/net/ethernet/marvell/mvpp2.c
> +++ b/drivers/net/ethernet/marvell/mvpp2.c
> @@ -791,6 +791,8 @@ struct mvpp2_txq_pcpu {
> /* Array of transmitted buffers' physical addresses */
> dma_addr_t *tx_buffs;
>
> + size_t *tx_data_size;
> +
You're really destroying cache locality, and making things overly
complicated, by having two arrays. Actually this is now the third in
this structure alone. That's crazy.
Just have one array for the TX ring software state:
struct tx_buff_info {
struct sk_buff *skb;
dma_addr_t dma_addr;
unsigned int size;
};
Then in the per-cpu TX struct:
struct tx_buff_info *info;
This way every data access by the cpu for processing a ring entry
will be localized, increasing cache hit rates.
This also significantly simplifies the code that allocates and
frees this memory.
^ permalink raw reply
* [PATCH] net: mvpp2: fix dma unmapping of TX buffers for fragments
From: Thomas Petazzoni @ 2016-12-17 15:26 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161217.102057.1497114610120684110.davem@davemloft.net>
Hello,
On Sat, 17 Dec 2016 10:20:57 -0500 (EST), David Miller wrote:
> You're really destroying cache locality, and making things overly
> complicated, by having two arrays. Actually this is now the third in
> this structure alone. That's crazy.
>
> Just have one array for the TX ring software state:
>
> struct tx_buff_info {
> struct sk_buff *skb;
> dma_addr_t dma_addr;
> unsigned int size;
> };
>
> Then in the per-cpu TX struct:
>
> struct tx_buff_info *info;
>
> This way every data access by the cpu for processing a ring entry
> will be localized, increasing cache hit rates.
>
> This also significantly simplifies the code that allocates and
> frees this memory.
Yes, I was thinking of moving towards a single array, as it's indeed
crazy to have three arrays for that. However, since it's a fix going
into stable, I also wanted to keep it as simple/straightforward as
possible and avoid refactoring other parts of the code.
If you however believe moving to one array should be done as part of
the fix, I'll do this.
Thanks for your feedback,
Thomas Petazzoni
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* [PATCH v2 00/11] add support for VBUS max current and min voltage limits AXP20X and AXP22X PMICs
From: Sebastian Reichel @ 2016-12-17 15:54 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161209110419.28981-1-quentin.schulz@free-electrons.com>
Hi Quentin,
On Fri, Dec 09, 2016 at 12:04:08PM +0100, Quentin Schulz wrote:
> The X-Powers AXP209 and AXP20X PMICs are able to set a limit for the
> VBUS power supply for both max current and min voltage supplied. This
> series of patch adds the possibility to set these limits from sysfs.
>
> Also, the AXP223 PMIC shares most of its behaviour with the AXP221 but
> the former can set the VBUS power supply max current to 100mA, unlike
> the latter. The AXP223 VBUS power supply driver used to probe on the
> AXP221 compatible. This series of patch introduces a new compatible for
> the AXP223 to be able to set the current max limit to 100mA.
>
> With that new compatible, boards having the AXP223 see their DT updated
> to use the VBUS power supply driver with the correct compatible.
>
> This series of patch also migrates from of_device_is_compatible function
> to the data field of of_device_id to identify the compatible used to
> probe. This improves the code readability.
>
> Mostly cosmetic changes in v2 and adding volatile and writeable regs to
> AXP20X and AXP22X MFD cells for the VBUS power supply driver.
>
> Quentin Schulz (11):
> power: supply: axp20x_usb_power: use of_device_id data field instead
> of device_is_compatible
> mfd: axp20x: add volatile and writeable reg ranges for VBUS power
> supply driver
> power: supply: axp20x_usb_power: set min voltage and max current from
> sysfs
> Documentation: DT: binding: axp20x_usb_power: add axp223 compatible
> power: supply: axp20x_usb_power: add 100mA max current limit for
> AXP223
> mfd: axp20x: add separate MFD cell for AXP223
> ARM: dtsi: add DTSI for AXP223
> ARM: dts: sun8i-a33-olinuxino: use AXP223 DTSI
> ARM: dts: sun8i-a33-sinlinx-sina33: use AXP223 DTSI
> ARM: dts: sun8i-r16-parrot: use AXP223 DTSI
> ARM: dtsi: sun8i-reference-design-tablet: use AXP223 DTSI
Thanks for your patchset. We are currently in the merge
window and patches 1 & 3-5 will appear in linux-next once
4.10-rc1 has been tagged by Linus Torvalds.
Until then I queued them into this branch:
https://git.kernel.org/cgit/linux/kernel/git/sre/linux-power-supply.git/log/?h=for-next-next
-- Sebastian
-- Sebastian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-arm-kernel/attachments/20161217/584ce732/attachment.sig>
^ permalink raw reply
* [PATCH] net: mvpp2: fix dma unmapping of TX buffers for fragments
From: David Miller @ 2016-12-17 16:35 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20161217162658.63bc2423@free-electrons.com>
From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date: Sat, 17 Dec 2016 16:26:58 +0100
> Yes, I was thinking of moving towards a single array, as it's indeed
> crazy to have three arrays for that. However, since it's a fix going
> into stable, I also wanted to keep it as simple/straightforward as
> possible and avoid refactoring other parts of the code.
By the same token, by adding a third array you are making the code
more complex, adding more error recovery paths, etc.
> If you however believe moving to one array should be done as part of
> the fix, I'll do this.
Please do.
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox