Re: [PATCH v2 1/5] media: mt9p031: Add support for 8 bit and 10 bit formats
Hi Laurent, On 30.09.20 13:42, Laurent Pinchart wrote: Hi Stefan, Thank you for the patch. On Wed, Sep 30, 2020 at 12:51:29PM +0200, Stefan Riedmueller wrote: From: Christian Hemp Aside from 12 bit monochrome or color format the sensor implicitly supports 10 and 8 bit formats as well by simply dropping the corresponding LSBs. That's not how it should work though. If you set the format on MEDIA_BUS_FMT_SGRBG8_1X8 through the pipeline for instance, you will end up capturing the 8 LSB, not the 8 MSB. What's your use case for this ? I use this sensor in combination with an i.MX 6 and i.MX 6UL. When the sensor is connected with 12 bit (or 10 bit on the i.MX 6UL) and I set MEDIA_BUS_FMT_SGRBG8_1X8 through the pipeline the CSI interface drops the unused 4 LSB (or 2 LSB on the i.MX 6UL) so I get the 8 MSB from my 12 bit sensor. Does this clarify things? Maybe the description in the commit message is not accurate enough or did I get something wrong? Thanks, Stefan Signed-off-by: Christian Hemp [j...@pengutronix.de: simplified by dropping v4l2_colorspace handling] Signed-off-by: Jan Luebbe Signed-off-by: Stefan Riedmueller --- Changes in v2: - Use unsigned int for num_fmts and loop variable in find_datafmt - Remove superfluous const qualifier from find_datafmt --- drivers/media/i2c/mt9p031.c | 50 + 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index dc23b9ed510a..2e6671ef877c 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -116,6 +116,18 @@ enum mt9p031_model { MT9P031_MODEL_MONOCHROME, }; +static const u32 mt9p031_color_fmts[] = { + MEDIA_BUS_FMT_SGRBG8_1X8, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SGRBG12_1X12, +}; + +static const u32 mt9p031_monochrome_fmts[] = { + MEDIA_BUS_FMT_Y8_1X8, + MEDIA_BUS_FMT_Y10_1X10, + MEDIA_BUS_FMT_Y12_1X12, +}; + struct mt9p031 { struct v4l2_subdev subdev; struct media_pad pad; @@ -138,6 +150,9 @@ struct mt9p031 { struct v4l2_ctrl *blc_auto; struct v4l2_ctrl *blc_offset; + const u32 *fmts; + unsigned int num_fmts; + /* Registers cache */ u16 output_control; u16 mode2; @@ -148,6 +163,17 @@ static struct mt9p031 *to_mt9p031(struct v4l2_subdev *sd) return container_of(sd, struct mt9p031, subdev); } +static u32 mt9p031_find_datafmt(struct mt9p031 *mt9p031, u32 code) +{ + unsigned int i; + + for (i = 0; i < mt9p031->num_fmts; i++) + if (mt9p031->fmts[i] == code) + return mt9p031->fmts[i]; + + return mt9p031->fmts[mt9p031->num_fmts-1]; +} + static int mt9p031_read(struct i2c_client *client, u8 reg) { return i2c_smbus_read_word_swapped(client, reg); @@ -476,10 +502,11 @@ static int mt9p031_enum_mbus_code(struct v4l2_subdev *subdev, { struct mt9p031 *mt9p031 = to_mt9p031(subdev); - if (code->pad || code->index) + if (code->pad || code->index >= mt9p031->num_fmts) return -EINVAL; - code->code = mt9p031->format.code; + code->code = mt9p031->fmts[code->index]; + return 0; } @@ -573,6 +600,8 @@ static int mt9p031_set_format(struct v4l2_subdev *subdev, __format->width = __crop->width / hratio; __format->height = __crop->height / vratio; + __format->code = mt9p031_find_datafmt(mt9p031, format->format.code); + format->format = *__format; return 0; @@ -951,10 +980,7 @@ static int mt9p031_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) format = v4l2_subdev_get_try_format(subdev, fh->pad, 0); - if (mt9p031->model == MT9P031_MODEL_MONOCHROME) - format->code = MEDIA_BUS_FMT_Y12_1X12; - else - format->code = MEDIA_BUS_FMT_SGRBG12_1X12; + format->code = mt9p031_find_datafmt(mt9p031, 0); format->width = MT9P031_WINDOW_WIDTH_DEF; format->height = MT9P031_WINDOW_HEIGHT_DEF; @@ -1121,10 +1147,14 @@ static int mt9p031_probe(struct i2c_client *client, mt9p031->crop.left = MT9P031_COLUMN_START_DEF; mt9p031->crop.top = MT9P031_ROW_START_DEF; - if (mt9p031->model == MT9P031_MODEL_MONOCHROME) - mt9p031->format.code = MEDIA_BUS_FMT_Y12_1X12; - else - mt9p031->format.code = MEDIA_BUS_FMT_SGRBG12_1X12; + if (mt9p031->model == MT9P031_MODEL_MONOCHROME) { + mt9p031->fmts = mt9p031_monochrome_fmts; + mt9p031->num_fmts = ARRAY_SIZE(mt9p031_monochrome_fmts); + } else { + mt9p031->fmts = mt9p031_color_fmts; + mt9p031->num_fmts = ARRAY_SIZE(mt9p031_color_fmts); + } + mt9p031->format.code = mt9p031_find_datafmt(mt9p031, 0); mt9p031->format.width = MT9P031_WINDOW_WIDTH_DEF; mt9p031->format.height = MT9P031_WINDOW_HEIGHT_DEF;
Re: [PATCH v2 3/5] media: mt9p031: Implement [gs]_register debug calls
Hi Laurent, On 30.09.20 13:38, Laurent Pinchart wrote: Hi Stefan, Thank you for the patch. On Wed, Sep 30, 2020 at 12:51:31PM +0200, Stefan Riedmueller wrote: From: Enrico Scholz Implement g_register and s_register v4l2_subdev_core_ops to access camera register directly from userspace for debug purposes. As the name of the operations imply, this is meant for debug purpose only. They are however prone to be abused to configure the sensor from userspace in production, which isn't a direction we want to take. What's your use case for this ? I'd rather drop this patch and see the driver extended with support for more controls if needed thanks for your feedback. I get your point. I myself solely use these operations for debugging purposes but I'm aware that others like to abuse them. I thought I send it anyway since for me the DEBUG config is enough to signalize that these operations are not to be used with a productive system. But I'm OK with dropping this patch if you think it might send the wrong signal. Regards, Stefan Signed-off-by: Enrico Scholz Signed-off-by: Stefan Riedmueller --- No changes in v2 --- drivers/media/i2c/mt9p031.c | 28 1 file changed, 28 insertions(+) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index b4c042f418c1..de36025260a8 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -703,6 +703,30 @@ static int mt9p031_restore_blc(struct mt9p031 *mt9p031) return 0; } +#ifdef CONFIG_VIDEO_ADV_DEBUG +static int mt9p031_g_register(struct v4l2_subdev *sd, + struct v4l2_dbg_register *reg) +{ + struct i2c_client *client = v4l2_get_subdevdata(sd); + int ret; + + ret = mt9p031_read(client, reg->reg); + if (ret < 0) + return ret; + + reg->val = ret; + return 0; +} + +static int mt9p031_s_register(struct v4l2_subdev *sd, + struct v4l2_dbg_register const *reg) +{ + struct i2c_client *client = v4l2_get_subdevdata(sd); + + return mt9p031_write(client, reg->reg, reg->val); +} +#endif + static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl) { struct mt9p031 *mt9p031 = @@ -1000,6 +1024,10 @@ static int mt9p031_close(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) static const struct v4l2_subdev_core_ops mt9p031_subdev_core_ops = { .s_power= mt9p031_set_power, +#ifdef CONFIG_VIDEO_ADV_DEBUG + .s_register = mt9p031_s_register, + .g_register = mt9p031_g_register, +#endif }; static const struct v4l2_subdev_video_ops mt9p031_subdev_video_ops = {
Re: [PATCH v5 80/80] ARM: dts: bcm2711: Enable the display pipeline
Hi, Am 30.09.20 um 18:38 schrieb Nathan Chancellor: > On Wed, Sep 30, 2020 at 04:07:58PM +0200, Maxime Ripard wrote: >> Hi Nathan, >> >> On Tue, Sep 29, 2020 at 03:15:26PM -0700, Nathan Chancellor wrote: >>> On Thu, Sep 03, 2020 at 10:01:52AM +0200, Maxime Ripard wrote: >>>> Now that all the drivers have been adjusted for it, let's bring in the >>>> necessary device tree changes. >>>> >>>> The VEC and PV3 are left out for now, since it will require a more specific >>>> clock setup. >>>> >>>> Reviewed-by: Dave Stevenson >>>> Tested-by: Chanwoo Choi >>>> Tested-by: Hoegeun Kwon >>>> Tested-by: Stefan Wahren >>>> Signed-off-by: Maxime Ripard >>> Apologies if this has already been reported or have a solution but this >>> patch (and presumably series) breaks output to the serial console after >>> a certain point during init. On Raspbian, I see systemd startup messages >>> then the output just turns into complete garbage. It looks like this >>> patch is merged first in linux-next, which is why my bisect fell on the >>> DRM merge. I am happy to provide whatever information could be helpful >>> for debugging this. I am on the latest version of the firmware >>> (currently 26620cc9a63c6cb9965374d509479b4ee2c30241). >> Unfortunately, the miniUART is in the same clock tree than the core >> clock and will thus have those kind of issues when the core clock is >> changed (which is also something that one should expect when using the >> DRM or other drivers). >> >> The only real workaround there would be to switch to one of the PL011 >> UARTs. I guess we can also somehow make the UART react to the core clock >> frequency changes, but that's going to require some effort >> >> Maxime > Ack, thank you for the reply! There does not really seem to be a whole > ton of documentation around using one of the other PL011 UARTs so for > now, I will just revert this commit locally. there was a patch series & discussion about this topic, but we finally didn't find a rock solid solution. You can have a look at "[RFC 5/5] serial: 8250: bcm2835aux: add notifier to follow clock changes" from 3.4.2019 on linux-rpi-kernel. Best regards > > Cheers, > Nathan
[PATCH] Revert "drm/i915: Force state->modeset=true when distrust_bios_wm==true"
The fix of flagging state->modeset whenever distrust_bios_wm is set causes a regression when initializing display(s) attached to a Lenovo USB-C docking station. The display remains blank until the dock is reattached. Revert to bring the behavior of the functional v5.6 stable. This reverts commit 0f8839f5f323da04a800e6ced1136e4b1e1689a9. BugLink: https://bugzilla.redhat.com/show_bug.cgi?id=1879442 Signed-off-by: Stefan Joosten --- drivers/gpu/drm/i915/display/intel_display.c | 14 -- 1 file changed, 14 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c index b18c5ac2934d..ece1c28278f7 100644 --- a/drivers/gpu/drm/i915/display/intel_display.c +++ b/drivers/gpu/drm/i915/display/intel_display.c @@ -14942,20 +14942,6 @@ static int intel_atomic_check(struct drm_device *dev, if (ret) goto fail; - /* -* distrust_bios_wm will force a full dbuf recomputation -* but the hardware state will only get updated accordingly -* if state->modeset==true. Hence distrust_bios_wm==true && -* state->modeset==false is an invalid combination which -* would cause the hardware and software dbuf state to get -* out of sync. We must prevent that. -* -* FIXME clean up this mess and introduce better -* state tracking for dbuf. -*/ - if (dev_priv->wm.distrust_bios_wm) - any_ms = true; - intel_fbc_choose_crtc(dev_priv, state); ret = calc_watermark_data(state); if (ret) -- 2.25.4
Re: [PATCH] xen/events: don't use chip_data for legacy IRQs
On 30.09.20 11:16, Juergen Gross wrote: > Since commit c330fb1ddc0a ("XEN uses irqdesc::irq_data_common::handler_data > to store a per interrupt XEN data pointer which contains XEN specific > information.") > Xen is using the chip_data pointer for storing IRQ specific data. When > running as a HVM domain this can result in problems for legacy IRQs, as > those might use chip_data for their own purposes. > > Use a local array for this purpose in case of legacy IRQs, avoiding the > double use. > > Cc: sta...@vger.kernel.org > Fixes: c330fb1ddc0a ("XEN uses irqdesc::irq_data_common::handler_data to > store a per interrupt XEN data pointer which contains XEN specific > information.") > Signed-off-by: Juergen Gross Tested-by: Stefan Bader > --- > drivers/xen/events/events_base.c | 29 + > 1 file changed, 21 insertions(+), 8 deletions(-) > > diff --git a/drivers/xen/events/events_base.c > b/drivers/xen/events/events_base.c > index 90b8f56fbadb..6f02c18fa65c 100644 > --- a/drivers/xen/events/events_base.c > +++ b/drivers/xen/events/events_base.c > @@ -92,6 +92,8 @@ static bool (*pirq_needs_eoi)(unsigned irq); > /* Xen will never allocate port zero for any purpose. */ > #define VALID_EVTCHN(chn)((chn) != 0) > > +static struct irq_info *legacy_info_ptrs[NR_IRQS_LEGACY]; > + > static struct irq_chip xen_dynamic_chip; > static struct irq_chip xen_percpu_chip; > static struct irq_chip xen_pirq_chip; > @@ -156,7 +158,18 @@ int get_evtchn_to_irq(evtchn_port_t evtchn) > /* Get info for IRQ */ > struct irq_info *info_for_irq(unsigned irq) > { > - return irq_get_chip_data(irq); > + if (irq < nr_legacy_irqs()) > + return legacy_info_ptrs[irq]; > + else > + return irq_get_chip_data(irq); > +} > + > +static void set_info_for_irq(unsigned int irq, struct irq_info *info) > +{ > + if (irq < nr_legacy_irqs()) > + legacy_info_ptrs[irq] = info; > + else > + irq_set_chip_data(irq, info); > } > > /* Constructors for packed IRQ information. */ > @@ -377,7 +390,7 @@ static void xen_irq_init(unsigned irq) > info->type = IRQT_UNBOUND; > info->refcnt = -1; > > - irq_set_chip_data(irq, info); > + set_info_for_irq(irq, info); > > list_add_tail(>list, _irq_list_head); > } > @@ -426,14 +439,14 @@ static int __must_check xen_allocate_irq_gsi(unsigned > gsi) > > static void xen_free_irq(unsigned irq) > { > - struct irq_info *info = irq_get_chip_data(irq); > + struct irq_info *info = info_for_irq(irq); > > if (WARN_ON(!info)) > return; > > list_del(>list); > > - irq_set_chip_data(irq, NULL); > + set_info_for_irq(irq, NULL); > > WARN_ON(info->refcnt > 0); > > @@ -603,7 +616,7 @@ EXPORT_SYMBOL_GPL(xen_irq_from_gsi); > static void __unbind_from_irq(unsigned int irq) > { > evtchn_port_t evtchn = evtchn_from_irq(irq); > - struct irq_info *info = irq_get_chip_data(irq); > + struct irq_info *info = info_for_irq(irq); > > if (info->refcnt > 0) { > info->refcnt--; > @@ -1108,7 +1121,7 @@ int bind_ipi_to_irqhandler(enum ipi_vector ipi, > > void unbind_from_irqhandler(unsigned int irq, void *dev_id) > { > - struct irq_info *info = irq_get_chip_data(irq); > + struct irq_info *info = info_for_irq(irq); > > if (WARN_ON(!info)) > return; > @@ -1142,7 +1155,7 @@ int evtchn_make_refcounted(evtchn_port_t evtchn) > if (irq == -1) > return -ENOENT; > > - info = irq_get_chip_data(irq); > + info = info_for_irq(irq); > > if (!info) > return -ENOENT; > @@ -1170,7 +1183,7 @@ int evtchn_get(evtchn_port_t evtchn) > if (irq == -1) > goto done; > > - info = irq_get_chip_data(irq); > + info = info_for_irq(irq); > > if (!info) > goto done; > signature.asc Description: OpenPGP digital signature
[PATCH v2 5/5] media: mt9p031: Fix corrupted frame after restarting stream
From: Dirk Bender To prevent corrupted frames after starting and stopping the sensor it's datasheet specifies a specific pause sequence to follow: Stopping: Set Pause_Restart Bit -> Set Restart Bit -> Set Chip_Enable Off Restarting: Set Chip_Enable On -> Clear Pause_Restart Bit The Restart Bit is cleared automatically and must not be cleared manually as this would cause undefined behavior. Signed-off-by: Dirk Bender Signed-off-by: Stefan Riedmueller --- No changes in v2 --- drivers/media/i2c/mt9p031.c | 25 + 1 file changed, 25 insertions(+) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index d10457361e6c..d59f66e3dcf3 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -80,6 +80,8 @@ #defineMT9P031_PIXEL_CLOCK_SHIFT(n)((n) << 8) #defineMT9P031_PIXEL_CLOCK_DIVIDE(n) ((n) << 0) #define MT9P031_FRAME_RESTART 0x0b +#defineMT9P031_FRAME_RESTART_SET (1 << 0) +#defineMT9P031_FRAME_PAUSE_RESTART_SET (1 << 1) #define MT9P031_SHUTTER_DELAY 0x0c #define MT9P031_RST0x0d #defineMT9P031_RST_ENABLE 1 @@ -483,9 +485,25 @@ static int mt9p031_set_params(struct mt9p031 *mt9p031) static int mt9p031_s_stream(struct v4l2_subdev *subdev, int enable) { struct mt9p031 *mt9p031 = to_mt9p031(subdev); + struct i2c_client *client = v4l2_get_subdevdata(subdev); + int val; int ret; if (!enable) { + val = mt9p031_read(client, MT9P031_FRAME_RESTART); + + /* enable pause restart */ + val |= MT9P031_FRAME_PAUSE_RESTART_SET; + ret = mt9p031_write(client, MT9P031_FRAME_RESTART, val); + if (ret < 0) + return ret; + + /* enable restart + keep pause restart set */ + val |= MT9P031_FRAME_RESTART_SET; + ret = mt9p031_write(client, MT9P031_FRAME_RESTART, val); + if (ret < 0) + return ret; + /* Stop sensor readout */ ret = mt9p031_set_output_control(mt9p031, MT9P031_OUTPUT_CONTROL_CEN, 0); @@ -505,6 +523,13 @@ static int mt9p031_s_stream(struct v4l2_subdev *subdev, int enable) if (ret < 0) return ret; + val = mt9p031_read(client, MT9P031_FRAME_RESTART); + /* disable reset + pause restart */ + val &= ~MT9P031_FRAME_PAUSE_RESTART_SET; + ret = mt9p031_write(client, MT9P031_FRAME_RESTART, val); + if (ret < 0) + return ret; + return mt9p031_pll_enable(mt9p031); } -- 2.25.1
[PATCH v2 3/5] media: mt9p031: Implement [gs]_register debug calls
From: Enrico Scholz Implement g_register and s_register v4l2_subdev_core_ops to access camera register directly from userspace for debug purposes. Signed-off-by: Enrico Scholz Signed-off-by: Stefan Riedmueller --- No changes in v2 --- drivers/media/i2c/mt9p031.c | 28 1 file changed, 28 insertions(+) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index b4c042f418c1..de36025260a8 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -703,6 +703,30 @@ static int mt9p031_restore_blc(struct mt9p031 *mt9p031) return 0; } +#ifdef CONFIG_VIDEO_ADV_DEBUG +static int mt9p031_g_register(struct v4l2_subdev *sd, + struct v4l2_dbg_register *reg) +{ + struct i2c_client *client = v4l2_get_subdevdata(sd); + int ret; + + ret = mt9p031_read(client, reg->reg); + if (ret < 0) + return ret; + + reg->val = ret; + return 0; +} + +static int mt9p031_s_register(struct v4l2_subdev *sd, + struct v4l2_dbg_register const *reg) +{ + struct i2c_client *client = v4l2_get_subdevdata(sd); + + return mt9p031_write(client, reg->reg, reg->val); +} +#endif + static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl) { struct mt9p031 *mt9p031 = @@ -1000,6 +1024,10 @@ static int mt9p031_close(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) static const struct v4l2_subdev_core_ops mt9p031_subdev_core_ops = { .s_power= mt9p031_set_power, +#ifdef CONFIG_VIDEO_ADV_DEBUG + .s_register = mt9p031_s_register, + .g_register = mt9p031_g_register, +#endif }; static const struct v4l2_subdev_video_ops mt9p031_subdev_video_ops = { -- 2.25.1
[PATCH v2 4/5] media: mt9p031: Make pixel clock polarity configurable by DT
From: Christian Hemp Evaluate the desired pixel clock polarity from the device tree. Signed-off-by: Christian Hemp Signed-off-by: Stefan Riedmueller --- Changes in v2: - Initialise endpoint bus_type field to V4L2_MBUS_PARALLEL since the sensor only supports a parallel interface --- drivers/media/i2c/Kconfig | 1 + drivers/media/i2c/mt9p031.c | 20 +++- include/media/i2c/mt9p031.h | 1 + 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index c7ba76fee599..7c026daeacf0 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -1103,6 +1103,7 @@ config VIDEO_MT9P031 select MEDIA_CONTROLLER select VIDEO_V4L2_SUBDEV_API select VIDEO_APTINA_PLL + select V4L2_FWNODE help This is a Video4Linux2 sensor driver for the Aptina (Micron) mt9p031 5 Mpixel camera. diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index de36025260a8..d10457361e6c 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include "aptina-pll.h" @@ -399,6 +400,14 @@ static int __mt9p031_set_power(struct mt9p031 *mt9p031, bool on) return ret; } + /* Configure the pixel clock polarity */ + if (mt9p031->pdata && mt9p031->pdata->pixclk_pol) { + ret = mt9p031_write(client, MT9P031_PIXEL_CLOCK_CONTROL, + MT9P031_PIXEL_CLOCK_INVERT); + if (ret < 0) + return ret; + } + return v4l2_ctrl_handler_setup(>ctrls); } @@ -1062,8 +1071,11 @@ static const struct v4l2_subdev_internal_ops mt9p031_subdev_internal_ops = { static struct mt9p031_platform_data * mt9p031_get_pdata(struct i2c_client *client) { - struct mt9p031_platform_data *pdata; + struct mt9p031_platform_data *pdata = NULL; struct device_node *np; + struct v4l2_fwnode_endpoint endpoint = { + .bus_type = V4L2_MBUS_PARALLEL + }; if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node) return client->dev.platform_data; @@ -1072,6 +1084,9 @@ mt9p031_get_pdata(struct i2c_client *client) if (!np) return NULL; + if (v4l2_fwnode_endpoint_parse(of_fwnode_handle(np), ) < 0) + goto done; + pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL); if (!pdata) goto done; @@ -1079,6 +1094,9 @@ mt9p031_get_pdata(struct i2c_client *client) of_property_read_u32(np, "input-clock-frequency", >ext_freq); of_property_read_u32(np, "pixel-clock-frequency", >target_freq); + pdata->pixclk_pol = !!(endpoint.bus.parallel.flags & + V4L2_MBUS_PCLK_SAMPLE_RISING); + done: of_node_put(np); return pdata; diff --git a/include/media/i2c/mt9p031.h b/include/media/i2c/mt9p031.h index 7c29c53aa988..f933cd0be8e5 100644 --- a/include/media/i2c/mt9p031.h +++ b/include/media/i2c/mt9p031.h @@ -10,6 +10,7 @@ struct v4l2_subdev; * @target_freq: Pixel clock frequency */ struct mt9p031_platform_data { + unsigned int pixclk_pol:1; int ext_freq; int target_freq; }; -- 2.25.1
[PATCH v2 2/5] media: mt9p031: Read back the real clock rate
From: Enrico Scholz The real and requested clock can differ and because it is used to calculate PLL values, the real clock rate should be read. Signed-off-by: Enrico Scholz Signed-off-by: Stefan Riedmueller --- No changes in v2 --- drivers/media/i2c/mt9p031.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 2e6671ef877c..b4c042f418c1 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -255,6 +255,7 @@ static int mt9p031_clk_setup(struct mt9p031 *mt9p031) struct i2c_client *client = v4l2_get_subdevdata(>subdev); struct mt9p031_platform_data *pdata = mt9p031->pdata; + unsigned long ext_freq; int ret; mt9p031->clk = devm_clk_get(>dev, NULL); @@ -265,13 +266,15 @@ static int mt9p031_clk_setup(struct mt9p031 *mt9p031) if (ret < 0) return ret; + ext_freq = clk_get_rate(mt9p031->clk); + /* If the external clock frequency is out of bounds for the PLL use the * pixel clock divider only and disable the PLL. */ - if (pdata->ext_freq > limits.ext_clock_max) { + if (ext_freq > limits.ext_clock_max) { unsigned int div; - div = DIV_ROUND_UP(pdata->ext_freq, pdata->target_freq); + div = DIV_ROUND_UP(ext_freq, pdata->target_freq); div = roundup_pow_of_two(div) / 2; mt9p031->clk_div = min_t(unsigned int, div, 64); @@ -280,7 +283,7 @@ static int mt9p031_clk_setup(struct mt9p031 *mt9p031) return 0; } - mt9p031->pll.ext_clock = pdata->ext_freq; + mt9p031->pll.ext_clock = ext_freq; mt9p031->pll.pix_clock = pdata->target_freq; mt9p031->use_pll = true; -- 2.25.1
[PATCH v2 1/5] media: mt9p031: Add support for 8 bit and 10 bit formats
From: Christian Hemp Aside from 12 bit monochrome or color format the sensor implicitly supports 10 and 8 bit formats as well by simply dropping the corresponding LSBs. Signed-off-by: Christian Hemp [j...@pengutronix.de: simplified by dropping v4l2_colorspace handling] Signed-off-by: Jan Luebbe Signed-off-by: Stefan Riedmueller --- Changes in v2: - Use unsigned int for num_fmts and loop variable in find_datafmt - Remove superfluous const qualifier from find_datafmt --- drivers/media/i2c/mt9p031.c | 50 + 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index dc23b9ed510a..2e6671ef877c 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -116,6 +116,18 @@ enum mt9p031_model { MT9P031_MODEL_MONOCHROME, }; +static const u32 mt9p031_color_fmts[] = { + MEDIA_BUS_FMT_SGRBG8_1X8, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SGRBG12_1X12, +}; + +static const u32 mt9p031_monochrome_fmts[] = { + MEDIA_BUS_FMT_Y8_1X8, + MEDIA_BUS_FMT_Y10_1X10, + MEDIA_BUS_FMT_Y12_1X12, +}; + struct mt9p031 { struct v4l2_subdev subdev; struct media_pad pad; @@ -138,6 +150,9 @@ struct mt9p031 { struct v4l2_ctrl *blc_auto; struct v4l2_ctrl *blc_offset; + const u32 *fmts; + unsigned int num_fmts; + /* Registers cache */ u16 output_control; u16 mode2; @@ -148,6 +163,17 @@ static struct mt9p031 *to_mt9p031(struct v4l2_subdev *sd) return container_of(sd, struct mt9p031, subdev); } +static u32 mt9p031_find_datafmt(struct mt9p031 *mt9p031, u32 code) +{ + unsigned int i; + + for (i = 0; i < mt9p031->num_fmts; i++) + if (mt9p031->fmts[i] == code) + return mt9p031->fmts[i]; + + return mt9p031->fmts[mt9p031->num_fmts-1]; +} + static int mt9p031_read(struct i2c_client *client, u8 reg) { return i2c_smbus_read_word_swapped(client, reg); @@ -476,10 +502,11 @@ static int mt9p031_enum_mbus_code(struct v4l2_subdev *subdev, { struct mt9p031 *mt9p031 = to_mt9p031(subdev); - if (code->pad || code->index) + if (code->pad || code->index >= mt9p031->num_fmts) return -EINVAL; - code->code = mt9p031->format.code; + code->code = mt9p031->fmts[code->index]; + return 0; } @@ -573,6 +600,8 @@ static int mt9p031_set_format(struct v4l2_subdev *subdev, __format->width = __crop->width / hratio; __format->height = __crop->height / vratio; + __format->code = mt9p031_find_datafmt(mt9p031, format->format.code); + format->format = *__format; return 0; @@ -951,10 +980,7 @@ static int mt9p031_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) format = v4l2_subdev_get_try_format(subdev, fh->pad, 0); - if (mt9p031->model == MT9P031_MODEL_MONOCHROME) - format->code = MEDIA_BUS_FMT_Y12_1X12; - else - format->code = MEDIA_BUS_FMT_SGRBG12_1X12; + format->code = mt9p031_find_datafmt(mt9p031, 0); format->width = MT9P031_WINDOW_WIDTH_DEF; format->height = MT9P031_WINDOW_HEIGHT_DEF; @@ -1121,10 +1147,14 @@ static int mt9p031_probe(struct i2c_client *client, mt9p031->crop.left = MT9P031_COLUMN_START_DEF; mt9p031->crop.top = MT9P031_ROW_START_DEF; - if (mt9p031->model == MT9P031_MODEL_MONOCHROME) - mt9p031->format.code = MEDIA_BUS_FMT_Y12_1X12; - else - mt9p031->format.code = MEDIA_BUS_FMT_SGRBG12_1X12; + if (mt9p031->model == MT9P031_MODEL_MONOCHROME) { + mt9p031->fmts = mt9p031_monochrome_fmts; + mt9p031->num_fmts = ARRAY_SIZE(mt9p031_monochrome_fmts); + } else { + mt9p031->fmts = mt9p031_color_fmts; + mt9p031->num_fmts = ARRAY_SIZE(mt9p031_color_fmts); + } + mt9p031->format.code = mt9p031_find_datafmt(mt9p031, 0); mt9p031->format.width = MT9P031_WINDOW_WIDTH_DEF; mt9p031->format.height = MT9P031_WINDOW_HEIGHT_DEF; -- 2.25.1
Re: [PATCH 1/5] media: mt9p031: Add support for 8 bit and 10 bit formats
Hi Sakari, On 25.09.20 22:11, Sakari Ailus wrote: Hi Stefan, On Fri, Sep 25, 2020 at 09:50:25AM +0200, Stefan Riedmueller wrote: From: Christian Hemp Aside from 12 bit monochrome or color format the sensor implicitly supports 10 and 8 bit formats as well by simply dropping the corresponding LSBs. Signed-off-by: Christian Hemp [j...@pengutronix.de: simplified by dropping v4l2_colorspace handling] Signed-off-by: Jan Luebbe Signed-off-by: Stefan Riedmueller --- drivers/media/i2c/mt9p031.c | 50 + 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index dc23b9ed510a..0002dd299ffa 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -116,6 +116,18 @@ enum mt9p031_model { MT9P031_MODEL_MONOCHROME, }; +static const u32 mt9p031_color_fmts[] = { + MEDIA_BUS_FMT_SGRBG8_1X8, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SGRBG12_1X12, +}; + +static const u32 mt9p031_monochrome_fmts[] = { + MEDIA_BUS_FMT_Y8_1X8, + MEDIA_BUS_FMT_Y10_1X10, + MEDIA_BUS_FMT_Y12_1X12, +}; + struct mt9p031 { struct v4l2_subdev subdev; struct media_pad pad; @@ -138,6 +150,9 @@ struct mt9p031 { struct v4l2_ctrl *blc_auto; struct v4l2_ctrl *blc_offset; + const u32 *fmts; + int num_fmts; Unsigned int, please. + /* Registers cache */ u16 output_control; u16 mode2; @@ -148,6 +163,17 @@ static struct mt9p031 *to_mt9p031(struct v4l2_subdev *sd) return container_of(sd, struct mt9p031, subdev); } +static const u32 mt9p031_find_datafmt(struct mt9p031 *mt9p031, u32 code) +{ + int i; Same here. I'll fix it in v2. Thanks, Stefan + + for (i = 0; i < mt9p031->num_fmts; i++) + if (mt9p031->fmts[i] == code) + return mt9p031->fmts[i]; + + return mt9p031->fmts[mt9p031->num_fmts-1]; +} + static int mt9p031_read(struct i2c_client *client, u8 reg) { return i2c_smbus_read_word_swapped(client, reg); @@ -476,10 +502,11 @@ static int mt9p031_enum_mbus_code(struct v4l2_subdev *subdev, { struct mt9p031 *mt9p031 = to_mt9p031(subdev); - if (code->pad || code->index) + if (code->pad || code->index >= mt9p031->num_fmts) return -EINVAL; - code->code = mt9p031->format.code; + code->code = mt9p031->fmts[code->index]; + return 0; } @@ -573,6 +600,8 @@ static int mt9p031_set_format(struct v4l2_subdev *subdev, __format->width = __crop->width / hratio; __format->height = __crop->height / vratio; + __format->code = mt9p031_find_datafmt(mt9p031, format->format.code); + format->format = *__format; return 0; @@ -951,10 +980,7 @@ static int mt9p031_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) format = v4l2_subdev_get_try_format(subdev, fh->pad, 0); - if (mt9p031->model == MT9P031_MODEL_MONOCHROME) - format->code = MEDIA_BUS_FMT_Y12_1X12; - else - format->code = MEDIA_BUS_FMT_SGRBG12_1X12; + format->code = mt9p031_find_datafmt(mt9p031, 0); format->width = MT9P031_WINDOW_WIDTH_DEF; format->height = MT9P031_WINDOW_HEIGHT_DEF; @@ -1121,10 +1147,14 @@ static int mt9p031_probe(struct i2c_client *client, mt9p031->crop.left = MT9P031_COLUMN_START_DEF; mt9p031->crop.top = MT9P031_ROW_START_DEF; - if (mt9p031->model == MT9P031_MODEL_MONOCHROME) - mt9p031->format.code = MEDIA_BUS_FMT_Y12_1X12; - else - mt9p031->format.code = MEDIA_BUS_FMT_SGRBG12_1X12; + if (mt9p031->model == MT9P031_MODEL_MONOCHROME) { + mt9p031->fmts = mt9p031_monochrome_fmts; + mt9p031->num_fmts = ARRAY_SIZE(mt9p031_monochrome_fmts); + } else { + mt9p031->fmts = mt9p031_color_fmts; + mt9p031->num_fmts = ARRAY_SIZE(mt9p031_color_fmts); + } + mt9p031->format.code = mt9p031_find_datafmt(mt9p031, 0); mt9p031->format.width = MT9P031_WINDOW_WIDTH_DEF; mt9p031->format.height = MT9P031_WINDOW_HEIGHT_DEF;
Re: [PATCH 4.4 50/62] XEN uses irqdesc::irq_data_common::handler_data to store a per interrupt XEN data pointer which contains XEN specific information.
On 29.09.20 16:05, Jürgen Groß wrote: > On 29.09.20 15:13, Stefan Bader wrote: >> On 01.09.20 17:10, Greg Kroah-Hartman wrote: >>> From: Thomas Gleixner >>> >>> commit c330fb1ddc0a922f044989492b7fcca77ee1db46 upstream. >>> >>> handler data is meant for interrupt handlers and not for storing irq chip >>> specific information as some devices require handler data to store internal >>> per interrupt information, e.g. pinctrl/GPIO chained interrupt handlers. >>> >>> This obviously creates a conflict of interests and crashes the machine >>> because the XEN pointer is overwritten by the driver pointer. >> >> I cannot say whether this applies the same for the vanilla 4.4 stable kernels >> but once this had been applied to our 4.4 based kernels, we observed Xen HVM >> guests crashing on boot with: >> >> [ 0.927538] ACPI: bus type PCI registered >> [ 0.936008] acpiphp: ACPI Hot Plug PCI Controller Driver version: 0.5 >> [ 0.948739] PCI: Using configuration type 1 for base access >> [ 0.960007] PCI: Using configuration type 1 for extended access >> [ 0.984340] ACPI: Added _OSI(Module Device) >> [ 0.988010] ACPI: Added _OSI(Processor Device) >> [ 0.992007] ACPI: Added _OSI(3.0 _SCP Extensions) >> [ 0.996013] ACPI: Added _OSI(Processor Aggregator Device) >> [ 1.002103] BUG: unable to handle kernel paging request at >> ff5f3000 >> [ 1.004000] IP: [] mp_irqdomain_activate+0x5f/0xa0 >> [ 1.004000] PGD 1e0f067 PUD 1e11067 PMD 1e12067 PTE 0 >> [ 1.004000] Oops: 0002 [#1] SMP >> [ 1.004000] Modules linked in: >> [ 1.004000] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.4.0-191-generic >> #221-Ubuntu >> [ 1.004000] Hardware name: Xen HVM domU, BIOS 4.6.5 04/18/2018 >> [ 1.004000] task: 880107db ti: 880107dac000 task.ti: >> 880107dac000 >> [ 1.004000] RIP: 0010:[] [] >> mp_irqdomain_activate+0x5f/0xa0 >> [ 1.004000] RSP: 0018:880107dafc48 EFLAGS: 00010086 >> [ 1.004000] RAX: 0086 RBX: 8800eb852140 RCX: >> >> [ 1.004000] RDX: ff5f3000 RSI: 0001 RDI: >> 0020c000 >> [ 1.004000] RBP: 880107dafc50 R08: 81ebdfd0 R09: >> >> [ 1.004000] R10: 0011 R11: 0009 R12: >> 88010880d400 >> [ 1.004000] R13: 0001 R14: 0009 R15: >> 8800eb880080 >> [ 1.004000] FS: () GS:880108ec() >> knlGS: >> [ 1.004000] CS: 0010 DS: ES: CR0: 80050033 >> [ 1.004000] CR2: ff5f3000 CR3: 01e0a000 CR4: >> 06f0 >> [ 1.004000] Stack: >> [ 1.004000] 88010880bc58 880107dafc70 810ea644 >> 88010880bc00 >> [ 1.004000] 88010880bc58 880107dafca0 810e6d88 >> 810e1009 >> [ 1.004000] 88010880bc00 88010880bca0 8800eb880080 >> 880107dafd38 >> [ 1.004000] Call Trace: >> [ 1.004000] [] irq_domain_activate_irq+0x44/0x50 >> [ 1.004000] [] irq_startup+0x38/0x90 >> [ 1.004000] [] ? vprintk_default+0x29/0x40 >> [ 1.004000] [] __setup_irq+0x5a2/0x650 >> [ 1.004000] [] ? kmem_cache_alloc_trace+0x1d4/0x1f0 >> [ 1.004000] [] ? acpi_osi_handler+0xb0/0xb0 >> [ 1.004000] [] request_threaded_irq+0xfb/0x1a0 >> [ 1.004000] [] ? acpi_osi_handler+0xb0/0xb0 >> [ 1.004000] [] ? acpi_ev_sci_dispatch+0x64/0x64 >> [ 1.004000] [] >> acpi_os_install_interrupt_handler+0xaa/0x100 >> [ 1.004000] [] ? acpi_sleep_proc_init+0x28/0x28 >> [ 1.004000] [] acpi_ev_install_sci_handler+0x23/0x25 >> [ 1.004000] [] acpi_ev_install_xrupt_handlers+0x1c/0x6c >> [ 1.004000] [] acpi_enable_subsystem+0x8f/0x93 >> [ 1.004000] [] acpi_init+0x8b/0x2c4 >> [ 1.004000] [] ? kasprintf+0x4e/0x70 >> [ 1.004000] [] ? acpi_sleep_proc_init+0x28/0x28 >> [ 1.004000] [] do_one_initcall+0xb5/0x200 >> [ 1.004000] [] ? parse_args+0x29a/0x4a0 >> [ 1.004000] [] kernel_init_freeable+0x177/0x218 >> [ 1.004000] [] ? rest_init+0x80/0x80 >> [ 1.004000] [] kernel_init+0xe/0xe0 >> [ 1.004000] [] ret_from_fork+0x42/0x80 >> [ 1.004000] [] ? rest_init+0x80/0x80 >> [ 1.004000] Code: 8d 1c d2 8d ba 0b 02 00 00 44 8d 51 11 42 8b 14 dd 74 >> ec 10 >> 82 c1 e7 0c 48 63 ff 81 e2 ff 0f 00 00 48 81 ea 00 10 80 00 48 29 fa <44> 89 >> 12 >> 89 72 10 42 8b 14 dd 74 ec
Re: [PATCH 4/5] media: mt9p031: Make pixel clock polarity configurable by DT
Hi Sakari, thanks for your review. On 25.09.20 22:07, Sakari Ailus wrote: Hi Stefan, Thanks for the patchset. On Fri, Sep 25, 2020 at 09:50:28AM +0200, Stefan Riedmueller wrote: From: Christian Hemp Evaluate the desired pixel clock polarity from the device tree. Signed-off-by: Christian Hemp Signed-off-by: Stefan Riedmueller --- drivers/media/i2c/Kconfig | 1 + drivers/media/i2c/mt9p031.c | 19 ++- include/media/i2c/mt9p031.h | 1 + 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index c7ba76fee599..7c026daeacf0 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -1103,6 +1103,7 @@ config VIDEO_MT9P031 select MEDIA_CONTROLLER select VIDEO_V4L2_SUBDEV_API select VIDEO_APTINA_PLL + select V4L2_FWNODE help This is a Video4Linux2 sensor driver for the Aptina (Micron) mt9p031 5 Mpixel camera. diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index f5d6a7890c47..8f8ee37a2dd2 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include "aptina-pll.h" @@ -399,6 +400,14 @@ static int __mt9p031_set_power(struct mt9p031 *mt9p031, bool on) return ret; } + /* Configure the pixel clock polarity */ + if (mt9p031->pdata && mt9p031->pdata->pixclk_pol) { + ret = mt9p031_write(client, MT9P031_PIXEL_CLOCK_CONTROL, + MT9P031_PIXEL_CLOCK_INVERT); + if (ret < 0) + return ret; + } + return v4l2_ctrl_handler_setup(>ctrls); } @@ -1062,7 +1071,8 @@ static const struct v4l2_subdev_internal_ops mt9p031_subdev_internal_ops = { static struct mt9p031_platform_data * mt9p031_get_pdata(struct i2c_client *client) { - struct mt9p031_platform_data *pdata; + struct mt9p031_platform_data *pdata = NULL; + struct v4l2_fwnode_endpoint endpoint; Could you initialise the bus_type field to a valid value? I suppose this sensor only supports one of them? That way you'll also initialise the rest of the struct fields to zero. Yes, I'll do that and send a v2. Thanks, Stefan struct device_node *np; if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node) @@ -1072,6 +1082,10 @@ mt9p031_get_pdata(struct i2c_client *client) if (!np) return NULL; + endpoint.bus_type = V4L2_MBUS_UNKNOWN; + if (v4l2_fwnode_endpoint_parse(of_fwnode_handle(np), ) < 0) + goto done; + pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL); if (!pdata) goto done; @@ -1079,6 +1093,9 @@ mt9p031_get_pdata(struct i2c_client *client) of_property_read_u32(np, "input-clock-frequency", >ext_freq); of_property_read_u32(np, "pixel-clock-frequency", >target_freq); + pdata->pixclk_pol = !!(endpoint.bus.parallel.flags & + V4L2_MBUS_PCLK_SAMPLE_RISING); + done: of_node_put(np); return pdata; diff --git a/include/media/i2c/mt9p031.h b/include/media/i2c/mt9p031.h index 7c29c53aa988..f933cd0be8e5 100644 --- a/include/media/i2c/mt9p031.h +++ b/include/media/i2c/mt9p031.h @@ -10,6 +10,7 @@ struct v4l2_subdev; * @target_freq: Pixel clock frequency */ struct mt9p031_platform_data { + unsigned int pixclk_pol:1; int ext_freq; int target_freq; };
Re: [PATCH 4.4 50/62] XEN uses irqdesc::irq_data_common::handler_data to store a per interrupt XEN data pointer which contains XEN specific information.
On 29.09.20 16:05, Jürgen Groß wrote: > On 29.09.20 15:13, Stefan Bader wrote: >> On 01.09.20 17:10, Greg Kroah-Hartman wrote: >>> From: Thomas Gleixner >>> >>> commit c330fb1ddc0a922f044989492b7fcca77ee1db46 upstream. >>> >>> handler data is meant for interrupt handlers and not for storing irq chip >>> specific information as some devices require handler data to store internal >>> per interrupt information, e.g. pinctrl/GPIO chained interrupt handlers. >>> >>> This obviously creates a conflict of interests and crashes the machine >>> because the XEN pointer is overwritten by the driver pointer. >> >> I cannot say whether this applies the same for the vanilla 4.4 stable kernels >> but once this had been applied to our 4.4 based kernels, we observed Xen HVM >> guests crashing on boot with: >> >> [ 0.927538] ACPI: bus type PCI registered >> [ 0.936008] acpiphp: ACPI Hot Plug PCI Controller Driver version: 0.5 >> [ 0.948739] PCI: Using configuration type 1 for base access >> [ 0.960007] PCI: Using configuration type 1 for extended access >> [ 0.984340] ACPI: Added _OSI(Module Device) >> [ 0.988010] ACPI: Added _OSI(Processor Device) >> [ 0.992007] ACPI: Added _OSI(3.0 _SCP Extensions) >> [ 0.996013] ACPI: Added _OSI(Processor Aggregator Device) >> [ 1.002103] BUG: unable to handle kernel paging request at >> ff5f3000 >> [ 1.004000] IP: [] mp_irqdomain_activate+0x5f/0xa0 >> [ 1.004000] PGD 1e0f067 PUD 1e11067 PMD 1e12067 PTE 0 >> [ 1.004000] Oops: 0002 [#1] SMP >> [ 1.004000] Modules linked in: >> [ 1.004000] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.4.0-191-generic >> #221-Ubuntu >> [ 1.004000] Hardware name: Xen HVM domU, BIOS 4.6.5 04/18/2018 >> [ 1.004000] task: 880107db ti: 880107dac000 task.ti: >> 880107dac000 >> [ 1.004000] RIP: 0010:[] [] >> mp_irqdomain_activate+0x5f/0xa0 >> [ 1.004000] RSP: 0018:880107dafc48 EFLAGS: 00010086 >> [ 1.004000] RAX: 0086 RBX: 8800eb852140 RCX: >> >> [ 1.004000] RDX: ff5f3000 RSI: 0001 RDI: >> 0020c000 >> [ 1.004000] RBP: 880107dafc50 R08: 81ebdfd0 R09: >> >> [ 1.004000] R10: 0011 R11: 0009 R12: >> 88010880d400 >> [ 1.004000] R13: 0001 R14: 0009 R15: >> 8800eb880080 >> [ 1.004000] FS: () GS:880108ec() >> knlGS: >> [ 1.004000] CS: 0010 DS: ES: CR0: 80050033 >> [ 1.004000] CR2: ff5f3000 CR3: 01e0a000 CR4: >> 06f0 >> [ 1.004000] Stack: >> [ 1.004000] 88010880bc58 880107dafc70 810ea644 >> 88010880bc00 >> [ 1.004000] 88010880bc58 880107dafca0 810e6d88 >> 810e1009 >> [ 1.004000] 88010880bc00 88010880bca0 8800eb880080 >> 880107dafd38 >> [ 1.004000] Call Trace: >> [ 1.004000] [] irq_domain_activate_irq+0x44/0x50 >> [ 1.004000] [] irq_startup+0x38/0x90 >> [ 1.004000] [] ? vprintk_default+0x29/0x40 >> [ 1.004000] [] __setup_irq+0x5a2/0x650 >> [ 1.004000] [] ? kmem_cache_alloc_trace+0x1d4/0x1f0 >> [ 1.004000] [] ? acpi_osi_handler+0xb0/0xb0 >> [ 1.004000] [] request_threaded_irq+0xfb/0x1a0 >> [ 1.004000] [] ? acpi_osi_handler+0xb0/0xb0 >> [ 1.004000] [] ? acpi_ev_sci_dispatch+0x64/0x64 >> [ 1.004000] [] >> acpi_os_install_interrupt_handler+0xaa/0x100 >> [ 1.004000] [] ? acpi_sleep_proc_init+0x28/0x28 >> [ 1.004000] [] acpi_ev_install_sci_handler+0x23/0x25 >> [ 1.004000] [] acpi_ev_install_xrupt_handlers+0x1c/0x6c >> [ 1.004000] [] acpi_enable_subsystem+0x8f/0x93 >> [ 1.004000] [] acpi_init+0x8b/0x2c4 >> [ 1.004000] [] ? kasprintf+0x4e/0x70 >> [ 1.004000] [] ? acpi_sleep_proc_init+0x28/0x28 >> [ 1.004000] [] do_one_initcall+0xb5/0x200 >> [ 1.004000] [] ? parse_args+0x29a/0x4a0 >> [ 1.004000] [] kernel_init_freeable+0x177/0x218 >> [ 1.004000] [] ? rest_init+0x80/0x80 >> [ 1.004000] [] kernel_init+0xe/0xe0 >> [ 1.004000] [] ret_from_fork+0x42/0x80 >> [ 1.004000] [] ? rest_init+0x80/0x80 >> [ 1.004000] Code: 8d 1c d2 8d ba 0b 02 00 00 44 8d 51 11 42 8b 14 dd 74 >> ec 10 >> 82 c1 e7 0c 48 63 ff 81 e2 ff 0f 00 00 48 81 ea 00 10 80 00 48 29 fa <44> 89 >> 12 >> 89 72 10 42 8b 14 dd 74 ec
Re: [PATCH 4.4 50/62] XEN uses irqdesc::irq_data_common::handler_data to store a per interrupt XEN data pointer which contains XEN specific information.
On 01.09.20 17:10, Greg Kroah-Hartman wrote: > From: Thomas Gleixner > > commit c330fb1ddc0a922f044989492b7fcca77ee1db46 upstream. > > handler data is meant for interrupt handlers and not for storing irq chip > specific information as some devices require handler data to store internal > per interrupt information, e.g. pinctrl/GPIO chained interrupt handlers. > > This obviously creates a conflict of interests and crashes the machine > because the XEN pointer is overwritten by the driver pointer. I cannot say whether this applies the same for the vanilla 4.4 stable kernels but once this had been applied to our 4.4 based kernels, we observed Xen HVM guests crashing on boot with: [0.927538] ACPI: bus type PCI registered [0.936008] acpiphp: ACPI Hot Plug PCI Controller Driver version: 0.5 [0.948739] PCI: Using configuration type 1 for base access [0.960007] PCI: Using configuration type 1 for extended access [0.984340] ACPI: Added _OSI(Module Device) [0.988010] ACPI: Added _OSI(Processor Device) [0.992007] ACPI: Added _OSI(3.0 _SCP Extensions) [0.996013] ACPI: Added _OSI(Processor Aggregator Device) [1.002103] BUG: unable to handle kernel paging request at ff5f3000 [1.004000] IP: [] mp_irqdomain_activate+0x5f/0xa0 [1.004000] PGD 1e0f067 PUD 1e11067 PMD 1e12067 PTE 0 [1.004000] Oops: 0002 [#1] SMP [1.004000] Modules linked in: [1.004000] CPU: 3 PID: 1 Comm: swapper/0 Not tainted 4.4.0-191-generic #221-Ubuntu [1.004000] Hardware name: Xen HVM domU, BIOS 4.6.5 04/18/2018 [1.004000] task: 880107db ti: 880107dac000 task.ti: 880107dac000 [1.004000] RIP: 0010:[] [] mp_irqdomain_activate+0x5f/0xa0 [1.004000] RSP: 0018:880107dafc48 EFLAGS: 00010086 [1.004000] RAX: 0086 RBX: 8800eb852140 RCX: [1.004000] RDX: ff5f3000 RSI: 0001 RDI: 0020c000 [1.004000] RBP: 880107dafc50 R08: 81ebdfd0 R09: [1.004000] R10: 0011 R11: 0009 R12: 88010880d400 [1.004000] R13: 0001 R14: 0009 R15: 8800eb880080 [1.004000] FS: () GS:880108ec() knlGS: [1.004000] CS: 0010 DS: ES: CR0: 80050033 [1.004000] CR2: ff5f3000 CR3: 01e0a000 CR4: 06f0 [1.004000] Stack: [1.004000] 88010880bc58 880107dafc70 810ea644 88010880bc00 [1.004000] 88010880bc58 880107dafca0 810e6d88 810e1009 [1.004000] 88010880bc00 88010880bca0 8800eb880080 880107dafd38 [1.004000] Call Trace: [1.004000] [] irq_domain_activate_irq+0x44/0x50 [1.004000] [] irq_startup+0x38/0x90 [1.004000] [] ? vprintk_default+0x29/0x40 [1.004000] [] __setup_irq+0x5a2/0x650 [1.004000] [] ? kmem_cache_alloc_trace+0x1d4/0x1f0 [1.004000] [] ? acpi_osi_handler+0xb0/0xb0 [1.004000] [] request_threaded_irq+0xfb/0x1a0 [1.004000] [] ? acpi_osi_handler+0xb0/0xb0 [1.004000] [] ? acpi_ev_sci_dispatch+0x64/0x64 [1.004000] [] acpi_os_install_interrupt_handler+0xaa/0x100 [1.004000] [] ? acpi_sleep_proc_init+0x28/0x28 [1.004000] [] acpi_ev_install_sci_handler+0x23/0x25 [1.004000] [] acpi_ev_install_xrupt_handlers+0x1c/0x6c [1.004000] [] acpi_enable_subsystem+0x8f/0x93 [1.004000] [] acpi_init+0x8b/0x2c4 [1.004000] [] ? kasprintf+0x4e/0x70 [1.004000] [] ? acpi_sleep_proc_init+0x28/0x28 [1.004000] [] do_one_initcall+0xb5/0x200 [1.004000] [] ? parse_args+0x29a/0x4a0 [1.004000] [] kernel_init_freeable+0x177/0x218 [1.004000] [] ? rest_init+0x80/0x80 [1.004000] [] kernel_init+0xe/0xe0 [1.004000] [] ret_from_fork+0x42/0x80 [1.004000] [] ? rest_init+0x80/0x80 [1.004000] Code: 8d 1c d2 8d ba 0b 02 00 00 44 8d 51 11 42 8b 14 dd 74 ec 10 82 c1 e7 0c 48 63 ff 81 e2 ff 0f 00 00 48 81 ea 00 10 80 00 48 29 fa <44> 89 12 89 72 10 42 8b 14 dd 74 ec 10 82 83 c1 10 81 e2 ff 0f [1.004000] RIP [] mp_irqdomain_activate+0x5f/0xa0 [1.004000] RSP [1.004000] CR2: ff5f3000 [1.004000] ---[ end trace 3201cae5b6bd7be1 ]--- [1.592027] Kernel panic - not syncing: Attempted to kill init! exitcode=0x0009 [1.592027] This is from a local server but same stack-trace happens on AWS instances while initializing ACPI SCI. mp_irqdomain_activate is accessing chip_data expecting ioapic data there. Oddly enough more recent kernels seem to do the same but not crashing as HVM guest (neither seen for our 4.15 nor the 5.4). Maybe someone could make sure the 4.4.y stable series is not also broken. -Stefan > > As the XEN data is not handler specific it should be stored in > irqdesc::irq_data::chip_data instead. > > A simple sed s/irq_[sg]et_handler_data/irq_[sg]et_chip_data/ cures that. &
[PATCH 4/5] media: mt9p031: Make pixel clock polarity configurable by DT
From: Christian Hemp Evaluate the desired pixel clock polarity from the device tree. Signed-off-by: Christian Hemp Signed-off-by: Stefan Riedmueller --- drivers/media/i2c/Kconfig | 1 + drivers/media/i2c/mt9p031.c | 19 ++- include/media/i2c/mt9p031.h | 1 + 3 files changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/media/i2c/Kconfig b/drivers/media/i2c/Kconfig index c7ba76fee599..7c026daeacf0 100644 --- a/drivers/media/i2c/Kconfig +++ b/drivers/media/i2c/Kconfig @@ -1103,6 +1103,7 @@ config VIDEO_MT9P031 select MEDIA_CONTROLLER select VIDEO_V4L2_SUBDEV_API select VIDEO_APTINA_PLL + select V4L2_FWNODE help This is a Video4Linux2 sensor driver for the Aptina (Micron) mt9p031 5 Mpixel camera. diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index f5d6a7890c47..8f8ee37a2dd2 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -27,6 +27,7 @@ #include #include #include +#include #include #include "aptina-pll.h" @@ -399,6 +400,14 @@ static int __mt9p031_set_power(struct mt9p031 *mt9p031, bool on) return ret; } + /* Configure the pixel clock polarity */ + if (mt9p031->pdata && mt9p031->pdata->pixclk_pol) { + ret = mt9p031_write(client, MT9P031_PIXEL_CLOCK_CONTROL, + MT9P031_PIXEL_CLOCK_INVERT); + if (ret < 0) + return ret; + } + return v4l2_ctrl_handler_setup(>ctrls); } @@ -1062,7 +1071,8 @@ static const struct v4l2_subdev_internal_ops mt9p031_subdev_internal_ops = { static struct mt9p031_platform_data * mt9p031_get_pdata(struct i2c_client *client) { - struct mt9p031_platform_data *pdata; + struct mt9p031_platform_data *pdata = NULL; + struct v4l2_fwnode_endpoint endpoint; struct device_node *np; if (!IS_ENABLED(CONFIG_OF) || !client->dev.of_node) @@ -1072,6 +1082,10 @@ mt9p031_get_pdata(struct i2c_client *client) if (!np) return NULL; + endpoint.bus_type = V4L2_MBUS_UNKNOWN; + if (v4l2_fwnode_endpoint_parse(of_fwnode_handle(np), ) < 0) + goto done; + pdata = devm_kzalloc(>dev, sizeof(*pdata), GFP_KERNEL); if (!pdata) goto done; @@ -1079,6 +1093,9 @@ mt9p031_get_pdata(struct i2c_client *client) of_property_read_u32(np, "input-clock-frequency", >ext_freq); of_property_read_u32(np, "pixel-clock-frequency", >target_freq); + pdata->pixclk_pol = !!(endpoint.bus.parallel.flags & + V4L2_MBUS_PCLK_SAMPLE_RISING); + done: of_node_put(np); return pdata; diff --git a/include/media/i2c/mt9p031.h b/include/media/i2c/mt9p031.h index 7c29c53aa988..f933cd0be8e5 100644 --- a/include/media/i2c/mt9p031.h +++ b/include/media/i2c/mt9p031.h @@ -10,6 +10,7 @@ struct v4l2_subdev; * @target_freq: Pixel clock frequency */ struct mt9p031_platform_data { + unsigned int pixclk_pol:1; int ext_freq; int target_freq; }; -- 2.25.1
[PATCH 3/5] media: mt9p031: Implement [gs]_register debug calls
From: Enrico Scholz Implement g_register and s_register v4l2_subdev_core_ops to access camera register directly from userspace for debug purposes. Signed-off-by: Enrico Scholz Signed-off-by: Stefan Riedmueller --- drivers/media/i2c/mt9p031.c | 28 1 file changed, 28 insertions(+) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index ce192be4531f..f5d6a7890c47 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -703,6 +703,30 @@ static int mt9p031_restore_blc(struct mt9p031 *mt9p031) return 0; } +#ifdef CONFIG_VIDEO_ADV_DEBUG +static int mt9p031_g_register(struct v4l2_subdev *sd, + struct v4l2_dbg_register *reg) +{ + struct i2c_client *client = v4l2_get_subdevdata(sd); + int ret; + + ret = mt9p031_read(client, reg->reg); + if (ret < 0) + return ret; + + reg->val = ret; + return 0; +} + +static int mt9p031_s_register(struct v4l2_subdev *sd, + struct v4l2_dbg_register const *reg) +{ + struct i2c_client *client = v4l2_get_subdevdata(sd); + + return mt9p031_write(client, reg->reg, reg->val); +} +#endif + static int mt9p031_s_ctrl(struct v4l2_ctrl *ctrl) { struct mt9p031 *mt9p031 = @@ -1000,6 +1024,10 @@ static int mt9p031_close(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) static const struct v4l2_subdev_core_ops mt9p031_subdev_core_ops = { .s_power= mt9p031_set_power, +#ifdef CONFIG_VIDEO_ADV_DEBUG + .s_register = mt9p031_s_register, + .g_register = mt9p031_g_register, +#endif }; static const struct v4l2_subdev_video_ops mt9p031_subdev_video_ops = { -- 2.25.1
[PATCH 5/5] media: mt9p031: Fix corrupted frame after restarting stream
From: Dirk Bender To prevent corrupted frames after starting and stopping the sensor it's datasheet specifies a specific pause sequence to follow: Stopping: Set Pause_Restart Bit -> Set Restart Bit -> Set Chip_Enable Off Restarting: Set Chip_Enable On -> Clear Pause_Restart Bit The Restart Bit is cleared automatically and must not be cleared manually as this would cause undefined behavior. Signed-off-by: Dirk Bender Signed-off-by: Stefan Riedmueller --- drivers/media/i2c/mt9p031.c | 25 + 1 file changed, 25 insertions(+) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 8f8ee37a2dd2..2f2daf95dcd3 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -80,6 +80,8 @@ #defineMT9P031_PIXEL_CLOCK_SHIFT(n)((n) << 8) #defineMT9P031_PIXEL_CLOCK_DIVIDE(n) ((n) << 0) #define MT9P031_FRAME_RESTART 0x0b +#defineMT9P031_FRAME_RESTART_SET (1 << 0) +#defineMT9P031_FRAME_PAUSE_RESTART_SET (1 << 1) #define MT9P031_SHUTTER_DELAY 0x0c #define MT9P031_RST0x0d #defineMT9P031_RST_ENABLE 1 @@ -483,9 +485,25 @@ static int mt9p031_set_params(struct mt9p031 *mt9p031) static int mt9p031_s_stream(struct v4l2_subdev *subdev, int enable) { struct mt9p031 *mt9p031 = to_mt9p031(subdev); + struct i2c_client *client = v4l2_get_subdevdata(subdev); + int val; int ret; if (!enable) { + val = mt9p031_read(client, MT9P031_FRAME_RESTART); + + /* enable pause restart */ + val |= MT9P031_FRAME_PAUSE_RESTART_SET; + ret = mt9p031_write(client, MT9P031_FRAME_RESTART, val); + if (ret < 0) + return ret; + + /* enable restart + keep pause restart set */ + val |= MT9P031_FRAME_RESTART_SET; + ret = mt9p031_write(client, MT9P031_FRAME_RESTART, val); + if (ret < 0) + return ret; + /* Stop sensor readout */ ret = mt9p031_set_output_control(mt9p031, MT9P031_OUTPUT_CONTROL_CEN, 0); @@ -505,6 +523,13 @@ static int mt9p031_s_stream(struct v4l2_subdev *subdev, int enable) if (ret < 0) return ret; + val = mt9p031_read(client, MT9P031_FRAME_RESTART); + /* disable reset + pause restart */ + val &= ~MT9P031_FRAME_PAUSE_RESTART_SET; + ret = mt9p031_write(client, MT9P031_FRAME_RESTART, val); + if (ret < 0) + return ret; + return mt9p031_pll_enable(mt9p031); } -- 2.25.1
[PATCH 2/5] media: mt9p031: Read back the real clock rate
From: Enrico Scholz The real and requested clock can differ and because it is used to calculate PLL values, the real clock rate should be read. Signed-off-by: Enrico Scholz Signed-off-by: Stefan Riedmueller --- drivers/media/i2c/mt9p031.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index 0002dd299ffa..ce192be4531f 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -255,6 +255,7 @@ static int mt9p031_clk_setup(struct mt9p031 *mt9p031) struct i2c_client *client = v4l2_get_subdevdata(>subdev); struct mt9p031_platform_data *pdata = mt9p031->pdata; + unsigned long ext_freq; int ret; mt9p031->clk = devm_clk_get(>dev, NULL); @@ -265,13 +266,15 @@ static int mt9p031_clk_setup(struct mt9p031 *mt9p031) if (ret < 0) return ret; + ext_freq = clk_get_rate(mt9p031->clk); + /* If the external clock frequency is out of bounds for the PLL use the * pixel clock divider only and disable the PLL. */ - if (pdata->ext_freq > limits.ext_clock_max) { + if (ext_freq > limits.ext_clock_max) { unsigned int div; - div = DIV_ROUND_UP(pdata->ext_freq, pdata->target_freq); + div = DIV_ROUND_UP(ext_freq, pdata->target_freq); div = roundup_pow_of_two(div) / 2; mt9p031->clk_div = min_t(unsigned int, div, 64); @@ -280,7 +283,7 @@ static int mt9p031_clk_setup(struct mt9p031 *mt9p031) return 0; } - mt9p031->pll.ext_clock = pdata->ext_freq; + mt9p031->pll.ext_clock = ext_freq; mt9p031->pll.pix_clock = pdata->target_freq; mt9p031->use_pll = true; -- 2.25.1
[PATCH 1/5] media: mt9p031: Add support for 8 bit and 10 bit formats
From: Christian Hemp Aside from 12 bit monochrome or color format the sensor implicitly supports 10 and 8 bit formats as well by simply dropping the corresponding LSBs. Signed-off-by: Christian Hemp [j...@pengutronix.de: simplified by dropping v4l2_colorspace handling] Signed-off-by: Jan Luebbe Signed-off-by: Stefan Riedmueller --- drivers/media/i2c/mt9p031.c | 50 + 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/drivers/media/i2c/mt9p031.c b/drivers/media/i2c/mt9p031.c index dc23b9ed510a..0002dd299ffa 100644 --- a/drivers/media/i2c/mt9p031.c +++ b/drivers/media/i2c/mt9p031.c @@ -116,6 +116,18 @@ enum mt9p031_model { MT9P031_MODEL_MONOCHROME, }; +static const u32 mt9p031_color_fmts[] = { + MEDIA_BUS_FMT_SGRBG8_1X8, + MEDIA_BUS_FMT_SGRBG10_1X10, + MEDIA_BUS_FMT_SGRBG12_1X12, +}; + +static const u32 mt9p031_monochrome_fmts[] = { + MEDIA_BUS_FMT_Y8_1X8, + MEDIA_BUS_FMT_Y10_1X10, + MEDIA_BUS_FMT_Y12_1X12, +}; + struct mt9p031 { struct v4l2_subdev subdev; struct media_pad pad; @@ -138,6 +150,9 @@ struct mt9p031 { struct v4l2_ctrl *blc_auto; struct v4l2_ctrl *blc_offset; + const u32 *fmts; + int num_fmts; + /* Registers cache */ u16 output_control; u16 mode2; @@ -148,6 +163,17 @@ static struct mt9p031 *to_mt9p031(struct v4l2_subdev *sd) return container_of(sd, struct mt9p031, subdev); } +static const u32 mt9p031_find_datafmt(struct mt9p031 *mt9p031, u32 code) +{ + int i; + + for (i = 0; i < mt9p031->num_fmts; i++) + if (mt9p031->fmts[i] == code) + return mt9p031->fmts[i]; + + return mt9p031->fmts[mt9p031->num_fmts-1]; +} + static int mt9p031_read(struct i2c_client *client, u8 reg) { return i2c_smbus_read_word_swapped(client, reg); @@ -476,10 +502,11 @@ static int mt9p031_enum_mbus_code(struct v4l2_subdev *subdev, { struct mt9p031 *mt9p031 = to_mt9p031(subdev); - if (code->pad || code->index) + if (code->pad || code->index >= mt9p031->num_fmts) return -EINVAL; - code->code = mt9p031->format.code; + code->code = mt9p031->fmts[code->index]; + return 0; } @@ -573,6 +600,8 @@ static int mt9p031_set_format(struct v4l2_subdev *subdev, __format->width = __crop->width / hratio; __format->height = __crop->height / vratio; + __format->code = mt9p031_find_datafmt(mt9p031, format->format.code); + format->format = *__format; return 0; @@ -951,10 +980,7 @@ static int mt9p031_open(struct v4l2_subdev *subdev, struct v4l2_subdev_fh *fh) format = v4l2_subdev_get_try_format(subdev, fh->pad, 0); - if (mt9p031->model == MT9P031_MODEL_MONOCHROME) - format->code = MEDIA_BUS_FMT_Y12_1X12; - else - format->code = MEDIA_BUS_FMT_SGRBG12_1X12; + format->code = mt9p031_find_datafmt(mt9p031, 0); format->width = MT9P031_WINDOW_WIDTH_DEF; format->height = MT9P031_WINDOW_HEIGHT_DEF; @@ -1121,10 +1147,14 @@ static int mt9p031_probe(struct i2c_client *client, mt9p031->crop.left = MT9P031_COLUMN_START_DEF; mt9p031->crop.top = MT9P031_ROW_START_DEF; - if (mt9p031->model == MT9P031_MODEL_MONOCHROME) - mt9p031->format.code = MEDIA_BUS_FMT_Y12_1X12; - else - mt9p031->format.code = MEDIA_BUS_FMT_SGRBG12_1X12; + if (mt9p031->model == MT9P031_MODEL_MONOCHROME) { + mt9p031->fmts = mt9p031_monochrome_fmts; + mt9p031->num_fmts = ARRAY_SIZE(mt9p031_monochrome_fmts); + } else { + mt9p031->fmts = mt9p031_color_fmts; + mt9p031->num_fmts = ARRAY_SIZE(mt9p031_color_fmts); + } + mt9p031->format.code = mt9p031_find_datafmt(mt9p031, 0); mt9p031->format.width = MT9P031_WINDOW_WIDTH_DEF; mt9p031->format.height = MT9P031_WINDOW_HEIGHT_DEF; -- 2.25.1
Re: [RFC PATCH 00/24] Control VQ support in vDPA
On Thu, Sep 24, 2020 at 11:21:01AM +0800, Jason Wang wrote: > This series tries to add the support for control virtqueue in vDPA. Please include documentation for both driver authors and vhost-vdpa ioctl users. vhost-vdpa ioctls are only documented with a single sentence. Please add full information on arguments, return values, and a high-level explanation of the feature (like this cover letter) to introduce the API. What is the policy for using virtqueue groups? My guess is: 1. virtio_vdpa simply enables all virtqueue groups. 2. vhost_vdpa relies on userspace policy on how to use virtqueue groups. Are the semantics of virtqueue groups documented somewhere so userspace knows what to do? If a vDPA driver author decides to create N virtqueue groups, N/2 virtqueue groups, or just 1 virtqueue group, how will userspace know what to do? Maybe a document is needed to describe the recommended device-specific virtqueue groups that vDPA drivers should implement (e.g. "put the net control vq into its own virtqueue group")? This could become messy with guidelines. For example, drivers might be shipped that aren't usable for certain use cases just because the author didn't know that a certain virtqueue grouping is advantageous. BTW I like how general this feature is. It seems to allow vDPA devices to be split into sub-devices for further passthrough. Who will write the first vDPA-on-vDPA driver? :) Stefan signature.asc Description: PGP signature
Re: [EXT] Re: [PATCH] net: fec: Keep device numbering consistent with datasheet
Hi Andy, David and Andrew, first of all, thanks for your review. I really appreciate it! On 24.09.20 08:36, Andy Duan wrote: From: David Miller Sent: Thursday, September 24, 2020 4:32 AM From: Stefan Riedmueller Date: Wed, 23 Sep 2020 16:25:28 +0200 From: Christian Hemp Make use of device tree alias for device enumeration to keep the device order consistent with the naming in the datasheet. Otherwise for the i.MX 6UL/ULL the ENET1 interface is enumerated as eth1 and ENET2 as eth0. Signed-off-by: Christian Hemp Signed-off-by: Stefan Riedmueller Device naming and ordering for networking devices was never, ever, guaranteed. Use udev or similar. @@ -3691,6 +3692,10 @@ fec_probe(struct platform_device *pdev) ndev->max_mtu = PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN; + eth_id = of_alias_get_id(pdev->dev.of_node, "ethernet"); + if (eth_id >= 0) + sprintf(ndev->name, "eth%d", eth_id); You can't ever just write into ndev->name, what if another networking device is already using that name? This change is incorrect on many levels. David is correct. For example, imx8DXL has ethernet0 is EQOS TSN, ethernet1 is FEC. EQOS TSN is andother driver and is registered early, the dev->name is eth0. So the patch will bring conflict in such case. I was not aware of that conflict, but now that you mention it it makes total sense. I wanted to make life a little easier for myself but underestimated the global context. I will try to find a solution with udev or something similar. So please drop this patch and sorry for the noise. Stefan Andy
[PATCH] net: fec: Keep device numbering consistent with datasheet
From: Christian Hemp Make use of device tree alias for device enumeration to keep the device order consistent with the naming in the datasheet. Otherwise for the i.MX 6UL/ULL the ENET1 interface is enumerated as eth1 and ENET2 as eth0. Signed-off-by: Christian Hemp Signed-off-by: Stefan Riedmueller --- drivers/net/ethernet/freescale/fec_main.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c index fb37816a74db..97dd41bed70a 100644 --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c @@ -3504,6 +3504,7 @@ fec_probe(struct platform_device *pdev) char irq_name[8]; int irq_cnt; struct fec_devinfo *dev_info; + int eth_id; fec_enet_get_queue_num(pdev, _tx_qs, _rx_qs); @@ -3691,6 +3692,10 @@ fec_probe(struct platform_device *pdev) ndev->max_mtu = PKT_MAXBUF_SIZE - ETH_HLEN - ETH_FCS_LEN; + eth_id = of_alias_get_id(pdev->dev.of_node, "ethernet"); + if (eth_id >= 0) + sprintf(ndev->name, "eth%d", eth_id); + ret = register_netdev(ndev); if (ret) goto failed_register; -- 2.25.1
Re: [PATCH 08/14] dasd: cleanup dasd_scan_partitions
Am 21.09.20 um 09:19 schrieb Christoph Hellwig: > Use blkdev_get_by_dev instead of bdget_disk + blkdev_get. > > Signed-off-by: Christoph Hellwig Beside what Sergei mentioned... Reviewed-by: Stefan Haberland > --- > drivers/s390/block/dasd_genhd.c | 15 --- > 1 file changed, 4 insertions(+), 11 deletions(-) > > diff --git a/drivers/s390/block/dasd_genhd.c b/drivers/s390/block/dasd_genhd.c > index af5b0ecb8f8923..a9698fba9b76ce 100644 > --- a/drivers/s390/block/dasd_genhd.c > +++ b/drivers/s390/block/dasd_genhd.c > @@ -101,18 +101,11 @@ int dasd_scan_partitions(struct dasd_block *block) > struct block_device *bdev; > int rc; > > - bdev = bdget_disk(block->gdp, 0); > - if (!bdev) { > - DBF_DEV_EVENT(DBF_ERR, block->base, "%s", > - "scan partitions error, bdget returned NULL"); > - return -ENODEV; > - } > - > - rc = blkdev_get(bdev, FMODE_READ, NULL); > - if (rc < 0) { > + bdev = blkdev_get_by_dev(disk_devt(block->gdp), FMODE_READ, NULL); > + if (IS_ERR(bdev)) { > DBF_DEV_EVENT(DBF_ERR, block->base, > - "scan partitions error, blkdev_get returned %d", > - rc); > + "scan partitions error, blkdev_get returned %ld", > + PTR_ERR(bdev)); > return -ENODEV; > } >
[PATCH] Input: stmpe: Add axis inversion and swapping capability
Make use of generic touchscreen_properties structure to add axis inversion and swapping capabilities. It's configurable via devicetree properties: touchscreen-inverted-x touchscreen-inverted-y touchscreen-swapped-x-y Signed-off-by: Stefan Riedmueller --- drivers/input/touchscreen/stmpe-ts.c | 7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/drivers/input/touchscreen/stmpe-ts.c b/drivers/input/touchscreen/stmpe-ts.c index 7e16fcfe3b95..cd747725589b 100644 --- a/drivers/input/touchscreen/stmpe-ts.c +++ b/drivers/input/touchscreen/stmpe-ts.c @@ -14,6 +14,7 @@ #include #include #include +#include #include #include #include @@ -72,6 +73,7 @@ struct stmpe_touch { struct input_dev *idev; struct delayed_work work; struct device *dev; + struct touchscreen_properties prop; u8 ave_ctrl; u8 touch_det_delay; u8 settling; @@ -150,8 +152,7 @@ static irqreturn_t stmpe_ts_handler(int irq, void *data) y = ((data_set[1] & 0xf) << 8) | data_set[2]; z = data_set[3]; - input_report_abs(ts->idev, ABS_X, x); - input_report_abs(ts->idev, ABS_Y, y); + touchscreen_report_pos(ts->idev, >prop, x, y, false); input_report_abs(ts->idev, ABS_PRESSURE, z); input_report_key(ts->idev, BTN_TOUCH, 1); input_sync(ts->idev); @@ -337,6 +338,8 @@ static int stmpe_input_probe(struct platform_device *pdev) input_set_abs_params(idev, ABS_Y, 0, XY_MASK, 0, 0); input_set_abs_params(idev, ABS_PRESSURE, 0x0, 0xff, 0, 0); + touchscreen_parse_properties(idev, false, >prop); + error = input_register_device(idev); if (error) { dev_err(>dev, "Could not register input device\n"); -- 2.25.1
Re: [PATCH] dasd: remove the unused dasd_enable_device export
Am 03.09.20 um 10:30 schrieb Christoph Hellwig: > Signed-off-by: Christoph Hellwig Acked-by:Stefan Haberland > --- > drivers/s390/block/dasd.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/drivers/s390/block/dasd.c b/drivers/s390/block/dasd.c > index eb17fea8075c6f..6ab992b3eed004 100644 > --- a/drivers/s390/block/dasd.c > +++ b/drivers/s390/block/dasd.c > @@ -683,7 +683,6 @@ void dasd_enable_device(struct dasd_device *device) > if (device->discipline->kick_validate) > device->discipline->kick_validate(device); > } > -EXPORT_SYMBOL(dasd_enable_device); > > /* > * SECTION: device operation (interrupt handler, start i/o, term i/o ...)
boot interrupt quirk (also in 4.19.y) breaks serial ports (was: [PATCH v2 0/2] pci: Add boot interrupt quirk mechanism for Xeon chipsets)
Hi, this quirk breaks our serial ports PCIe card (i.e. we don't see any output from the connected devices; no idea whether anything we send reaches them): 05:00.0 PCI bridge: PLX Technology, Inc. PEX8112 x1 Lane PCI Express-to-PCI Bridge (rev aa) 06:00.0 Serial controller: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Uart) 06:00.1 Bridge: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Disabled) 06:01.0 Serial controller: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Uart) 06:01.1 Bridge: Oxford Semiconductor Ltd OX16PCI954 (Quad 16950 UART) function 0 (Disabled) The hardware is "MP8-PCIe-RS232-R2" (we also have the 16-port variant) from https://rs-232.de/sercom_e.htm#MP8PCIERS232 The CPU is an "Intel(R) Xeon(R) CPU D-1518 @ 2.20GHz". Disabling the quirk for 0x6f28 (Xeon D-1500) on 4.19.145 gets the serial ports working again (patch attached for reference). The box is running debian stable; we build ixgbe 5.6.1 through dkms (local package) due to issues with the SFP+ ports (usually our uplink...). Known broken versions: 4.19.115-00065, 4.19.118, 4.19.145, 5.8.9 Known good versions: 4.19.115-00064 Basically if we see "kernel: pci :00:05.0: disabled boot interrupts on device [8086:6f28]" in the kernel log it's broken. Attached lspci, lscpu, lsmod and kernel log outputs while running 5.8.9. Can we disable the quirk somehow via the kernel commandline? Anything else I can do to help getting this fixed properly? (We have the same card in other boxes with CPUs not affected by this quirk; they are working just fine.) cheers, Stefan -- Stefan BühlerMail/xmpp: stefan.bueh...@tik.uni-stuttgart.de Netze und Kommunikationssysteme der Universität Stuttgart (NKS) https://www.tik.uni-stuttgart.de/Telefon: +49 711 685 60854 00:00.0 Host bridge: Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D DMI2 (rev 03) 00:01.0 PCI bridge: Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D PCI Express Root Port 1 (rev 03) 00:01.1 PCI bridge: Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D PCI Express Root Port 1 (rev 03) 00:02.0 PCI bridge: Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D PCI Express Root Port 2 (rev 03) 00:02.2 PCI bridge: Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D PCI Express Root Port 2 (rev 03) 00:03.0 PCI bridge: Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D PCI Express Root Port 3 (rev 03) 00:03.2 PCI bridge: Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D PCI Express Root Port 3 (rev 03) 00:05.0 System peripheral: Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D Map/VTd_Misc/System Management (rev 03) 00:05.1 System peripheral: Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D IIO Hot Plug (rev 03) 00:05.2 System peripheral: Intel Corporation Xeon E7 v4/Xeon E5 v4/Xeon E3 v4/Xeon D IIO RAS/Control Status/Global Errors (rev 03) 00:14.0 USB controller: Intel Corporation 8 Series/C220 Series Chipset Family USB xHCI (rev 05) 00:16.0 Communication controller: Intel Corporation 8 Series/C220 Series Chipset Family MEI Controller #1 (rev 04) 00:16.1 Communication controller: Intel Corporation 8 Series/C220 Series Chipset Family MEI Controller #2 (rev 04) 00:1a.0 USB controller: Intel Corporation 8 Series/C220 Series Chipset Family USB EHCI #2 (rev 05) 00:1c.0 PCI bridge: Intel Corporation 8 Series/C220 Series Chipset Family PCI Express Root Port #1 (rev d5) 00:1c.1 PCI bridge: Intel Corporation 8 Series/C220 Series Chipset Family PCI Express Root Port #2 (rev d5) 00:1c.3 PCI bridge: Intel Corporation 8 Series/C220 Series Chipset Family PCI Express Root Port #4 (rev d5) 00:1c.4 PCI bridge: Intel Corporation 8 Series/C220 Series Chipset Family PCI Express Root Port #5 (rev d5) 00:1d.0 USB controller: Intel Corporation 8 Series/C220 Series Chipset Family USB EHCI #1 (rev 05) 00:1f.0 ISA bridge: Intel Corporation C224 Series Chipset Family Server Standard SKU LPC Controller (rev 05) 00:1f.3 SMBus: Intel Corporation 8 Series/C220 Series Chipset Family SMBus Controller (rev 05) 00:1f.6 Signal processing controller: Intel Corporation 8 Series Chipset Family Thermal Management Controller (rev 05) 01:00.0 Non-Volatile memory controller: Intel Corporation SSD 600P Series (rev 03) 03:00.0 System peripheral: Intel Corporation Xeon Processor D Family QuickData Technology Register DMA Channel 0 03:00.1 System peripheral: Intel Corporation Xeon Processor D Family QuickData Technology Register DMA Channel 1 03:00.2 System peripheral: Intel Corporation Xeon Processor D Family QuickData Technology Register DMA Channel 2 03:00.3 System peripheral: Intel Corporation Xeon Processor D Family QuickData Technology Register DMA Channel 3 04:00.0 Ethernet controller: Intel Corporation Ethernet Connection X552 10 GbE SFP+ 04:00.1 Ethernet controller: Intel Corporation Ethernet Connection X552 10 GbE
Re: [PATCH v3] drm: mxsfb: check framebuffer pitch
On 2020-09-08 16:16, Stefan Agner wrote: > The lcdif IP does not support a framebuffer pitch (stride) other than > framebuffer width. Check for equality and reject the framebuffer > otherwise. > > This prevents a distorted picture when using 640x800 and running the > Mesa graphics stack. Mesa tries to use a cache aligned stride, which > leads at that particular resolution to width != stride. Currently > Mesa has no fallback behavior, but rejecting this configuration allows > userspace to handle the issue correctly. > > Fixes: 45d59d704080 ("drm: Add new driver for MXSFB controller") > Signed-off-by: Stefan Agner > Reviewed-by: Laurent Pinchart Applied to drm-misc-next. -- Stefan > --- > Changes in v3: > - Fix typo in commit message > - Add fixes tag > > Changes in v2: > - Validate pitch in fb_create callback > > drivers/gpu/drm/mxsfb/mxsfb_drv.c | 21 - > 1 file changed, 20 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c > b/drivers/gpu/drm/mxsfb/mxsfb_drv.c > index 8c549c3931af..35122aef037b 100644 > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c > @@ -21,6 +21,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -81,8 +82,26 @@ void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb) > clk_disable_unprepare(mxsfb->clk_axi); > } > > +static struct drm_framebuffer * > +mxsfb_fb_create(struct drm_device *dev, struct drm_file *file_priv, > + const struct drm_mode_fb_cmd2 *mode_cmd) > +{ > + const struct drm_format_info *info; > + > + info = drm_get_format_info(dev, mode_cmd); > + if (!info) > + return ERR_PTR(-EINVAL); > + > + if (mode_cmd->width * info->cpp[0] != mode_cmd->pitches[0]) { > + dev_dbg(dev->dev, "Invalid pitch: fb width must match pitch\n"); > + return ERR_PTR(-EINVAL); > + } > + > + return drm_gem_fb_create(dev, file_priv, mode_cmd); > +} > + > static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = { > - .fb_create = drm_gem_fb_create, > + .fb_create = mxsfb_fb_create, > .atomic_check = drm_atomic_helper_check, > .atomic_commit = drm_atomic_helper_commit, > };
[BUG] NULL ptr deref in css_free_rwork_fn
Hi, i noticed a regular crash (NULL pointer dereference) on my ARM i.MX28 board using the current mainline kernel (last tested version Linux 5.9-rc5). I also tested Linux 5.8, but i wasn't able to reproduce with this version, so i assume this is a regression. The crash happens mostly short after boot or during shutdown. Unfortunately i don't have a specific scenario to trigger this issue. The issue was reproducable with arm/mxs_defconfig. Here is the crash output: [ 58.670665] 8<--- cut here --- [ 58.673844] Unable to handle kernel NULL pointer dereference at virtual address 02c0 [ 58.682130] pgd = aa9080fa [ 58.684863] [02c0] *pgd= [ 58.688467] Internal error: Oops: 5 [#1] ARM [ 58.692743] Modules linked in: [ 58.695826] CPU: 0 PID: 41 Comm: kworker/0:2 Not tainted 5.9.0-rc5-9-g9d3c35e #15 [ 58.703660] Hardware name: Freescale MXS (Device Tree) [ 58.708830] Workqueue: cgroup_destroy css_free_rwork_fn [ 58.714089] PC is at kernfs_put+0xb8/0x1c4 [ 58.718195] LR is at 0xc6f4c7c0 [ 58.721342] pc : [] lr : [] psr: 0013 [ 58.727614] sp : c7787ed0 ip : 6013 fp : c6f47a10 [ 58.732843] r10: c6f4c9c0 r9 : c0ae95fc r8 : c0ae95f8 [ 58.738072] r7 : c74249c8 r6 : c6f486e0 r5 : c670b898 r4 : 02c0 [ 58.744603] r3 : r2 : 0001 r1 : 0001 r0 : c7ee4030 [ 58.751138] Flags: nzcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 58.758280] Control: 0005317f Table: 44d18000 DAC: 0053 [ 58.764043] Process kworker/0:2 (pid: 41, stack limit = 0x6fb11f96) [ 58.770322] Stack: (0xc7787ed0 to 0xc7788000) [ 58.774697] 7ec0: 6013 c0ae2664 c7ee4d00 [ 58.782896] 7ee0: c653b800 c74249c0 c0ab2df4 c653b864 c653b800 c7ee4d00 [ 58.791096] 7f00: c653b868 c0ab2df4 c0083574 c653b864 c773d720 c7ee4d00 [ 58.799295] 7f20: c00309e8 c0a21370 c7786000 c0a28fa0 c773d720 c0a2135c c773d734 [ 58.807493] 7f40: c0a21370 c7786000 c0a28fa0 0008 c0a2135c c0031158 c773d720 c7497eb8 [ 58.815693] 7f60: c7787f7c c778c100 c7781200 c7786000 c0031108 c773d720 c7497eb8 [ 58.823890] 7f80: c778c120 c00371d4 6013 c7781200 c00370dc [ 58.832087] 7fa0: c0008540 [ 58.840282] 7fc0: [ 58.848478] 7fe0: 0013 [ 58.856698] [] (kernfs_put) from [] (css_free_rwork_fn+0x19c/0x3ac) [ 58.864739] [] (css_free_rwork_fn) from [] (process_one_work+0x14c/0x4d4) [ 58.873293] [] (process_one_work) from [] (worker_thread+0x50/0x590) [ 58.881421] [] (worker_thread) from [] (kthread+0xf8/0x134) [ 58.888764] [] (kthread) from [] (ret_from_fork+0x14/0x34) [ 58.895996] Exception stack(0xc7787fb0 to 0xc7787ff8) [ 58.901062] 7fa0: [ 58.909259] 7fc0: [ 58.917452] 7fe0: 0013 [ 58.924088] Code: ebfdce1f e1aa ebfce9aa e154000b (e5943000) [ 58.930410] ---[ end trace 73455566ad3e0105 ]---
Re: [PATCH 12/24] getgrent_r.3: Use sizeof() to get buffer size (instead of hardcoding macro name)
Hi, On Fri, Sep 11, 2020 at 6:28 PM Alejandro Colomar wrote: > > Hi Stefan, > > On 2020-09-11 16:35, Stefan Puiu wrote: > > Hi, > > > > On Fri, Sep 11, 2020 at 12:15 AM Alejandro Colomar > > wrote: > >> > >> Signed-off-by: Alejandro Colomar > >> --- > >> man3/getgrent_r.3 | 2 +- > >> 1 file changed, 1 insertion(+), 1 deletion(-) > >> > >> diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3 > >> index 81d81a851..76deec370 100644 > >> --- a/man3/getgrent_r.3 > >> +++ b/man3/getgrent_r.3 > >> @@ -186,7 +186,7 @@ main(void) > >> > >> setgrent(); > >> while (1) { > >> -i = getgrent_r(, buf, BUFLEN, ); > >> +i = getgrent_r(, buf, sizeof(buf), ); > > > > I'm worried that less attentive people might copy/paste parts of this > > in their code, where maybe buf is just a pointer, and expect it to > > work. Maybe leaving BUFLEN here is useful as a reminder that they need > > to change something to adapt the code? > > > > Just my 2 cents, > > Stefan. > > > That's a very good point. > > So we have 3 options and I will propose now a 4th one. Let's see all > of them and see which one is better for the man pages. > > 1.- Use the macro everywhere. > > pros: > - It is still valid when the buffer is a pointer and not an array. > cons: > - Hardcodes the initializer. If the array is later initialized with a >different value, it may produce a silent bug, or a compilation break. > > 2.- Use sizeof() everywhere, and the macro for the initializer. > > pros: > - It is valid as long as the buffer is an array. > cons: > - If the code gets into a function, and the buffer is then a pointer, >it will definitively produce a silent bug. > > 3.- Use sizeof() everywhere, and a magic number for the initializer. > > The same as 2. > > 4.- Use ARRAY_BYTES() macro > > pros: > - It is always safe and when code changes, it may break compilation, but >never a silent bug. > cons: > - Add a few lines of code. Maybe too much complexity for an example. >But I'd say that it is the only safe option, and in real code it >should probably be used more, so maybe it's good to show a good practice. If you ask me, I think examples should be simple and easy to understand, and easy to copy/paste in your code. I'd settle for easy enough, not perfect or completely foolproof. If you need to look up obscure gcc features to understand an example, that's not very helpful. So I'd be more inclined to prefer version 1 above. But let's see Michael's opinion on this. Just my 2c, Stefan.
greetings
-- My name is Mrs.Julianna Stefan Ndoi. I am a cancer patient who had decided to donate what I have to you for God's works. I want to donate $8.5 million to you so that you will use part of the this fund to help the poor ones,while you use the rest for your family.If you are interested,Respond now for more details on how to receive this fund. Regards, Mrs.Julianna,greeting from the sick bed
Re: [PATCH 12/24] getgrent_r.3: Use sizeof() to get buffer size (instead of hardcoding macro name)
Hi, On Fri, Sep 11, 2020 at 12:15 AM Alejandro Colomar wrote: > > Signed-off-by: Alejandro Colomar > --- > man3/getgrent_r.3 | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/man3/getgrent_r.3 b/man3/getgrent_r.3 > index 81d81a851..76deec370 100644 > --- a/man3/getgrent_r.3 > +++ b/man3/getgrent_r.3 > @@ -186,7 +186,7 @@ main(void) > > setgrent(); > while (1) { > -i = getgrent_r(, buf, BUFLEN, ); > +i = getgrent_r(, buf, sizeof(buf), ); I'm worried that less attentive people might copy/paste parts of this in their code, where maybe buf is just a pointer, and expect it to work. Maybe leaving BUFLEN here is useful as a reminder that they need to change something to adapt the code? Just my 2 cents, Stefan.
Re: [PATCH] drm: mxsfb: check framebuffer pitch
On 2020-09-08 10:48, Daniel Vetter wrote: > On Tue, Sep 08, 2020 at 11:18:25AM +0300, Tomi Valkeinen wrote: >> Hi, >> >> On 08/09/2020 10:55, Stefan Agner wrote: >> > On 2020-09-07 20:18, Daniel Vetter wrote: >> >> On Mon, Sep 07, 2020 at 07:17:12PM +0300, Laurent Pinchart wrote: >> >>> Hi Stefan, >> >>> >> >>> Thank you for the patch. >> >>> >> >>> On Mon, Sep 07, 2020 at 06:03:43PM +0200, Stefan Agner wrote: >> >>>> The lcdif IP does not support a framebuffer pitch (stride) other than >> >>>> the CRTC width. Check for equality and reject the state otherwise. >> >>>> >> >>>> This prevents a distorted picture when using 640x800 and running the >> >>>> Mesa graphics stack. Mesa tires to use a cache aligned stride, which >> >>> >> >>> s/tires/tries/ >> >>> >> >>>> leads at that particular resolution to width != stride. Currently >> >>>> Mesa has no fallback behavior, but rejecting this configuration allows >> >>>> userspace to handle the issue correctly. >> >>> >> >>> I'm increasingly impressed by how featureful this IP core is :-) >> >>> >> >>>> Signed-off-by: Stefan Agner >> >>>> --- >> >>>> drivers/gpu/drm/mxsfb/mxsfb_kms.c | 22 ++ >> >>>> 1 file changed, 18 insertions(+), 4 deletions(-) >> >>>> >> >>>> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c >> >>>> b/drivers/gpu/drm/mxsfb/mxsfb_kms.c >> >>>> index b721b8b262ce..79aa14027f91 100644 >> >>>> --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c >> >>>> +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c >> >>>> @@ -403,14 +403,28 @@ static int mxsfb_plane_atomic_check(struct >> >>>> drm_plane *plane, >> >>>> { >> >>>> struct mxsfb_drm_private *mxsfb = >> >>>> to_mxsfb_drm_private(plane->dev); >> >>>> struct drm_crtc_state *crtc_state; >> >>>> + unsigned int pitch; >> >>>> + int ret; >> >>>> >> >>>> crtc_state = drm_atomic_get_new_crtc_state(plane_state->state, >> >>>>>crtc); >> >>>> >> >>>> - return drm_atomic_helper_check_plane_state(plane_state, >> >>>> crtc_state, >> >>>> - >> >>>> DRM_PLANE_HELPER_NO_SCALING, >> >>>> - >> >>>> DRM_PLANE_HELPER_NO_SCALING, >> >>>> - false, true); >> >>>> + ret = drm_atomic_helper_check_plane_state(plane_state, >> >>>> crtc_state, >> >>>> + >> >>>> DRM_PLANE_HELPER_NO_SCALING, >> >>>> + >> >>>> DRM_PLANE_HELPER_NO_SCALING, >> >>>> + false, true); >> >>>> + if (ret || !plane_state->visible) >> >>> >> >>> Would it be more explict to check for !plane_state->fb ? Otherwise I'll >> >>> have to verify that !fb always implies !visible :-) >> >>> >> >>>> + return ret; >> >>>> + >> >>>> + pitch = crtc_state->mode.hdisplay * >> >>>> + plane_state->fb->format->cpp[0]; >> >>> >> >>> This holds on a single line. >> >>> >> >>>> + if (plane_state->fb->pitches[0] != pitch) { >> >>>> + dev_err(plane->dev->dev, >> >>>> + "Invalid pitch: fb and crtc widths must be the >> >>>> same"); >> >>> >> >>> I'd turn this into a dev_dbg(), printing error messages to the kernel >> >>> log in response to user-triggered conditions is a bit too verbose and >> >>> could flood the log. >> >>> >> >>> Wouldn't it be best to catch this issue when creating the framebuffer ? >> >> >> >> Yeah this should be verified at addfb time. We try to validate as early as >> >> possible. >> >> -Daniel >> >> >> > >> > Sounds sensible. From what I can tell fb_create is the proper callback >> > to implement this at addfb time. Will give this a try. >> > >> > FWIW, I got the idea from drivers/gpu/drm/tilcdc/tilcdc_plane.c. Maybe >> > should be moved to addfb there too? >> >> But you don't know the crtc width when creating the framebuffer. > > Hm right this is a different check. What we could check in fb_create for > both is that the logical fb size matches exactly the pitch. That's not > sufficient criteria, but it will at least catch some of them already. > > But yeah we'd need both here. After validating width of framebuffer against pitch, the only thing we need to check here is that the width matches. From what I can tell, least for mxsfb, this should be covered by drm_atomic_helper_check_plane_state's can_position parameter set to false. So I think in my case I can get away by only checking the framebuffer. -- Stefan > -Daniel > >> >> Tomi >> >> -- >> Texas Instruments Finland Oy, Porkkalankatu 22, 00180 Helsinki. >> Y-tunnus/Business ID: 0615521-4. Kotipaikka/Domicile: Helsinki
[PATCH v3] drm: mxsfb: check framebuffer pitch
The lcdif IP does not support a framebuffer pitch (stride) other than framebuffer width. Check for equality and reject the framebuffer otherwise. This prevents a distorted picture when using 640x800 and running the Mesa graphics stack. Mesa tries to use a cache aligned stride, which leads at that particular resolution to width != stride. Currently Mesa has no fallback behavior, but rejecting this configuration allows userspace to handle the issue correctly. Fixes: 45d59d704080 ("drm: Add new driver for MXSFB controller") Signed-off-by: Stefan Agner Reviewed-by: Laurent Pinchart --- Changes in v3: - Fix typo in commit message - Add fixes tag Changes in v2: - Validate pitch in fb_create callback drivers/gpu/drm/mxsfb/mxsfb_drv.c | 21 - 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index 8c549c3931af..35122aef037b 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -81,8 +82,26 @@ void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb) clk_disable_unprepare(mxsfb->clk_axi); } +static struct drm_framebuffer * +mxsfb_fb_create(struct drm_device *dev, struct drm_file *file_priv, + const struct drm_mode_fb_cmd2 *mode_cmd) +{ + const struct drm_format_info *info; + + info = drm_get_format_info(dev, mode_cmd); + if (!info) + return ERR_PTR(-EINVAL); + + if (mode_cmd->width * info->cpp[0] != mode_cmd->pitches[0]) { + dev_dbg(dev->dev, "Invalid pitch: fb width must match pitch\n"); + return ERR_PTR(-EINVAL); + } + + return drm_gem_fb_create(dev, file_priv, mode_cmd); +} + static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = { - .fb_create = drm_gem_fb_create, + .fb_create = mxsfb_fb_create, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, }; -- 2.28.0
Re: [PATCH] drm: mxsfb: check framebuffer pitch
On 2020-09-08 14:33, Laurent Pinchart wrote: > On Tue, Sep 08, 2020 at 02:29:02PM +0200, Daniel Vetter wrote: >> On Tue, Sep 8, 2020 at 2:07 PM Stefan Agner wrote: >> > On 2020-09-08 10:48, Daniel Vetter wrote: >> >> On Tue, Sep 08, 2020 at 11:18:25AM +0300, Tomi Valkeinen wrote: >> >>> On 08/09/2020 10:55, Stefan Agner wrote: >> >>>> On 2020-09-07 20:18, Daniel Vetter wrote: >> >>>>> On Mon, Sep 07, 2020 at 07:17:12PM +0300, Laurent Pinchart wrote: >> >>>>>> Hi Stefan, >> >>>>>> >> >>>>>> Thank you for the patch. >> >>>>>> >> >>>>>> On Mon, Sep 07, 2020 at 06:03:43PM +0200, Stefan Agner wrote: >> >>>>>>> The lcdif IP does not support a framebuffer pitch (stride) other than >> >>>>>>> the CRTC width. Check for equality and reject the state otherwise. >> >>>>>>> >> >>>>>>> This prevents a distorted picture when using 640x800 and running the >> >>>>>>> Mesa graphics stack. Mesa tires to use a cache aligned stride, which >> >>>>>> >> >>>>>> s/tires/tries/ >> >>>>>> >> >>>>>>> leads at that particular resolution to width != stride. Currently >> >>>>>>> Mesa has no fallback behavior, but rejecting this configuration >> >>>>>>> allows >> >>>>>>> userspace to handle the issue correctly. >> >>>>>> >> >>>>>> I'm increasingly impressed by how featureful this IP core is :-) >> >>>>>> >> >>>>>>> Signed-off-by: Stefan Agner >> >>>>>>> --- >> >>>>>>> drivers/gpu/drm/mxsfb/mxsfb_kms.c | 22 ++ >> >>>>>>> 1 file changed, 18 insertions(+), 4 deletions(-) >> >>>>>>> >> >>>>>>> diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c >> >>>>>>> b/drivers/gpu/drm/mxsfb/mxsfb_kms.c >> >>>>>>> index b721b8b262ce..79aa14027f91 100644 >> >>>>>>> --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c >> >>>>>>> +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c >> >>>>>>> @@ -403,14 +403,28 @@ static int mxsfb_plane_atomic_check(struct >> >>>>>>> drm_plane *plane, >> >>>>>>> { >> >>>>>>> struct mxsfb_drm_private *mxsfb = >> >>>>>>> to_mxsfb_drm_private(plane->dev); >> >>>>>>> struct drm_crtc_state *crtc_state; >> >>>>>>> + unsigned int pitch; >> >>>>>>> + int ret; >> >>>>>>> >> >>>>>>> crtc_state = >> >>>>>>> drm_atomic_get_new_crtc_state(plane_state->state, >> >>>>>>>>crtc); >> >>>>>>> >> >>>>>>> - return drm_atomic_helper_check_plane_state(plane_state, >> >>>>>>> crtc_state, >> >>>>>>> - >> >>>>>>> DRM_PLANE_HELPER_NO_SCALING, >> >>>>>>> - >> >>>>>>> DRM_PLANE_HELPER_NO_SCALING, >> >>>>>>> - false, true); >> >>>>>>> + ret = drm_atomic_helper_check_plane_state(plane_state, >> >>>>>>> crtc_state, >> >>>>>>> + >> >>>>>>> DRM_PLANE_HELPER_NO_SCALING, >> >>>>>>> + >> >>>>>>> DRM_PLANE_HELPER_NO_SCALING, >> >>>>>>> + false, true); >> >>>>>>> + if (ret || !plane_state->visible) >> >>>>>> >> >>>>>> Would it be more explict to check for !plane_state->fb ? Otherwise >> >>>>>> I'll >> >>>>>> have to verify that !fb always implies !visible :-) >
[PATCH v2] drm: mxsfb: check framebuffer pitch
The lcdif IP does not support a framebuffer pitch (stride) other than framebuffer width. Check for equality and reject the framebuffer otherwise. This prevents a distorted picture when using 640x800 and running the Mesa graphics stack. Mesa tires to use a cache aligned stride, which leads at that particular resolution to width != stride. Currently Mesa has no fallback behavior, but rejecting this configuration allows userspace to handle the issue correctly. Signed-off-by: Stefan Agner --- drivers/gpu/drm/mxsfb/mxsfb_drv.c | 21 - 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c b/drivers/gpu/drm/mxsfb/mxsfb_drv.c index 8c549c3931af..fa6798d21029 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c @@ -21,6 +21,7 @@ #include #include #include +#include #include #include #include @@ -81,8 +82,26 @@ void mxsfb_disable_axi_clk(struct mxsfb_drm_private *mxsfb) clk_disable_unprepare(mxsfb->clk_axi); } +static struct drm_framebuffer * +mxsfb_fb_create(struct drm_device *dev, struct drm_file *file_priv, + const struct drm_mode_fb_cmd2 *mode_cmd) +{ + const struct drm_format_info *info; + + info = drm_get_format_info(dev, mode_cmd); + if (!info) + return ERR_PTR(-EINVAL); + + if (mode_cmd->width * info->cpp[0] != mode_cmd->pitches[0]) { + dev_dbg(dev->dev, "Invalid pitch: fb width must match pitch\n"); + return ERR_PTR(-EINVAL); + } + + return drm_gem_fb_create(dev, file_priv, mode_cmd); +} + static const struct drm_mode_config_funcs mxsfb_mode_config_funcs = { - .fb_create = drm_gem_fb_create, + .fb_create = mxsfb_fb_create, .atomic_check = drm_atomic_helper_check, .atomic_commit = drm_atomic_helper_commit, }; -- 2.28.0
Re: [PATCH] drm: mxsfb: check framebuffer pitch
On 2020-09-07 20:18, Daniel Vetter wrote: > On Mon, Sep 07, 2020 at 07:17:12PM +0300, Laurent Pinchart wrote: >> Hi Stefan, >> >> Thank you for the patch. >> >> On Mon, Sep 07, 2020 at 06:03:43PM +0200, Stefan Agner wrote: >> > The lcdif IP does not support a framebuffer pitch (stride) other than >> > the CRTC width. Check for equality and reject the state otherwise. >> > >> > This prevents a distorted picture when using 640x800 and running the >> > Mesa graphics stack. Mesa tires to use a cache aligned stride, which >> >> s/tires/tries/ >> >> > leads at that particular resolution to width != stride. Currently >> > Mesa has no fallback behavior, but rejecting this configuration allows >> > userspace to handle the issue correctly. >> >> I'm increasingly impressed by how featureful this IP core is :-) >> >> > Signed-off-by: Stefan Agner >> > --- >> > drivers/gpu/drm/mxsfb/mxsfb_kms.c | 22 ++ >> > 1 file changed, 18 insertions(+), 4 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c >> > b/drivers/gpu/drm/mxsfb/mxsfb_kms.c >> > index b721b8b262ce..79aa14027f91 100644 >> > --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c >> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c >> > @@ -403,14 +403,28 @@ static int mxsfb_plane_atomic_check(struct drm_plane >> > *plane, >> > { >> >struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(plane->dev); >> >struct drm_crtc_state *crtc_state; >> > + unsigned int pitch; >> > + int ret; >> > >> >crtc_state = drm_atomic_get_new_crtc_state(plane_state->state, >> > >crtc); >> > >> > - return drm_atomic_helper_check_plane_state(plane_state, crtc_state, >> > - DRM_PLANE_HELPER_NO_SCALING, >> > - DRM_PLANE_HELPER_NO_SCALING, >> > - false, true); >> > + ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state, >> > +DRM_PLANE_HELPER_NO_SCALING, >> > +DRM_PLANE_HELPER_NO_SCALING, >> > +false, true); >> > + if (ret || !plane_state->visible) >> >> Would it be more explict to check for !plane_state->fb ? Otherwise I'll >> have to verify that !fb always implies !visible :-) >> >> > + return ret; >> > + >> > + pitch = crtc_state->mode.hdisplay * >> > + plane_state->fb->format->cpp[0]; >> >> This holds on a single line. >> >> > + if (plane_state->fb->pitches[0] != pitch) { >> > + dev_err(plane->dev->dev, >> > + "Invalid pitch: fb and crtc widths must be the same"); >> >> I'd turn this into a dev_dbg(), printing error messages to the kernel >> log in response to user-triggered conditions is a bit too verbose and >> could flood the log. >> >> Wouldn't it be best to catch this issue when creating the framebuffer ? > > Yeah this should be verified at addfb time. We try to validate as early as > possible. > -Daniel > Sounds sensible. From what I can tell fb_create is the proper callback to implement this at addfb time. Will give this a try. FWIW, I got the idea from drivers/gpu/drm/tilcdc/tilcdc_plane.c. Maybe should be moved to addfb there too? [added Jyri/Tomi] -- Stefan >> >> > + return -EINVAL; >> > + } >> > + >> > + return 0; >> > } >> > >> > static void mxsfb_plane_primary_atomic_update(struct drm_plane *plane, >> >> -- >> Regards, >> >> Laurent Pinchart
[PATCH] drm: mxsfb: check framebuffer pitch
The lcdif IP does not support a framebuffer pitch (stride) other than the CRTC width. Check for equality and reject the state otherwise. This prevents a distorted picture when using 640x800 and running the Mesa graphics stack. Mesa tires to use a cache aligned stride, which leads at that particular resolution to width != stride. Currently Mesa has no fallback behavior, but rejecting this configuration allows userspace to handle the issue correctly. Signed-off-by: Stefan Agner --- drivers/gpu/drm/mxsfb/mxsfb_kms.c | 22 ++ 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/mxsfb/mxsfb_kms.c b/drivers/gpu/drm/mxsfb/mxsfb_kms.c index b721b8b262ce..79aa14027f91 100644 --- a/drivers/gpu/drm/mxsfb/mxsfb_kms.c +++ b/drivers/gpu/drm/mxsfb/mxsfb_kms.c @@ -403,14 +403,28 @@ static int mxsfb_plane_atomic_check(struct drm_plane *plane, { struct mxsfb_drm_private *mxsfb = to_mxsfb_drm_private(plane->dev); struct drm_crtc_state *crtc_state; + unsigned int pitch; + int ret; crtc_state = drm_atomic_get_new_crtc_state(plane_state->state, >crtc); - return drm_atomic_helper_check_plane_state(plane_state, crtc_state, - DRM_PLANE_HELPER_NO_SCALING, - DRM_PLANE_HELPER_NO_SCALING, - false, true); + ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state, + DRM_PLANE_HELPER_NO_SCALING, + DRM_PLANE_HELPER_NO_SCALING, + false, true); + if (ret || !plane_state->visible) + return ret; + + pitch = crtc_state->mode.hdisplay * + plane_state->fb->format->cpp[0]; + if (plane_state->fb->pitches[0] != pitch) { + dev_err(plane->dev->dev, + "Invalid pitch: fb and crtc widths must be the same"); + return -EINVAL; + } + + return 0; } static void mxsfb_plane_primary_atomic_update(struct drm_plane *plane, -- 2.28.0
Re: [PATCH v2 1/1] ARM: dts: bcm2711: Enable ddr modes on emmc2 controller
Hi Tobias, Am 31.08.20 um 23:59 schrieb Tobias Schramm: > This commit enables ddr modes for eMMC storage on emmc2. > The bcm2711 supports eMMC storage using ddr modes. The board > layout of the Raspberry Pi 4 supports them, too. i want to inform you that Ulf Hansson already applied my driver change to mmc-next [1]. So this patch won't be necessary anymore. Best regards [1] - https://marc.info/?l=linux-mmc=159903747923131=2 > > Signed-off-by: Tobias Schramm > --- > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > index 222d7825e1ab..1851efebe9c6 100644 > --- a/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > +++ b/arch/arm/boot/dts/bcm2711-rpi-4-b.dts > @@ -191,6 +191,7 @@ { > vqmmc-supply = <_io_1v8_reg>; > vmmc-supply = <_vcc_reg>; > broken-cd; > + mmc-ddr-3_3v; > status = "okay"; > }; >
Re: [PATCH v4 29/78] drm/vc4: crtc: Add a delay after disabling the PixelValve output
Hi Maxime, Am 01.09.20 um 11:58 schrieb Maxime Ripard: > Hi Stefan > > On Tue, Aug 25, 2020 at 11:30:58PM +0200, Stefan Wahren wrote: >> Am 25.08.20 um 17:06 schrieb Maxime Ripard: >>> Hi Stefan, >>> >>> On Wed, Jul 29, 2020 at 05:50:31PM +0200, Stefan Wahren wrote: >>>> Am 29.07.20 um 16:42 schrieb Maxime Ripard: >>>>> Hi, >>>>> >>>>> On Wed, Jul 29, 2020 at 03:09:21PM +0100, Dave Stevenson wrote: >>>>>> On Wed, 8 Jul 2020 at 18:43, Maxime Ripard wrote: >>>>>>> In order to avoid pixels getting stuck in the (unflushable) FIFO between >>>>>>> the HVS and the PV, we need to add some delay after disabling the PV >>>>>>> output >>>>>>> and before disabling the HDMI controller. 20ms seems to be good enough >>>>>>> so >>>>>>> let's use that. >>>>>>> >>>>>>> Signed-off-by: Maxime Ripard >>>>>>> --- >>>>>>> drivers/gpu/drm/vc4/vc4_crtc.c | 2 ++ >>>>>>> 1 file changed, 2 insertions(+) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c >>>>>>> b/drivers/gpu/drm/vc4/vc4_crtc.c >>>>>>> index d0b326e1df0a..7b178d67187f 100644 >>>>>>> --- a/drivers/gpu/drm/vc4/vc4_crtc.c >>>>>>> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c >>>>>>> @@ -403,6 +403,8 @@ static void vc4_crtc_atomic_disable(struct drm_crtc >>>>>>> *crtc, >>>>>>> ret = wait_for(!(CRTC_READ(PV_V_CONTROL) & PV_VCONTROL_VIDEN), >>>>>>> 1); >>>>>>> WARN_ONCE(ret, "Timeout waiting for !PV_VCONTROL_VIDEN\n"); >>>>>>> >>>>>>> + mdelay(20); >>>>>> mdelay for 20ms seems a touch unfriendly as it's a busy wait. Can we >>>>>> not msleep instead? >>>>> Since the timing was fairly critical, sleeping didn't seem like a good >>>>> solution since there's definitely some chance you overshoot and end up >>>>> with a higher time than the one you targeted. >>>> usleep_range(min, max) isn't a solution? >>> My understanding of usleep_range was that you can still overshoot, even >>> though it's backed by an HR timer so the resolution is not a jiffy. Are >>> we certain that we're going to be in that range? >> you are right there is no guarantee about the upper wake up time. >> >> And it's not worth the effort to poll the FIFO state until its empty >> (using 20 ms as timeout)? > I know this isn't really a great argument there, but getting this to > work has been quite painful, and the timing is very sensitive. If we > fail to wait for enough time, there's going to be a pixel shift that we > can't get rid of unless we reboot, which is pretty bad (and would fail > any CI test that checks for the output integrity). > > I know busy-looping for 20ms isn't ideal, but it's not really in a > hot-path (it's only done when changing a mode), with the sync time of > the display likely to be much more than that, and if it can avoid having > to look into it ever again or avoid random failures, I'd say it's worth > it. i don't want to delay this series. Could you please add a small comment to the delay to clarify the timing is very sensitive? Thanks > > Maxime
Re: [PATCH 3/3] drm/vc4: hdmi: Add pixel bvb clock control
Hi Maxime, Am 28.08.20 um 17:25 schrieb Maxime Ripard: > Hi, > > On Fri, Aug 28, 2020 at 02:45:49PM +0200, Stefan Wahren wrote: >> Am 28.08.20 um 08:30 schrieb Hoegeun Kwon: >>> On 8/27/20 6:49 PM, Stefan Wahren wrote: >>>> Am 27.08.20 um 06:35 schrieb Hoegeun Kwon: >>>>> Hi Stefan, >>>>> >>>>> Thank you for your review. >>>>> >>>>> >>>>> On 8/26/20 7:04 PM, Stefan Wahren wrote: >>>>>> Hi Hoeguen, >>>>>> >>>>>> Am 21.08.20 um 09:10 schrieb Hoegeun Kwon: >>>>>>> There is a problem that the output does not work at a resolution >>>>>>> exceeding FHD. To solve this, we need to adjust the bvb clock at a >>>>>>> resolution exceeding FHD. >>>>>> this patch introduces a mandatory clock, please update >>>>>> brcm,bcm2835-hdmi.yaml first. >>>>>> >>>>>> Is this clock physically available on BCM283x or only on BCM2711? >>>>> As far as I know, BCM2711 raspberry pi 4 supports 4k, >>>>> >>>>> don't supported on pi 3 and pi 3+. >>>>> >>>>> Since 4k is not supported in versions prior to Raspberry Pi 4, >>>>> >>>>> I don't think we need to modify the bvb clock. >>>>> >>>>> >>>>> So I think it is better to update 'brcm,bcm2711-hdmi.yaml' >>>>> >>>>> instead of 'brcm,bcm2835-hdmi.yaml'. >>>> You are correct please update only brcm,bcm2711-hdmi.yaml. >>>> >>>> My concern was that the function vc4_hdmi_encoder_pre_crtc_configure() >>>> is called on a non-bcm2711 platform or on a Raspberry Pi 4 with an older >>>> DTB. So making the BVB clock optional might be better? >>> You are right, if use old dtb, we have a problem with the hdmi driver. >>> >>> So how about modifying it like this? >>> >>> @@ -1614,8 +1614,8 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi >>> *vc4_hdmi) >>> >>> vc4_hdmi->pixel_bvb_clock = devm_clk_get(dev, "bvb"); >>> if (IS_ERR(vc4_hdmi->pixel_bvb_clock)) { >>> - DRM_ERROR("Failed to get pixel bvb clock\n"); >>> - return PTR_ERR(vc4_hdmi->pixel_bvb_clock); >>> + DRM_WARN("Failed to get pixel bvb clock\n"); >>> + vc4_hdmi->pixel_bvb_clock = NULL; >>> } >> i think the better solution would be devm_clk_get_optional(), which >> return NULL in case the clock doesn't exist. > It's not really optional though. BCM2711 will require it in order to run > properly (as Hoegeun experienced), and the previous SoCs won't. > > If we use clk_get_optional and that the DT is missing the clock on the > BCM2711, we will silently ignore it which doesn't sound great. you are right. Sorry for the noise. Best regards > > Maxime
[PATCH v2] clk: meson: g12a: mark fclk_div2 as critical
On Amlogic Meson G12b platform, similar to fclk_div3, the fclk_div2 seems to be necessary for the system to operate correctly as well. Typically, the clock also gets chosen by the eMMC peripheral. This probably masked the problem so far. However, when booting from a SD card the clock seems to get disabled which leads to a system freeze. Let's mark this clock as critical, fixing boot from SD card on G12b platforms. Fixes: 085a4ea93d54 ("clk: meson: g12a: add peripheral clock controller") Cc: Marek Szyprowski Signed-off-by: Stefan Agner Tested-by: Anand Moon --- drivers/clk/meson/g12a.c | 11 +++ 1 file changed, 11 insertions(+) diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c index fad616cac01e..6d44cadc06af 100644 --- a/drivers/clk/meson/g12a.c +++ b/drivers/clk/meson/g12a.c @@ -298,6 +298,17 @@ static struct clk_regmap g12a_fclk_div2 = { _fclk_div2_div.hw }, .num_parents = 1, + /* +* Similar to fclk_div3, it seems that this clock is used by +* the resident firmware and is required by the platform to +* operate correctly. +* Until the following condition are met, we need this clock to +* be marked as critical: +* a) Mark the clock used by a firmware resource, if possible +* b) CCF has a clock hand-off mechanism to make the sure the +*clock stays on until the proper driver comes along +*/ + .flags = CLK_IS_CRITICAL, }, }; -- 2.28.0
Re: [PATCH 3/3] drm/vc4: hdmi: Add pixel bvb clock control
Hi, Am 28.08.20 um 08:30 schrieb Hoegeun Kwon: > On 8/27/20 6:49 PM, Stefan Wahren wrote: >> Am 27.08.20 um 06:35 schrieb Hoegeun Kwon: >>> Hi Stefan, >>> >>> Thank you for your review. >>> >>> >>> On 8/26/20 7:04 PM, Stefan Wahren wrote: >>>> Hi Hoeguen, >>>> >>>> Am 21.08.20 um 09:10 schrieb Hoegeun Kwon: >>>>> There is a problem that the output does not work at a resolution >>>>> exceeding FHD. To solve this, we need to adjust the bvb clock at a >>>>> resolution exceeding FHD. >>>> this patch introduces a mandatory clock, please update >>>> brcm,bcm2835-hdmi.yaml first. >>>> >>>> Is this clock physically available on BCM283x or only on BCM2711? >>> As far as I know, BCM2711 raspberry pi 4 supports 4k, >>> >>> don't supported on pi 3 and pi 3+. >>> >>> Since 4k is not supported in versions prior to Raspberry Pi 4, >>> >>> I don't think we need to modify the bvb clock. >>> >>> >>> So I think it is better to update 'brcm,bcm2711-hdmi.yaml' >>> >>> instead of 'brcm,bcm2835-hdmi.yaml'. >> You are correct please update only brcm,bcm2711-hdmi.yaml. >> >> My concern was that the function vc4_hdmi_encoder_pre_crtc_configure() >> is called on a non-bcm2711 platform or on a Raspberry Pi 4 with an older >> DTB. So making the BVB clock optional might be better? > You are right, if use old dtb, we have a problem with the hdmi driver. > > So how about modifying it like this? > > @@ -1614,8 +1614,8 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi > *vc4_hdmi) > > vc4_hdmi->pixel_bvb_clock = devm_clk_get(dev, "bvb"); > if (IS_ERR(vc4_hdmi->pixel_bvb_clock)) { > - DRM_ERROR("Failed to get pixel bvb clock\n"); > - return PTR_ERR(vc4_hdmi->pixel_bvb_clock); > + DRM_WARN("Failed to get pixel bvb clock\n"); > + vc4_hdmi->pixel_bvb_clock = NULL; > } i think the better solution would be devm_clk_get_optional(), which return NULL in case the clock doesn't exist. Best regards > > vc4_hdmi->reset = devm_reset_control_get(dev, NULL); > > If we modify like this, if there is no bvb clock in dtb, then > pixel_bvb_clock will be null > > and it will not affect the clk control function below. > > - clk_set_rate() > - clk_prepare_enable() > - clk_disable_unprepare() > > > Best regards > > Hoegeun >
Re: [PATCH 1/2] EDAC/aspeed: Fix handling of platform_get_irq() error
> platform_get_irq() returns -ERRNO on error. In such case comparison > to 0 would pass the check. > > Fixes: 9b7e6242ee4e ("EDAC, aspeed: Add an Aspeed AST2500 EDAC driver") > Signed-off-by: Krzysztof Kozlowski Reviewed-by: Stefan Schaeckeler > --- > drivers/edac/aspeed_edac.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/edac/aspeed_edac.c b/drivers/edac/aspeed_edac.c > index b194658b8b5c..fbec28dc661d 100644 > --- a/drivers/edac/aspeed_edac.c > +++ b/drivers/edac/aspeed_edac.c > @@ -209,8 +209,8 @@ static int config_irq(void *ctx, struct platform_device > *pdev) > /* register interrupt handler */ > irq = platform_get_irq(pdev, 0); > dev_dbg(>dev, "got irq %d\n", irq); > -if (!irq) > -return -ENODEV; > +if (irq < 0) > +return irq; > > rc = devm_request_irq(>dev, irq, mcr_isr, IRQF_TRIGGER_HIGH, > DRV_NAME, ctx); > -- > 2.17.1 > >
[PATCH] clk: meson: g12a: mark fclk_div2 as critical
On Amlogic Meson G12b platform, similar to fclk_div3, the fclk_div2 seems to be necessary for the system to operate correctly as well. Typically, the clock also gets chosen by the eMMC peripheral. This probably masked the problem so far. However, when booting from a SD card the clock seems to get disabled which leads to a system freeze. Let's mark this clock as critical, fixing boot from SD card on G12b platforms. Signed-off-by: Stefan Agner --- drivers/clk/meson/g12a.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/clk/meson/g12a.c b/drivers/clk/meson/g12a.c index fad616cac01e..2214b974f748 100644 --- a/drivers/clk/meson/g12a.c +++ b/drivers/clk/meson/g12a.c @@ -298,6 +298,7 @@ static struct clk_regmap g12a_fclk_div2 = { _fclk_div2_div.hw }, .num_parents = 1, + .flags = CLK_IS_CRITICAL, }, }; -- 2.28.0
Re: [PATCH 3/3] drm/vc4: hdmi: Add pixel bvb clock control
Am 27.08.20 um 06:35 schrieb Hoegeun Kwon: > Hi Stefan, > > Thank you for your review. > > > On 8/26/20 7:04 PM, Stefan Wahren wrote: >> Hi Hoeguen, >> >> Am 21.08.20 um 09:10 schrieb Hoegeun Kwon: >>> There is a problem that the output does not work at a resolution >>> exceeding FHD. To solve this, we need to adjust the bvb clock at a >>> resolution exceeding FHD. >> this patch introduces a mandatory clock, please update >> brcm,bcm2835-hdmi.yaml first. >> >> Is this clock physically available on BCM283x or only on BCM2711? > As far as I know, BCM2711 raspberry pi 4 supports 4k, > > don't supported on pi 3 and pi 3+. > > Since 4k is not supported in versions prior to Raspberry Pi 4, > > I don't think we need to modify the bvb clock. > > > So I think it is better to update 'brcm,bcm2711-hdmi.yaml' > > instead of 'brcm,bcm2835-hdmi.yaml'. You are correct please update only brcm,bcm2711-hdmi.yaml. My concern was that the function vc4_hdmi_encoder_pre_crtc_configure() is called on a non-bcm2711 platform or on a Raspberry Pi 4 with an older DTB. So making the BVB clock optional might be better? > > Please comment, what do you think? > >> I'm a little bit afraid, this change could break with older firmware >> versions on BCM283x. > Tested it several times with libdrm modetest. > > I expect there will be no problem. > > > Best regards, > > Hoegeun > >> Best regards >> Stefan >> >>> Signed-off-by: Hoegeun Kwon >>> --- >>> drivers/gpu/drm/vc4/vc4_hdmi.c | 25 + >>> drivers/gpu/drm/vc4/vc4_hdmi.h | 1 + >>> 2 files changed, 26 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c >>> index 95ec5eedea39..eb3192d1fd86 100644 >>> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c >>> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c >>> @@ -80,6 +80,7 @@ >>> # define VC4_HD_M_ENABLE BIT(0) >>> >>> #define CEC_CLOCK_FREQ 4 >>> +#define VC4_HSM_MID_CLOCK 149985000 >>> >>> static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused) >>> { >>> @@ -380,6 +381,7 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct >>> drm_encoder *encoder) >>> HDMI_WRITE(HDMI_VID_CTL, >>>HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE); >>> >>> + clk_disable_unprepare(vc4_hdmi->pixel_bvb_clock); >>> clk_disable_unprepare(vc4_hdmi->hsm_clock); >>> clk_disable_unprepare(vc4_hdmi->pixel_clock); >>> >>> @@ -638,6 +640,23 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct >>> drm_encoder *encoder) >>> return; >>> } >>> >>> + ret = clk_set_rate(vc4_hdmi->pixel_bvb_clock, >>> + (hsm_rate > VC4_HSM_MID_CLOCK ? 15000 : 7500)); >>> + if (ret) { >>> + DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret); >>> + clk_disable_unprepare(vc4_hdmi->hsm_clock); >>> + clk_disable_unprepare(vc4_hdmi->pixel_clock); >>> + return; >>> + } >>> + >>> + ret = clk_prepare_enable(vc4_hdmi->pixel_bvb_clock); >>> + if (ret) { >>> + DRM_ERROR("Failed to turn on pixel bvb clock: %d\n", ret); >>> + clk_disable_unprepare(vc4_hdmi->hsm_clock); >>> + clk_disable_unprepare(vc4_hdmi->pixel_clock); >>> + return; >>> + } >>> + >>> if (vc4_hdmi->variant->reset) >>> vc4_hdmi->variant->reset(vc4_hdmi); >>> >>> @@ -1593,6 +1612,12 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi >>> *vc4_hdmi) >>> return PTR_ERR(vc4_hdmi->audio_clock); >>> } >>> >>> + vc4_hdmi->pixel_bvb_clock = devm_clk_get(dev, "bvb"); >>> + if (IS_ERR(vc4_hdmi->pixel_bvb_clock)) { >>> + DRM_ERROR("Failed to get pixel bvb clock\n"); >>> + return PTR_ERR(vc4_hdmi->pixel_bvb_clock); >>> + } >>> + >>> vc4_hdmi->reset = devm_reset_control_get(dev, NULL); >>> if (IS_ERR(vc4_hdmi->reset)) { >>> DRM_ERROR("Failed to get HDMI reset line\n"); >>> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h >>> index 0806c6d9f24e..63c6f8bddf1d 100644 >>> --- a/drivers/gpu/drm/vc4/vc4_hdmi.h >>> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h >>> @@ -147,6 +147,7 @@ struct vc4_hdmi { >>> struct clk *pixel_clock; >>> struct clk *hsm_clock; >>> struct clk *audio_clock; >>> + struct clk *pixel_bvb_clock; >>> >>> struct reset_control *reset; >>> > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v3 11/18] fuse: implement FUSE_INIT map_alignment field
On Wed, Aug 26, 2020 at 11:51:42AM -0400, Vivek Goyal wrote: > On Wed, Aug 26, 2020 at 04:06:35PM +0200, Miklos Szeredi wrote: > > On Thu, Aug 20, 2020 at 12:21 AM Vivek Goyal wrote: > > > > > > The device communicates FUSE_SETUPMAPPING/FUSE_REMOVMAPPING alignment > > > constraints via the FUST_INIT map_alignment field. Parse this field and > > > ensure our DAX mappings meet the alignment constraints. > > > > > > We don't actually align anything differently since our mappings are > > > already 2MB aligned. Just check the value when the connection is > > > established. If it becomes necessary to honor arbitrary alignments in > > > the future we'll have to adjust how mappings are sized. > > > > > > The upshot of this commit is that we can be confident that mappings will > > > work even when emulating x86 on Power and similar combinations where the > > > host page sizes are different. > > > > > > Signed-off-by: Stefan Hajnoczi > > > Signed-off-by: Vivek Goyal > > > --- > > > fs/fuse/fuse_i.h | 5 - > > > fs/fuse/inode.c | 18 -- > > > include/uapi/linux/fuse.h | 4 +++- > > > 3 files changed, 23 insertions(+), 4 deletions(-) > > > > > > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h > > > index 478c940b05b4..4a46e35222c7 100644 > > > --- a/fs/fuse/fuse_i.h > > > +++ b/fs/fuse/fuse_i.h > > > @@ -47,7 +47,10 @@ > > > /** Number of dentries for each connection in the control filesystem */ > > > #define FUSE_CTL_NUM_DENTRIES 5 > > > > > > -/* Default memory range size, 2MB */ > > > +/* > > > + * Default memory range size. A power of 2 so it agrees with common > > > FUSE_INIT > > > + * map_alignment values 4KB and 64KB. > > > + */ > > > #define FUSE_DAX_SZ(2*1024*1024) > > > #define FUSE_DAX_SHIFT (21) > > > #define FUSE_DAX_PAGES (FUSE_DAX_SZ/PAGE_SIZE) > > > diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c > > > index b82eb61d63cc..947abdd776ca 100644 > > > --- a/fs/fuse/inode.c > > > +++ b/fs/fuse/inode.c > > > @@ -980,9 +980,10 @@ static void process_init_reply(struct fuse_conn *fc, > > > struct fuse_args *args, > > > { > > > struct fuse_init_args *ia = container_of(args, typeof(*ia), args); > > > struct fuse_init_out *arg = >out; > > > + bool ok = true; > > > > > > if (error || arg->major != FUSE_KERNEL_VERSION) > > > - fc->conn_error = 1; > > > + ok = false; > > > else { > > > unsigned long ra_pages; > > > > > > @@ -1045,6 +1046,13 @@ static void process_init_reply(struct fuse_conn > > > *fc, struct fuse_args *args, > > > min_t(unsigned int, > > > FUSE_MAX_MAX_PAGES, > > > max_t(unsigned int, > > > arg->max_pages, 1)); > > > } > > > + if ((arg->flags & FUSE_MAP_ALIGNMENT) && > > > + (FUSE_DAX_SZ % (1ul << arg->map_alignment))) { > > > > This just obfuscates "arg->map_alignment != FUSE_DAX_SHIFT". > > > > So the intention was that userspace can ask the kernel for a > > particular alignment, right? > > My understanding is that device will specify alignment for > the foffset/moffset fields in fuse_setupmapping_in/fuse_removemapping_one. > And DAX mapping can be any size meeting that alignment contraint. > > > > > In that case kernel can definitely succeed if the requested alignment > > is smaller than the kernel provided one, no? > > Yes. So if map_alignemnt is 64K and DAX mapping size is 2MB, that's just > fine because it meets 4K alignment contraint. Just that we can't use > 4K size DAX mapping in that case. > > > It would also make > > sense to make this a two way negotiation. I.e. send the largest > > alignment (FUSE_DAX_SHIFT in this implementation) that the kernel can > > provide in fuse_init_in. In that case the only error would be if > > userspace ignored the given constraints. > > We could make it two way negotiation if it helps. So if we support > multiple mapping sizes in future, say 4K, 64K, 2MB, 1GB. So idea is > to send alignment of largest mapping size to device/user_space (1GB) > in this case? And that will allow device to choose an a
Re: [PATCH 3/3] drm/vc4: hdmi: Add pixel bvb clock control
Hi Hoeguen, Am 21.08.20 um 09:10 schrieb Hoegeun Kwon: > There is a problem that the output does not work at a resolution > exceeding FHD. To solve this, we need to adjust the bvb clock at a > resolution exceeding FHD. this patch introduces a mandatory clock, please update brcm,bcm2835-hdmi.yaml first. Is this clock physically available on BCM283x or only on BCM2711? I'm a little bit afraid, this change could break with older firmware versions on BCM283x. Best regards Stefan > > Signed-off-by: Hoegeun Kwon > --- > drivers/gpu/drm/vc4/vc4_hdmi.c | 25 + > drivers/gpu/drm/vc4/vc4_hdmi.h | 1 + > 2 files changed, 26 insertions(+) > > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c > index 95ec5eedea39..eb3192d1fd86 100644 > --- a/drivers/gpu/drm/vc4/vc4_hdmi.c > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c > @@ -80,6 +80,7 @@ > # define VC4_HD_M_ENABLE BIT(0) > > #define CEC_CLOCK_FREQ 4 > +#define VC4_HSM_MID_CLOCK 149985000 > > static int vc4_hdmi_debugfs_regs(struct seq_file *m, void *unused) > { > @@ -380,6 +381,7 @@ static void vc4_hdmi_encoder_post_crtc_powerdown(struct > drm_encoder *encoder) > HDMI_WRITE(HDMI_VID_CTL, > HDMI_READ(HDMI_VID_CTL) & ~VC4_HD_VID_CTL_ENABLE); > > + clk_disable_unprepare(vc4_hdmi->pixel_bvb_clock); > clk_disable_unprepare(vc4_hdmi->hsm_clock); > clk_disable_unprepare(vc4_hdmi->pixel_clock); > > @@ -638,6 +640,23 @@ static void vc4_hdmi_encoder_pre_crtc_configure(struct > drm_encoder *encoder) > return; > } > > + ret = clk_set_rate(vc4_hdmi->pixel_bvb_clock, > + (hsm_rate > VC4_HSM_MID_CLOCK ? 15000 : 7500)); > + if (ret) { > + DRM_ERROR("Failed to set pixel bvb clock rate: %d\n", ret); > + clk_disable_unprepare(vc4_hdmi->hsm_clock); > + clk_disable_unprepare(vc4_hdmi->pixel_clock); > + return; > + } > + > + ret = clk_prepare_enable(vc4_hdmi->pixel_bvb_clock); > + if (ret) { > + DRM_ERROR("Failed to turn on pixel bvb clock: %d\n", ret); > + clk_disable_unprepare(vc4_hdmi->hsm_clock); > + clk_disable_unprepare(vc4_hdmi->pixel_clock); > + return; > + } > + > if (vc4_hdmi->variant->reset) > vc4_hdmi->variant->reset(vc4_hdmi); > > @@ -1593,6 +1612,12 @@ static int vc5_hdmi_init_resources(struct vc4_hdmi > *vc4_hdmi) > return PTR_ERR(vc4_hdmi->audio_clock); > } > > + vc4_hdmi->pixel_bvb_clock = devm_clk_get(dev, "bvb"); > + if (IS_ERR(vc4_hdmi->pixel_bvb_clock)) { > + DRM_ERROR("Failed to get pixel bvb clock\n"); > + return PTR_ERR(vc4_hdmi->pixel_bvb_clock); > + } > + > vc4_hdmi->reset = devm_reset_control_get(dev, NULL); > if (IS_ERR(vc4_hdmi->reset)) { > DRM_ERROR("Failed to get HDMI reset line\n"); > diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.h b/drivers/gpu/drm/vc4/vc4_hdmi.h > index 0806c6d9f24e..63c6f8bddf1d 100644 > --- a/drivers/gpu/drm/vc4/vc4_hdmi.h > +++ b/drivers/gpu/drm/vc4/vc4_hdmi.h > @@ -147,6 +147,7 @@ struct vc4_hdmi { > struct clk *pixel_clock; > struct clk *hsm_clock; > struct clk *audio_clock; > + struct clk *pixel_bvb_clock; > > struct reset_control *reset; >
Re: [PATCH 0/3] drm/vc4: Support HDMI QHD or higher output
Hi Hoegeun, Am 21.08.20 um 09:10 schrieb Hoegeun Kwon: > Hi everyone, > > There is a problem that the output does not work at a resolution > exceeding FHD. To solve this, we need to adjust the bvb clock at a > resolution exceeding FHD. thanks for providing this. > > Rebased on top of next-20200708 and [1]. > > [1] : [PATCH v4 00/78] drm/vc4: Support BCM2711 Display Pipeline (Maxime's > patchset) > > Hoegeun Kwon (3):the > clk: bcm: rpi: Add register to control pixel bvb clk > ARM: dts: bcm2711: Add bvb clock for hdmi-pixel > drm/vc4: hdmi: Add pixel bvb clock control Please change the order of your last 2 patches. First the driver changes and then the device tree sources. Regards Stefan > > arch/arm/boot/dts/bcm2711-rpi-4-b.dts | 6 -- > drivers/clk/bcm/clk-raspberrypi.c | 1 + > drivers/gpu/drm/vc4/vc4_hdmi.c| 25 + > drivers/gpu/drm/vc4/vc4_hdmi.h| 1 + > 4 files changed, 31 insertions(+), 2 deletions(-) >
Re: [PATCH v4 29/78] drm/vc4: crtc: Add a delay after disabling the PixelValve output
Hi Maxime, Am 25.08.20 um 17:06 schrieb Maxime Ripard: > Hi Stefan, > > On Wed, Jul 29, 2020 at 05:50:31PM +0200, Stefan Wahren wrote: >> Am 29.07.20 um 16:42 schrieb Maxime Ripard: >>> Hi, >>> >>> On Wed, Jul 29, 2020 at 03:09:21PM +0100, Dave Stevenson wrote: >>>> On Wed, 8 Jul 2020 at 18:43, Maxime Ripard wrote: >>>>> In order to avoid pixels getting stuck in the (unflushable) FIFO between >>>>> the HVS and the PV, we need to add some delay after disabling the PV >>>>> output >>>>> and before disabling the HDMI controller. 20ms seems to be good enough so >>>>> let's use that. >>>>> >>>>> Signed-off-by: Maxime Ripard >>>>> --- >>>>> drivers/gpu/drm/vc4/vc4_crtc.c | 2 ++ >>>>> 1 file changed, 2 insertions(+) >>>>> >>>>> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c >>>>> b/drivers/gpu/drm/vc4/vc4_crtc.c >>>>> index d0b326e1df0a..7b178d67187f 100644 >>>>> --- a/drivers/gpu/drm/vc4/vc4_crtc.c >>>>> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c >>>>> @@ -403,6 +403,8 @@ static void vc4_crtc_atomic_disable(struct drm_crtc >>>>> *crtc, >>>>> ret = wait_for(!(CRTC_READ(PV_V_CONTROL) & PV_VCONTROL_VIDEN), 1); >>>>> WARN_ONCE(ret, "Timeout waiting for !PV_VCONTROL_VIDEN\n"); >>>>> >>>>> + mdelay(20); >>>> mdelay for 20ms seems a touch unfriendly as it's a busy wait. Can we >>>> not msleep instead? >>> Since the timing was fairly critical, sleeping didn't seem like a good >>> solution since there's definitely some chance you overshoot and end up >>> with a higher time than the one you targeted. >> usleep_range(min, max) isn't a solution? > My understanding of usleep_range was that you can still overshoot, even > though it's backed by an HR timer so the resolution is not a jiffy. Are > we certain that we're going to be in that range? you are right there is no guarantee about the upper wake up time. And it's not worth the effort to poll the FIFO state until its empty (using 20 ms as timeout)? > > Maxime > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v4] dt-bindings: nvmem: Add syscon to Vybrid OCOTP driver
On 2020-08-25 05:04, Chris Healy wrote: > From: Chris Healy > > Add syscon compatibility with Vybrid OCOTP driver binding. This is > required to access the UID. > > Fixes: 623069946952 ("nvmem: Add DT binding documentation for Vybrid > OCOTP driver") > Cc: sta...@vger.kernel.org > Signed-off-by: Chris Healy Reviewed-by: Stefan Agner > --- > Changes in v4: > - Point to the appropriate commit for the Fixes: line > - Update the Required Properties to add the "syscon" compatible > --- > Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt > b/Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt > index 56ed481c3e26..72ba628f6d0b 100644 > --- a/Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt > +++ b/Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt > @@ -2,7 +2,7 @@ On-Chip OTP Memory for Freescale Vybrid > > Required Properties: >compatible: > - - "fsl,vf610-ocotp" for VF5xx/VF6xx > + - "fsl,vf610-ocotp", "syscon" for VF5xx/VF6xx >#address-cells : Should be 1 >#size-cells : Should be 1 >reg : Address and length of OTP controller and fuse map registers > @@ -11,7 +11,7 @@ Required Properties: > Example for Vybrid VF5xx/VF6xx: > > ocotp: ocotp@400a5000 { > - compatible = "fsl,vf610-ocotp"; > + compatible = "fsl,vf610-ocotp", "syscon"; > #address-cells = <1>; > #size-cells = <1>; > reg = <0x400a5000 0xCF0>;
[PATCH] ARM: dts: sun4i: Enable HDMI support on the Mele A1000
Enable the display pipeline and HDMI output. Signed-off-by: Stefan Monnier --- arch/arm/boot/dts/sun4i-a10-a1000.dts | 25 + 1 file changed, 25 insertions(+) diff --git a/arch/arm/boot/dts/sun4i-a10-a1000.dts b/arch/arm/boot/dts/sun4i-a10-a1000.dts index 8692b11a83c3..af8ab736fd3c 100644 --- a/arch/arm/boot/dts/sun4i-a10-a1000.dts +++ b/arch/arm/boot/dts/sun4i-a10-a1000.dts @@ -60,6 +60,17 @@ stdout-path = "serial0:115200n8"; }; + hdmi-connector { + compatible = "hdmi-connector"; + type = "a"; + + port { + hdmi_con_in: endpoint { + remote-endpoint = <_out_con>; + }; + }; + }; + leds { compatible = "gpio-leds"; @@ -133,6 +144,20 @@ status = "okay"; }; + { + status = "okay"; +}; + + { + status = "okay"; +}; + +_out { + hdmi_out_con: endpoint { + remote-endpoint = <_con_in>; + }; +}; + { status = "okay"; -- 2.20.1
Re: [PATCH v2] ARM: dts: vfxxx: Add syscon compatible with ocotp
On 2020-08-21 16:13, Chris Healy wrote: > On Fri, Aug 21, 2020 at 6:21 AM Stefan Agner wrote: >> >> On 2020-08-20 06:10, Chris Healy wrote: >> > From: Chris Healy >> > >> > Add syscon compatibility with Vybrid ocotp node. This is required to >> > access the UID. >> >> Hm, it seems today the SoC driver uses the specific compatible. It also >> should expose the UID as soc_id, see drivers/soc/imx/soc-imx.c. >> > Yes, until I added syscon, the soc_id was empty and I would get the > following line in dmesg: "failed to find vf610-ocotp regmap! > Ah I see, it looks up syscon, so that requires syscon to be in compatible. >> Maybe it does make sense exposing it as syscon, but then we should >> probably also adjust >> Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt. >> > Makes sense. I will update vf610-ocotp.txt in v3. Tnx > Ok, thx. With that you can add Reviewed-by: Stefan Agner as well. -- Stefan >> -- >> Stefan >> >> > >> > Fixes: fa8d20c8dbb77 ("ARM: dts: vfxxx: Add node corresponding to OCOTP") >> > Cc: sta...@vger.kernel.org >> > Signed-off-by: Chris Healy >> > --- >> > Changes in v2: >> > - Add Fixes line to commit message >> > >> > arch/arm/boot/dts/vfxxx.dtsi | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi >> > index 0fe03aa0367f..2259d11af721 100644 >> > --- a/arch/arm/boot/dts/vfxxx.dtsi >> > +++ b/arch/arm/boot/dts/vfxxx.dtsi >> > @@ -495,7 +495,7 @@ edma1: dma-controller@40098000 { >> > }; >> > >> > ocotp: ocotp@400a5000 { >> > - compatible = "fsl,vf610-ocotp"; >> > + compatible = "fsl,vf610-ocotp", "syscon"; >> > reg = <0x400a5000 0x1000>; >> > clocks = < VF610_CLK_OCOTP>; >> > };
Re: [PATCH] drm: fsl-dcu: enable PIXCLK on LS1021A
Hi Matthias, On 2020-08-20 12:58, Matthias Schiffer wrote: > The PIXCLK needs to be enabled in SCFG before accessing the DCU on LS1021A, > or the access will hang. Hm, this seems a rather ad-hoc access to SCFG from the DCU. We do support a pixel clock in the device tree bindings of fsl-dcu, so ideally we should enable the pixel clock through the clock framework. On the other hand, I guess that would mean adding a clock driver to flip a single bit, which seems a bit excessive too. I'd like a second opinion on that. Adding clk framework maintainers. -- Stefan > > Signed-off-by: Matthias Schiffer > --- > drivers/gpu/drm/fsl-dcu/Kconfig | 1 + > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c | 25 +++ > drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h | 3 +++ > 3 files changed, 29 insertions(+) > > diff --git a/drivers/gpu/drm/fsl-dcu/Kconfig b/drivers/gpu/drm/fsl-dcu/Kconfig > index d7dd8ba90e3a..9e5a35e7c00c 100644 > --- a/drivers/gpu/drm/fsl-dcu/Kconfig > +++ b/drivers/gpu/drm/fsl-dcu/Kconfig > @@ -8,6 +8,7 @@ config DRM_FSL_DCU > select DRM_PANEL > select REGMAP_MMIO > select VIDEOMODE_HELPERS > + select MFD_SYSCON if SOC_LS1021A > help > Choose this option if you have an Freescale DCU chipset. > If M is selected the module will be called fsl-dcu-drm. > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > index abbc1ddbf27f..8a7556655581 100644 > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.c > @@ -51,6 +51,23 @@ static const struct regmap_config fsl_dcu_regmap_config = { > .volatile_reg = fsl_dcu_drm_is_volatile_reg, > }; > > +static int fsl_dcu_scfg_config_ls1021a(struct device_node *np) > +{ > + struct regmap *scfg; > + > + scfg = syscon_regmap_lookup_by_compatible("fsl,ls1021a-scfg"); > + if (IS_ERR(scfg)) > + return PTR_ERR(scfg); > + > + /* > + * For simplicity, enable the PIXCLK unconditionally. It might > + * be possible to disable the clock in PM or on unload as a future > + * improvement. > + */ > + return regmap_update_bits(scfg, SCFG_PIXCLKCR, SCFG_PIXCLKCR_PXCEN, > + SCFG_PIXCLKCR_PXCEN); > +} > + > static void fsl_dcu_irq_uninstall(struct drm_device *dev) > { > struct fsl_dcu_drm_device *fsl_dev = dev->dev_private; > @@ -70,6 +87,14 @@ static int fsl_dcu_load(struct drm_device *dev, > unsigned long flags) > return ret; > } > > + if (of_device_is_compatible(fsl_dev->np, "fsl,ls1021a-dcu")) { > + ret = fsl_dcu_scfg_config_ls1021a(fsl_dev->np); > + if (ret < 0) { > + dev_err(dev->dev, "failed to enable pixclk\n"); > + goto done; > + } > + } > + > ret = drm_vblank_init(dev, dev->mode_config.num_crtc); > if (ret < 0) { > dev_err(dev->dev, "failed to initialize vblank\n"); > diff --git a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h > b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h > index e2049a0e8a92..566396013c04 100644 > --- a/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h > +++ b/drivers/gpu/drm/fsl-dcu/fsl_dcu_drm_drv.h > @@ -160,6 +160,9 @@ > #define FSL_DCU_ARGB 12 > #define FSL_DCU_YUV422 14 > > +#define SCFG_PIXCLKCR0x28 > +#define SCFG_PIXCLKCR_PXCEN BIT(31) > + > #define VF610_LAYER_REG_NUM 9 > #define LS1021A_LAYER_REG_NUM10
Re: [PATCH v2] ARM: dts: vfxxx: Add syscon compatible with ocotp
On 2020-08-20 06:10, Chris Healy wrote: > From: Chris Healy > > Add syscon compatibility with Vybrid ocotp node. This is required to > access the UID. Hm, it seems today the SoC driver uses the specific compatible. It also should expose the UID as soc_id, see drivers/soc/imx/soc-imx.c. Maybe it does make sense exposing it as syscon, but then we should probably also adjust Documentation/devicetree/bindings/nvmem/vf610-ocotp.txt. -- Stefan > > Fixes: fa8d20c8dbb77 ("ARM: dts: vfxxx: Add node corresponding to OCOTP") > Cc: sta...@vger.kernel.org > Signed-off-by: Chris Healy > --- > Changes in v2: > - Add Fixes line to commit message > > arch/arm/boot/dts/vfxxx.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/vfxxx.dtsi b/arch/arm/boot/dts/vfxxx.dtsi > index 0fe03aa0367f..2259d11af721 100644 > --- a/arch/arm/boot/dts/vfxxx.dtsi > +++ b/arch/arm/boot/dts/vfxxx.dtsi > @@ -495,7 +495,7 @@ edma1: dma-controller@40098000 { > }; > > ocotp: ocotp@400a5000 { > - compatible = "fsl,vf610-ocotp"; > + compatible = "fsl,vf610-ocotp", "syscon"; > reg = <0x400a5000 0x1000>; > clocks = < VF610_CLK_OCOTP>; > };
Re: [PATCH v3] docs: trusted-encrypted.rst: update parameters for command examples
On 8/19/20 5:02 PM, Jarkko Sakkinen wrote: On Wed, Aug 19, 2020 at 01:00:02AM +0800, Coly Li wrote: The parameters in command examples for tpm2_createprimary and tpm2_evictcontrol are outdated, people (like me) are not able to create trusted key by these command examples. This patch updates the parameters of command example tpm2_createprimary and tpm2_evictcontrol in trusted-encrypted.rst. With Linux kernel v5.8 and tpm2-tools-4.1, people can create a trusted key by following the examples in this document. Signed-off-by: Coly Li Cc: Dan Williams Cc: James Bottomley Cc: Jarkko Sakkinen Cc: Mimi Zohar Cc: Stefan Berger OK, now it is clear. Thank you. Reviewed-by: Jarkko Sakkinen Reviewed-by: Stefan Berger /Jarkko --- Changelog: v3: update commit log with review comments from Jarkko Sakkinen. v2: remove the change of trusted key related operation. v1: initial version. Documentation/security/keys/trusted-encrypted.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst index 9483a7425ad5..1da879a68640 100644 --- a/Documentation/security/keys/trusted-encrypted.rst +++ b/Documentation/security/keys/trusted-encrypted.rst @@ -39,10 +39,9 @@ With the IBM TSS 2 stack:: Or with the Intel TSS 2 stack:: - #> tpm2_createprimary --hierarchy o -G rsa2048 -o key.ctxt + #> tpm2_createprimary --hierarchy o -G rsa2048 -c key.ctxt [...] - handle: 0x80FF - #> tpm2_evictcontrol -c key.ctxt -p 0x8101 + #> tpm2_evictcontrol -c key.ctxt 0x8101 persistentHandle: 0x8101 Usage:: -- 2.26.2
Re: [PATCH] HID: roccat: add bounds checking in kone_sysfs_write_settings()
Please remove the settings->size check. The driver sometimes just writes a modified version of the read settings data. This would fail if stored size is invalid. This value of size is not solely in my hands, I can't guarantee Windows driver does a sanity check, also cases of flash data corruption are known. My general design agenda is that in my userspace tools I make sure only valid data (to the best of knowledge) is prepared for the device. In the kernel driver I don't (or didn't) do additional checks and eventually let the hardware reject things it doesn't like. This way the kernel driver doesn't need an update if firmware updates change the binary interface. This happend in the past, but won't for this device anymore because it's oop for years. Am Mittwoch, den 05.08.2020, 12:55 +0300 schrieb Dan Carpenter: > In the original code we didn't check if the new value for > "settings->startup_profile" was within bounds that could possibly > result in an out of bounds array acccess. What we did was we checked > if > we could write the data to the firmware and it's possibly the > firmware > checks these values but there is no way to know. It's safer and > easier > to read if we check it in the kernel as well. > > I also added a check to ensure that "settings->size" was > correct. The > comments say that the only valid value is 36 which is sizeof(struct > kone_settings). > > Fixes: 14bf62cde794 ("HID: add driver for Roccat Kone gaming mouse") > Signed-off-by: Dan Carpenter > --- > This isn't tested. Checking settings->size seems like a good idea, > but > there is a slight risky because maybe invalid values used to be > allowed > and I don't want to break user space. > > drivers/hid/hid-roccat-kone.c | 24 +--- > 1 file changed, 17 insertions(+), 7 deletions(-) > > diff --git a/drivers/hid/hid-roccat-kone.c b/drivers/hid/hid-roccat- > kone.c > index 1a6e600197d0..5e8e1517f23c 100644 > --- a/drivers/hid/hid-roccat-kone.c > +++ b/drivers/hid/hid-roccat-kone.c > @@ -294,31 +294,41 @@ static ssize_t kone_sysfs_write_settings(struct > file *fp, struct kobject *kobj, > struct kone_device *kone = > hid_get_drvdata(dev_get_drvdata(dev)); > struct usb_device *usb_dev = > interface_to_usbdev(to_usb_interface(dev)); > int retval = 0, difference, old_profile; > + struct kone_settings *settings = (struct kone_settings > *)buf; > > /* I need to get my data in one piece */ > if (off != 0 || count != sizeof(struct kone_settings)) > return -EINVAL; > > mutex_lock(>kone_lock); > - difference = memcmp(buf, >settings, sizeof(struct > kone_settings)); > + difference = memcmp(settings, >settings, > + sizeof(struct kone_settings)); > if (difference) { > - retval = kone_set_settings(usb_dev, > - (struct kone_settings const *)buf); > - if (retval) { > - mutex_unlock(>kone_lock); > - return retval; > + if (settings->size != sizeof(struct kone_settings) > || > + settings->startup_profile < 1 || > + settings->startup_profile > 5) { > + retval = -EINVAL; > + goto unlock; > } > > + retval = kone_set_settings(usb_dev, settings); > + if (retval) > + goto unlock; > + > old_profile = kone->settings.startup_profile; > - memcpy(>settings, buf, sizeof(struct > kone_settings)); > + memcpy(>settings, settings, sizeof(struct > kone_settings)); > > kone_profile_activated(kone, kone- > >settings.startup_profile); > > if (kone->settings.startup_profile != old_profile) > kone_profile_report(kone, kone- > >settings.startup_profile); > } > +unlock: > mutex_unlock(>kone_lock); > > + if (retval) > + return retval; > + > return sizeof(struct kone_settings); > } > static BIN_ATTR(settings, 0660, kone_sysfs_read_settings,
Re: [PATCH v2] docs: update trusted-encrypted.rst
On 8/17/20 10:28 AM, Coly Li wrote: The parameters in tmp2 commands are outdated, people are not able to create trusted key by the example commands. This patch updates the paramerters of tpm2 commands, they are verified by tpm2-tools-4.1 with Linux v5.8 kernel. Signed-off-by: Coly Li Cc: Dan Williams Cc: James Bottomley Cc: Jarkko Sakkinen Cc: Mimi Zohar Cc: Stefan Berger --- Changelog: v2: remove the change of trusted key related operation. v1: initial version. Documentation/security/keys/trusted-encrypted.rst | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst index 9483a7425ad5..1da879a68640 100644 --- a/Documentation/security/keys/trusted-encrypted.rst +++ b/Documentation/security/keys/trusted-encrypted.rst @@ -39,10 +39,9 @@ With the IBM TSS 2 stack:: Or with the Intel TSS 2 stack:: - #> tpm2_createprimary --hierarchy o -G rsa2048 -o key.ctxt + #> tpm2_createprimary --hierarchy o -G rsa2048 -c key.ctxt [...] - handle: 0x80FF - #> tpm2_evictcontrol -c key.ctxt -p 0x8101 + #> tpm2_evictcontrol -c key.ctxt 0x8101 persistentHandle: 0x8101 Usage:: Reviewed-by: Stefan Berger
Re: [PATCH 4/8] net: mac802154: convert tasklets to use new tasklet_setup() API
Hello. On 17.08.20 10:51, Allen Pais wrote: From: Allen Pais In preparation for unconditionally passing the struct tasklet_struct pointer to all tasklet callbacks, switch to using the new tasklet_setup() and from_tasklet() to pass the tasklet pointer explicitly. Signed-off-by: Romain Perier Signed-off-by: Allen Pais --- net/mac802154/main.c | 8 +++- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/net/mac802154/main.c b/net/mac802154/main.c index 06ea0f8bfd5c..520cedc594e1 100644 --- a/net/mac802154/main.c +++ b/net/mac802154/main.c @@ -20,9 +20,9 @@ #include "ieee802154_i.h" #include "cfg.h" -static void ieee802154_tasklet_handler(unsigned long data) +static void ieee802154_tasklet_handler(struct tasklet_struct *t) { - struct ieee802154_local *local = (struct ieee802154_local *)data; + struct ieee802154_local *local = from_tasklet(local, t, tasklet); struct sk_buff *skb; while ((skb = skb_dequeue(>skb_queue))) { @@ -91,9 +91,7 @@ ieee802154_alloc_hw(size_t priv_data_len, const struct ieee802154_ops *ops) INIT_LIST_HEAD(>interfaces); mutex_init(>iflist_mtx); - tasklet_init(>tasklet, -ieee802154_tasklet_handler, -(unsigned long)local); + tasklet_setup(>tasklet, ieee802154_tasklet_handler); skb_queue_head_init(>skb_queue); Acked-by: Stefan Schmidt regards Stefan Schmidt
Re: [PATCH RESEND] docs: update trusted-encrypted.rst
On 8/15/20 3:51 AM, Coly Li wrote: The parameters in tmp2 commands are outdated, people are not able to create trusted key by the example commands. This patch updates the paramerters of tpm2 commands, they are verified by tpm2-tools-4.1 with Linux v5.8 kernel. Signed-off-by: Coly Li Cc: Dan Williams Cc: James Bottomley Cc: Jarkko Sakkinen Cc: Mimi Zohar Cc: Stefan Berger --- Documentation/security/keys/trusted-encrypted.rst | 9 - 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/Documentation/security/keys/trusted-encrypted.rst b/Documentation/security/keys/trusted-encrypted.rst index 9483a7425ad5..442a2775156e 100644 --- a/Documentation/security/keys/trusted-encrypted.rst +++ b/Documentation/security/keys/trusted-encrypted.rst @@ -39,10 +39,9 @@ With the IBM TSS 2 stack:: Or with the Intel TSS 2 stack:: - #> tpm2_createprimary --hierarchy o -G rsa2048 -o key.ctxt + #> tpm2_createprimary --hierarchy o -G rsa2048 key.ctxt [...] - handle: 0x80FF Are you sure about this? My documentation for 4.1.3 on F32 states -c, --key-context=FILE: The file path to save the object context of the generated primary object. - #> tpm2_evictcontrol -c key.ctxt -p 0x8101 + #> tpm2_evictcontrol -c key.ctxt 0x8101 persistentHandle: 0x8101 This seems correct. Usage:: @@ -115,7 +114,7 @@ append 'keyhandle=0x8101' to statements between quotes, such as A note in this file states this: Note: When using a TPM 2.0 with a persistent key with handle 0x8101, append 'keyhandle=0x8101' to statements between quotes, such as "new 32 keyhandle=0x8101". Now if someone was (still) interested in TPM 1.2 then the below changes you are proposing wouldn't work for them. Maybe you should adapt the note to state that these keyhandle=... should be removed for the TPM 1.2 case. :: -$ keyctl add trusted kmk "new 32" @u +$ keyctl add trusted kmk "new 32 keyhandle=0x8101" @u 440502848 $ keyctl show @@ -138,7 +137,7 @@ append 'keyhandle=0x8101' to statements between quotes, such as Load a trusted key from the saved blob:: -$ keyctl add trusted kmk "load `cat kmk.blob`" @u +$ keyctl add trusted kmk "load `cat kmk.blob` keyhandle=0x8101" @u 268728824 $ keyctl print 268728824
Re: [PATCH] ieee802154/adf7242: check status of adf7242_read_reg
Hello. On 02.08.20 16:23, t...@redhat.com wrote: From: Tom Rix Clang static analysis reports this error adf7242.c:887:6: warning: Assigned value is garbage or undefined len = len_u8; ^ ~~ len_u8 is set in adf7242_read_reg(lp, 0, _u8); When this call fails, len_u8 is not set. So check the return code. Fixes: 7302b9d90117 ("ieee802154/adf7242: Driver for ADF7242 MAC IEEE802154") Signed-off-by: Tom Rix --- drivers/net/ieee802154/adf7242.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c index c11f32f644db..7db9cbd0f5de 100644 --- a/drivers/net/ieee802154/adf7242.c +++ b/drivers/net/ieee802154/adf7242.c @@ -882,7 +882,9 @@ static int adf7242_rx(struct adf7242_local *lp) int ret; u8 lqi, len_u8, *data; - adf7242_read_reg(lp, 0, _u8); + ret = adf7242_read_reg(lp, 0, _u8); + if (ret) + return ret; len = len_u8; This patch has been applied to the wpan tree and will be part of the next pull request to net. Thanks! regards Stefan Schmidt
Re: [PATCH v2 0/5] virtio mmio specification enhancement
On Thu, Jul 30, 2020 at 08:15:12PM +, Pincus, Josh wrote: > We were looking into a similar enhancement for the Virt I/O MMIO transport > and came across this project. > This enhancement would be perfect for us. > > Has there been any progress since Feb, 2020? It looks like the effort might > have stalled? CCing Alex Bennee. I think he recently asked the same question. Stefan signature.asc Description: PGP signature
Re: [RFC PATCH 0/5] madvise MADV_DOEXEC
lls it as a kvm.ko memory region so that the guest directly accesses the memory view. Now virtiofsd can map portions of files into the DAX Window without coordinating with the QEMU process. This simplifies the virtio-fs code and should also improve DAX map/unmap performance. Stefan signature.asc Description: PGP signature
Re: [PATCH v4 29/78] drm/vc4: crtc: Add a delay after disabling the PixelValve output
Hi Maxime, Am 29.07.20 um 16:42 schrieb Maxime Ripard: > Hi, > > On Wed, Jul 29, 2020 at 03:09:21PM +0100, Dave Stevenson wrote: >> On Wed, 8 Jul 2020 at 18:43, Maxime Ripard wrote: >>> In order to avoid pixels getting stuck in the (unflushable) FIFO between >>> the HVS and the PV, we need to add some delay after disabling the PV output >>> and before disabling the HDMI controller. 20ms seems to be good enough so >>> let's use that. >>> >>> Signed-off-by: Maxime Ripard >>> --- >>> drivers/gpu/drm/vc4/vc4_crtc.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/gpu/drm/vc4/vc4_crtc.c b/drivers/gpu/drm/vc4/vc4_crtc.c >>> index d0b326e1df0a..7b178d67187f 100644 >>> --- a/drivers/gpu/drm/vc4/vc4_crtc.c >>> +++ b/drivers/gpu/drm/vc4/vc4_crtc.c >>> @@ -403,6 +403,8 @@ static void vc4_crtc_atomic_disable(struct drm_crtc >>> *crtc, >>> ret = wait_for(!(CRTC_READ(PV_V_CONTROL) & PV_VCONTROL_VIDEN), 1); >>> WARN_ONCE(ret, "Timeout waiting for !PV_VCONTROL_VIDEN\n"); >>> >>> + mdelay(20); >> mdelay for 20ms seems a touch unfriendly as it's a busy wait. Can we >> not msleep instead? > Since the timing was fairly critical, sleeping didn't seem like a good > solution since there's definitely some chance you overshoot and end up > with a higher time than the one you targeted. usleep_range(min, max) isn't a solution? Stefan > > Maxime > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH 02/10] block: virtio-blk: check logical block size
On Tue, Jul 21, 2020 at 01:52:31PM +0300, Maxim Levitsky wrote: > Linux kernel only supports logical block sizes which are power of two, > at least 512 bytes and no more that PAGE_SIZE. > > Check this instead of crashing later on. > > Note that there is no need to check physical block size since it is > only a hint, and virtio-blk already only supports power of two values. > > Bugzilla link: https://bugzilla.redhat.com/show_bug.cgi?id=1664619 > > Signed-off-by: Maxim Levitsky > --- > drivers/block/virtio_blk.c | 15 +-- > 1 file changed, 13 insertions(+), 2 deletions(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: strace of io_uring events?
On Tue, Jul 21, 2020 at 05:58:48PM +0200, Stefano Garzarella wrote: > On Tue, Jul 21, 2020 at 08:27:34AM -0700, Andy Lutomirski wrote: > > On Fri, Jul 17, 2020 at 1:02 AM Stefano Garzarella > > wrote: > > > > > > On Thu, Jul 16, 2020 at 08:12:35AM -0700, Kees Cook wrote: > > > > On Thu, Jul 16, 2020 at 03:14:04PM +0200, Stefano Garzarella wrote: > > > > > > access (IIUC) is possible without actually calling any of the io_uring > > > > syscalls. Is that correct? A process would receive an fd (via > > > > SCM_RIGHTS, > > > > pidfd_getfd, or soon seccomp addfd), and then call mmap() on it to gain > > > > access to the SQ and CQ, and off it goes? (The only glitch I see is > > > > waking up the worker thread?) > > > > > > It is true only if the io_uring istance is created with SQPOLL flag (not > > > the > > > default behaviour and it requires CAP_SYS_ADMIN). In this case the > > > kthread is created and you can also set an higher idle time for it, so > > > also the waking up syscall can be avoided. > > > > I stared at the io_uring code for a while, and I'm wondering if we're > > approaching this the wrong way. It seems to me that most of the > > complications here come from the fact that io_uring SQEs don't clearly > > belong to any particular security principle. (We have struct creds, > > but we don't really have a task or mm.) But I'm also not convinced > > that io_uring actually supports cross-mm submission except by accident > > -- as it stands, unless a user is very careful to only submit SQEs > > that don't use user pointers, the results will be unpredictable. > > Perhaps we can get away with this: > > > > diff --git a/fs/io_uring.c b/fs/io_uring.c > > index 74bc4a04befa..92266f869174 100644 > > --- a/fs/io_uring.c > > +++ b/fs/io_uring.c > > @@ -7660,6 +7660,20 @@ SYSCALL_DEFINE6(io_uring_enter, unsigned int, > > fd, u32, to_submit, > > if (!percpu_ref_tryget(>refs)) > > goto out_fput; > > > > +if (unlikely(current->mm != ctx->sqo_mm)) { > > +/* > > + * The mm used to process SQEs will be current->mm or > > + * ctx->sqo_mm depending on which submission path is used. > > + * It's also unclear who is responsible for an SQE submitted > > + * out-of-process from a security and auditing perspective. > > + * > > + * Until a real usecase emerges and there are clear semantics > > + * for out-of-process submission, disallow it. > > + */ > > +ret = -EACCES; > > +goto out; > > +} > > + > > /* > > * For SQ polling, the thread will do all submissions and completions. > > * Just return the requested submit count, and wake the thread if > > > > If we can do that, then we could bind seccomp-like io_uring filters to > > an mm, and we get obvious semantics that ought to cover most of the > > bases. > > > > Jens, Christoph? > > > > Stefano, what's your intended usecase for your restriction patchset? > > > > Hi Andy, > my use case concerns virtualization. The idea, that I described in the > proposal of io-uring restrictions [1], is to share io_uring CQ and SQ queues > with a guest VM for block operations. > > In the PoC that I realized, there is a block device driver in the guest that > uses io_uring queues coming from the host to submit block requests. > > Since the guest is not trusted, we need restrictions to allow only > a subset of syscalls on a subset of file descriptors and memory. BTW there's only a single mm in the kvm.ko use case. Stefan signature.asc Description: PGP signature
Re: [PATCH 3/3] arm64: dts: meson: add support for the ODROID-N2+
Hi Christian, On 2020-07-19 16:10, Christian Hewitt wrote: > HardKernel ODROID-N2+ uses an Amlogic S922X rev. C chip capable of higher > clock speeds than the original ODROID-N2. Hardkernel supports the big cpu > cluster at 2.4GHz and the little cpu cluster at 2.0GHz. Opp points and > regulator changess are from the HardKernel Linux kernel sources. According to the ODROID wiki those values are already in the overclocking range: https://wiki.odroid.com/odroid-n2/hardware/overclocking >From what I can tell, for ODROID-N2 upstream Linux so far used defaults from meson-g12b-s922x.dtsi, which were 1896MHz for the A53 and 1704MHz for the A73 (so it seems currently the A73 running even 100MHz below "Stock"). I guess we should pick either Stock or Overclock for the two models. Unless there is another good reason not to? -- Stefan > > Suggested-by: Dongjin Kim > Signed-off-by: Christian Hewitt > --- > arch/arm64/boot/dts/amlogic/Makefile | 1 + > .../dts/amlogic/meson-g12b-odroid-n2-plus.dts | 53 +++ > 2 files changed, 54 insertions(+) > create mode 100644 arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2-plus.dts > > diff --git a/arch/arm64/boot/dts/amlogic/Makefile > b/arch/arm64/boot/dts/amlogic/Makefile > index 5cac4d1d487d..6dc508b80133 100644 > --- a/arch/arm64/boot/dts/amlogic/Makefile > +++ b/arch/arm64/boot/dts/amlogic/Makefile > @@ -8,6 +8,7 @@ dtb-$(CONFIG_ARCH_MESON) += meson-g12b-gtking-pro.dtb > dtb-$(CONFIG_ARCH_MESON) += meson-g12b-a311d-khadas-vim3.dtb > dtb-$(CONFIG_ARCH_MESON) += meson-g12b-s922x-khadas-vim3.dtb > dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2.dtb > +dtb-$(CONFIG_ARCH_MESON) += meson-g12b-odroid-n2-plus.dtb > dtb-$(CONFIG_ARCH_MESON) += meson-g12b-ugoos-am6.dtb > dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-kii-pro.dtb > dtb-$(CONFIG_ARCH_MESON) += meson-gxbb-nanopi-k2.dtb > diff --git a/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2-plus.dts > b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2-plus.dts > new file mode 100644 > index ..99e96be509f8 > --- /dev/null > +++ b/arch/arm64/boot/dts/amlogic/meson-g12b-odroid-n2-plus.dts > @@ -0,0 +1,53 @@ > +// SPDX-License-Identifier: (GPL-2.0+ OR MIT) > +/* > + * Copyright (c) 2019 BayLibre, SAS > + * Author: Neil Armstrong > + */ > + > +/dts-v1/; > + > +#include "meson-g12b-odroid-n2.dtsi" > + > +/ { > + compatible = "hardkernel,odroid-n2-plus", "amlogic,s922x", > "amlogic,g12b"; > + model = "Hardkernel ODROID-N2+"; > + > + vddcpu_a: regulator-vddcpu-a { > + regulator-min-microvolt = <68>; > + regulator-max-microvolt = <104>; > + > + pwms = <_ab 0 1500 0>; > + }; > + > + vddcpu_b: regulator-vddcpu-b { > + regulator-min-microvolt = <68>; > + regulator-max-microvolt = <104>; > + > + pwms = <_AO_cd 1 1500 0>; > + }; > + > + cpu_opp_table_0: opp-table-0 { > + opp-190800 { > + opp-hz = /bits/ 64 <190800>; > + opp-microvolt = <103>; > + }; > + > + opp-201600 { > + opp-hz = /bits/ 64 <201600>; > + opp-microvolt = <104>; > + }; > + }; > + > + cpub_opp_table_1: opp-table-1 { > + opp-230400 { > + opp-hz = /bits/ 64 <230400>; > + opp-microvolt = <103>; > + }; > + > + opp-24 { > + opp-hz = /bits/ 64 <24>; > + opp-microvolt = <104>; > + }; > + }; > +}; > +
Re: [PATCH 24/24] net: pass a sockptr_t into ->setsockopt
ockptr(, optval, sizeof(int))) return -EFAULT; if (optname == DCCP_SOCKOPT_SERVICE) @@ -563,8 +563,8 @@ static int do_dccp_setsockopt(struct sock *sk, int level, int optname, return err; } -int dccp_setsockopt(struct sock *sk, int level, int optname, - char __user *optval, unsigned int optlen) +int dccp_setsockopt(struct sock *sk, int level, int optname, sockptr_t optval, + unsigned int optlen) { if (level != SOL_DCCP) return inet_csk(sk)->icsk_af_ops->setsockopt(sk, level, diff --git a/net/decnet/af_decnet.c b/net/decnet/af_decnet.c index 7d51ab608fb3f1..3b53d766789d47 100644 --- a/net/decnet/af_decnet.c +++ b/net/decnet/af_decnet.c @@ -150,7 +150,8 @@ static struct hlist_head dn_sk_hash[DN_SK_HASH_SIZE]; static struct hlist_head dn_wild_sk; static atomic_long_t decnet_memory_allocated; -static int __dn_setsockopt(struct socket *sock, int level, int optname, char __user *optval, unsigned int optlen, int flags); +static int __dn_setsockopt(struct socket *sock, int level, int optname, + sockptr_t optval, unsigned int optlen, int flags); static int __dn_getsockopt(struct socket *sock, int level, int optname, char __user *optval, int __user *optlen, int flags); static struct hlist_head *dn_find_list(struct sock *sk) @@ -1320,7 +1321,8 @@ static int dn_shutdown(struct socket *sock, int how) return err; } -static int dn_setsockopt(struct socket *sock, int level, int optname, char __user *optval, unsigned int optlen) +static int dn_setsockopt(struct socket *sock, int level, int optname, + sockptr_t optval, unsigned int optlen) { struct sock *sk = sock->sk; int err; @@ -1332,14 +1334,14 @@ static int dn_setsockopt(struct socket *sock, int level, int optname, char __use /* we need to exclude all possible ENOPROTOOPTs except default case */ if (err == -ENOPROTOOPT && optname != DSO_LINKINFO && optname != DSO_STREAM && optname != DSO_SEQPACKET) - err = nf_setsockopt(sk, PF_DECnet, optname, - USER_SOCKPTR(optval), optlen); + err = nf_setsockopt(sk, PF_DECnet, optname, optval, optlen); #endif return err; } -static int __dn_setsockopt(struct socket *sock, int level,int optname, char __user *optval, unsigned int optlen, int flags) +static int __dn_setsockopt(struct socket *sock, int level, int optname, + sockptr_t optval, unsigned int optlen, int flags) { struct sock *sk = sock->sk; struct dn_scp *scp = DN_SK(sk); @@ -1355,13 +1357,13 @@ static int __dn_setsockopt(struct socket *sock, int level,int optname, char __us } u; int err; - if (optlen && !optval) + if (optlen && sockptr_is_null(optval)) return -EINVAL; if (optlen > sizeof(u)) return -EINVAL; - if (copy_from_user(, optval, optlen)) + if (copy_from_sockptr(, optval, optlen)) return -EFAULT; switch (optname) { diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c index 94ae9662133e30..a45a0401adc50b 100644 --- a/net/ieee802154/socket.c +++ b/net/ieee802154/socket.c @@ -382,7 +382,7 @@ static int raw_getsockopt(struct sock *sk, int level, int optname, } static int raw_setsockopt(struct sock *sk, int level, int optname, - char __user *optval, unsigned int optlen) + sockptr_t optval, unsigned int optlen) { return -EOPNOTSUPP; } @@ -872,7 +872,7 @@ static int dgram_getsockopt(struct sock *sk, int level, int optname, } static int dgram_setsockopt(struct sock *sk, int level, int optname, - char __user *optval, unsigned int optlen) + sockptr_t optval, unsigned int optlen) { struct dgram_sock *ro = dgram_sk(sk); struct net *net = sock_net(sk); @@ -882,7 +882,7 @@ static int dgram_setsockopt(struct sock *sk, int level, int optname, if (optlen < sizeof(int)) return -EINVAL; - if (get_user(val, (int __user *)optval)) + if (copy_from_sockptr(, optval, sizeof(int))) return -EFAULT; For the ieee802154 part: Acked-by: Stefan Schmidt regards Stefan Schmidt
Re: [PATCH] drm/mxsfb: Make supported modifiers explicit
On 2020-07-18 19:14, Guido Günther wrote: > Hi, > On Mon, Mar 23, 2020 at 04:51:05PM +0100, Lucas Stach wrote: >> Am Montag, den 23.03.2020, 15:52 +0100 schrieb Guido Günther: >> > In contrast to other display controllers on imx like DCSS and ipuv3 >> > lcdif/mxsfb does not support detiling e.g. vivante tiled layouts. >> > Since mesa might assume otherwise make it explicit that only >> > DRM_FORMAT_MOD_LINEAR is supported. >> > >> > Signed-off-by: Guido Günther >> >> Reviewed-by: Lucas Stach > > Can i do anything to get this applied? > Cheers, > -- Guido Sorry about the delay, I was thinking to apply it with another patchset which is not ready though. Pushed this patch to drm-misc-next just now. -- Stefan > >> >> > --- >> > drivers/gpu/drm/mxsfb/mxsfb_drv.c | 9 +++-- >> > 1 file changed, 7 insertions(+), 2 deletions(-) >> > >> > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c >> > b/drivers/gpu/drm/mxsfb/mxsfb_drv.c >> > index 762379530928..fc71e7a7a02e 100644 >> > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c >> > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c >> > @@ -73,6 +73,11 @@ static const uint32_t mxsfb_formats[] = { >> >DRM_FORMAT_RGB565 >> > }; >> > >> > +static const uint64_t mxsfb_modifiers[] = { >> > + DRM_FORMAT_MOD_LINEAR, >> > + DRM_FORMAT_MOD_INVALID >> > +}; >> > + >> > static struct mxsfb_drm_private * >> > drm_pipe_to_mxsfb_drm_private(struct drm_simple_display_pipe *pipe) >> > { >> > @@ -334,8 +339,8 @@ static int mxsfb_load(struct drm_device *drm, unsigned >> > long flags) >> >} >> > >> >ret = drm_simple_display_pipe_init(drm, >pipe, _funcs, >> > - mxsfb_formats, ARRAY_SIZE(mxsfb_formats), NULL, >> > - mxsfb->connector); >> > + mxsfb_formats, ARRAY_SIZE(mxsfb_formats), >> > + mxsfb_modifiers, mxsfb->connector); >> >if (ret < 0) { >> >dev_err(drm->dev, "Cannot setup simple display pipe\n"); >> >goto err_vblank; >>
Re: [PATCH] drm/mxsfb: miss err handle in probe
On 2020-06-11 14:23, Bernard Zhao wrote: > There are three err return values in drm_fbdev_generic_setup. > In mxsfb_probe we called this function, but didn`t handle the > return value, this change is to add err handle, maybe make code > a bit more readable. This got recently changed, so I guess checking the return value isn't required anymore: https://patchwork.freedesktop.org/patch/msgid/20200408082641.590-11-tzimmerm...@suse.de -- Stefan > > Signed-off-by: Bernard Zhao > --- > drivers/gpu/drm/mxsfb/mxsfb_drv.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/mxsfb/mxsfb_drv.c > b/drivers/gpu/drm/mxsfb/mxsfb_drv.c > index 497cf443a9af..a45f3b85f725 100644 > --- a/drivers/gpu/drm/mxsfb/mxsfb_drv.c > +++ b/drivers/gpu/drm/mxsfb/mxsfb_drv.c > @@ -415,7 +415,9 @@ static int mxsfb_probe(struct platform_device *pdev) > if (ret) > goto err_unload; > > - drm_fbdev_generic_setup(drm, 32); > + ret = drm_fbdev_generic_setup(drm, 32); > + if (ret) > + goto err_unload; > > return 0;
Re: [PATCH for v5.9] net: ieee802154: adf7242: Replace HTTP links with HTTPS ones
Hello. On 19.07.20 13:31, Alexander A. Klimov wrote: Rationale: Reduces attack surface on kernel devs opening the links for MITM as HTTPS traffic is much harder to manipulate. Deterministic algorithm: For each file: If not .svg: For each line: If doesn't contain `\bxmlns\b`: For each link, `\bhttp://[^# \t\r\n]*(?:\w|/)`: If neither `\bgnu\.org/license`, nor `\bmozilla\.org/MPL\b`: If both the HTTP and HTTPS versions return 200 OK and serve the same content: Replace HTTP with HTTPS. Signed-off-by: Alexander A. Klimov --- Continuing my work started at 93431e0607e5. See also: git log --oneline '--author=Alexander A. Klimov ' v5.7..master (Actually letting a shell for loop submit all this stuff for me.) If there are any URLs to be removed completely or at least not (just) HTTPSified: Just clearly say so and I'll *undo my change*. See also: https://lkml.org/lkml/2020/6/27/64 If there are any valid, but yet not changed URLs: See: https://lkml.org/lkml/2020/6/26/837 If you apply the patch, please let me know. Sorry again to all maintainers who complained about subject lines. Now I realized that you want an actually perfect prefixes, not just subsystem ones. I tried my best... And yes, *I could* (at least half-)automate it. Impossible is nothing! :) drivers/net/ieee802154/adf7242.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/ieee802154/adf7242.c b/drivers/net/ieee802154/adf7242.c index 5a37514e4234..60a016a6806a 100644 --- a/drivers/net/ieee802154/adf7242.c +++ b/drivers/net/ieee802154/adf7242.c @@ -4,7 +4,7 @@ * * Copyright 2009-2017 Analog Devices Inc. * - * http://www.analog.com/ADF7242 + * https://www.analog.com/ADF7242 */ #include This patch has been applied to the wpan tree and will be part of the next pull request to net. Thanks! regards Stefan Schmidt
Re: [PATCH 05/22] net: remove compat_sock_common_{get,set}sockopt
e(struct sock *sk) { diff --git a/net/dccp/ipv4.c b/net/dccp/ipv4.c index a7e989919c5307..316cc5ac0da72b 100644 --- a/net/dccp/ipv4.c +++ b/net/dccp/ipv4.c @@ -999,10 +999,6 @@ static const struct proto_ops inet_dccp_ops = { .recvmsg = sock_common_recvmsg, .mmap = sock_no_mmap, .sendpage = sock_no_sendpage, -#ifdef CONFIG_COMPAT - .compat_setsockopt = compat_sock_common_setsockopt, - .compat_getsockopt = compat_sock_common_getsockopt, -#endif }; static struct inet_protosw dccp_v4_protosw = { diff --git a/net/dccp/ipv6.c b/net/dccp/ipv6.c index 650187d688519c..b50f85a72cd5fc 100644 --- a/net/dccp/ipv6.c +++ b/net/dccp/ipv6.c @@ -1083,8 +1083,6 @@ static const struct proto_ops inet6_dccp_ops = { .sendpage = sock_no_sendpage, #ifdef CONFIG_COMPAT .compat_ioctl = inet6_compat_ioctl, - .compat_setsockopt = compat_sock_common_setsockopt, - .compat_getsockopt = compat_sock_common_getsockopt, #endif }; diff --git a/net/ieee802154/socket.c b/net/ieee802154/socket.c index d93d4531aa9bc5..94ae9662133e30 100644 --- a/net/ieee802154/socket.c +++ b/net/ieee802154/socket.c @@ -423,10 +423,6 @@ static const struct proto_ops ieee802154_raw_ops = { .recvmsg = sock_common_recvmsg, .mmap = sock_no_mmap, .sendpage = sock_no_sendpage, -#ifdef CONFIG_COMPAT - .compat_setsockopt = compat_sock_common_setsockopt, - .compat_getsockopt = compat_sock_common_getsockopt, -#endif }; /* DGRAM Sockets (802.15.4 dataframes) */ @@ -986,10 +982,6 @@ static const struct proto_ops ieee802154_dgram_ops = { .recvmsg = sock_common_recvmsg, .mmap = sock_no_mmap, .sendpage = sock_no_sendpage, -#ifdef CONFIG_COMPAT - .compat_setsockopt = compat_sock_common_setsockopt, - .compat_getsockopt = compat_sock_common_getsockopt, -#endif For the ieee802154 part: Acked-by: Stefan Schmidt regards Stefan Schmidt
Re: [PATCH v5 01/18] nitro_enclaves: Add ioctl interface definition
On Wed, Jul 15, 2020 at 10:45:23PM +0300, Andra Paraschiv wrote: > + * A NE CPU pool has be set before calling this function. The pool can be set s/has be/has to be/ Thanks, this looks good! Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [PATCH 07/17] dma: altera-msgdma: Fix struct documentation blocks
On 14.07.20 13:15, Lee Jones wrote: Fix some misspelling/description issues, demote non-kerneldoc header to standard comment block and provide a new description for msgdma_desc_config()'s 'stride' parameter. Fixes the following W=1 kernel build warning(s): drivers/dma/altera-msgdma.c:163: warning: Function parameter or member 'node' not described in 'msgdma_sw_desc' drivers/dma/altera-msgdma.c:163: warning: Function parameter or member 'tx_list' not described in 'msgdma_sw_desc' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'lock' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'dev' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'irq_tasklet' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'pending_list' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'free_list' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'active_list' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'done_list' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'desc_free_cnt' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'idle' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'dmadev' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'dmachan' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'hw_desq' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'sw_desq' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'npendings' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'slave_cfg' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'irq' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'csr' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'desc' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:197: warning: Function parameter or member 'resp' not described in 'msgdma_device' drivers/dma/altera-msgdma.c:265: warning: Function parameter or member 'stride' not described in 'msgdma_desc_config' Cc: Stefan Roese Signed-off-by: Lee Jones Reviewed-by: Stefan Roese Thanks, Stefan --- drivers/dma/altera-msgdma.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/dma/altera-msgdma.c b/drivers/dma/altera-msgdma.c index 539e785039cac..321ac3a7aa418 100644 --- a/drivers/dma/altera-msgdma.c +++ b/drivers/dma/altera-msgdma.c @@ -153,7 +153,8 @@ struct msgdma_extended_desc { * struct msgdma_sw_desc - implements a sw descriptor * @async_tx: support for the async_tx api * @hw_desc: assosiated HW descriptor - * @free_list: node of the free SW descriprots list + * @node: node to move from the free list to the tx list + * @tx_list: transmit list node */ struct msgdma_sw_desc { struct dma_async_tx_descriptor async_tx; @@ -162,7 +163,7 @@ struct msgdma_sw_desc { struct list_head tx_list; }; -/** +/* * struct msgdma_device - DMA device structure */ struct msgdma_device { @@ -258,6 +259,7 @@ static void msgdma_free_desc_list(struct msgdma_device *mdev, * @dst: Destination buffer address * @src: Source buffer address * @len: Transfer length + * @stride: Read/write stride value to set */ static void msgdma_desc_config(struct msgdma_extended_desc *desc, dma_addr_t dst, dma_addr_t src, size_t len, Viele Grüße, Stefan -- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-51 Fax: (+49)-8142-66989-80 Email: s...@denx.de
Re: [PATCH v9 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
On 7/14/20 7:20 AM, Jarkko Sakkinen wrote: On Wed, Jul 08, 2020 at 10:17:17AM -0400, Stefan Berger wrote: ❯ swtpm-mvo.swtpm socket --tpmstate dir=/tmp/mytpm1 \ --ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock \ --log level=20 swtpm: Could not open UnixIO socket: No such file or directory Did you create the directory '/tmp/mytpm1' ? Yes. It's the socket file that it is complain because it does not exist beforehand. The socket file is created by the swtpm program. /Jarkko
Re: [PATCH] vhost/scsi: fix up req type endian-ness
On Fri, Jul 10, 2020 at 06:48:51AM -0400, Michael S. Tsirkin wrote: > vhost/scsi doesn't handle type conversion correctly > for request type when using virtio 1.0 and up for BE, > or cross-endian platforms. > > Fix it up using vhost_32_to_cpu. > > Cc: sta...@vger.kernel.org > Signed-off-by: Michael S. Tsirkin > --- > drivers/vhost/scsi.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [PATCH] vsock/virtio: annotate 'the_virtio_vsock' RCU pointer
On Fri, Jul 10, 2020 at 02:12:43PM +0200, Stefano Garzarella wrote: > Commit 0deab087b16a ("vsock/virtio: use RCU to avoid use-after-free > on the_virtio_vsock") starts to use RCU to protect 'the_virtio_vsock' > pointer, but we forgot to annotate it. > > This patch adds the annotation to fix the following sparse errors: > > net/vmw_vsock/virtio_transport.c:73:17: error: incompatible types in > comparison expression (different address spaces): > net/vmw_vsock/virtio_transport.c:73:17:struct virtio_vsock [noderef] > __rcu * > net/vmw_vsock/virtio_transport.c:73:17:struct virtio_vsock * > net/vmw_vsock/virtio_transport.c:171:17: error: incompatible types in > comparison expression (different address spaces): > net/vmw_vsock/virtio_transport.c:171:17:struct virtio_vsock [noderef] > __rcu * > net/vmw_vsock/virtio_transport.c:171:17:struct virtio_vsock * > net/vmw_vsock/virtio_transport.c:207:17: error: incompatible types in > comparison expression (different address spaces): > net/vmw_vsock/virtio_transport.c:207:17:struct virtio_vsock [noderef] > __rcu * > net/vmw_vsock/virtio_transport.c:207:17:struct virtio_vsock * > net/vmw_vsock/virtio_transport.c:561:13: error: incompatible types in > comparison expression (different address spaces): > net/vmw_vsock/virtio_transport.c:561:13:struct virtio_vsock [noderef] > __rcu * > net/vmw_vsock/virtio_transport.c:561:13:struct virtio_vsock * > net/vmw_vsock/virtio_transport.c:612:9: error: incompatible types in > comparison expression (different address spaces): > net/vmw_vsock/virtio_transport.c:612:9:struct virtio_vsock [noderef] > __rcu * > net/vmw_vsock/virtio_transport.c:612:9:struct virtio_vsock * > net/vmw_vsock/virtio_transport.c:631:9: error: incompatible types in > comparison expression (different address spaces): > net/vmw_vsock/virtio_transport.c:631:9:struct virtio_vsock [noderef] > __rcu * > net/vmw_vsock/virtio_transport.c:631:9:struct virtio_vsock * > > Fixes: 0deab087b16a ("vsock/virtio: use RCU to avoid use-after-free on > the_virtio_vsock") > Reported-by: Michael S. Tsirkin > Signed-off-by: Stefano Garzarella > --- > net/vmw_vsock/virtio_transport.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
Re: [PATCH RFC 0/3] io_uring: add restrictions to support untrusted applications and guests
On Fri, Jul 10, 2020 at 06:20:17PM +0200, Stefano Garzarella wrote: > On Fri, Jul 10, 2020 at 11:33:09AM -0400, Konrad Rzeszutek Wilk wrote: > > .snip.. > > > Just to recap the proposal, the idea is to add some restrictions to the > > > operations (sqe, register, fixed file) to safely allow untrusted > > > applications > > > or guests to use io_uring queues. > > > > Hi! > > > > This is neat and quite cool - but one thing that keeps nagging me is > > what how much overhead does this cut from the existing setup when you use > > virtio (with guests obviously)? > > I need to do more tests, but the preliminary results that I reported on > the original proposal [1] show an overhead of ~ 4.17 uS (with iodepth=1) > when I'm using virtio ring processed in a dedicated iothread: > > - 73 kIOPS using virtio-blk + QEMU iothread + io_uring backend > - 104 kIOPS using io_uring passthrough > > > That is from a high level view the > > beaty of io_uring being passed in the guest is you don't have the > > virtio ring -> io_uring processing, right? > > Right, and potentially we can share the io_uring queues directly to the > guest userspace applications, cutting down the cost of Linux block > layer in the guest. Another factor is that the guest submits requests directly to the host kernel sqpoll thread. When a virtqueue is used the sqpoll thread cannot poll it directly so another host thread (QEMU) needs to poll the virtqueue. The same applies for the completion code path. Stefan signature.asc Description: PGP signature
Re: [PATCH v4 00/78] drm/vc4: Support BCM2711 Display Pipeline
Hi Maxime, Am 08.07.20 um 19:41 schrieb Maxime Ripard: > Hi everyone, > > Here's a (pretty long) series to introduce support in the VC4 DRM driver > for the display pipeline found in the BCM2711 (and thus the RaspberryPi 4). > > The main differences are that there's two HDMI controllers and that there's > more pixelvalve now. Those pixelvalve come with a mux in the HVS that still > have only 3 FIFOs. Both of those differences are breaking a bunch of > expectations in the driver, so we first need a good bunch of cleanup and > reworks to introduce support for the new controllers. > > Similarly, the HDMI controller has all its registers shuffled and split in > multiple controllers now, so we need a bunch of changes to support this as > well. > > Only the HDMI support is enabled for now (even though the DPI and DSI > outputs have been tested too). > > Let me know if you have any comments > Maxime > > Cc: bcm-kernel-feedback-l...@broadcom.com > Cc: devicet...@vger.kernel.org > Cc: Kamal Dasu > Cc: linux-...@vger.kernel.org > Cc: Michael Turquette > Cc: Philipp Zabel > Cc: Rob Herring > Cc: Stephen Boyd > > Changes from v3: > - Rebased on top of next-20200708 > - Added a name to the HDMI audio codec component > - Only disable the BCM2711 HDMI pixelvalves at boot > - Fixed an error in the HVS binding > - Fix a framebuffer size condition that was inverted > - Changed the channel allocation algorithm using Eric's suggestion > - Always write the muxing values instead of updating if needed > - Improved a bit the hvs_available_channels comment in the structure > - Change atomic_complete_commit code to use for_each_new_crtc_in_state > - Change the muxing code to take into account disparities between the > BCM2711 and previous SoCs. > - Only change the clock rate on BCM2711 during a modeset > - Fix a crash at atomic_disable > - Use clk_set_min_rate for the core clock too > - Add a few defines, and simplify the FIFO level stuff > - Reordered the patches according to Eric's reviews > - Fixed a regression with VID_CTL setting on RPI3 > i additionally applied "drm/vc4/vc4_hdmi: fill ASoC card owner" on top of your series (potential merge conflict). I didn't see any issues with a RPI 3B or RPI 4B. So this whole series is Tested-by: Stefan Wahren Regards Stefan
Re: [PATCH v9 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
On 7/8/20 10:07 AM, Jarkko Sakkinen wrote: On Tue, Jul 07, 2020 at 12:09:11AM -0400, Stefan Berger wrote: On 7/7/20 12:03 AM, Jarkko Sakkinen wrote: On Mon, Jul 06, 2020 at 11:08:12PM -0400, Stefan Berger wrote: On 7/6/20 10:24 PM, Jarkko Sakkinen wrote: On Mon, Jul 06, 2020 at 07:55:26PM -0400, Stefan Berger wrote: On 7/6/20 7:09 PM, Jarkko Sakkinen wrote: On Mon, Jul 06, 2020 at 02:19:53PM -0400, Stefan Berger wrote: From: Stefan Berger In case a TPM2 is attached, search for a TPM2 ACPI table when trying to get the event log from ACPI. If one is found, use it to get the start and length of the log area. This allows non-UEFI systems, such as SeaBIOS, to pass an event log when using a TPM2. Signed-off-by: Stefan Berger Do you think that QEMU with TPM 1.2 emulator turned on would be a viable way to test this? Yes. Is the emulator bundled with QEMU or does it have to be installed separately? It has to be installed separately. On Fedora 31 it would just be a `sudo dnf -y install swtpm-tools` and you should be good to go with libvirt / virt-manager. Is there some packaging for Debian/Ubuntu available? So far may not be available yet. I had *experimented* with a PPA once: https://launchpad.net/~stefanberger/+archive/ubuntu/swtpm-focal There is a snap available: name: swtpm-mvo summary: Libtpms-based TPM emulator publisher: Michael Vogt (mvo) store-url: https://snapcraft.io/swtpm-mvo license: unset description: | Libtpms-based TPM emulator with socket, character device, and Linux CUSE interface. commands: - swtpm-mvo.swtpm services: swtpm-mvo.swtpm-sock: simple, enabled, active snap-id: HNl1TwHRBk3OtXQ8OriRB93FDZ6vman7 tracking: latest/edge refresh-date: today at 02:05 EEST channels: latest/stable:– latest/candidate: – latest/beta: 0.1.0 2019-07-26 (11) 3MB - latest/edge: 0.1.0 2020-07-08 (75) 3MB - installed: 0.1.0(74) 3MB - This is the version information: ❯ swtpm-mvo.swtpm --version TPM emulator version 0.4.0, Copyright (c) 2014 IBM Corp. However, if I try to run the first example from [*], I get: ❯ swtpm-mvo.swtpm socket --tpmstate dir=/tmp/mytpm1 \ --ctrl type=unixio,path=/tmp/mytpm1/swtpm-sock \ --log level=20 swtpm: Could not open UnixIO socket: No such file or directory Did you create the directory '/tmp/mytpm1' ? [*] https://www.qemu.org/docs/master/specs/tpm.html /Jarkko
Re: [PATCH][next] s390/dasd: Use struct_size() helper
Am 19.06.20 um 18:56 schrieb Gustavo A. R. Silva: > Make use of the struct_size() helper instead of an open-coded version > in order to avoid any potential type mistakes. Also, remove unnecessary > variable _datasize_. > > This code was detected with the help of Coccinelle and, audited and > fixed manually. > > Signed-off-by: Gustavo A. R. Silva Thanks for the patch. Tested and applied. > --- > drivers/s390/block/dasd_diag.c | 8 +++- > 1 file changed, 3 insertions(+), 5 deletions(-) > > diff --git a/drivers/s390/block/dasd_diag.c b/drivers/s390/block/dasd_diag.c > index facb588d09e4..7f53ba015300 100644 > --- a/drivers/s390/block/dasd_diag.c > +++ b/drivers/s390/block/dasd_diag.c > @@ -506,7 +506,7 @@ static struct dasd_ccw_req *dasd_diag_build_cp(struct > dasd_device *memdev, > struct req_iterator iter; > struct bio_vec bv; > char *dst; > - unsigned int count, datasize; > + unsigned int count; > sector_t recid, first_rec, last_rec; > unsigned int blksize, off; > unsigned char rw_cmd; > @@ -534,10 +534,8 @@ static struct dasd_ccw_req *dasd_diag_build_cp(struct > dasd_device *memdev, > if (count != last_rec - first_rec + 1) > return ERR_PTR(-EINVAL); > /* Build the request */ > - datasize = sizeof(struct dasd_diag_req) + > - count*sizeof(struct dasd_diag_bio); > - cqr = dasd_smalloc_request(DASD_DIAG_MAGIC, 0, datasize, memdev, > -blk_mq_rq_to_pdu(req)); > + cqr = dasd_smalloc_request(DASD_DIAG_MAGIC, 0, struct_size(dreq, bio, > count), > +memdev, blk_mq_rq_to_pdu(req)); > if (IS_ERR(cqr)) > return cqr; >
Re: [PATCH v9 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
On 7/7/20 12:03 AM, Jarkko Sakkinen wrote: On Mon, Jul 06, 2020 at 11:08:12PM -0400, Stefan Berger wrote: On 7/6/20 10:24 PM, Jarkko Sakkinen wrote: On Mon, Jul 06, 2020 at 07:55:26PM -0400, Stefan Berger wrote: On 7/6/20 7:09 PM, Jarkko Sakkinen wrote: On Mon, Jul 06, 2020 at 02:19:53PM -0400, Stefan Berger wrote: From: Stefan Berger In case a TPM2 is attached, search for a TPM2 ACPI table when trying to get the event log from ACPI. If one is found, use it to get the start and length of the log area. This allows non-UEFI systems, such as SeaBIOS, to pass an event log when using a TPM2. Signed-off-by: Stefan Berger Do you think that QEMU with TPM 1.2 emulator turned on would be a viable way to test this? Yes. Is the emulator bundled with QEMU or does it have to be installed separately? It has to be installed separately. On Fedora 31 it would just be a `sudo dnf -y install swtpm-tools` and you should be good to go with libvirt / virt-manager. Is there some packaging for Debian/Ubuntu available? So far may not be available yet. I had *experimented* with a PPA once: https://launchpad.net/~stefanberger/+archive/ubuntu/swtpm-focal /Jarkko
Re: [PATCH v9 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
On 7/6/20 10:24 PM, Jarkko Sakkinen wrote: On Mon, Jul 06, 2020 at 07:55:26PM -0400, Stefan Berger wrote: On 7/6/20 7:09 PM, Jarkko Sakkinen wrote: On Mon, Jul 06, 2020 at 02:19:53PM -0400, Stefan Berger wrote: From: Stefan Berger In case a TPM2 is attached, search for a TPM2 ACPI table when trying to get the event log from ACPI. If one is found, use it to get the start and length of the log area. This allows non-UEFI systems, such as SeaBIOS, to pass an event log when using a TPM2. Signed-off-by: Stefan Berger Do you think that QEMU with TPM 1.2 emulator turned on would be a viable way to test this? Yes. Is the emulator bundled with QEMU or does it have to be installed separately? It has to be installed separately. On Fedora 31 it would just be a `sudo dnf -y install swtpm-tools` and you should be good to go with libvirt / virt-manager. /Jarkko
[RESEND,PATCH v9 0/2] tpm2: Make TPM2 logs accessible for non-UEFI firmware
From: Stefan Berger This series of patches adds an optional extensions for the TPM2 ACPI table with additional fields found in the TPM2 TCG ACPI specification (reference is in the patch) that allow access to the log's address and its size. We then modify the code that so far only enables access to a TPM 1.2's log for a TPM2 as well. This then enables access to the TPM2's log on non-UEFI system that for example run SeaBIOS. Stefan v8->v9: - Renamed variable - Added R-b v7->v8: - Added empty line. v6->v7: - Added empty lines and R-b. v5->v6: - Moved extensions of TPM2 table into acpi_tpm2_phy. v4->v5: - Added R-bs and A-bs. v3->v4: - Repost as one series v2->v3: - Split the series into two separate patches - Added comments to ACPI table fields - Added check for null pointer to log area and zero log size v1->v2: - Repost of the series Stefan Berger (2): acpi: Extend TPM2 ACPI table with missing log fields tpm: Add support for event log pointer found in TPM2 ACPI table drivers/char/tpm/eventlog/acpi.c | 63 +--- include/acpi/actbl3.h| 7 2 files changed, 49 insertions(+), 21 deletions(-) -- 2.26.2
[RESEND,PATCH v9 1/2] acpi: Extend TPM2 ACPI table with missing log fields
From: Stefan Berger Recent extensions of the TPM2 ACPI table added 3 more fields including 12 bytes of start method specific parameters and Log Area Minimum Length (u32) and Log Area Start Address (u64). So, we define a new structure acpi_tpm2_phy that holds these optional new fields. The new fields allow non-UEFI systems to access the TPM2's log. The specification that has the new fields is the following: TCG ACPI Specification Family "1.2" and "2.0" Version 1.2, Revision 8 https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf Signed-off-by: Stefan Berger Cc: linux-a...@vger.kernel.org Cc: Len Brown Acked-by: Rafael J. Wysocki Reviewed-by: Jarkko Sakkinen Reviewed-by: Jerry Snitselaar --- include/acpi/actbl3.h | 7 +++ 1 file changed, 7 insertions(+) diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h index b0b163b9efc6..bdcac69fa6bd 100644 --- a/include/acpi/actbl3.h +++ b/include/acpi/actbl3.h @@ -415,6 +415,13 @@ struct acpi_table_tpm2 { /* Platform-specific data follows */ }; +/* Optional trailer for revision 4 holding platform-specific data */ +struct acpi_tpm2_phy { + u8 start_method_specific[12]; + u32 log_area_minimum_length; + u64 log_area_start_address; +}; + /* Values for start_method above */ #define ACPI_TPM2_NOT_ALLOWED 0 -- 2.26.2
[RESEND,PATCH v9 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
From: Stefan Berger In case a TPM2 is attached, search for a TPM2 ACPI table when trying to get the event log from ACPI. If one is found, use it to get the start and length of the log area. This allows non-UEFI systems, such as SeaBIOS, to pass an event log when using a TPM2. Signed-off-by: Stefan Berger Reviewed-by: Jerry Snitselaar Cc: Peter Huewe Cc: Jason Gunthorpe --- drivers/char/tpm/eventlog/acpi.c | 63 +--- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c index 63ada5e53f13..3633ed70f48f 100644 --- a/drivers/char/tpm/eventlog/acpi.c +++ b/drivers/char/tpm/eventlog/acpi.c @@ -49,9 +49,9 @@ int tpm_read_log_acpi(struct tpm_chip *chip) void __iomem *virt; u64 len, start; struct tpm_bios_log *log; - - if (chip->flags & TPM_CHIP_FLAG_TPM2) - return -ENODEV; + struct acpi_table_tpm2 *tbl; + struct acpi_tpm2_phy *tpm2_phy; + int format; log = >log; @@ -61,23 +61,44 @@ int tpm_read_log_acpi(struct tpm_chip *chip) if (!chip->acpi_dev_handle) return -ENODEV; - /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */ - status = acpi_get_table(ACPI_SIG_TCPA, 1, - (struct acpi_table_header **)); - - if (ACPI_FAILURE(status)) - return -ENODEV; - - switch(buff->platform_class) { - case BIOS_SERVER: - len = buff->server.log_max_len; - start = buff->server.log_start_addr; - break; - case BIOS_CLIENT: - default: - len = buff->client.log_max_len; - start = buff->client.log_start_addr; - break; + if (chip->flags & TPM_CHIP_FLAG_TPM2) { + status = acpi_get_table("TPM2", 1, + (struct acpi_table_header **)); + if (ACPI_FAILURE(status)) + return -ENODEV; + + if (tbl->header.length < + sizeof(*tbl) + sizeof(struct acpi_tpm2_phy)) + return -ENODEV; + + tpm2_phy = (void *)tbl + sizeof(*tbl); + len = tpm2_phy->log_area_minimum_length; + + start = tpm2_phy->log_area_start_address; + if (!start || !len) + return -ENODEV; + + format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2; + } else { + /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */ + status = acpi_get_table(ACPI_SIG_TCPA, 1, + (struct acpi_table_header **)); + if (ACPI_FAILURE(status)) + return -ENODEV; + + switch (buff->platform_class) { + case BIOS_SERVER: + len = buff->server.log_max_len; + start = buff->server.log_start_addr; + break; + case BIOS_CLIENT: + default: + len = buff->client.log_max_len; + start = buff->client.log_start_addr; + break; + } + + format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; } if (!len) { dev_warn(>dev, "%s: TCPA log area empty\n", __func__); @@ -98,7 +119,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip) memcpy_fromio(log->bios_event_log, virt, len); acpi_os_unmap_iomem(virt, len); - return EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; + return format; err: kfree(log->bios_event_log); -- 2.26.2
Re: [PATCH v9 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
On 7/6/20 7:09 PM, Jarkko Sakkinen wrote: On Mon, Jul 06, 2020 at 02:19:53PM -0400, Stefan Berger wrote: From: Stefan Berger In case a TPM2 is attached, search for a TPM2 ACPI table when trying to get the event log from ACPI. If one is found, use it to get the start and length of the log area. This allows non-UEFI systems, such as SeaBIOS, to pass an event log when using a TPM2. Signed-off-by: Stefan Berger Do you think that QEMU with TPM 1.2 emulator turned on would be a viable way to test this? Yes. I'm anyway more worried about breaking existing TPM 1.2 functionality and that requires only QEMU without extras. /Jarkko
[PATCH v9 1/2] acpi: Extend TPM2 ACPI table with missing log fields
From: Stefan Berger Recent extensions of the TPM2 ACPI table added 3 more fields including 12 bytes of start method specific parameters and Log Area Minimum Length (u32) and Log Area Start Address (u64). So, we define a new structure acpi_tpm2_phy that holds these optional new fields. The new fields allow non-UEFI systems to access the TPM2's log. The specification that has the new fields is the following: TCG ACPI Specification Family "1.2" and "2.0" Version 1.2, Revision 8 https://trustedcomputinggroup.org/wp-content/uploads/TCG_ACPIGeneralSpecification_v1.20_r8.pdf Signed-off-by: Stefan Berger Cc: linux-a...@vger.kernel.org Acked-by: Rafael J. Wysocki Reviewed-by: Jarkko Sakkinen --- include/acpi/actbl3.h | 7 +++ 1 file changed, 7 insertions(+) diff --git a/include/acpi/actbl3.h b/include/acpi/actbl3.h index b0b163b9efc6..bdcac69fa6bd 100644 --- a/include/acpi/actbl3.h +++ b/include/acpi/actbl3.h @@ -415,6 +415,13 @@ struct acpi_table_tpm2 { /* Platform-specific data follows */ }; +/* Optional trailer for revision 4 holding platform-specific data */ +struct acpi_tpm2_phy { + u8 start_method_specific[12]; + u32 log_area_minimum_length; + u64 log_area_start_address; +}; + /* Values for start_method above */ #define ACPI_TPM2_NOT_ALLOWED 0 -- 2.26.2
[PATCH v9 2/2] tpm: Add support for event log pointer found in TPM2 ACPI table
From: Stefan Berger In case a TPM2 is attached, search for a TPM2 ACPI table when trying to get the event log from ACPI. If one is found, use it to get the start and length of the log area. This allows non-UEFI systems, such as SeaBIOS, to pass an event log when using a TPM2. Signed-off-by: Stefan Berger --- drivers/char/tpm/eventlog/acpi.c | 63 +--- 1 file changed, 42 insertions(+), 21 deletions(-) diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c index 63ada5e53f13..3633ed70f48f 100644 --- a/drivers/char/tpm/eventlog/acpi.c +++ b/drivers/char/tpm/eventlog/acpi.c @@ -49,9 +49,9 @@ int tpm_read_log_acpi(struct tpm_chip *chip) void __iomem *virt; u64 len, start; struct tpm_bios_log *log; - - if (chip->flags & TPM_CHIP_FLAG_TPM2) - return -ENODEV; + struct acpi_table_tpm2 *tbl; + struct acpi_tpm2_phy *tpm2_phy; + int format; log = >log; @@ -61,23 +61,44 @@ int tpm_read_log_acpi(struct tpm_chip *chip) if (!chip->acpi_dev_handle) return -ENODEV; - /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */ - status = acpi_get_table(ACPI_SIG_TCPA, 1, - (struct acpi_table_header **)); - - if (ACPI_FAILURE(status)) - return -ENODEV; - - switch(buff->platform_class) { - case BIOS_SERVER: - len = buff->server.log_max_len; - start = buff->server.log_start_addr; - break; - case BIOS_CLIENT: - default: - len = buff->client.log_max_len; - start = buff->client.log_start_addr; - break; + if (chip->flags & TPM_CHIP_FLAG_TPM2) { + status = acpi_get_table("TPM2", 1, + (struct acpi_table_header **)); + if (ACPI_FAILURE(status)) + return -ENODEV; + + if (tbl->header.length < + sizeof(*tbl) + sizeof(struct acpi_tpm2_phy)) + return -ENODEV; + + tpm2_phy = (void *)tbl + sizeof(*tbl); + len = tpm2_phy->log_area_minimum_length; + + start = tpm2_phy->log_area_start_address; + if (!start || !len) + return -ENODEV; + + format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2; + } else { + /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */ + status = acpi_get_table(ACPI_SIG_TCPA, 1, + (struct acpi_table_header **)); + if (ACPI_FAILURE(status)) + return -ENODEV; + + switch (buff->platform_class) { + case BIOS_SERVER: + len = buff->server.log_max_len; + start = buff->server.log_start_addr; + break; + case BIOS_CLIENT: + default: + len = buff->client.log_max_len; + start = buff->client.log_start_addr; + break; + } + + format = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; } if (!len) { dev_warn(>dev, "%s: TCPA log area empty\n", __func__); @@ -98,7 +119,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip) memcpy_fromio(log->bios_event_log, virt, len); acpi_os_unmap_iomem(virt, len); - return EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; + return format; err: kfree(log->bios_event_log); -- 2.26.2
[PATCH v9 0/2] tpm2: Make TPM2 logs accessible for non-UEFI firmware
From: Stefan Berger This series of patches adds an optional extensions for the TPM2 ACPI table with additional fields found in the TPM2 TCG ACPI specification (reference is in the patch) that allow access to the log's address and its size. We then modify the code that so far only enables access to a TPM 1.2's log for a TPM2 as well. This then enables access to the TPM2's log on non-UEFI system that for example run SeaBIOS. Stefan v8->v9: - Renamed variable v7->v8: - Added empty line. v6->v7: - Added empty lines and R-b. v5->v6: - Moved extensions of TPM2 table into acpi_tpm2_phy. v4->v5: - Added R-bs and A-bs. v3->v4: - Repost as one series v2->v3: - Split the series into two separate patches - Added comments to ACPI table fields - Added check for null pointer to log area and zero log size v1->v2: - Repost of the series Stefan Berger (2): acpi: Extend TPM2 ACPI table with missing log fields tpm: Add support for event log pointer found in TPM2 ACPI table drivers/char/tpm/eventlog/acpi.c | 63 +--- include/acpi/actbl3.h| 7 2 files changed, 49 insertions(+), 21 deletions(-) -- 2.26.2
Re: [PATCH] tpm: Define TPM2_SPACE_BUFFER_SIZE to replace the use of PAGE_SIZE
On 7/3/20 11:56 PM, Jarkko Sakkinen wrote: On Thu, Jul 02, 2020 at 04:55:44PM -0700, Jerry Snitselaar wrote: On Fri Jul 03 20, Jarkko Sakkinen wrote: The size of the buffers for storing context's and sessions can vary from arch to arch as PAGE_SIZE can be anything between 4 kB and 256 kB (the maximum for PPC64). Define a fixed buffer size set to 16 kB. This should be enough for most use with three handles (that is how many we allow at the moment). Parametrize the buffer size while doing this, so that it is easier to revisit this later on if required. Reported-by: Stefan Berger Cc: sta...@vger.kernel.org Fixes: 745b361e989a ("tpm: infrastructure for TPM spaces") Signed-off-by: Jarkko Sakkinen Reviewed-by: Jerry Snitselaar Thank you. Now only needs tested-by from Stefan. Tested-by: Stefan Berger /Jarkko
Re: [PATCH v3 02/14] iommu: Report domain nesting info
On Tue, Jun 30, 2020 at 02:00:49AM +, Tian, Kevin wrote: > > From: Liu, Yi L > > Sent: Monday, June 29, 2020 8:23 PM > > > > Hi Stefan, > > > > > From: Stefan Hajnoczi > > > Sent: Monday, June 29, 2020 5:25 PM > > > > > > On Wed, Jun 24, 2020 at 01:55:15AM -0700, Liu Yi L wrote: > > > > +/* > > > > + * struct iommu_nesting_info - Information for nesting-capable IOMMU. > > > > + * user space should check it before using > > > > + * nesting capability. > > > > + * > > > > + * @size: size of the whole structure > > > > + * @format:PASID table entry format, the same definition with > > > > + * @format of struct iommu_gpasid_bind_data. > > > > + * @features: supported nesting features. > > > > + * @flags: currently reserved for future extension. > > > > + * @data: vendor specific cap info. > > > > + * > > > > + * > > > > +---++ > > > > + * | feature | Notes > > > > | > > > > + * > > > > > +===+=== > > > > > =+ > > > > + * | SYSWIDE_PASID | Kernel manages PASID in system wide, PASIDs > > used | > > > > + * | | in the system should be allocated by host kernel > > > > | > > > > + * > > > > +---++ > > > > + * | BIND_PGTBL| bind page tables to host PASID, the PASID could > > > > | > > > > + * | | either be a host PASID passed in bind request or > > > > | > > > > + * | | default PASIDs (e.g. default PASID of > > > > aux-domain) | > > > > + * > > > > +---++ > > > > + * | CACHE_INVLD | mandatory feature for nesting capable IOMMU > > | > > > > + * > > > > +---++ > > > > > > This feature description is vague about what CACHE_INVLD does and how > > to > > > use it. If I understand correctly, the presence of this feature means > > > that VFIO_IOMMU_NESTING_OP_CACHE_INVLD must be used? > > > > > > The same kind of clarification could be done for SYSWIDE_PASID and > > > BIND_PGTBL too. > > > > For SYSWIDE_PASID and BIND_PGTBL, yes, presence of the feature bit > > means must use. So the two are requirements to user space if it wants > > to setup nesting. While for CACHE_INVLD, it's kind of availability > > here. How about removing CACHE_INVLD as presence of BIND_PGTBL should > > indicates support of CACHE_INVLD? > > > > So far this assumption is correct but it may not be true when thinking > forward. > For example, a vendor might find a way to allow the owner of 1st-level page > table to directly invalidate cache w/o going through host IOMMU driver. From > this angle I feel explicitly reporting this capability is more robust. > > Regarding to the description, what about below? > > -- > SYSWIDE_PASID: PASIDs are managed in system-wide, instead of per device. > When a device is assigned to userspace or VM, proper uAPI (provided by > userspace driver framework, e.g. VFIO) must be used to allocate/free PASIDs > for the assigned device. > > BIND_PGTBL: The owner of the first-level/stage-1 page table must explicitly > bind the page table to associated PASID (either the one specified in bind > request or the default PASID of the iommu domain), through VFIO_IOMMU > _NESTING_OP > > CACHE_INVLD: The owner of the first-level/stage-1 page table must > explicitly invalidate the IOMMU cache through VFIO_IOMMU_NESTING_OP, > according to vendor-specific requirement when changing the page table. > -- Mentioning the API to allocate/free PASIDs and VFIO_IOMMU_NESTING_OP has made this clearer. This lets someone reading the documentation know where to look for further information on using these features. Thank you! Stefan signature.asc Description: PGP signature
Re: [PATCH v3 02/14] iommu: Report domain nesting info
On Wed, Jun 24, 2020 at 01:55:15AM -0700, Liu Yi L wrote: > +/* > + * struct iommu_nesting_info - Information for nesting-capable IOMMU. > + * user space should check it before using > + * nesting capability. > + * > + * @size:size of the whole structure > + * @format: PASID table entry format, the same definition with > + * @format of struct iommu_gpasid_bind_data. > + * @features:supported nesting features. > + * @flags: currently reserved for future extension. > + * @data:vendor specific cap info. > + * > + * +---++ > + * | feature | Notes | > + * +===++ > + * | SYSWIDE_PASID | Kernel manages PASID in system wide, PASIDs used | > + * | | in the system should be allocated by host kernel | > + * +---++ > + * | BIND_PGTBL| bind page tables to host PASID, the PASID could | > + * | | either be a host PASID passed in bind request or | > + * | | default PASIDs (e.g. default PASID of aux-domain) | > + * +---++ > + * | CACHE_INVLD | mandatory feature for nesting capable IOMMU | > + * +---++ This feature description is vague about what CACHE_INVLD does and how to use it. If I understand correctly, the presence of this feature means that VFIO_IOMMU_NESTING_OP_CACHE_INVLD must be used? The same kind of clarification could be done for SYSWIDE_PASID and BIND_PGTBL too. Stefan signature.asc Description: PGP signature
Re: [PATCH v3 13/14] vfio: Document dual stage control
On Wed, Jun 24, 2020 at 01:55:26AM -0700, Liu Yi L wrote: > +Details can be found in Documentation/userspace-api/iommu.rst. For Intel > +VT-d, each stage 1 page table is bound to host by: > + > +nesting_op->flags = VFIO_IOMMU_NESTING_OP_BIND_PGTBL; > +memcpy(_op->data, _data, sizeof(bind_data)); > +ioctl(container->fd, VFIO_IOMMU_NESTING_OP, nesting_op); > + > +As mentioned above, guest OS may use stage 1 for GIOVA->GPA or GVA->GPA. > +GVA->GPA page tables are available when PASID (Process Address Space ID) > +is exposed to guest. e.g. guest with PASID-capable devices assigned. For > +such page table binding, the bind_data should include PASID info, which > +is allocated by guest itself or by host. This depends on hardware vendor > +e.g. Intel VT-d requires to allocate PASID from host. This requirement is > +defined by the Virtual Command Support in VT-d 3.0 spec, guest software > +running on VT-d should allocate PASID from host kernel. To allocate PASID > +from host, user space should +check the IOMMU_NESTING_FEAT_SYSWIDE_PASID s/+check/check/g Reviewed-by: Stefan Hajnoczi signature.asc Description: PGP signature
[PATCH v3 2/3] ARM: use VFP assembler mnemonics in register load/store macros
The integrated assembler of Clang 10 and earlier do not allow to access the VFP registers through the coprocessor load/store instructions: :4:6: error: invalid operand for instruction LDC p11, cr0, [r10],#32*4 @ FLDMIAD r10!, {d0-d15} ^ This has been addressed with Clang 11 [0]. However, to support earlier versions of Clang and for better readability use of VFP assembler mnemonics still is preferred. Replace the coprocessor load/store instructions with explicit assembler mnemonics to accessing the floating point coprocessor registers. Use assembler directives to select the appropriate FPU version. This allows to build these macros with GNU assembler as well as with Clang's built-in assembler. [0] https://reviews.llvm.org/D59733 Link: https://github.com/ClangBuiltLinux/linux/issues/905 Signed-off-by: Stefan Agner --- Changes in v3: - Reworded commit message, adding hint that Clang 11 won't have this restriction any longer Changes in v2: - Add link in commit message arch/arm/include/asm/vfpmacros.h | 19 +++ 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/arch/arm/include/asm/vfpmacros.h b/arch/arm/include/asm/vfpmacros.h index 628c336e8e3b..947ee5395e1f 100644 --- a/arch/arm/include/asm/vfpmacros.h +++ b/arch/arm/include/asm/vfpmacros.h @@ -19,23 +19,25 @@ @ read all the working registers back into the VFP .macro VFPFLDMIA, base, tmp + .fpuvfpv2 #if __LINUX_ARM_ARCH__ < 6 - LDC p11, cr0, [\base],#33*4 @ FLDMIAX \base!, {d0-d15} + fldmiax \base!, {d0-d15} #else - LDC p11, cr0, [\base],#32*4 @ FLDMIAD \base!, {d0-d15} + vldmia \base!, {d0-d15} #endif #ifdef CONFIG_VFPv3 + .fpuvfpv3 #if __LINUX_ARM_ARCH__ <= 6 ldr \tmp, =elf_hwcap@ may not have MVFR regs ldr \tmp, [\tmp, #0] tst \tmp, #HWCAP_VFPD32 - ldclne p11, cr0, [\base],#32*4 @ FLDMIAD \base!, {d16-d31} + vldmiane \base!, {d16-d31} addeq \base, \base, #32*4 @ step over unused register space #else VFPFMRX \tmp, MVFR0 @ Media and VFP Feature Register 0 and \tmp, \tmp, #MVFR0_A_SIMD_MASK @ A_SIMD field cmp \tmp, #2@ 32 x 64bit registers? - ldcleq p11, cr0, [\base],#32*4 @ FLDMIAD \base!, {d16-d31} + vldmiaeq \base!, {d16-d31} addne \base, \base, #32*4 @ step over unused register space #endif #endif @@ -44,22 +46,23 @@ @ write all the working registers out of the VFP .macro VFPFSTMIA, base, tmp #if __LINUX_ARM_ARCH__ < 6 - STC p11, cr0, [\base],#33*4 @ FSTMIAX \base!, {d0-d15} + fstmiax \base!, {d0-d15} #else - STC p11, cr0, [\base],#32*4 @ FSTMIAD \base!, {d0-d15} + vstmia \base!, {d0-d15} #endif #ifdef CONFIG_VFPv3 + .fpuvfpv3 #if __LINUX_ARM_ARCH__ <= 6 ldr \tmp, =elf_hwcap@ may not have MVFR regs ldr \tmp, [\tmp, #0] tst \tmp, #HWCAP_VFPD32 - stclne p11, cr0, [\base],#32*4 @ FSTMIAD \base!, {d16-d31} + vstmiane \base!, {d16-d31} addeq \base, \base, #32*4 @ step over unused register space #else VFPFMRX \tmp, MVFR0 @ Media and VFP Feature Register 0 and \tmp, \tmp, #MVFR0_A_SIMD_MASK @ A_SIMD field cmp \tmp, #2@ 32 x 64bit registers? - stcleq p11, cr0, [\base],#32*4 @ FSTMIAD \base!, {d16-d31} + vstmiaeq \base!, {d16-d31} addne \base, \base, #32*4 @ step over unused register space #endif #endif -- 2.27.0
[PATCH v3 0/3] ARM: make use of UAL VFP mnemonics when possible
To build the kernel with the integrated assembler of Clang 10 and earlier the VFP code needs to make use of the unified assembler language (UAL) VFP mnemonics. As pointed out by Russell, even for ARMv7 blocking VFP access through MCR/MRC is not correct. This has been addressed in upstream Clang and VFP access will be possible through MCR/MRC (see [0]). At first I tried to replace all co-processor instructions to access the floating point unit along with the macros. However, due to missing FPINST/FPINST2 argument support in older binutils versions we have to keep them around. Once we drop support for binutils 2.24 and older, the move to UAL VFP mnemonics will be straight forward with this changes applied. Tested using Clang with integrated assembler as well as external (binutils assembler), various gcc/binutils version down to 4.7/2.23. Disassembled and compared the object files in arch/arm/vfp/ to make sure this changes leads to the same code. Besides different inlining behavior I was not able to spot a difference. In v2 the check for FPINST argument support is now made in Kconfig. In v3 reworded commit message and addressed review feedback in patch 1. -- Stefan [0] https://reviews.llvm.org/D59733 Stefan Agner (3): ARM: use .fpu assembler directives instead of assembler arguments ARM: use VFP assembler mnemonics in register load/store macros ARM: use VFP assembler mnemonics if available arch/arm/Kconfig | 2 ++ arch/arm/Kconfig.assembler | 6 ++ arch/arm/include/asm/vfp.h | 2 ++ arch/arm/include/asm/vfpmacros.h | 31 ++- arch/arm/vfp/Makefile| 2 -- arch/arm/vfp/vfphw.S | 31 +-- arch/arm/vfp/vfpinstr.h | 23 +++ 7 files changed, 72 insertions(+), 25 deletions(-) create mode 100644 arch/arm/Kconfig.assembler -- 2.27.0
[PATCH v3 3/3] ARM: use VFP assembler mnemonics if available
The integrated assembler of Clang 10 and earlier do not allow to access the VFP registers through the coprocessor load/store instructions: arch/arm/vfp/vfpmodule.c:342:2: error: invalid operand for instruction fmxr(FPEXC, fpexc & ~(FPEXC_EX|FPEXC_DEX|FPEXC_FP2V|FPEXC_VV|FPEXC_TRAP_MASK)); ^ arch/arm/vfp/vfpinstr.h:79:6: note: expanded from macro 'fmxr' asm("mcr p10, 7, %0, " vfpreg(_vfp_) ", cr0, 0 @ fmxr " #_vfp_ ", %0" \ ^ :1:6: note: instantiated into assembly here mcr p10, 7, r0, cr8, cr0, 0 @ fmxr FPEXC, r0 ^ This has been addressed with Clang 11 [0]. However, to support earlier versions of Clang and for better readability use of VFP assembler mnemonics still is preferred. Ideally we would replace this code with the unified assembler language mnemonics vmrs/vmsr on call sites along with .fpu assembler directives. The GNU assembler supports the .fpu directive at least since 2.17 (when documentation has been added). Since Linux requires binutils 2.21 it is safe to use .fpu directive. However, binutils does not allow to use FPINST or FPINST2 as an argument to vmrs/vmsr instructions up to binutils 2.24 (see binutils commit 16d02dc907c5): arch/arm/vfp/vfphw.S: Assembler messages: arch/arm/vfp/vfphw.S:162: Error: operand 0 must be FPSID or FPSCR pr FPEXC -- `vmsr FPINST,r6' arch/arm/vfp/vfphw.S:165: Error: operand 0 must be FPSID or FPSCR pr FPEXC -- `vmsr FPINST2,r8' arch/arm/vfp/vfphw.S:235: Error: operand 1 must be a VFP extension System Register -- `vmrs r3,FPINST' arch/arm/vfp/vfphw.S:238: Error: operand 1 must be a VFP extension System Register -- `vmrs r12,FPINST2' Use as-instr in Kconfig to check if FPINST/FPINST2 can be used. If they can be used make use of .fpu directives and UAL VFP mnemonics for register access. This allows to build vfpmodule.c with Clang and its integrated assembler. [0] https://reviews.llvm.org/D59733 Link: https://github.com/ClangBuiltLinux/linux/issues/905 Signed-off-by: Stefan Agner --- Changes in v3: - Reworded commit message, adding hint that Clang 11 won't have this restriction any longer Changes in v2: - Check assembler capabilities in Kconfig instead of Makefile arch/arm/Kconfig | 2 ++ arch/arm/Kconfig.assembler | 6 ++ arch/arm/include/asm/vfp.h | 2 ++ arch/arm/include/asm/vfpmacros.h | 12 +++- arch/arm/vfp/vfphw.S | 1 + arch/arm/vfp/vfpinstr.h | 23 +++ 6 files changed, 41 insertions(+), 5 deletions(-) create mode 100644 arch/arm/Kconfig.assembler diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig index 2ac74904a3ce..911f55a11c63 100644 --- a/arch/arm/Kconfig +++ b/arch/arm/Kconfig @@ -2097,3 +2097,5 @@ source "drivers/firmware/Kconfig" if CRYPTO source "arch/arm/crypto/Kconfig" endif + +source "arch/arm/Kconfig.assembler" diff --git a/arch/arm/Kconfig.assembler b/arch/arm/Kconfig.assembler new file mode 100644 index ..5cb31aae1188 --- /dev/null +++ b/arch/arm/Kconfig.assembler @@ -0,0 +1,6 @@ +# SPDX-License-Identifier: GPL-2.0 + +config AS_VFP_VMRS_FPINST + def_bool $(as-instr,.fpu vfpv2\nvmrs r0$(comma)FPINST) + help + Supported by binutils >= 2.24 and LLVM integrated assembler. diff --git a/arch/arm/include/asm/vfp.h b/arch/arm/include/asm/vfp.h index 7157d2a30a49..19928bfb4f9c 100644 --- a/arch/arm/include/asm/vfp.h +++ b/arch/arm/include/asm/vfp.h @@ -9,6 +9,7 @@ #ifndef __ASM_VFP_H #define __ASM_VFP_H +#ifndef CONFIG_AS_VFP_VMRS_FPINST #define FPSID cr0 #define FPSCR cr1 #define MVFR1 cr6 @@ -16,6 +17,7 @@ #define FPEXC cr8 #define FPINST cr9 #define FPINST2cr10 +#endif /* FPSID bits */ #define FPSID_IMPLEMENTER_BIT (24) diff --git a/arch/arm/include/asm/vfpmacros.h b/arch/arm/include/asm/vfpmacros.h index 947ee5395e1f..ba0d4cb5377e 100644 --- a/arch/arm/include/asm/vfpmacros.h +++ b/arch/arm/include/asm/vfpmacros.h @@ -8,7 +8,16 @@ #include -@ Macros to allow building with old toolkits (with no VFP support) +#ifdef CONFIG_AS_VFP_VMRS_FPINST + .macro VFPFMRX, rd, sysreg, cond + vmrs\cond \rd, \sysreg + .endm + + .macro VFPFMXR, sysreg, rd, cond + vmsr\cond \sysreg, \rd + .endm +#else + @ Macros to allow building with old toolkits (with no VFP support) .macro VFPFMRX, rd, sysreg, cond MRC\condp10, 7, \rd, \sysreg, cr0, 0@ FMRX \rd, \sysreg .endm @@ -16,6 +25,7 @@ .macro VFPFMXR, sysreg, rd, cond MCR\condp10, 7, \rd, \sysreg, cr0, 0@ FMXR \sysreg, \rd .endm +#endif @ read all the working registers back into the VFP .macro VFPFLDMIA, base, tmp diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S index 29ed36b99d1d
[PATCH v3 1/3] ARM: use .fpu assembler directives instead of assembler arguments
Explicit FPU selection has been introduced in commit 1a6be26d5b1a ("[ARM] Enable VFP to be built when non-VFP capable CPUs are selected") to make use of assembler mnemonics for VFP instructions. However, clang currently does not support passing assembler flags like this and errors out with: clang-10: error: the clang compiler does not support '-Wa,-mfpu=softvfp+vfp' Make use of the .fpu assembler directives to select the floating point hardware selectively. Also use the new unified assembler language mnemonics. This allows to build these procedures with Clang. Link: https://github.com/ClangBuiltLinux/linux/issues/762 Signed-off-by: Stefan Agner --- Changes in V3: - Drop unnecessary comma - Add .fpu directive also in single precision macros to avoid Clang error error: instruction requires: fp registers Changes in v2: - Add link in commit message arch/arm/vfp/Makefile | 2 -- arch/arm/vfp/vfphw.S | 30 -- 2 files changed, 20 insertions(+), 12 deletions(-) diff --git a/arch/arm/vfp/Makefile b/arch/arm/vfp/Makefile index 9975b63ac3b0..749901a72d6d 100644 --- a/arch/arm/vfp/Makefile +++ b/arch/arm/vfp/Makefile @@ -8,6 +8,4 @@ # ccflags-y := -DDEBUG # asflags-y := -DDEBUG -KBUILD_AFLAGS :=$(KBUILD_AFLAGS:-msoft-float=-Wa,-mfpu=softvfp+vfp -mfloat-abi=soft) - obj-y += vfpmodule.o entry.o vfphw.o vfpsingle.o vfpdouble.o diff --git a/arch/arm/vfp/vfphw.S b/arch/arm/vfp/vfphw.S index b2e560290860..29ed36b99d1d 100644 --- a/arch/arm/vfp/vfphw.S +++ b/arch/arm/vfp/vfphw.S @@ -258,11 +258,14 @@ vfp_current_hw_state_address: ENTRY(vfp_get_float) tbl_branch r0, r3, #3 + .fpuvfpv2 .irpdr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 -1: mrc p10, 0, r0, c\dr, c0, 0 @ fmrs r0, s0 +1: vmovr0, s\dr ret lr .org1b + 8 -1: mrc p10, 0, r0, c\dr, c0, 4 @ fmrs r0, s1 + .endr + .irpdr,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31 +1: vmovr0, s\dr ret lr .org1b + 8 .endr @@ -270,11 +273,14 @@ ENDPROC(vfp_get_float) ENTRY(vfp_put_float) tbl_branch r1, r3, #3 + .fpuvfpv2 .irpdr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 -1: mcr p10, 0, r0, c\dr, c0, 0 @ fmsr r0, s0 +1: vmovs\dr, r0 ret lr .org1b + 8 -1: mcr p10, 0, r0, c\dr, c0, 4 @ fmsr r0, s1 + .endr + .irpdr,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31 +1: vmovs\dr, r0 ret lr .org1b + 8 .endr @@ -282,15 +288,17 @@ ENDPROC(vfp_put_float) ENTRY(vfp_get_double) tbl_branch r0, r3, #3 + .fpuvfpv2 .irpdr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 -1: fmrrd r0, r1, d\dr +1: vmovr0, r1, d\dr ret lr .org1b + 8 .endr #ifdef CONFIG_VFPv3 @ d16 - d31 registers - .irpdr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 -1: mrrcp11, 3, r0, r1, c\dr@ fmrrd r0, r1, d\dr + .fpuvfpv3 + .irpdr,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31 +1: vmovr0, r1, d\dr ret lr .org1b + 8 .endr @@ -304,15 +312,17 @@ ENDPROC(vfp_get_double) ENTRY(vfp_put_double) tbl_branch r2, r3, #3 + .fpuvfpv2 .irpdr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 -1: fmdrr d\dr, r0, r1 +1: vmovd\dr, r0, r1 ret lr .org1b + 8 .endr #ifdef CONFIG_VFPv3 + .fpuvfpv3 @ d16 - d31 registers - .irpdr,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15 -1: mcrrp11, 3, r0, r1, c\dr@ fmdrr r0, r1, d\dr + .irpdr,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31 +1: vmovd\dr, r0, r1 ret lr .org1b + 8 .endr -- 2.27.0
Re: [PATCH] ARM: dts: vf610: Align L2 cache-controller nodename with dtschema
On 2020-06-26 10:05, Krzysztof Kozlowski wrote: > Fix dtschema validator warnings like: > l2-cache@40006000: $nodename:0: > 'l2-cache@40006000' does not match > '^(cache-controller|cpu)(@[0-9a-f,]+)*$' > > Signed-off-by: Krzysztof Kozlowski Acked-by: Stefan Agner -- Stefan > --- > arch/arm/boot/dts/vf610.dtsi | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/boot/dts/vf610.dtsi b/arch/arm/boot/dts/vf610.dtsi > index 7fd39817f8ab..956182d08e74 100644 > --- a/arch/arm/boot/dts/vf610.dtsi > +++ b/arch/arm/boot/dts/vf610.dtsi > @@ -10,7 +10,7 @@ > }; > > { > - L2: l2-cache@40006000 { > + L2: cache-controller@40006000 { > compatible = "arm,pl310-cache"; > reg = <0x40006000 0x1000>; > cache-unified;
Re: [PATCH] tpm: Define TPM2_SPACE_BUFFER_SIZE to replace the use of PAGE_SIZE
On 6/26/20 10:34 AM, Jarkko Sakkinen wrote: The size of the buffers for storing context's and sessions can vary from arch to arch as PAGE_SIZE can be anything between 4 kB and 256 kB (the maximum for PPC64). Define a fixed buffer size set to 16 kB. This should be enough for most use with three handles (that is how many we allow at the moment). Reported-by: Stefan Berger Cc: sta...@vger.kernel.org Fixes: 745b361e989a ("tpm: infrastructure for TPM spaces") Signed-off-by: Jarkko Sakkinen --- drivers/char/tpm/tpm2-space.c | 27 --- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c index 982d341d8837..9bef646093d1 100644 --- a/drivers/char/tpm/tpm2-space.c +++ b/drivers/char/tpm/tpm2-space.c @@ -15,6 +15,8 @@ #include #include "tpm.h" +#define TPM2_SPACE_BUFFER_SIZE 16384 /* 16 kB */ + enum tpm2_handle_types { TPM2_HT_HMAC_SESSION= 0x0200, TPM2_HT_POLICY_SESSION = 0x0300, @@ -40,11 +42,11 @@ static void tpm2_flush_sessions(struct tpm_chip *chip, struct tpm_space *space) int tpm2_init_space(struct tpm_space *space) { - space->context_buf = kzalloc(PAGE_SIZE, GFP_KERNEL); + space->context_buf = kzalloc(TPM2_SPACE_BUFFER_SIZE, GFP_KERNEL); if (!space->context_buf) return -ENOMEM; - space->session_buf = kzalloc(PAGE_SIZE, GFP_KERNEL); + space->session_buf = kzalloc(TPM2_SPACE_BUFFER_SIZE, GFP_KERNEL); if (space->session_buf == NULL) { kfree(space->context_buf); return -ENOMEM; @@ -311,8 +313,10 @@ int tpm2_prepare_space(struct tpm_chip *chip, struct tpm_space *space, u8 *cmd, sizeof(space->context_tbl)); memcpy(>work_space.session_tbl, >session_tbl, sizeof(space->session_tbl)); - memcpy(chip->work_space.context_buf, space->context_buf, PAGE_SIZE); - memcpy(chip->work_space.session_buf, space->session_buf, PAGE_SIZE); + memcpy(chip->work_space.context_buf, space->context_buf, + TPM2_SPACE_BUFFER_SIZE); + memcpy(chip->work_space.session_buf, space->session_buf, + TPM2_SPACE_BUFFER_SIZE); rc = tpm2_load_space(chip); if (rc) { @@ -492,8 +496,8 @@ static int tpm2_save_space(struct tpm_chip *chip) continue; rc = tpm2_save_context(chip, space->context_tbl[i], - space->context_buf, PAGE_SIZE, - ); + space->context_buf, + TPM2_SPACE_BUFFER_SIZE, ); if (rc == -ENOENT) { space->context_tbl[i] = 0; continue; @@ -509,9 +513,8 @@ static int tpm2_save_space(struct tpm_chip *chip) continue; rc = tpm2_save_context(chip, space->session_tbl[i], - space->session_buf, PAGE_SIZE, - ); - + space->session_buf, + TPM2_SPACE_BUFFER_SIZE, ); if (rc == -ENOENT) { /* handle error saving session, just forget it */ space->session_tbl[i] = 0; @@ -557,8 +560,10 @@ int tpm2_commit_space(struct tpm_chip *chip, struct tpm_space *space, sizeof(space->context_tbl)); memcpy(>session_tbl, >work_space.session_tbl, sizeof(space->session_tbl)); - memcpy(space->context_buf, chip->work_space.context_buf, PAGE_SIZE); - memcpy(space->session_buf, chip->work_space.session_buf, PAGE_SIZE); + memcpy(space->context_buf, chip->work_space.context_buf, + TPM2_SPACE_BUFFER_SIZE); + memcpy(space->session_buf, chip->work_space.session_buf, + TPM2_SPACE_BUFFER_SIZE); work_space.session_buf and context_buf also need allocation changes, otherwise we read from a smaller buffer here. See comment to other patch about tpm-chip.c . return 0; out: