Re: [PATCH AUTOSEL 5.10 04/16] powerpc/hw_breakpoint: Avoid relying on caller synchronization
Hi! > From: Marco Elver > > [ Upstream commit f95e5a3d59011eec1257d0e76de1e1f8969d426f ] > > Internal data structures (cpu_bps, task_bps) of powerpc's hw_breakpoint > implementation have relied on nr_bp_mutex serializing access to them. > > Before overhauling synchronization of kernel/events/hw_breakpoint.c, > introduce 2 spinlocks to synchronize cpu_bps and task_bps respectively, > thus avoiding reliance on callers synchronizing powerpc's > hw_breakpoint. This is preparation, not a bugfix, not sure why it was picked for 5.10. Best regards, Pavel -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany signature.asc Description: PGP signature
Re: [PATCH 6/6] i2c: Make remove callback return void
Hi! > From: Uwe Kleine-König > > The value returned by an i2c driver's remove function is mostly ignored. > (Only an error message is printed if the value is non-zero that the > error is ignored.) > > So change the prototype of the remove function to return no value. This > way driver authors are not tempted to assume that passing an error to > the upper layer is a good idea. All drivers are adapted accordingly. > There is no intended change of behaviour, all callbacks were prepared to > return 0 before. > > Signed-off-by: Uwe Kleine-König 2-4: Acked-by: Pavel Machek Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: PGP signature
Re: [PATCH] ASoC: fsl: using pm_runtime_resume_and_get instead of pm_runtime_get_sync
Hi! > From: Minghao Chi > > Using pm_runtime_resume_and_get is more appropriate > for simplifing code > > Reported-by: Zeal Robot > Signed-off-by: Minghao Chi > --- > sound/soc/fsl/fsl_esai.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/sound/soc/fsl/fsl_esai.c b/sound/soc/fsl/fsl_esai.c > index ed444e8f1d6b..1a2bdf8e76f0 100644 > --- a/sound/soc/fsl/fsl_esai.c > +++ b/sound/soc/fsl/fsl_esai.c > @@ -1050,11 +1050,9 @@ static int fsl_esai_probe(struct platform_device *pdev) > goto err_pm_disable; > } > > - ret = pm_runtime_get_sync(>dev); > - if (ret < 0) { > - pm_runtime_put_noidle(>dev); > + ret = pm_runtime_resume_and_get(>dev); > + if (ret < 0) > goto err_pm_get_sync; > - } > > ret = fsl_esai_hw_init(esai_priv); > if (ret) Please take a closer look at that function. a) error labels are now misnamed b) there's leak if ret = fsl_esai_hw_init(esai_priv); if (ret) goto err_pm_get_sync; happens. Best regards, Pavel -- People of Russia, stop Putin before his war on Ukraine escalates. signature.asc Description: Digital signature
Re: [PATCH AUTOSEL 5.10 01/26] ibmvnic: check failover_pending in login response
Hi! Something went wrong with this series. I only see first 7 patches. I thought it might be local problem, but I only see 7 patches on lore... https://lore.kernel.org/lkml/20210923033839.1421034-1-sas...@kernel.org/ Best regards, Pavel -- http://www.livejournal.com/~pavelmachek signature.asc Description: Digital signature
Re: [PATCH] Raise the minimum GCC version to 5.2
On Sun 2021-05-02 00:15:38, Masahiro Yamada wrote: > The current minimum GCC version is 4.9 except ARCH=arm64 requiring > GCC 5.1. Please don't. I'm still on 4.9 on machine I can't easily update, > Documentation/process/changes.rst | 2 +- > arch/arm64/Kconfig| 2 +- > arch/powerpc/Kconfig | 2 +- > arch/riscv/Kconfig| 2 +- > include/linux/compiler-gcc.h | 6 +- > lib/Kconfig.debug | 2 +- > scripts/min-tool-version.sh | 8 +--- > 7 files changed, 7 insertions(+), 17 deletions(-) and 10 lines of cleanups is really not worth that. Best regards, Pavel -- http://www.livejournal.com/~pavelmachek signature.asc Description: Digital signature
Re: [PATCH 11/20] Documentation: leds/ledtrig-transient: eliminate duplicated word
On Tue 2020-07-07 11:04:05, Randy Dunlap wrote: > Drop the doubled word "for". > > Signed-off-by: Randy Dunlap > Cc: Jonathan Corbet > Cc: linux-...@vger.kernel.org > Cc: Jacek Anaszewski Acked-by: Pavel Machek (I expect documentation people take this, not me). -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH v6 3/3] soc: fsl: add RCPM driver
Hi! > > + /* Begin with first registered wakeup source */ > > + ws = wakeup_source_get_start(); > > Since I have mad some change in this version, could you please take a look on > this. > If it's OK to you, I would re-add 'Acked-by: Pavel Machek ' I'm sorry, I'm a bit busy at the moment and this is not really my area. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH V2 3/3] soc: fsl: add RCPM driver
Hi! > > > > You are right, but the current code is "interesting". What about > > > > > > > > ws = NULL; > > > > while (ws = wakeup_source_get_next(NULL)) ... > > > > > > > > then? > > > > > > Did you mean: > > > ws = NULL; > > > while (ws = wakeup_source_get_next(ws)) ... > > > > > >Yes, that will be the same to my original logic, do you recommend > > > to change to this? :) > > > > Yes please. It will be less confusing to the reader. > > OK, if no other comment, I will work out v4, fix this and extra ',' > > > Thanks (and sorry for cross-talk), > > That's OK, thanks for your time. You can add Acked-by: Pavel Machek to that version. Best regards, Pavel -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany signature.asc Description: Digital signature
Re: [PATCH V2 3/3] soc: fsl: add RCPM driver
On Mon 2019-05-20 09:03:50, Ran Wang wrote: > Hi Pavel, > > On Monday, May 20, 2019 16:57, Pavel Machek wrote: > > > > Hi! > > > > > > > +static int rcpm_pm_prepare(struct device *dev) { > > > > > + struct device_node *np = dev->of_node; > > > > > + struct wakeup_source *ws; > > > > > + struct rcpm *rcpm; > > > > > + u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1], tmp; > > > > > + int i, ret; > > > > > + > > > > > + rcpm = dev_get_drvdata(dev); > > > > > + if (!rcpm) > > > > > + return -EINVAL; > > > > > + > > > > > + /* Begin with first registered wakeup source */ > > > > > + ws = wakeup_source_get_next(NULL); > > > > > + while (ws) { > > > > > > > > while (ws = wakeup_source_get_next(NULL)) ? > > > > > > Actually, we only pass NULL to wakeup_source_get_next() at very first > > > call to get 1st wakeup source. Then in the while loop, we will fetch > > > next source but not 1st, that's different. I am afraid your suggestion > > > is not quite correct. > > > > Sorry, I seen your next version before seeing this explanation. > > > > You are right, but the current code is "interesting". What about > > > > ws = NULL; > > while (ws = wakeup_source_get_next(NULL)) ... > > > > then? > > Did you mean: > ws = NULL; > while (ws = wakeup_source_get_next(ws)) ... > >Yes, that will be the same to my original logic, do you recommend to change > to this? :) Yes please. It will be less confusing to the reader. Thanks (and sorry for cross-talk), Pavel -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany signature.asc Description: Digital signature
Re: [PATCH V2 3/3] soc: fsl: add RCPM driver
Hi! > > > +static int rcpm_pm_prepare(struct device *dev) { > > > + struct device_node *np = dev->of_node; > > > + struct wakeup_source *ws; > > > + struct rcpm *rcpm; > > > + u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1], tmp; > > > + int i, ret; > > > + > > > + rcpm = dev_get_drvdata(dev); > > > + if (!rcpm) > > > + return -EINVAL; > > > + > > > + /* Begin with first registered wakeup source */ > > > + ws = wakeup_source_get_next(NULL); > > > + while (ws) { > > > > while (ws = wakeup_source_get_next(NULL)) ? > > Actually, we only pass NULL to wakeup_source_get_next() at very first > call to get 1st wakeup source. Then in the while loop, we will fetch > next source but not 1st, that's different. I am afraid your suggestion > is not quite correct. Sorry, I seen your next version before seeing this explanation. You are right, but the current code is "interesting". What about ws = NULL; while (ws = wakeup_source_get_next(NULL)) ... then? Best regards, Pavel -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany signature.asc Description: Digital signature
Re: [PATCH v3 3/3] soc: fsl: add RCPM driver
Hi! > The NXP's QorIQ Processors based on ARM Core have RCPM module > (Run Control and Power Management), which performs all device-level > tasks associated with power management such as wakeup source control. > > This driver depends on PM wakeup source framework which help to > collect wake information. > > Signed-off-by: Ran Wang > +// Copyright 2019 NXP > +// > +// Author: Ran Wang , extra , > + rcpm = dev_get_drvdata(dev); > + if (!rcpm) > + return -EINVAL; > + > + /* Begin with first registered wakeup source */ > + ws = wakeup_source_get_next(NULL); > + while (ws) { while (ws = wakeup_source_get_next(NULL)) ? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH V2 1/3] PM: wakeup: Add routine to help fetch wakeup source object.
> --- a/include/linux/pm_wakeup.h > @@ -70,6 +71,7 @@ struct wakeup_source { > unsigned long wakeup_count; > boolactive:1; > boolautosleep_enabled:1; > + struct device *attached_dev; > }; > > #ifdef CONFIG_PM_SLEEP You might want to format this similary to the rest... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH V2 3/3] soc: fsl: add RCPM driver
Hi! > + > +struct rcpm { > + unsigned int wakeup_cells; > + void __iomem *ippdexpcr_base; > + boollittle_endian; > +}; Inconsistent whitespace > +static int rcpm_pm_prepare(struct device *dev) > +{ > + struct device_node *np = dev->of_node; > + struct wakeup_source *ws; > + struct rcpm *rcpm; > + u32 value[RCPM_WAKEUP_CELL_MAX_SIZE + 1], tmp; > + int i, ret; > + > + rcpm = dev_get_drvdata(dev); > + if (!rcpm) > + return -EINVAL; > + > + /* Begin with first registered wakeup source */ > + ws = wakeup_source_get_next(NULL); > + while (ws) { while (ws = wakeup_source_get_next(NULL)) ? > +static int rcpm_probe(struct platform_device *pdev) > +{ > + struct device *dev = >dev; > + struct resource *r; > + struct rcpm *rcpm; > + int ret; Whitespace. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH v10 02/18] counter: Documentation: Add Generic Counter sysfs documentation
On Tue 2019-04-02 15:30:37, William Breathitt Gray wrote: > This patch adds standard documentation for the userspace sysfs > attributes of the Generic Counter interface. > > Reviewed-by: Jonathan Cameron > Signed-off-by: William Breathitt Gray > --- > Documentation/ABI/testing/sysfs-bus-counter | 230 > MAINTAINERS | 1 + > 2 files changed, 231 insertions(+) > create mode 100644 Documentation/ABI/testing/sysfs-bus-counter > > diff --git a/Documentation/ABI/testing/sysfs-bus-counter > b/Documentation/ABI/testing/sysfs-bus-counter > new file mode 100644 > index ..566bd99fe0a5 > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-bus-counter > @@ -0,0 +1,230 @@ > +What:/sys/bus/counter/devices/counterX/countY/count > +KernelVersion: 5.2 > +Contact: linux-...@vger.kernel.org > +Description: > + Count data of Count Y represented as a string. > + > +What:/sys/bus/counter/devices/counterX/countY/ceiling > +KernelVersion: 5.2 > +Contact: linux-...@vger.kernel.org > +Description: > + Count value ceiling for Count Y. This is the upper limit for the > + respective counter. > + > +What:/sys/bus/counter/devices/counterX/countY/floor > +KernelVersion: 5.2 > +Contact: linux-...@vger.kernel.org > +Description: > + Count value floor for Count Y. This is the lower limit for the > + respective counter. > + > +What:/sys/bus/counter/devices/counterX/countY/count_mode > +KernelVersion: 5.2 > +Contact: linux-...@vger.kernel.org > +Description: > + Count mode for channel Y. The ceiling and floor values for > + Count Y are used by the count mode where required. The following > + count modes are available: > + > + normal: > + Counting is continuous in either direction. > + > + range limit: > + An upper or lower limit is set, mimicking limit switches > + in the mechanical counterpart. The upper limit is set to > + the Count Y ceiling value, while the lower limit is set > + to the Count Y floor value. The counter freezes at > + count = ceiling when counting up, and at count = floor > + when counting down. At either of these limits, the > + counting is resumed only when the count direction is > + reversed. > + > + non-recycle: > + The counter is disabled whenever a counter overflow or > + underflow takes place. The counter is re-enabled when a > + new count value is loaded to the counter via a preset > + operation or direct write. > + > + modulo-n: > + A count value boundary is set between the Count Y floor > + value and the Count Y ceiling value. The counter is > + reset to the Count Y floor value at count = ceiling when > + counting up, while the counter is set to the Count Y > + ceiling value at count = floor when counting down; the > + counter does not freeze at the boundary points, but > + counts continuously throughout. > + > +What: > /sys/bus/counter/devices/counterX/countY/count_mode_available > +What: > /sys/bus/counter/devices/counterX/countY/error_noise_available > +What: > /sys/bus/counter/devices/counterX/countY/function_available > +What: > /sys/bus/counter/devices/counterX/countY/signalZ_action_available > +KernelVersion: 5.2 > +Contact: linux-...@vger.kernel.org > +Description: > + Discrete set of available values for the respective Count Y > + configuration are listed in this file. Values are delimited by > + newline characters. Elsewhere in sysfs we do space-separated: pavel@amd:~$ cat /sys/power/state freeze mem disk And we use [] to mark current selection: pavel@amd:~$ cat /sys/class/leds/tpacpi\:\:thinkvantage/trigger [none] bluetooth-power rfkill-any rfkill-none kbd-scrolllock kbd-numlock kbd-capslock kbd-kanalock kbd-shiftlock kbd-altgrlock kbd-ctrllock kbd-altlock kbd-shiftllock kbd-shiftrlock kbd-ctrlllock kbd-ctrlrlock AC-online BAT0-charging-or-full BAT0-charging BAT0-full BAT0-charging-blink-full-solid rfkill0 phy0rx phy0tx phy0assoc phy0radio phy0tpt mmc0 timer heartbeat audio-mute audio-micmute rfkill1 Note this only works if you have less than PAGE_SIZE of entries... and will never have more. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures)
Re: [PATCH v10 01/18] counter: Introduce the Generic Counter interface
Hi! > +const char *const counter_count_direction_str[2] = { > + [COUNTER_COUNT_DIRECTION_FORWARD] = "forward", > + [COUNTER_COUNT_DIRECTION_BACKWARD] = "backward" > +}; > +EXPORT_SYMBOL_GPL(counter_count_direction_str); > + > +const char *const counter_count_mode_str[4] = { > + [COUNTER_COUNT_MODE_NORMAL] = "normal", > + [COUNTER_COUNT_MODE_RANGE_LIMIT] = "range limit", > + [COUNTER_COUNT_MODE_NON_RECYCLE] = "non-recycle", > + [COUNTER_COUNT_MODE_MODULO_N] = "modulo-n" > +}; > +EXPORT_SYMBOL_GPL(counter_count_mode_str); Dunno. Exporting const tables saying "forward" and "backward". Can we ... somehow make it work without need to export this? -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH] [RFC v2] Drop all 00-INDEX files from Documentation/
Hi! > The 00-INDEX files are supposed to give a summary of all files present > in a directory, but these files are horribly out of date and their > usefulness is brought into question. Often a simple "ls" would reveal > the same information as the filenames are generally quite descriptive as > a short introduction to what the file covers (it should not surprise > -SAK.txt > - - info on Secure Attention Keys. > -SM501.txt > - - Silicon Motion SM501 multimedia companion chip > -btmrvl.txt > - - info on Marvell Bluetooth driver usage. Well, I don't know what sm501 is, but description helps me. SAK is similar. .. as is btmrvl. I'd say this is still quite valueable, and it might be worth fixing, rather then removing completely. And yes, moving stuff to subdirectories and naming files reasonably would help, too. "btmrvl" is really bad name... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.
On Fri 2018-03-30 12:07:58, Ilya Smith wrote: > Hi > > > On 30 Mar 2018, at 10:55, Pavel Machek <pa...@ucw.cz> wrote: > > > > Hi! > > > >> Current implementation doesn't randomize address returned by mmap. > >> All the entropy ends with choosing mmap_base_addr at the process > >> creation. After that mmap build very predictable layout of address > >> space. It allows to bypass ASLR in many cases. This patch make > >> randomization of address on any mmap call. > > > > How will this interact with people debugging their application, and > > getting different behaviours based on memory layout? > > > > strace, strace again, get different results? > > > > Honestly I’m confused about your question. If the only one way for debugging > application is to use predictable mmap behaviour, then something went wrong > in > this live and we should stop using computers at all. I'm not saying "only way". I'm saying one way, and you are breaking that. There's advanced stuff like debuggers going "back in time". Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [RFC PATCH v2 0/2] Randomization of address chosen by mmap.
Hi! > Current implementation doesn't randomize address returned by mmap. > All the entropy ends with choosing mmap_base_addr at the process > creation. After that mmap build very predictable layout of address > space. It allows to bypass ASLR in many cases. This patch make > randomization of address on any mmap call. How will this interact with people debugging their application, and getting different behaviours based on memory layout? strace, strace again, get different results? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: Linux 4.16: Reported regressions as of Monday, 2018-02-26 (Was: Linux 4.16-rc3)
Hi! > On Nokia N900:/dev/input/event6 aka AV Jack support disappeared > Status: quite new > Reported: 2018-02-24 > https://marc.info/?l=linux-omap=151950886524308=2 > Cause: 14e3e295b2b9 > Linux-Regression-ID: 4b650f This one is now solved in the mainline. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH] fix double ;;s in code
Hi! > > diff --git a/drivers/soc/imx/gpc.c b/drivers/soc/imx/gpc.c > > index 53f7275..cfb42f5 100644 > > --- a/drivers/soc/imx/gpc.c > > +++ b/drivers/soc/imx/gpc.c > > @@ -348,7 +348,7 @@ static int imx_gpc_old_dt_init(struct device *dev, > > struct regmap *regmap, > > if (i == 1) { > > domain->supply = devm_regulator_get(dev, "pu"); > > if (IS_ERR(domain->supply)) > > - return PTR_ERR(domain->supply);; > > + return PTR_ERR(domain->supply); > > > > ret = imx_pgc_get_clocks(dev, domain); > > if (ret) > > > > Considering the controversy how the changes should be merged, I'm going > to send a separate patch just for IMX GPC driver with a reported-by-you > tag. Thanks for catching this. That works for me. Alternatively... Andrew Morton merged the patch to his -mm tree (thanks!), so you don't need to take any action, and it will be eventually fixed. If you merge your own version of the patch, that's ok, too, since Andrew will just drop his version. Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH] fix double ;;s in code
On Mon 2018-02-19 16:41:35, Daniel Vetter wrote: > On Sun, Feb 18, 2018 at 11:00:56AM +0100, Christophe LEROY wrote: > > > > > > Le 17/02/2018 à 22:19, Pavel Machek a écrit : > > > > > > Fix double ;;'s in code. > > > > > > Signed-off-by: Pavel Machek <pa...@ucw.cz> > > > > A summary of the files modified on top of the patch would help understand > > the impact. > > > > A maybe there should be one patch by area, eg one for each arch specific > > modif and one for drivers/ and one for block/ ? > > Yeah, pls split this into one patch per area, with a suitable patch > subject prefix. Look at git log of each file to get a feeling for what's > the standard in each area. Yeah I can spend hour spliting it, and then people will ignore it anyway. If you care about one of the files being modified, please fix the bug, ";;" is a clear bug. If you don't care ... well I don't care either. drivers/gpu/ has four entries, i guess that's something for you. Pavel > > > diff --git a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > > > b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > > > index 61e8c3e..33d91e4 100644 > > > --- a/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > > > +++ b/drivers/gpu/drm/amd/display/dc/core/dc_link_dp.c > > > @@ -718,7 +718,7 @@ static enum link_training_result > > > perform_channel_equalization_sequence( > > > uint32_t retries_ch_eq; > > > enum dc_lane_count lane_count = > > > lt_settings->link_settings.lane_count; > > > union lane_align_status_updated dpcd_lane_status_updated = > > > {{0}}; > > > - union lane_status dpcd_lane_status[LANE_COUNT_DP_MAX] = {{{0}}};; > > > + union lane_status dpcd_lane_status[LANE_COUNT_DP_MAX] = {{{0}}}; > > > hw_tr_pattern = get_supported_tp(link); > > > diff --git a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > > > b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > > > index 4c3223a..adb6e7b 100644 > > > --- a/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > > > +++ b/drivers/gpu/drm/amd/powerplay/amd_powerplay.c > > > @@ -162,7 +162,7 @@ static int pp_hw_init(void *handle) > > > if(hwmgr->smumgr_funcs->start_smu(pp_handle->hwmgr)) { > > > pr_err("smc start failed\n"); > > > hwmgr->smumgr_funcs->smu_fini(pp_handle->hwmgr); > > > - return -EINVAL;; > > > + return -EINVAL; > > > } > > > if (ret == PP_DPM_DISABLED) > > > goto exit; > > > diff --git a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c > > > b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c > > > index 3e9bba4..6d8e3a9 100644 > > > --- a/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c > > > +++ b/drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.c > > > @@ -680,7 +680,7 @@ struct msm_kms *mdp5_kms_init(struct drm_device *dev) > > > } else { > > > dev_info(>dev, > > >"no iommu, fallback to phys contig buffers for > > > scanout\n"); > > > - aspace = NULL;; > > > + aspace = NULL; > > > } > > > pm_runtime_put_sync(>dev); > > > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c > > > b/drivers/gpu/drm/scheduler/gpu_scheduler.c > > > index 2c18996..0d95888 100644 > > > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c > > > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c > > > @@ -461,7 +461,7 @@ void drm_sched_hw_job_reset(struct drm_gpu_scheduler > > > *sched, struct drm_sched_jo > > > { > > > struct drm_sched_job *s_job; > > > struct drm_sched_entity *entity, *tmp; > > > - int i;; > > > + int i; > > > spin_lock(>job_list_lock); > > > list_for_each_entry_reverse(s_job, >ring_mirror_list, > > > node) { -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
[PATCH] fix double ;;s in code
Fix double ;;'s in code. Signed-off-by: Pavel Machek <pa...@ucw.cz> diff --git a/arch/arc/kernel/setup.c b/arch/arc/kernel/setup.c index 9d27331..ec12fe1 100644 --- a/arch/arc/kernel/setup.c +++ b/arch/arc/kernel/setup.c @@ -373,7 +373,7 @@ static void arc_chk_core_config(void) { struct cpuinfo_arc *cpu = _arc700[smp_processor_id()]; int saved = 0, present = 0; - char *opt_nm = NULL;; + char *opt_nm = NULL; if (!cpu->extn.timer0) panic("Timer0 is not present!\n"); diff --git a/arch/arc/kernel/unwind.c b/arch/arc/kernel/unwind.c index 333daab..183391d 100644 --- a/arch/arc/kernel/unwind.c +++ b/arch/arc/kernel/unwind.c @@ -366,7 +366,7 @@ static void init_unwind_hdr(struct unwind_table *table, return; ret_err: - panic("Attention !!! Dwarf FDE parsing errors\n");; + panic("Attention !!! Dwarf FDE parsing errors\n"); } #ifdef CONFIG_MODULES diff --git a/arch/arm/kernel/time.c b/arch/arm/kernel/time.c index 629f8e9..cf2701c 100644 --- a/arch/arm/kernel/time.c +++ b/arch/arm/kernel/time.c @@ -83,7 +83,7 @@ static void dummy_clock_access(struct timespec64 *ts) } static clock_access_fn __read_persistent_clock = dummy_clock_access; -static clock_access_fn __read_boot_clock = dummy_clock_access;; +static clock_access_fn __read_boot_clock = dummy_clock_access; void read_persistent_clock64(struct timespec64 *ts) { diff --git a/arch/arm64/kernel/ptrace.c b/arch/arm64/kernel/ptrace.c index 6618036..9ae31f7 100644 --- a/arch/arm64/kernel/ptrace.c +++ b/arch/arm64/kernel/ptrace.c @@ -1419,7 +1419,7 @@ static int compat_ptrace_hbp_get(unsigned int note_type, u64 addr = 0; u32 ctrl = 0; - int err, idx = compat_ptrace_hbp_num_to_idx(num);; + int err, idx = compat_ptrace_hbp_num_to_idx(num); if (num & 1) { err = ptrace_hbp_get_addr(note_type, tsk, idx, ); diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c index f0f5cd4..f9818d7 100644 --- a/arch/powerpc/kvm/book3s_xive.c +++ b/arch/powerpc/kvm/book3s_xive.c @@ -188,7 +188,7 @@ static int xive_provision_queue(struct kvm_vcpu *vcpu, u8 prio) if (!qpage) { pr_err("Failed to allocate queue %d for VCPU %d\n", prio, xc->server_num); - return -ENOMEM;; + return -ENOMEM; } memset(qpage, 0, 1 << xive->q_order); diff --git a/arch/powerpc/platforms/powernv/pci-ioda.c b/arch/powerpc/platforms/powernv/pci-ioda.c index 496e476..a6c92c7 100644 --- a/arch/powerpc/platforms/powernv/pci-ioda.c +++ b/arch/powerpc/platforms/powernv/pci-ioda.c @@ -1854,7 +1854,7 @@ static int pnv_pci_ioda_dma_set_mask(struct pci_dev *pdev, u64 dma_mask) s64 rc; if (WARN_ON(!pdn || pdn->pe_number == IODA_INVALID_PE)) - return -ENODEV;; + return -ENODEV; pe = >ioda.pe_array[pdn->pe_number]; if (pe->tce_bypass_enabled) { diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c index 353e20c..886a911 100644 --- a/arch/x86/boot/compressed/eboot.c +++ b/arch/x86/boot/compressed/eboot.c @@ -439,7 +439,7 @@ setup_uga32(void **uga_handle, unsigned long size, u32 *width, u32 *height) struct efi_uga_draw_protocol *uga = NULL, *first_uga; efi_guid_t uga_proto = EFI_UGA_PROTOCOL_GUID; unsigned long nr_ugas; - u32 *handles = (u32 *)uga_handle;; + u32 *handles = (u32 *)uga_handle; efi_status_t status = EFI_INVALID_PARAMETER; int i; @@ -484,7 +484,7 @@ setup_uga64(void **uga_handle, unsigned long size, u32 *width, u32 *height) struct efi_uga_draw_protocol *uga = NULL, *first_uga; efi_guid_t uga_proto = EFI_UGA_PROTOCOL_GUID; unsigned long nr_ugas; - u64 *handles = (u64 *)uga_handle;; + u64 *handles = (u64 *)uga_handle; efi_status_t status = EFI_INVALID_PARAMETER; int i; diff --git a/block/sed-opal.c b/block/sed-opal.c index 9ed51d0c..e4929ee 100644 --- a/block/sed-opal.c +++ b/block/sed-opal.c @@ -490,7 +490,7 @@ static int opal_discovery0_end(struct opal_dev *dev) if (!found_com_id) { pr_debug("Could not find OPAL comid for device. Returning early\n"); - return -EOPNOTSUPP;; + return -EOPNOTSUPP; } dev->comid = comid; diff --git a/drivers/clocksource/mips-gic-timer.c b/drivers/clocksource/mips-gic-timer.c index a04808a..65e18c8 100644 --- a/drivers/clocksource/mips-gic-timer.c +++ b/drivers/clocksource/mips-gic-timer.c @@ -205,12 +205,12 @@ static int __init gic_clocksource_of_init(struct device_node *node) } else if (of_property_read_u32(node, "clock-frequency", _frequency)) { pr_err("GIC frequency not specifi
Re: [PATCH v3 1/2] livepatch: send a fake signal to all blocking tasks
Hi! > --- a/Documentation/ABI/testing/sysfs-kernel-livepatch > +++ b/Documentation/ABI/testing/sysfs-kernel-livepatch > @@ -33,6 +33,15 @@ Contact: live-patch...@vger.kernel.org > An attribute which indicates whether the patch is currently in > transition. > > +What:/sys/kernel/livepatch//signal > +Date:Oct 2017 > +KernelVersion: 4.15.0 > +Contact: live-patch...@vger.kernel.org > +Description: > + A writable attribute that allows administrator to affect the > + course of an existing transition. Writing 1 sends a signal to > + all remaining blocking tasks. What kind of signal? > What:/sys/kernel/livepatch// > Date:Nov 2014 > KernelVersion: 3.19.0 > diff --git a/Documentation/livepatch/livepatch.txt > b/Documentation/livepatch/livepatch.txt > index ecdb18104ab0..6694530d0894 100644 > --- a/Documentation/livepatch/livepatch.txt > +++ b/Documentation/livepatch/livepatch.txt > @@ -178,6 +178,12 @@ transition, it shows -1. Any tasks which are blocking > the transition > can be signaled with SIGSTOP and SIGCONT to force them to change their > patched state. > > +Administrator can also affect a transition through > +/sys/kernel/livepatch//signal attribute. Writing 1 to the attribute > sends > +a signal to all remaining blocking tasks. This is an alternative for > +SIGSTOP/SIGCONT approach mentioned in the previous paragraph. It should also > be > +less harmful to the system. Well... If SIGSTOP / SIGCONT is considered harmful (it probably is), it should be mentioned above, and not in note here... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH] leds-PowerNV: Delete an error message for a failed memory allocation in powernv_led_create()
On Sun 2017-08-27 22:10:08, SF Markus Elfring wrote: > From: Markus Elfring <elfr...@users.sourceforge.net> > Date: Sun, 27 Aug 2017 22:00:22 +0200 > > Omit an extra message for a memory allocation failure in this function. > > This issue was detected by using the Coccinelle software. > > Signed-off-by: Markus Elfring <elfr...@users.sourceforge.net> Acked-by: Pavel Machek <pa...@ucw.cz> -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: Linux 4.13: Reported regressions as of Sunday, 2017-08-06
Hi! > Hi! Find below my second regression report for Linux 4.13. It lists 10 > regressions I'm currently aware of (albeit in one case it's not entirely > clear yet if it's a regression in 4.13). One regression got fixed since > last weeks report. You can also find the report at > http://bit.ly/lnxregrep413 where I try to update it every now and then. > > As always: Are you aware of any other regressions? Then please let me > know. For details see http://bit.ly/lnxregtrackid And please tell me if > there is anything in the report that shouldn't be there. There's compile-time regression in et8ek8, with patch available. On 2017-06-08 02:01, Arnd Bergmann wrote: > This one got applied twice, causing a build error with clang: > > drivers/media/i2c/et8ek8/et8ek8_driver.c:1499:1: error: redefinition > of '__mod_of__et8ek8_of_table_device_table' > > Fixes: 9ae05fd1e791 ("[media] et8ek8: Export OF device ID as module aliases") > Signed-off-by: Arnd Bergmann <a...@arndb.de> > Acked-by: Sakari Ailus <sakari.ai...@linux.intel.com> > Acked-by: Pavel Machek <pa...@ucw.cz> Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [RFC PATCH 0/7 v1] powerpc: Memory Protection Keys
Hi! > Memory protection keys enable applications to protect its > address space from inadvertent access or corruption from > itself. > > The overall idea: > > A process allocates a key and associates it with > a address range withinits address space. > The process than can dynamically set read/write > permissions on the key without involving the > kernel. Any code that violates the permissions > off the address space; as defined by its associated > key, will receive a segmentation fault. Do you have some documentation how userspace should use this? Will it be possible to hide details in libc so that it works across architectures? Do you have some kind of library that hides them? Where would you like it to be used? Web browsers? How does it interact with ptrace()? With /dev/mem? With /proc/XXX/mem? Will it enable malware to become very hard to understand? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH] powerpc/sequoia: fix NAND partitions not to overlap
Hi! > >>> > > Signed-off-by: Pavel Machek <pa...@ucw.cz> > >>> > > >>> > Ping? Two partitions at same place are bad news... > >>> > >>> Please expand on "bad news"? What are the runtime effects of this > >>> change? Decisions about which kernel(s) to patch depend on this info. > >> > >> Well... two partitions at same place. If you use one, you will corrupt > >> information on the other one. > >> > >> OTOH this moves partition around (so that they don't overlap) so it is > >> probably not stable candidate. > >> > >> I guess this is not huge issue; people using these boards probably > >> have custom dts changes, anyway... > > > > Or no one's even using it anymore. > > > > I can take this via powerpc, I won't mark it for stable etc. > > It became: > > Author: Pavel Machek <pa...@denx.de> > > powerpc/sequoia: Fix NAND partitions not to overlap > > Currently the DTS defines two partitions at the same addresses, if you > use one, you will corrupt information on the other one. Fix it by > shifting the second partition. > > Signed-off-by: Pavel Machek <pa...@ucw.cz> > [mpe: Reconstruct change log from email thread] > Signed-off-by: Michael Ellerman <m...@ellerman.id.au> > > > Minor nit, your From: address doesn't match your Signed-off-by: address, > which trips my "check patch is signed-off-by author" script. I can > just ignore it, but I think it's preferable if they match. Sorry about that (I have automatic scripts changing From:). It is still me, but I guess denx should get the credit here, so you can use. Signed-off-by: Pavel Machek <pa...@denx.de> Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH] powerpc/sequoia: fix NAND partitions not to overlap
Hi! > > Well... two partitions at same place. If you use one, you will corrupt > > information on the other one. > > > > OTOH this moves partition around (so that they don't overlap) so it is > > probably not stable candidate. > > > > I guess this is not huge issue; people using these boards probably > > have custom dts changes, anyway... > > Or no one's even using it anymore. > > I can take this via powerpc, I won't mark it for stable etc. That sounds good. Thanks, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH] powerpc/sequoia: fix NAND partitions not to overlap
On Wed 2017-05-17 14:37:17, Andrew Morton wrote: > On Wed, 17 May 2017 14:06:13 +0200 Pavel Machek <pa...@denx.de> wrote: > > > On Sun 2017-04-02 12:05:36, Pavel Machek wrote: > > > Fix overlapping NAND partitions. > > > > > > Signed-off-by: Pavel Machek <pa...@ucw.cz> > > > > Ping? Two partitions at same place are bad news... > > Please expand on "bad news"? What are the runtime effects of this > change? Decisions about which kernel(s) to patch depend on this info. Well... two partitions at same place. If you use one, you will corrupt information on the other one. OTOH this moves partition around (so that they don't overlap) so it is probably not stable candidate. I guess this is not huge issue; people using these boards probably have custom dts changes, anyway... Best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: [PATCH] powerpc/sequoia: fix NAND partitions not to overlap
On Sun 2017-04-02 12:05:36, Pavel Machek wrote: > Fix overlapping NAND partitions. > > Signed-off-by: Pavel Machek <pa...@ucw.cz> Ping? Two partitions at same place are bad news... Pavel > diff --git a/arch/powerpc/boot/dts/sequoia.dts > b/arch/powerpc/boot/dts/sequoia.dts > index b1d3292..e41b88a 100644 > --- a/arch/powerpc/boot/dts/sequoia.dts > +++ b/arch/powerpc/boot/dts/sequoia.dts > @@ -229,7 +229,7 @@ > }; > partition@84000 { > label = "user"; > - reg = <0x > 0x01f7c000>; > + reg = <0x00084000 > 0x01f7c000>; > }; > }; > }; > > > -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
[PATCH] powerpc/sequoia: fix NAND partitions not to overlap
Fix overlapping NAND partitions. Signed-off-by: Pavel Machek <pa...@ucw.cz> diff --git a/arch/powerpc/boot/dts/sequoia.dts b/arch/powerpc/boot/dts/sequoia.dts index b1d3292..e41b88a 100644 --- a/arch/powerpc/boot/dts/sequoia.dts +++ b/arch/powerpc/boot/dts/sequoia.dts @@ -229,7 +229,7 @@ }; partition@84000 { label = "user"; - reg = <0x 0x01f7c000>; + reg = <0x00084000 0x01f7c000>; }; }; }; -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html signature.asc Description: Digital signature
Re: Linux 4.8: Reported regressions as of Sunday, 2016-08-28
Hi! > Hi! Here is my second regression report for Linux 4.8. It lists 11 > regressions. 5 of them are new; 5 mentioned in the last report two > weeks ago got fixed. > > FWIW: A small detail: I did not include "Regression - SATA disks behind > USB ones on v4.8-rc1, breaking boot. [Re: Who reordered my disks]" > (http://www.spinics.net/lists/linux-usb/msg144871.html ) in below list > report. The discussion mentions that device names like /dev/sd? are not > considered stable as they might change depending on various factors -- > like the order in which modules are loaded or other timing issues (like > in this case). That is how it is afaik (even if it's not well known), > and that's why I didn't include the issue; let me know if you think it > should be on the list. > > OTOH I included "Commit cb4f71c429 deliberately changes order of > network interfaces" (http://www.spinics.net/lists/kernel/msg2325600.html ) > for now, as I think traditional network interface names (eth0, eth1, ...) > might be considered stable -- but I'm not sure, that's why I raise it > here. > > Anyway, you know the drill: Are you aware of any other regressions? > Then please let me know. And tell me if there is anything in the > report that shouldn't be there. > > Ciao, Thorsten > > P.S.: Thanks to all those that Aaro Koskinen, Hans de Goede, Pavel > Machek for CCing me when reporting regressions. Much appreciated! Ohh, > and thx to all those that replied when I asked them for status updates > when things look stuck. Hmm, and there's one more apparently. See Date: Tue, 13 Sep 2016 22:38:45 +0200 From: Martin Steigerwald <mar...@lichtvoll.de> To: Pavel Machek <pa...@ucw.cz> Cc: kernel list <linux-ker...@vger.kernel.org>, daniel.vet...@intel.com, jani.nik...@linux.intel.com, intel-...@lists.freedesktop.org, dri-de...@lists.freedesktop.org, "Rafael J. Wysocki" <r...@rjwysocki.net> Subject: Re: 4.8-rc1: it is now common that machine needs re-run of xrandr after resume User-Agent: KMail/5.2.3 (Linux/4.8.0-rc6-tp520-btrfstrim+; KDE/5.25.0; x86_64; ; ) I'm glad I'm not the only one seeing it, but I don't have idea how to actually debug it. Thanks and best regards, Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
Re: [PATCH 01/44] kernel: Add support for poweroff handler call chain
Hi! +/** + * register_poweroff_handler_simple - Register function to be called to power off + * the system + * @handler: Function to be called to power off the system + * @priority: Handler priority. For priority guidelines see + * register_poweroff_handler. + * + * This is a simplified version of register_poweroff_handler. It does not + * take a notifier as argument, but a function pointer. The function + * registers a poweroff handler with specified priority. Poweroff + * handlers registered with this function can not be unregistered, + * and only a single poweroff handler can be installed using it. + * + * This function must not be called from modules and is therefore + * not exported. + * + * Returns -EBUSY if a poweroff handler has already been registered + * using register_poweroff_handler_simple. Otherwise returns zero, + * since atomic_notifier_chain_register() currently always returns zero. + */ +int register_poweroff_handler_simple(void (*handler)(void), int priority) +{ + char symname[KSYM_NAME_LEN]; + + if (poweroff_handler_data.handler) { + lookup_symbol_name((unsigned long)poweroff_handler_data.handler, +symname); + pr_warn(Poweroff function already registered (%s), symname); + lookup_symbol_name((unsigned long)handler, symname); + pr_cont(, cannot register %s\n, symname); + return -EBUSY; + } Dunno, are you maybe overdoing the debugging infrastructure a bit? This is not going to happen in production, and if it does happen, developer can look the symbol name himself. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 03/44] hibernate: Call have_kernel_poweroff instead of checking pm_power_off
On Mon 2014-10-06 22:28:05, Guenter Roeck wrote: Poweroff handlers may now be installed with register_poweroff_handler. Use the new API function have_kernel_poweroff to determine if a poweroff handler has been installed. Cc: Rafael J. Wysocki r...@rjwysocki.net Cc: Pavel Machek pa...@ucw.cz Cc: Len Brown len.br...@intel.com Signed-off-by: Guenter Roeck li...@roeck-us.net --- kernel/power/hibernate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kernel/power/hibernate.c b/kernel/power/hibernate.c index a9dfa79..20353c5 100644 --- a/kernel/power/hibernate.c +++ b/kernel/power/hibernate.c @@ -602,7 +602,7 @@ static void power_down(void) case HIBERNATION_PLATFORM: hibernation_platform_enter(); case HIBERNATION_SHUTDOWN: - if (pm_power_off) + if (have_kernel_poweroff()) kernel_power_off(); break; poweroff - power_off. But if you are playing with this, anyway... does it make sense to introduce kernel_power_off() that just works, no need to check have_..? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 08/44] kernel: Move pm_power_off to common code
Hi! @@ -184,6 +179,8 @@ machine_halt(void) void machine_power_off(void) { + do_kernel_poweroff(); + poweroff - power_off for consistency. index c4f50a3..1da27d1 100644 --- a/arch/blackfin/kernel/reboot.c +++ b/arch/blackfin/kernel/reboot.c @@ -106,6 +107,7 @@ void machine_halt(void) __attribute__((weak)) void native_machine_power_off(void) { + do_kernel_poweroff(); idle_with_irq_disabled(); } So here we handle do_kernel_poweroff() returning, diff --git a/arch/cris/kernel/process.c b/arch/cris/kernel/process.c index b78498e..eaafad0 100644 --- a/arch/cris/kernel/process.c +++ b/arch/cris/kernel/process.c @@ -60,6 +57,7 @@ void machine_halt(void) void machine_power_off(void) { + do_kernel_poweroff(); } Here we don't. diff --git a/arch/frv/kernel/process.c b/arch/frv/kernel/process.c index 5d40aeb77..a673725 100644 --- a/arch/frv/kernel/process.c +++ b/arch/frv/kernel/process.c @@ -107,6 +104,8 @@ void machine_power_off(void) gdbstub_exit(0); #endif + do_kernel_poweroff(); + for (;;); } And here we do. What is right? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 08/44] kernel: Move pm_power_off to common code
Hi! @@ -184,6 +179,8 @@ machine_halt(void) void machine_power_off(void) { + do_kernel_poweroff(); + poweroff - power_off for consistency. Dunno; matter of personal preference. I started with that, but ultimately went with poweroff to distinguish poweroff handler functions from existing code, specifically kernel_power_off(). That works for you, but once it is merged, it is ugly/confusing typo. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: bit fields data tearing
On Fri 2014-09-05 12:17:16, Peter Hurley wrote: On 09/05/2014 08:37 AM, David Laight wrote: From: Peter Hurley On 09/05/2014 04:30 AM, David Laight wrote: I've seen gcc generate 32bit accesses for 16bit structure members on arm. It does this because of the more limited range of the offsets for the 16bit access. OTOH I don't know if it ever did this for writes - so it may be moot. Can you recall the particulars, like what ARM config or what code? I tried an overly-simple test to see if gcc would bump up to the word load for the 12-bit offset mode, but it stuck with register offset rather than immediate offset. [I used the compiler options for allmodconfig and a 4.8 cross-compiler.] Maybe the test doesn't generate enough register pressure on the compiler? Dunno, I would have been using a much older version of the compiler. It is possible that it doesn't do it any more. It might only have done it for loads. The compiler used to use misaligned 32bit loads for structure members on large 4n+2 byte boundaries as well. I'm pretty sure it doesn't do that either. There have been a lot of compiler versions since I was compiling anything for arm. Yeah, it seems gcc for ARM no longer uses the larger operand size as a substitute for 12-bit immediate offset addressing mode, even for reads. While this test: struct x { short b[12]; }; short load_b(struct x *p) { return p-b[8]; } generates the 8-bit immediate offset form, short load_b(struct x *p) { 0: e1d001f0ldrsh r0, [r0, #16] 4: e12fff1ebx lr pushing the offset out past 256: struct x { long unused[64]; short b[12]; }; short load_b(struct x *p) { return p-b[8]; } generates the register offset addressing mode instead of 12-bit immediate: short load_b(struct x *p) { 0: e3a03e11mov r3, #272; 0x110 4: e19000f3ldrsh r0, [r0, r3] 8: e12fff1ebx lr If we rely on new gcc features for correctness, does minimum compiler version need to be updated? -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/7] fault-injection: notifier error injection
+ for (action = enb-actions; action-name; action++) { + struct dentry *file = debugfs_create_int(action-name, mode, + enb-dir, action-error); + + if (!file) { + debugfs_remove_recursive(enb-dir); + return -ENOMEM; + } Few lines how this work would be welcome...? -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 3/5] v2 seccomp_filters: Enable ftrace-based system call filtering
On Mon 2011-05-16 10:36:05, James Morris wrote: On Fri, 13 May 2011, Ingo Molnar wrote: How do you reason about the behavior of the system as a whole? I argue that this is the LSM and audit subsystems designed right: in the long run it could allow everything that LSM does at the moment - and so much more ... Now you're proposing a redesign of the security subsystem. That's a significant undertaking. In the meantime, we have a simple, well-defined enhancement to seccomp which will be very useful to current users in reducing their kernel attack surface. Well, you can do the same with subterfugue, even without kernel changes. But that's ptrace -- slow. (And it already shows that syscall based filters are extremely tricky to configure). If yu want speed, seccomp+server for non-permitted operations seems like reasonable way. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: register long sp asm(r1) incorrect
On Mon 2010-02-15 14:15:17, H. Peter Anvin wrote: On 02/15/2010 01:04 PM, Benjamin Herrenschmidt wrote: It's true that most other use of it we have are global scope (local_paca in r13, glibc use of r2/r13, etc...) afaik, but since r1 itself is the stack pointer always, I think they pretty much guarantee it works. It should work, because r1, being the stack pointer, is already marked a reserved register in gcc. The reference Pavel is citing bascially states that gcc won't globally reserve the register, which is true, but it is already reserved anyway. Ok, thanks for clarification. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: register long sp asm(r1) incorrect
On Tue 2010-02-16 06:59:52, Benjamin Herrenschmidt wrote: On Mon, 2010-02-15 at 08:34 +0100, Pavel Machek wrote: On Tue, 2010-02-09 at 16:24 +0100, Pavel Machek wrote: ...according to gcc docs, sp should be global, or placement in register is not guaranteed (except at asm boundaries, but there are none). Sorry I'm not sure I grok what you mean. Well, according to gcc doscs and my experience, local register int __asm() variables only work by accident (or not at all). Hrm... we definitely rely on that for our thread_info() access, and so far it has worked well for us, but I'll poke our gcc folks just in case. Thanks, and let me know about any results. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: register long sp asm(r1) incorrect
On Tue, 2010-02-09 at 16:24 +0100, Pavel Machek wrote: ...according to gcc docs, sp should be global, or placement in register is not guaranteed (except at asm boundaries, but there are none). Sorry I'm not sure I grok what you mean. Well, according to gcc doscs and my experience, local register int __asm() variables only work by accident (or not at all). Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [v9 PATCH 4/9]: x86: refactor x86 idle power management code and remove all instances of pm_idle.
+static int local_idle_loop(struct cpuidle_device *dev, struct cpuidle_state *st) +{ + ktime_t t1, t2; + s64 diff; + int ret; + + t1 = ktime_get(); + local_idle(); + t2 = ktime_get(); + + diff = ktime_to_us(ktime_sub(t2, t1)); + if (diff INT_MAX) + diff = INT_MAX; + ret = (int) diff; + + return ret; +} So we get this routine essentially 3 times. Is there no way to share the code? We can move this code to a common place, but that would mean exporting the idle function pointer to be called from within this routine, which is exactly what we wanted to avoid. Any suggestions are welcome. You can just pass idle routine as a parameter...? int common_idle_loop(struct cpuidle_device *dev, struct cpuidle_state *st, void *idle(void)) ...? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [v9 PATCH 4/9]: x86: refactor x86 idle power management code and remove all instances of pm_idle.
On Fri 2009-10-16 15:13:08, Arun R Bharadwaj wrote: * Arun R Bharadwaj a...@linux.vnet.ibm.com [2009-10-16 15:08:50]: This patch cleans up x86 of all instances of pm_idle. pm_idle which was earlier called from cpu_idle() idle loop is replaced by cpuidle_idle_call. x86 also registers to cpuidle when the idle routine is selected, by populating the cpuidle_device data structure for each cpu. This is replicated for apm module and for xen, which also used pm_idle. Signed-off-by: Arun R Bharadwaj a...@linux.vnet.ibm.com --- arch/x86/kernel/apm_32.c | 55 - arch/x86/kernel/process.c | 90 -- arch/x86/kernel/process_32.c |3 - arch/x86/kernel/process_64.c |3 - arch/x86/xen/setup.c | 40 ++ drivers/acpi/processor_core.c |9 ++-- drivers/cpuidle/cpuidle.c | 16 +-- 7 files changed, 182 insertions(+), 34 deletions(-) ... +static int local_idle_loop(struct cpuidle_device *dev, struct cpuidle_state *st) +{ + ktime_t t1, t2; + s64 diff; + int ret; + + t1 = ktime_get(); + local_idle(); + t2 = ktime_get(); + + diff = ktime_to_us(ktime_sub(t2, t1)); + if (diff INT_MAX) + diff = INT_MAX; + ret = (int) diff; + + return ret; +} So we get this routine essentially 3 times. Is there no way to share the code? -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v3 0/3] cpu: pseries: Cpu offline states framework
On Tue 2009-09-15 14:11:41, Peter Zijlstra wrote: On Tue, 2009-09-15 at 17:36 +0530, Gautham R Shenoy wrote: This patchset contains the offline state driver implemented for pSeries. For pSeries, we define three available_hotplug_states. They are: online: The processor is online. offline: This is the the default behaviour when the cpu is offlined even in the absense of this driver. The CPU would call make an rtas_stop_self() call and hand over the CPU back to the resource pool, thereby effectively deallocating that vCPU from the LPAR. NOTE: This would result in a configuration change to the LPAR which is visible to the outside world. inactive: This cedes the vCPU to the hypervisor with a cede latency specifier value 2. NOTE: This option does not result in a configuration change and the vCPU would be still entitled to the LPAR to which it earlier belong to. Any feedback on the patchset will be immensely valuable. I still think its a layering violation... its the hypervisor manager that should be bothered in what state an off-lined cpu is in. Agreed. Proposed interface is ugly. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH v2 0/2] cpu: pseries: Offline state framework.
On Wed 2009-09-02 07:33:31, Peter Zijlstra wrote: On Fri, 2009-08-28 at 15:30 +0530, Gautham R Shenoy wrote: Hi, This is the version 2 of the patch series to provide a cpu-offline framework that enables the administrators choose the state the offline CPU must be put into when multiple such states are exposed by the underlying architecture. Version 1 of the Patch can be found here: http://lkml.org/lkml/2009/8/6/236 The patch-series exposes the following sysfs tunables to allow the system-adminstrator to choose the state of a CPU: To query the available hotplug states, one needs to read the sysfs tunable: /sys/devices/system/cpu/cpunumber/available_hotplug_states To query or set the current state, on needs to read/write the sysfs tunable: /sys/devices/system/cpu/cpunumber/current_states The patchset ensures that the writes to the current_state sysfs file are serialized against the writes to the online file. This patchset also contains the offline state driver implemented for pSeries. For pSeries, we define three available_hotplug_states. They are: online: The processor is online. deallocate: This is the the default behaviour when the cpu is offlined even in the absense of this driver. The CPU would call make an rtas_stop_self() call and hand over the CPU back to the resource pool, thereby effectively deallocating that vCPU from the LPAR. NOTE: This would result in a configuration change to the LPAR which is visible to the outside world. deactivate: This cedes the vCPU to the hypervisor which in turn can put the vCPU time to the best use. NOTE: This option DOES NOT result in a configuration change and the vCPU would be still entitled to the LPAR to which it earlier belong to. Awaiting your feedback. I'm still thinking this is a bad idea. The OS should only know about online/offline. Use the hypervisor interface to deal with the cpu once its offline. That is, I think this interface you propose is a layering violation. Agreed. Plus having interface like 'go to this state during offliine' then 'go offline' is strange/stupid. For hypervisor case, you might want to change 'state' of cpu that is already offline. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/3] cpu: idle state framework for offline CPUs.
2. A low-power state where the guest indicates it doesn't need the CPU (and can be put in low power state) but doesn't want to give up its allocated cpu share. IOW, no visible configuration changes. So, in any case we would probably want more than one states. How are #1 and #2 different when the hypervisor gets control in all idle states? I assert that they are the same, and thus 1 state will suffice. It depends on the hypervisor implementation. On pseries (powerpc) hypervisor, for example, they are different. By offlining a vcpu (and in turn shutting a cpu), you will actually create a configuration change in the VM that is visible to other systems management tools which may not be what the system administrator wanted. Ideally, we would like to distinguish between these two states. Hope that suffices as an example. So... you have something like physically pulling out hotplug cpu on powerpc. But maybe it is useful to take already offline cpus (from linux side), and make that visible to hypervisor, too. So maybe something like echo 1 /sys/devices/system/cpu/cpu1/unplug would be more useful for hypervisor case? -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/3] cpu: idle state framework for offline CPUs.
Hi! May be having (to pick a number) 3 possible offline states for all platforms with one for halt equivalent and one for deepest possible that CPU can handle and one for deepest possible that platform likes for C-states may make sense. Will keeps things simpler in terms of usage expectations and possibly reduce the misuse oppurtubity? Maybe just going to the deepest offline state automatically is the easiest option? cpu hotplug/unplug should be rare-enough operation that the latencies do not really matter, right? Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/3] cpu: idle state framework for offline CPUs.
On Sun 2009-08-09 15:22:02, Rafael J. Wysocki wrote: On Sunday 09 August 2009, Pavel Machek wrote: Hi! Also, approaches such as [1] can make use of this extended infrastructure instead of putting the CPU to an arbitrary C-state when it is offlined, thereby providing the system administrator a rope to hang himself with should he feel the need to do so. I didn't see the reason why administrator needs to know which state offline cpu should stay. Don't know about powerpc side, but in x86 side, it appears deepest C-state is already preferred. Agreed, deepest c-state is always best, there's no need to make it configurable. Unless it doesn't work. If it does not work, machine will not boot. We already have max_cstate= kernel command line option to work around that... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/3] cpu: idle state framework for offline CPUs.
Hi! Also, approaches such as [1] can make use of this extended infrastructure instead of putting the CPU to an arbitrary C-state when it is offlined, thereby providing the system administrator a rope to hang himself with should he feel the need to do so. I didn't see the reason why administrator needs to know which state offline cpu should stay. Don't know about powerpc side, but in x86 side, it appears deepest C-state is already preferred. Yes that is what we would expect, but the deepest sleep state may be restricted by BIOS or other system level parameters. This was the main objection to Venki's deepest sleep state for offline cpus patch. 'May be restricted'? Kernel already needs to know about that restriction, just do the right thing... -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 0/3] cpu: idle state framework for offline CPUs.
Hi! Also, approaches such as [1] can make use of this extended infrastructure instead of putting the CPU to an arbitrary C-state when it is offlined, thereby providing the system administrator a rope to hang himself with should he feel the need to do so. I didn't see the reason why administrator needs to know which state offline cpu should stay. Don't know about powerpc side, but in x86 side, it appears deepest C-state is already preferred. Agreed, deepest c-state is always best, there's no need to make it configurable. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 2/2] x86-64: seccomp: fix 32/64 syscall hole
On Thu 2009-05-07 12:11:29, Ingo Molnar wrote: * Nicholas Miell nmi...@comcast.net wrote: On Wed, 2009-05-06 at 15:21 -0700, Markus Gutschke (?) wrote: On Wed, May 6, 2009 at 15:13, Ingo Molnar mi...@elte.hu wrote: doing a (per arch) bitmap of harmless syscalls and replacing the mode1_syscalls[] check with that in kernel/seccomp.c would be a pretty reasonable extension. (.config controllable perhaps, for old-style-seccomp) It would probably be faster than the current loop over mode1_syscalls[] as well. This would be a great option to improve performance of our sandbox. I can detect the availability of the new kernel API dynamically, and then not intercept the bulk of the system calls. This would allow the sandbox to work both with existing and with newer kernels. We'll post a kernel patch for discussion in the next few days, I suspect the correct thing to do would be to leave seccomp mode 1 alone and introduce a mode 2 with a less restricted set of system calls -- the interface was designed to be extended in this way, after all. Yes, that is what i alluded to above via the '.config controllable' aspect. Mode 2 could be implemented like this: extend prctl_set_seccomp() with a bitmap pointer, and copy it to a per task seccomp context structure. a bitmap for 300 syscalls takes only about 40 bytes. Please take care to implement nesting properly: if a seccomp context does a seccomp call (which mode 2 could allow), then the resulting bitmap should be the logical-AND of the parent and child bitmaps. There's no reason why seccomp couldnt be used in hiearachy of sandboxes, in a gradually less permissive fashion. I don't think seccomp nesting (at kernel level) has any value. First, syscalls are wrong level of abstraction for sandboxing. There are multiple ways to read from file, for example. If you wanted to do hierarchical sandboxes, asking your monitor to restrict your seccomp mask would seem like a way to go... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: suspend-to-mem on the mpc8349e-mitx-gp?
On Wed 2009-03-25 18:42:41, Li Yang wrote: On Tue, Mar 24, 2009 at 12:54 AM, Scott Wood scottw...@freescale.com wrote: On Sun, Mar 22, 2009 at 10:45:23PM -0700, Li Yang-R58472 wrote: I don't think so, in this case. The user is not asking for sleep or deep sleep; they are asking for a power state that meets the definition of standby (which sleep does) or which meets the definition of mem (which both sleep and deep sleep do). When the user asks for mem, we provide the lowest power mode that qualifies. In my understanding, mem which is suspend-to-ram means all CPU states and registers are kept in memory and the CPU is completely off during suspension. I don't think the sleep mode of 8349 qualifies, does it? Is there a difference visible to software or to the user (other than not achieving power savings that the board does not support)? It seems simpler for userspace to just specify the heaviest sleep state it wants deal with (though some feedback to an administrator of what actually happens would be nice). I agree that it's handy to have a sleep state in kernel to automatically enter the heaviest sleep state supported. However it is also very simple for user space script or application to check the available states first and then enter explicitly the heaviest sleep state. Pavel, what's the preferred way for current PM sub-system? If you have single sleep state, use mem /sys/power/state. If you have two, use mem and standby. Do you have more? And if we want to be really pedantic, neither sleep nor deep sleep meet the definitions for either standby or mem, because they specify acceptable latency ranges in seconds, and (in the absence of a disk) we are much faster than that (it doesn't say up to 1-2 seconds). :-) Are there any existing suspend drivers that suppord standby but not mem? I see omap1 as a counterexample that treats them both the same. -Scott -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: suspend-to-mem on the mpc8349e-mitx-gp?
On Wed 2009-03-25 11:31:57, Scott Wood wrote: Pavel Machek wrote: Pavel, what's the preferred way for current PM sub-system? If you have single sleep state, use mem /sys/power/state. If you have two, use mem and standby. Do you have more? Some of our chips have two, and some have one. However, the sleep state of the chips that have only one is the same as the standby state of the chips that have two, not the mem state. Accepting mem and only Ok, I guess it makes sense to use standby for the single sleep state, then... It is not really a big deal, anyway. mem for the one-state chip seems like the most confusing of the options discussed. There would be no consistent way for userspace to request the milder suspend state. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 4/4] leds: Let GPIO LEDs keep their current state
On Thu 2008-11-20 17:05:56, Trent Piepho wrote: On Mon, 17 Nov 2008, Richard Purdie wrote: On Fri, 2008-10-24 at 16:09 -0700, Trent Piepho wrote: + if (template-keep_state) + state = !!gpio_get_value(led_dat-gpio) ^ led_dat-active_low; + else + state = template-default_state; state = of_get_property(child, default-state, NULL); led.default_state = state !strcmp(state, on); + led.keep_state = state !strcmp(state, keep); +++ b/include/linux/leds.h @@ -138,7 +138,8 @@ struct gpio_led { const char *default_trigger; unsignedgpio; u8 active_low; - u8 default_state; + u8 default_state; /* 0 = off, 1 = on */ + u8 keep_state; /* overrides default_state */ }; How about something simpler here, just make default state have three different values - keep, on and off? I'm not keen on having two different state variables like this. I thought of that, but it ends up being more complex. Instead of just using: static const struct gpio_led myled = { .name = something, .keep_state = 1, } You'd do something like this: .default_state = LEDS_GPIO_DEFSTATE_KEEP, Is that better? Yes. -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: MMIO and gcc re-ordering issue
Hi! Though it's my understanding that at least ia64 does require the explicit barriers anyway, so we are still in a dodgy situation here where it's not clear what drivers should do and we end up with possibly excessive barriers on powerpc where I end up with both the wmb/rmb/mb that were added for ia64 -and- the ones I have in readl/writel to make them look synchronous... Not nice. ia64 is a disaster with a slightly different ordering problem -- the mmiowb() issue. I know Ben knows far too much about this, but for big SGI boxes, you sometimes need mmiowb() to avoid problems with driver code that does totally sane stuff like spin_lock(mmio_lock); writel(val1, reg1); writel(val2, reg2); spin_unlock(mmio_lock); If that snippet is called on two CPUs at the same time, then the device might see a sequence like CPU1 -- write reg1 CPU2 -- write reg1 CPU1 -- write reg2 CPU2 -- write reg2 in spite of the fact that everything is totally ordered on the CPUs by the spin lock. The reason this is such a disaster is because the code looks right, makes sense, and works fine on 99.99% of all systems out there. So I would bet that 99% of our drivers have missing mmiowb() bugs -- no one has plugged the hardware into an Altix box and cared enough to stress test it. Yes, ia64 is a disaster, and needs its spinlock implementation fixed :-). Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: MMIO and gcc re-ordering issue
Hi! The only way to guarantee ordering in the above setup, is to either make writel() fully ordered or adding the mmiowb()'s inbetween the two writel's. On Altix you have to go and read from the PCI brige to ensure all writes to it have been flushed, which is also what mmiowb() is doing. If writel() was to guarantee this ordering, it would make every writel() call extremely expensive :-( Interesting. I've always been taught by ia64 people that mmiowb() was intended to be used solely between writel() and spin_unlock(). I think in the above case, you really should make writel() ordered. Anything else is asking for trouble, for the exact same reasons that I made it fully ordered on powerpc at least vs. previous stores. I only kept it relaxed vs. subsequent cacheable stores (ie, spin_unlock), for which I use the trick mentioned before. Hmmm I hope I didn't mess up the description of this and added to the confusion. The net result of that would be to kill performance completely, I really don't like that idea Having each writel() go out and poll the PCI bridge is going to make every driver used on Altix slow as a dog. In addition it's still not going to solve the problem for userland mapped stuff such as infinibug. Yes, this has some cost (can be fairly significant on powerpc too) but I think it's a very basic assumption from drivers that consecutive writel's, especially issued by the same CPU, will get to the device in order. In this case the cost is more than just significant, it's pretty crippling. If this is a performance problem, then provide relaxed variants and use them in selected drivers. We'd have to make major changes to drivers like e1000, tg3, mptsas, the qla2/3/4xxx and a bunch of others to get performance back. I really think the code maintenance issue there will get far worse than what we have today :( Still better than changing semantics of writel for _all_ drivers. If you are really sure driver does not depend on writel order, it is just a sed/// command, so I don't see any big code maintenance issues... Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] Fake NUMA emulation for PowerPC
On Sat 2007-12-08 09:52:06, Balbir Singh wrote: David Rientjes wrote: On Sat, 8 Dec 2007, Balbir Singh wrote: To be able to test the memory controller under NUMA, I use fake NUMA nodes. x86-64 has a similar feature, the code I have here is the simplest I could come up with for PowerPC. Magnus Damm had patches from over a year ago that, I believe, made much of the x86_64 fake NUMA code generic so that it could be extended for architectures such as i386. Perhaps he could resurrect those patches if there is wider interest in such a tool. That would be a very interesting patch, but what I have here is the simplest patch and we could build on it incrementally. The interface is non-standard but it does amazing things for 59 lines of code change. Well, maybe it is amazing, but having non-standard interface is also wrong... -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [linux-pm] [RFC] powermac: proper sleep management
Hi! This adds platform_suspend_ops for PMU based machines, directly in the PMU driver. This finally allows suspending via /sys/power/state on powerbooks. Thanks for doing this! -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [linux-pm] [PATCH, RFC] linkstation: implement standby
On Sun 2007-08-26 02:03:49, Guennadi Liakhovetski wrote: Implement suspend/resume for mpc10x compatible fsl host PCI controllers, use it for linkstation standby. Signed-off-by: Guennadi Liakhovetski [EMAIL PROTECTED] Looks okay to me. I'm glad to see standby is present on non-i386 machines, too. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Sleep problems with kernels = 2.6.21 on powerpc
Hi! I didn't know who to include as the wizards of the matter. If I, on the other hand, use Debian's kernel 2.6.22 or compile my own kernel with just the necessary parts for my work (version 2.6.23-rc3 taken from kernel.org), then I can't make the machine sleep: when I press the button, it acts like if I had, in sequence, pressed anything to wake it up (say, like pressing shift). Things are getting now a little bit fishy. Before, I was using gnome and all the little daemons that come with it (even though I just prefer a plain window manager), but, as I mentioned before, it didn't matter if I pressed the power button on X or on a console. I had the gnomee power-thingy sounding like an alarm (an ambulance!) on me when I pressed the power button with Debian kernels 2.6.22, for instance. I compiled my own (this time, from kernel.org) 2.6.23-rc3, with many modules and subsystems removed (e.g., bluetooth, as this laptop doesn't have it) and I got the exact same behavior. If I booted, OTOH, with Debian's kernel 2.6.18, things were fine and the machine would go to sleep without any problems. I am now suspecting of some module that prevented the machine from going to sleep and I now using just a fluxbox as my window manager. This time, even with a 2.6.23-rc3 kernel (again, from kernel.org) the machine went to sleep normally. I did 13 compiles with git bisect and some of them were unsucessfuly Rather than doing bisect, it might be more interesting to find out which userspace part makes it fail to sleep, then see what is it that makes the difference. Pavel -- (english) http://www.livejournal.com/~pavelmachek (cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev