Re: [PATCH 31/51] DMA-API: media: omap3isp: use dma_coerce_mask_and_coherent()
Hi Russell, Thank you for the patch. On Thursday 19 September 2013 22:56:02 Russell King wrote: > The code sequence: > isp->raw_dmamask = DMA_BIT_MASK(32); > isp->dev->dma_mask = &isp->raw_dmamask; > isp->dev->coherent_dma_mask = DMA_BIT_MASK(32); > bypasses the architectures check on the DMA mask. It can be replaced > with dma_coerce_mask_and_coherent(), avoiding the direct initialization > of this mask. > > Signed-off-by: Russell King Acked-by: Laurent Pinchart > --- > drivers/media/platform/omap3isp/isp.c |6 +++--- > drivers/media/platform/omap3isp/isp.h |3 --- > 2 files changed, 3 insertions(+), 6 deletions(-) > > diff --git a/drivers/media/platform/omap3isp/isp.c > b/drivers/media/platform/omap3isp/isp.c index df3a0ec..1c36080 100644 > --- a/drivers/media/platform/omap3isp/isp.c > +++ b/drivers/media/platform/omap3isp/isp.c > @@ -2182,9 +2182,9 @@ static int isp_probe(struct platform_device *pdev) > isp->pdata = pdata; > isp->ref_count = 0; > > - isp->raw_dmamask = DMA_BIT_MASK(32); > - isp->dev->dma_mask = &isp->raw_dmamask; > - isp->dev->coherent_dma_mask = DMA_BIT_MASK(32); > + ret = dma_coerce_mask_and_coherent(isp->dev, DMA_BIT_MASK(32)); > + if (ret) > + return ret; > > platform_set_drvdata(pdev, isp); > > diff --git a/drivers/media/platform/omap3isp/isp.h > b/drivers/media/platform/omap3isp/isp.h index cd3eff4..ce65d3a 100644 > --- a/drivers/media/platform/omap3isp/isp.h > +++ b/drivers/media/platform/omap3isp/isp.h > @@ -152,7 +152,6 @@ struct isp_xclk { > * @mmio_base_phys: Array with physical L4 bus addresses for ISP register > * regions. > * @mmio_size: Array with ISP register regions size in bytes. > - * @raw_dmamask: Raw DMA mask > * @stat_lock: Spinlock for handling statistics > * @isp_mutex: Mutex for serializing requests to ISP. > * @crashed: Bitmask of crashed entities (indexed by entity ID) > @@ -190,8 +189,6 @@ struct isp_device { > unsigned long mmio_base_phys[OMAP3_ISP_IOMEM_LAST]; > resource_size_t mmio_size[OMAP3_ISP_IOMEM_LAST]; > > - u64 raw_dmamask; > - > /* ISP Obj */ > spinlock_t stat_lock; /* common lock for statistic drivers */ > struct mutex isp_mutex; /* For handling ref_count field */ -- Regards, Laurent Pinchart ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware
Hi, On Monday 22 October 2012 03:33:19 Li Yang-R58472 wrote: > On Saturday, October 20, 2012 1:37 AM Felipe Balbi wrote: > > On Fri, Oct 19, 2012 at 06:19:26PM +0100, Simon Haggett wrote: > > > Some gadget drivers may attempt to dequeue requests for an endpoint > > > that has already been disabled. For example, in the UVC gadget driver, > > > uvc_function_set_alt() will call usb_ep_disable() when alt setting 0 > > > is selected. When the userspace application subsequently issues the > > > VIDIOC_STREAMOFF ioctl, uvc_video_enable() invokes usb_ep_dequeue() to > > > > ensure that all requests have been cancelled. > > > > bug is on uvc gadget, then. Laurent ? We've discussed this topic a couple of months before. I believe that's not a bug. http://68.183.106.108/lists/linux-usb/msg68869.html > > Also, fsl should be removed from the tree, I'm trying to persuade iMX > > folks to use drivers/usb/chipidea instead. > > Besides the iMX usage, the driver is also being used by many Freescale > PowerPC/Coldfire SoCs. I agree that it's ideal to move to a common driver. > But it is a large task to make the chipidea driver works for all the > hardware that fsl_udc had supported and been tested on. > > > > For the Freescale High Speed Dual-Role USB controller, > > > fsl_ep_dequeue() provides the implementation of usb_ep_dequeue(). If > > > this is called for a disabled endpoint, a kernel oops will occur when > > > > the ep->ep.desc field is dereferenced (by ep_index()). > > > > > fsl_ep_disable() sets this field to NULL, as well as deleting all > > > pending requests for the endpoint. > > > > > > This patch adds an additional check to fsl_ep_dequeue() to ensure that > > > the endpoint has not already been disabled before attempting to dequeue > > > > requests. > > > > > Signed-off-by: Simon Haggett > > > --- > > > > > > drivers/usb/gadget/fsl_udc_core.c |5 - > > > 1 files changed, 4 insertions(+), 1 deletions(-) > > > > > > diff --git a/drivers/usb/gadget/fsl_udc_core.c > > > b/drivers/usb/gadget/fsl_udc_core.c > > > index 6ae70cb..acd513b 100644 > > > --- a/drivers/usb/gadget/fsl_udc_core.c > > > +++ b/drivers/usb/gadget/fsl_udc_core.c > > > @@ -955,7 +955,10 @@ static int fsl_ep_dequeue(struct usb_ep *_ep, > > struct usb_request *_req) > > > > > int ep_num, stopped, ret = 0; > > > u32 epctrl; > > > > > > - if (!_ep || !_req) > > > + /* Ensure that the ep and request are valid, and the ep is not > > > + * disabled > > > + */ > > > + if (!_ep || !_req || !ep->ep.desc) > > > return -EINVAL; Shouldn't that last check be done with a lock taken ? > > > spin_lock_irqsave(&ep->udc->lock, flags); -- Regards, Laurent Pinchart ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware
Hi Simon, On Monday 22 October 2012 12:49:51 Simon Haggett wrote: > On 22/10/12 11:47, Laurent Pinchart wrote: > > On Monday 22 October 2012 03:33:19 Li Yang-R58472 wrote: > >> On Saturday, October 20, 2012 1:37 AM Felipe Balbi wrote: > >>> On Fri, Oct 19, 2012 at 06:19:26PM +0100, Simon Haggett wrote: > >>>> Some gadget drivers may attempt to dequeue requests for an endpoint > >>>> that has already been disabled. For example, in the UVC gadget driver, > >>>> uvc_function_set_alt() will call usb_ep_disable() when alt setting 0 > >>>> is selected. When the userspace application subsequently issues the > >>>> VIDIOC_STREAMOFF ioctl, uvc_video_enable() invokes usb_ep_dequeue() to > >>> > >>> ensure that all requests have been cancelled. > >>> > >>> bug is on uvc gadget, then. Laurent ? > > > > We've discussed this topic a couple of months before. I believe that's not > > a bug. > > > > http://68.183.106.108/lists/linux-usb/msg68869.html > > > >>> Also, fsl should be removed from the tree, I'm trying to persuade iMX > >>> folks to use drivers/usb/chipidea instead. > >> > >> Besides the iMX usage, the driver is also being used by many Freescale > >> PowerPC/Coldfire SoCs. I agree that it's ideal to move to a common > >> driver. > >> But it is a large task to make the chipidea driver works for all the > >> hardware that fsl_udc had supported and been tested on. > >> > >>>> For the Freescale High Speed Dual-Role USB controller, > >>>> fsl_ep_dequeue() provides the implementation of usb_ep_dequeue(). If > >>>> this is called for a disabled endpoint, a kernel oops will occur when > >>> > >>> the ep->ep.desc field is dereferenced (by ep_index()). > >>> > >>>> fsl_ep_disable() sets this field to NULL, as well as deleting all > >>>> pending requests for the endpoint. > >>>> > >>>> This patch adds an additional check to fsl_ep_dequeue() to ensure that > >>>> the endpoint has not already been disabled before attempting to dequeue > >>> > >>> requests. > >>> > >>>> Signed-off-by: Simon Haggett > >>>> --- > >>>> > >>>> drivers/usb/gadget/fsl_udc_core.c |5 - > >>>> 1 files changed, 4 insertions(+), 1 deletions(-) > >>>> > >>>> diff --git a/drivers/usb/gadget/fsl_udc_core.c > >>>> b/drivers/usb/gadget/fsl_udc_core.c > >>>> index 6ae70cb..acd513b 100644 > >>>> --- a/drivers/usb/gadget/fsl_udc_core.c > >>>> +++ b/drivers/usb/gadget/fsl_udc_core.c > >>>> @@ -955,7 +955,10 @@ static int fsl_ep_dequeue(struct usb_ep *_ep, > >>> > >>> struct usb_request *_req) > >>> > >>>> int ep_num, stopped, ret = 0; > >>>> u32 epctrl; > >>>> > >>>> -if (!_ep || !_req) > >>>> +/* Ensure that the ep and request are valid, and the ep is not > >>>> + * disabled > >>>> + */ > >>>> +if (!_ep || !_req || !ep->ep.desc) > >>>> > >>>> return -EINVAL; > > > > Shouldn't that last check be done with a lock taken ? > > I had presumed a lock wasn't necessary because ep->ep.desc is only set > to NULL by fsl_ep_disable() which, since it is called by > usb_ep_disable(), should only be invoked when no other task is using the > endpoint (according to include/linux/usb/gadget.h). Furthermore, the > chipidea UDC driver does check the equivalent of this field is not NULL > without taking a lock (ep_dequeue() in drivers/usb/chipidea/udc.c). > > However, it is possible that I'm misunderstanding something here, so > apologies if I am. I might be wrong as well :-) I just wanted to point out something that appeared to me as a possible issue. > >>>> spin_lock_irqsave(&ep->udc->lock, flags); -- Regards, Laurent Pinchart ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 1/1] usb: gadget: Don't attempt to dequeue requests for a disabled USB endpoint on Freescale hardware
Hi Felipe, On Monday 22 October 2012 13:56:01 Felipe Balbi wrote: > On Mon, Oct 22, 2012 at 12:47:21PM +0200, Laurent Pinchart wrote: > > On Monday 22 October 2012 03:33:19 Li Yang-R58472 wrote: > > > On Saturday, October 20, 2012 1:37 AM Felipe Balbi wrote: > > > > On Fri, Oct 19, 2012 at 06:19:26PM +0100, Simon Haggett wrote: > > > > > Some gadget drivers may attempt to dequeue requests for an endpoint > > > > > that has already been disabled. For example, in the UVC gadget > > > > > driver, uvc_function_set_alt() will call usb_ep_disable() when alt > > > > > setting 0 is selected. When the userspace application subsequently > > > > > issues the VIDIOC_STREAMOFF ioctl, uvc_video_enable() invokes > > > > > usb_ep_dequeue() to ensure that all requests have been cancelled. > > > > > > > > bug is on uvc gadget, then. Laurent ? > > > > We've discussed this topic a couple of months before. I believe that's not > > a bug. > > > > http://68.183.106.108/lists/linux-usb/msg68869.html > > fair enough :-) > > That's a different case, however. At the link above we're discussing > dequeueing a request which is already being dequeued. $SUBJECT is trying > to fix dequeueing of a request for an endpoint which isn't even enabled. You've got a point there :-) That's a different case indeed, I'm open to (at least evaluating) a fix in the UVC gadget driver if you think that's better. -- Regards, Laurent Pinchart signature.asc Description: This is a digitally signed message part. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH 11/13] sdhci: Add quirk for controllers that need IRQ re-init after reset
Hi Anton, On Friday 13 February 2009 15:47:38 Anton Vorontsov wrote: > FSL eSDHC controllers losing signal/interrupt enable states after > reset, so we should re-enable them. > > Signed-off-by: Anton Vorontsov > --- > drivers/mmc/host/sdhci.c |7 +++ > drivers/mmc/host/sdhci.h |2 ++ > 2 files changed, 9 insertions(+), 0 deletions(-) > > diff --git a/drivers/mmc/host/sdhci.c b/drivers/mmc/host/sdhci.c > index eff615d..b308dbf 100644 > --- a/drivers/mmc/host/sdhci.c > +++ b/drivers/mmc/host/sdhci.c > @@ -189,6 +189,7 @@ static void sdhci_disable_card_detection(struct > sdhci_host *host) static void sdhci_reset(struct sdhci_host *host, u8 mask) > { > unsigned long timeout; > + u32 ier = 0; /* shut up gcc */ You should use u32 uninitialized_var(ier); instead to avoid generating extra code. > > if (host->quirks & SDHCI_QUIRK_NO_CARD_NO_RESET) { > if (!(sdhci_readl(host, SDHCI_PRESENT_STATE) & > @@ -196,6 +197,9 @@ static void sdhci_reset(struct sdhci_host *host, u8 > mask) return; > } > > + if (host->quirks & SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET) > + ier = sdhci_readl(host, SDHCI_INT_ENABLE); > + > sdhci_writeb(host, mask, SDHCI_SOFTWARE_RESET); > > if (mask & SDHCI_RESET_ALL) > @@ -215,6 +219,9 @@ static void sdhci_reset(struct sdhci_host *host, u8 > mask) timeout--; > mdelay(1); > } > + > + if (host->quirks & SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET) > + sdhci_clear_set_irqs(host, SDHCI_INT_ALL_MASK, ier); > } > > static void sdhci_init(struct sdhci_host *host) > diff --git a/drivers/mmc/host/sdhci.h b/drivers/mmc/host/sdhci.h > index 44c820a..5c5a950 100644 > --- a/drivers/mmc/host/sdhci.h > +++ b/drivers/mmc/host/sdhci.h > @@ -229,6 +229,8 @@ struct sdhci_host { > #define SDHCI_QUIRK_NONSTANDARD_CLOCK(1<<19) > /* Controller does not like fast PIO transfers */ > #define SDHCI_QUIRK_PIO_NEEDS_DELAY (1<<20) > +/* Controller losing signal/interrupt enable states after reset */ > +#define SDHCI_QUIRK_RESTORE_IRQS_AFTER_RESET (1<<21) > > int irq;/* Device IRQ */ > void __iomem * ioaddr; /* Mapped address */ -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 signature.asc Description: This is a digitally signed message part. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [MPC8272ADS]Can not configure the ttyCPM0
Hi Jean-Michel, On Friday 13 March 2009 14:38:55 Jean-Michel Hautbois wrote: > Hi all ! > > I am currently facing a big problem on my MPC8272ADS development board. > I have tried to use the /dev/ttyCPM1 port in order to send data over > the serial cable to another device. > > This is just not working but nothing is sent, my DTR signal is down: > > cat /proc/tty/driver/ttyCPM > > 0: uart:CPM UART mmio:0xF0011A00 irq:40 tx:296 rx:0 RTS|CTS|DTR|DSR|CD > 1: uart:CPM UART mmio:0xF0011A60 irq:43 tx:0 rx:0 CTS|DSR|CD > > So, I have tried to use the serial port /dev/ttyCPM0. It is > successfully sending out my bytes, but I can't configure it !! > When I am trying to use the tcsetattr() function, I don't have any > error, but the signal is always the same (no effect, if you prefer). > > I have tried to disable the kernel console output, but it is not > working, either... Support for the modem control lines has been added in v2.6.27-rc2. You will need to declare the modem control lines in your device tree. See Documentation/powerpc/dts-bindings/fsl/cpm_qe/serial.txt for more information. Please note that hardware flow control is not supported by the CPM UART driver yet. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Waterloo Office Park Building M Dreve Richelle, 161 B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 signature.asc Description: This is a digitally signed message part. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [MPC8272ADS]Can not configure the ttyCPM0
Hi Jean-Michel, On Friday 13 March 2009 15:20:24 Jean-Michel Hautbois wrote: > 2009/3/13 Laurent Pinchart : > > Support for the modem control lines has been added in v2.6.27-rc2. You > > will need to declare the modem control lines in your device tree. See > > Documentation/powerpc/dts-bindings/fsl/cpm_qe/serial.txt for more > > information. > > > > Please note that hardware flow control is not supported by the CPM UART > > driver yet. > > OK, this is not easy, to modify... > My DTS already contains a configuration for the CPM1 port... > > ser...@11a00 { > device_type = "serial"; > compatible = "fsl,mpc8272-scc-uart", >"fsl,cpm2-scc-uart"; > reg = <0x11a00 0x20 0x8000 0x100>; > interrupts = <40 8>; > interrupt-parent = <&PIC>; > fsl,cpm-brg = <1>; > fsl,cpm-command = <0x80>; > }; > > ser...@11a60 { > device_type = "serial"; > compatible = "fsl,mpc8272-scc-uart", >"fsl,cpm2-scc-uart"; > reg = <0x11a60 0x20 0x8300 0x100>; > interrupts = <43 8>; > interrupt-parent = <&PIC>; > fsl,cpm-brg = <4>; > fsl,cpm-command = <0xce>; > }; > > I can't see what I have to do... > This is not obvious ;). Here are excerpts of my device tree for an MPC8248 based platform. You need to declare GPIO controllers for the pins used by modem control lines cpm2_pio_c: gpio_control...@10d40 { compatible = "fsl,mpc8248-pario-bank", "fsl,cpm2-pario-bank"; reg = <0x10d40 0x14>; #gpio-cells = <2>; gpio-controller; }; cpm2_pio_d: gpio_control...@10d60 { compatible = "fsl,mpc8248-pario-bank", "fsl,cpm2-pario-bank"; reg = <0x10d60 0x14>; #gpio-cells = <2>; gpio-controller; }; and add the GPIO mapping in your serial port controller nodes gpios = <&cpm2_pio_c 15 0 /* CTS */ &cpm2_pio_d 29 0 /* RTS */ &cpm2_pio_c 14 0 /* DCD */ 0 /* DSR */ &cpm2_pio_d 20 0 /* DTR */ &cpm2_pio_c 8 0/* RI */ >; Don't forget to configure the pins (direction, special purpose, ...) in your board-specific code. -- Laurent Pinchart CSE Semaphore Belgium Waterloo Office Park Building M Dreve Richelle, 161 B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 signature.asc Description: This is a digitally signed message part. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
g parameter which is > > +Q31.32 format. > > > > .. raw:: latex > > > > diff --git a/Documentation/userspace-api/media/videodev2.h.rst.exceptions > > b/Documentation/userspace-api/media/videodev2.h.rst.exceptions > > index e61152bb80d1..2faa5a2015eb 100644 > > --- a/Documentation/userspace-api/media/videodev2.h.rst.exceptions > > +++ b/Documentation/userspace-api/media/videodev2.h.rst.exceptions > > @@ -167,6 +167,7 @@ replace symbol V4L2_CTRL_TYPE_AV1_SEQUENCE > > :c:type:`v4l2_ctrl_type` > > replace symbol V4L2_CTRL_TYPE_AV1_TILE_GROUP_ENTRY :c:type:`v4l2_ctrl_type` > > replace symbol V4L2_CTRL_TYPE_AV1_FRAME :c:type:`v4l2_ctrl_type` > > replace symbol V4L2_CTRL_TYPE_AV1_FILM_GRAIN :c:type:`v4l2_ctrl_type` > > +replace symbol V4L2_CTRL_TYPE_FIXED_POINT :c:type:`v4l2_ctrl_type` > > > > # V4L2 capability defines > > replace define V4L2_CAP_VIDEO_CAPTURE device-capabilities > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-api.c > > b/drivers/media/v4l2-core/v4l2-ctrls-api.c > > index 002ea6588edf..e6a0fb8d6791 100644 > > --- a/drivers/media/v4l2-core/v4l2-ctrls-api.c > > +++ b/drivers/media/v4l2-core/v4l2-ctrls-api.c > > @@ -57,6 +57,7 @@ static int ptr_to_user(struct v4l2_ext_control *c, > > return copy_to_user(c->string, ptr.p_char, len + 1) ? > >-EFAULT : 0; > > case V4L2_CTRL_TYPE_INTEGER64: > > + case V4L2_CTRL_TYPE_FIXED_POINT: > > c->value64 = *ptr.p_s64; > > break; > > default: > > @@ -132,6 +133,7 @@ static int user_to_new(struct v4l2_ext_control *c, > > struct v4l2_ctrl *ctrl) > > > > switch (ctrl->type) { > > case V4L2_CTRL_TYPE_INTEGER64: > > + case V4L2_CTRL_TYPE_FIXED_POINT: > > *ctrl->p_new.p_s64 = c->value64; > > break; > > case V4L2_CTRL_TYPE_STRING: > > @@ -540,7 +542,8 @@ static int validate_ctrls(struct v4l2_ext_controls *cs, > > */ > > if (ctrl->is_ptr) > > continue; > > - if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64) > > + if (ctrl->type == V4L2_CTRL_TYPE_INTEGER64 || > > + ctrl->type == V4L2_CTRL_TYPE_FIXED_POINT) > > p_new.p_s64 = &cs->controls[i].value64; > > else > > p_new.p_s32 = &cs->controls[i].value; > > diff --git a/drivers/media/v4l2-core/v4l2-ctrls-core.c > > b/drivers/media/v4l2-core/v4l2-ctrls-core.c > > index a662fb60f73f..9d50df0d9874 100644 > > --- a/drivers/media/v4l2-core/v4l2-ctrls-core.c > > +++ b/drivers/media/v4l2-core/v4l2-ctrls-core.c > > @@ -1187,6 +1187,7 @@ static int std_validate_elem(const struct v4l2_ctrl > > *ctrl, u32 idx, > > case V4L2_CTRL_TYPE_INTEGER: > > return ROUND_TO_RANGE(ptr.p_s32[idx], u32, ctrl); > > case V4L2_CTRL_TYPE_INTEGER64: > > + case V4L2_CTRL_TYPE_FIXED_POINT: > > /* > > * We can't use the ROUND_TO_RANGE define here due to > > * the u64 divide that needs special care. > > @@ -1779,6 +1780,7 @@ static struct v4l2_ctrl *v4l2_ctrl_new(struct > > v4l2_ctrl_handler *hdl, > > /* Prefill elem_size for all types handled by std_type_ops */ > > switch ((u32)type) { > > case V4L2_CTRL_TYPE_INTEGER64: > > + case V4L2_CTRL_TYPE_FIXED_POINT: > > elem_size = sizeof(s64); > > break; > > case V4L2_CTRL_TYPE_STRING: > > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > > index cf8c44595a1d..9482ac66a675 100644 > > --- a/include/uapi/linux/videodev2.h > > +++ b/include/uapi/linux/videodev2.h > > @@ -1903,6 +1903,7 @@ enum v4l2_ctrl_type { > > V4L2_CTRL_TYPE_STRING= 7, > > V4L2_CTRL_TYPE_BITMASK = 8, > > V4L2_CTRL_TYPE_INTEGER_MENU = 9, > > + V4L2_CTRL_TYPE_FIXED_POINT = 10, > > > > /* Compound types are >= 0x0100 */ > > V4L2_CTRL_COMPOUND_TYPES = 0x0100, -- Regards, Laurent Pinchart
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > On 13/11/2023 11:42, Laurent Pinchart wrote: > > On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > >> Hi Shengjiu, > >> > >> On 10/11/2023 06:48, Shengjiu Wang wrote: > >>> Fixed point controls are used by the user to configure > >>> a fixed point value in 64bits, which Q31.32 format. > >>> > >>> Signed-off-by: Shengjiu Wang > >> > >> This patch adds a new control type. This is something that also needs to be > >> tested by v4l2-compliance, and for that we need to add support for this to > >> one of the media test-drivers. The best place for that is the vivid driver, > >> since that has already a bunch of test controls for other control types. > >> > >> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > >> > >> Can you add a patch adding a fixed point test control to vivid? > > > > I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to > > relate more to units than control types. We have lots of fixed-point > > values in controls already, using the 32-bit and 64-bit integer control > > types. They use various locations for the decimal point, depending on > > the control. If we want to make this more explicit to users, we should > > work on adding unit support to the V4L2 controls. > > "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. It's not a unit, but I think it's related to units. My point is that, without units support, I don't see why we need a formal definition of fixed-point types, and why this series couldn't just use VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 values as they see fit. > A quick "git grep -i "fixed point" Documentation/userspace-api/media/' > only shows a single driver specific control (dw100.rst). > > I'm not aware of other controls in mainline that use fixed point. The analog gain control for sensors for instance. > Note that V4L2_CTRL_TYPE_FIXED_POINT is a Q31.32 format. By setting > min/max/step you can easily map that to just about any QN.M format where > N <= 31 and M <= 32. > > In the case of dw100 it is a bit different in that it is quite specialized > and it had to fit in 16 bits. > > >>> --- > >>> .../userspace-api/media/v4l/vidioc-g-ext-ctrls.rst | 13 +++-- > >>> .../userspace-api/media/v4l/vidioc-queryctrl.rst| 9 - > >>> .../userspace-api/media/videodev2.h.rst.exceptions | 1 + > >>> drivers/media/v4l2-core/v4l2-ctrls-api.c| 5 - > >>> drivers/media/v4l2-core/v4l2-ctrls-core.c | 2 ++ > >>> include/uapi/linux/videodev2.h | 1 + > >>> 6 files changed, 23 insertions(+), 8 deletions(-) > >>> > >>> diff --git a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > >>> b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > >>> index e8475f9fd2cf..e7e5d78dc11e 100644 > >>> --- a/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > >>> +++ b/Documentation/userspace-api/media/v4l/vidioc-g-ext-ctrls.rst > >>> @@ -162,13 +162,13 @@ still cause this situation. > >>> * - __s32 > >>>- ``value`` > >>>- New value or current value. Valid if this control is not of type > >>> - ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is > >>> - not set. > >>> + ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and > >>> + ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set. > >>> * - __s64 > >>>- ``value64`` > >>>- New value or current value. Valid if this control is of type > >>> - ``V4L2_CTRL_TYPE_INTEGER64`` and ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is > >>> - not set. > >>> + ``V4L2_CTRL_TYPE_INTEGER64``, ``V4L2_CTRL_TYPE_FIXED_POINT`` and > >>> + ``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is not set. > >>> * - char * > >>>- ``string`` > >>>- A pointer to a string. Valid if this control is of type > >>> @@ -193,8 +193,9 @@ still cause this situation. > >>> * - __s64 * > >>>- ``p_s64`` > >>>- A pointer to a matrix control of signed 64-bit values. Valid if > >>> -this control is of type ``V4L2_CTRL_TYPE_INTEGER64`` and > >>> -``V4L2_CTRL_FLAG_HAS_PAYLOAD`` is set. > >
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > Hi Hans, > > On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > > On 13/11/2023 12:07, Laurent Pinchart wrote: > > > On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > > >> On 13/11/2023 11:42, Laurent Pinchart wrote: > > >>> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > > >>>> Hi Shengjiu, > > >>>> > > >>>> On 10/11/2023 06:48, Shengjiu Wang wrote: > > >>>>> Fixed point controls are used by the user to configure > > >>>>> a fixed point value in 64bits, which Q31.32 format. > > >>>>> > > >>>>> Signed-off-by: Shengjiu Wang > > >>>> > > >>>> This patch adds a new control type. This is something that also needs > > >>>> to be > > >>>> tested by v4l2-compliance, and for that we need to add support for > > >>>> this to > > >>>> one of the media test-drivers. The best place for that is the vivid > > >>>> driver, > > >>>> since that has already a bunch of test controls for other control > > >>>> types. > > >>>> > > >>>> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > > >>>> > > >>>> Can you add a patch adding a fixed point test control to vivid? > > >>> > > >>> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to > > >>> relate more to units than control types. We have lots of fixed-point > > >>> values in controls already, using the 32-bit and 64-bit integer control > > >>> types. They use various locations for the decimal point, depending on > > >>> the control. If we want to make this more explicit to users, we should > > >>> work on adding unit support to the V4L2 controls. > > >> > > >> "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. > > > > > > It's not a unit, but I think it's related to units. My point is that, > > > without units support, I don't see why we need a formal definition of > > > fixed-point types, and why this series couldn't just use > > > VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > > > values as they see fit. > > > > They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 > > (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > > is always interpreted as a 64 bit integer and nothing else. As it should. The most common case for control handling in drivers is taking the integer value and converting it to a register value, using device-specific encoding of the register value. It can be a fixed-point format or something else, depending on the device. My point is that drivers routinely convert a "plain" integer to something else, and that has never been considered as a cause of concern. I don't see why it would be different in this series. > > And while we do not have support for units (other than the documentation), > > we do have type support in the form of V4L2_CTRL_TYPE_*. > > > > >> A quick "git grep -i "fixed point" Documentation/userspace-api/media/' > > >> only shows a single driver specific control (dw100.rst). > > >> > > >> I'm not aware of other controls in mainline that use fixed point. > > > > > > The analog gain control for sensors for instance. > > > > Not really. The documentation is super vague: > > > > V4L2_CID_ANALOGUE_GAIN (integer) > > > > Analogue gain is gain affecting all colour components in the pixel > > matrix. The > > gain operation is performed in the analogue domain before A/D > > conversion. > > > > And the integer is just a range. Internally it might map to some fixed > > point value, but userspace won't see that, it's hidden in the driver AFAICT. It's hidden so well that libcamera has a database of the sensor it supports, with formulas to map a real gain value to the V4L2_CID_ANALOGUE_GAIN control. The encoding of the integer value does matter, and the kernel doesn't expose it. We may or may not consider that as a shortcoming of the V4L2 control API, but in any case it's the situation we have today. > I wonder if Laurent meant digital gain. No, I meant a
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Hans, On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: > On 13/11/2023 12:43, Laurent Pinchart wrote: > > On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > >> On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > >>> On 13/11/2023 12:07, Laurent Pinchart wrote: > >>>> On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > >>>>> On 13/11/2023 11:42, Laurent Pinchart wrote: > >>>>>> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > >>>>>>> On 10/11/2023 06:48, Shengjiu Wang wrote: > >>>>>>>> Fixed point controls are used by the user to configure > >>>>>>>> a fixed point value in 64bits, which Q31.32 format. > >>>>>>>> > >>>>>>>> Signed-off-by: Shengjiu Wang > >>>>>>> > >>>>>>> This patch adds a new control type. This is something that also needs > >>>>>>> to be > >>>>>>> tested by v4l2-compliance, and for that we need to add support for > >>>>>>> this to > >>>>>>> one of the media test-drivers. The best place for that is the vivid > >>>>>>> driver, > >>>>>>> since that has already a bunch of test controls for other control > >>>>>>> types. > >>>>>>> > >>>>>>> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > >>>>>>> > >>>>>>> Can you add a patch adding a fixed point test control to vivid? > >>>>>> > >>>>>> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems to > >>>>>> relate more to units than control types. We have lots of fixed-point > >>>>>> values in controls already, using the 32-bit and 64-bit integer control > >>>>>> types. They use various locations for the decimal point, depending on > >>>>>> the control. If we want to make this more explicit to users, we should > >>>>>> work on adding unit support to the V4L2 controls. > >>>>> > >>>>> "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. > >>>> > >>>> It's not a unit, but I think it's related to units. My point is that, > >>>> without units support, I don't see why we need a formal definition of > >>>> fixed-point types, and why this series couldn't just use > >>>> VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > >>>> values as they see fit. > >>> > >>> They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 > >>> (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it > > > > Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > > > >>> is always interpreted as a 64 bit integer and nothing else. As it should. > > > > The most common case for control handling in drivers is taking the > > integer value and converting it to a register value, using > > device-specific encoding of the register value. It can be a fixed-point > > format or something else, depending on the device. My point is that > > drivers routinely convert a "plain" integer to something else, and that > > has never been considered as a cause of concern. I don't see why it > > would be different in this series. > > > >>> And while we do not have support for units (other than the documentation), > >>> we do have type support in the form of V4L2_CTRL_TYPE_*. > >>> > >>>>> A quick "git grep -i "fixed point" Documentation/userspace-api/media/' > >>>>> only shows a single driver specific control (dw100.rst). > >>>>> > >>>>> I'm not aware of other controls in mainline that use fixed point. > >>>> > >>>> The analog gain control for sensors for instance. > >>> > >>> Not really. The documentation is super vague: > >>> > >>> V4L2_CID_ANALOGUE_GAIN (integer) > >>> > >>> Analogue gain is gain affecting all colour components in the pixel > >>> matrix. The > >>> gain operation is performed in the analogue domain before A/D > >>> conversion. > >>> > >>> A
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Hans, On Wed, Nov 15, 2023 at 09:09:42AM +0100, Hans Verkuil wrote: > On 13/11/2023 13:44, Laurent Pinchart wrote: > > On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: > >> On 13/11/2023 12:43, Laurent Pinchart wrote: > >>> On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > >>>> On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > >>>>> On 13/11/2023 12:07, Laurent Pinchart wrote: > >>>>>> On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > >>>>>>> On 13/11/2023 11:42, Laurent Pinchart wrote: > >>>>>>>> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > >>>>>>>>> On 10/11/2023 06:48, Shengjiu Wang wrote: > >>>>>>>>>> Fixed point controls are used by the user to configure > >>>>>>>>>> a fixed point value in 64bits, which Q31.32 format. > >>>>>>>>>> > >>>>>>>>>> Signed-off-by: Shengjiu Wang > >>>>>>>>> > >>>>>>>>> This patch adds a new control type. This is something that also > >>>>>>>>> needs to be > >>>>>>>>> tested by v4l2-compliance, and for that we need to add support for > >>>>>>>>> this to > >>>>>>>>> one of the media test-drivers. The best place for that is the vivid > >>>>>>>>> driver, > >>>>>>>>> since that has already a bunch of test controls for other control > >>>>>>>>> types. > >>>>>>>>> > >>>>>>>>> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > >>>>>>>>> > >>>>>>>>> Can you add a patch adding a fixed point test control to vivid? > >>>>>>>> > >>>>>>>> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This seems > >>>>>>>> to > >>>>>>>> relate more to units than control types. We have lots of fixed-point > >>>>>>>> values in controls already, using the 32-bit and 64-bit integer > >>>>>>>> control > >>>>>>>> types. They use various locations for the decimal point, depending on > >>>>>>>> the control. If we want to make this more explicit to users, we > >>>>>>>> should > >>>>>>>> work on adding unit support to the V4L2 controls. > >>>>>>> > >>>>>>> "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. > >>>>>> > >>>>>> It's not a unit, but I think it's related to units. My point is that, > >>>>>> without units support, I don't see why we need a formal definition of > >>>>>> fixed-point types, and why this series couldn't just use > >>>>>> VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > >>>>>> values as they see fit. > >>>>> > >>>>> They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 > >>>>> (I assume you meant that rather than VIVID_CID_INTEGER64) shows that it > >>> > >>> Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > >>> > >>>>> is always interpreted as a 64 bit integer and nothing else. As it > >>>>> should. > >>> > >>> The most common case for control handling in drivers is taking the > >>> integer value and converting it to a register value, using > >>> device-specific encoding of the register value. It can be a fixed-point > >>> format or something else, depending on the device. My point is that > >>> drivers routinely convert a "plain" integer to something else, and that > >>> has never been considered as a cause of concern. I don't see why it > >>> would be different in this series. > >>> > >>>>> And while we do not have support for units (other than the > >>>>> documentation), > >>>>> we do have type support in the form of V4L2_CTRL_TYPE_*. > >>>>> > >>>>>>> A quick "git grep -i "fixed point" Docum
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Hans, On Wed, Nov 15, 2023 at 12:19:31PM +0100, Hans Verkuil wrote: > On 11/15/23 11:55, Laurent Pinchart wrote: > > On Wed, Nov 15, 2023 at 09:09:42AM +0100, Hans Verkuil wrote: > >> On 13/11/2023 13:44, Laurent Pinchart wrote: > >>> On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: > >>>> On 13/11/2023 12:43, Laurent Pinchart wrote: > >>>>> On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > >>>>>> On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > >>>>>>> On 13/11/2023 12:07, Laurent Pinchart wrote: > >>>>>>>> On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > >>>>>>>>> On 13/11/2023 11:42, Laurent Pinchart wrote: > >>>>>>>>>> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > >>>>>>>>>>> On 10/11/2023 06:48, Shengjiu Wang wrote: > >>>>>>>>>>>> Fixed point controls are used by the user to configure > >>>>>>>>>>>> a fixed point value in 64bits, which Q31.32 format. > >>>>>>>>>>>> > >>>>>>>>>>>> Signed-off-by: Shengjiu Wang > >>>>>>>>>>> > >>>>>>>>>>> This patch adds a new control type. This is something that also > >>>>>>>>>>> needs to be > >>>>>>>>>>> tested by v4l2-compliance, and for that we need to add support > >>>>>>>>>>> for this to > >>>>>>>>>>> one of the media test-drivers. The best place for that is the > >>>>>>>>>>> vivid driver, > >>>>>>>>>>> since that has already a bunch of test controls for other control > >>>>>>>>>>> types. > >>>>>>>>>>> > >>>>>>>>>>> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > >>>>>>>>>>> > >>>>>>>>>>> Can you add a patch adding a fixed point test control to vivid? > >>>>>>>>>> > >>>>>>>>>> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This > >>>>>>>>>> seems to > >>>>>>>>>> relate more to units than control types. We have lots of > >>>>>>>>>> fixed-point > >>>>>>>>>> values in controls already, using the 32-bit and 64-bit integer > >>>>>>>>>> control > >>>>>>>>>> types. They use various locations for the decimal point, depending > >>>>>>>>>> on > >>>>>>>>>> the control. If we want to make this more explicit to users, we > >>>>>>>>>> should > >>>>>>>>>> work on adding unit support to the V4L2 controls. > >>>>>>>>> > >>>>>>>>> "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are units. > >>>>>>>> > >>>>>>>> It's not a unit, but I think it's related to units. My point is that, > >>>>>>>> without units support, I don't see why we need a formal definition of > >>>>>>>> fixed-point types, and why this series couldn't just use > >>>>>>>> VIVID_CID_INTEGER64. Drivers already interpret VIVID_CID_INTEGER64 > >>>>>>>> values as they see fit. > >>>>>>> > >>>>>>> They do? That's new to me. A quick grep for V4L2_CTRL_TYPE_INTEGER64 > >>>>>>> (I assume you meant that rather than VIVID_CID_INTEGER64) shows that > >>>>>>> it > >>>>> > >>>>> Yes, I meant V4L2_CTRL_TYPE_INTEGER64. Too hasty copy & paste :-) > >>>>> > >>>>>>> is always interpreted as a 64 bit integer and nothing else. As it > >>>>>>> should. > >>>>> > >>>>> The most common case for control handling in drivers is taking the > >>>>> integer value and converting it to a register value, using > >>>>> device-specific encoding of the regi
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
Hi Tomasz, On Thu, Nov 16, 2023 at 04:31:41PM +0900, Tomasz Figa wrote: > On Wed, Nov 15, 2023 at 8:49 PM Laurent Pinchart wrote: > > On Wed, Nov 15, 2023 at 12:19:31PM +0100, Hans Verkuil wrote: > > > On 11/15/23 11:55, Laurent Pinchart wrote: > > > > On Wed, Nov 15, 2023 at 09:09:42AM +0100, Hans Verkuil wrote: > > > >> On 13/11/2023 13:44, Laurent Pinchart wrote: > > > >>> On Mon, Nov 13, 2023 at 01:05:12PM +0100, Hans Verkuil wrote: > > > >>>> On 13/11/2023 12:43, Laurent Pinchart wrote: > > > >>>>> On Mon, Nov 13, 2023 at 11:28:51AM +, Sakari Ailus wrote: > > > >>>>>> On Mon, Nov 13, 2023 at 12:24:14PM +0100, Hans Verkuil wrote: > > > >>>>>>> On 13/11/2023 12:07, Laurent Pinchart wrote: > > > >>>>>>>> On Mon, Nov 13, 2023 at 11:56:49AM +0100, Hans Verkuil wrote: > > > >>>>>>>>> On 13/11/2023 11:42, Laurent Pinchart wrote: > > > >>>>>>>>>> On Mon, Nov 13, 2023 at 11:29:09AM +0100, Hans Verkuil wrote: > > > >>>>>>>>>>> On 10/11/2023 06:48, Shengjiu Wang wrote: > > > >>>>>>>>>>>> Fixed point controls are used by the user to configure > > > >>>>>>>>>>>> a fixed point value in 64bits, which Q31.32 format. > > > >>>>>>>>>>>> > > > >>>>>>>>>>>> Signed-off-by: Shengjiu Wang > > > >>>>>>>>>>> > > > >>>>>>>>>>> This patch adds a new control type. This is something that > > > >>>>>>>>>>> also needs to be > > > >>>>>>>>>>> tested by v4l2-compliance, and for that we need to add > > > >>>>>>>>>>> support for this to > > > >>>>>>>>>>> one of the media test-drivers. The best place for that is the > > > >>>>>>>>>>> vivid driver, > > > >>>>>>>>>>> since that has already a bunch of test controls for other > > > >>>>>>>>>>> control types. > > > >>>>>>>>>>> > > > >>>>>>>>>>> See e.g. VIVID_CID_INTEGER64 in vivid-ctrls.c. > > > >>>>>>>>>>> > > > >>>>>>>>>>> Can you add a patch adding a fixed point test control to > > > >>>>>>>>>>> vivid? > > > >>>>>>>>>> > > > >>>>>>>>>> I don't think V4L2_CTRL_TYPE_FIXED_POINT is a good idea. This > > > >>>>>>>>>> seems to > > > >>>>>>>>>> relate more to units than control types. We have lots of > > > >>>>>>>>>> fixed-point > > > >>>>>>>>>> values in controls already, using the 32-bit and 64-bit > > > >>>>>>>>>> integer control > > > >>>>>>>>>> types. They use various locations for the decimal point, > > > >>>>>>>>>> depending on > > > >>>>>>>>>> the control. If we want to make this more explicit to users, > > > >>>>>>>>>> we should > > > >>>>>>>>>> work on adding unit support to the V4L2 controls. > > > >>>>>>>>> > > > >>>>>>>>> "Fixed Point" is not a unit, it's a type. 'Db', 'Hz' etc. are > > > >>>>>>>>> units. > > > >>>>>>>> > > > >>>>>>>> It's not a unit, but I think it's related to units. My point is > > > >>>>>>>> that, > > > >>>>>>>> without units support, I don't see why we need a formal > > > >>>>>>>> definition of > > > >>>>>>>> fixed-point types, and why this series couldn't just use > > > >>>>>>>> VIVID_CID_INTEGER64. Drivers already interpret > > > >>>>>>>> VIVID_CID_INTEGER64 > > > >>>>&g
Re: [PATCH v9 10/15] media: uapi: Add V4L2_CTRL_TYPE_FIXED_POINT
ew_std_compound); > @@ -2173,7 +2219,7 @@ struct v4l2_ctrl *v4l2_ctrl_new_int_menu(struct > v4l2_ctrl_handler *hdl, > return NULL; > } > return v4l2_ctrl_new(hdl, ops, NULL, id, name, type, > - 0, max, 0, def, NULL, 0, > + 0, max, 0, def, NULL, 0, 0, >flags, NULL, qmenu_int, ptr_null, NULL); > } > EXPORT_SYMBOL(v4l2_ctrl_new_int_menu); > diff --git a/include/media/v4l2-ctrls.h b/include/media/v4l2-ctrls.h > index 59679a42b3e7..c35514c5bf88 100644 > --- a/include/media/v4l2-ctrls.h > +++ b/include/media/v4l2-ctrls.h > @@ -211,7 +211,8 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl > *ctrl, void *priv); > * except for dynamic arrays. In that case it is in the range of > * 1 to @p_array_alloc_elems. > * @dims:The size of each dimension. > - * @nr_of_dims:The number of dimensions in @dims. > + * @nr_of_dims: The number of dimensions in @dims. > + * @fraction_bits: The number of fraction bits for fixed point values. > * @menu_skip_mask: The control's skip mask for menu controls. This makes it > * easy to skip menu items that are not valid. If bit X is set, > * then menu item X is skipped. Of course, this only works for > @@ -228,6 +229,7 @@ typedef void (*v4l2_ctrl_notify_fnc)(struct v4l2_ctrl > *ctrl, void *priv); > * :math:`ceil(\frac{maximum - minimum}{step}) + 1`. > * Used only if the @type is %V4L2_CTRL_TYPE_INTEGER_MENU. > * @flags: The control's flags. > + * @fraction_bits: The number of fraction bits for fixed point values. > * @priv:The control's private pointer. For use by the driver. It is > * untouched by the control framework. Note that this pointer is > * not freed when the control is deleted. Should this be needed > @@ -286,6 +288,7 @@ struct v4l2_ctrl { > u32 new_elems; > u32 dims[V4L2_CTRL_MAX_DIMS]; > u32 nr_of_dims; > + u32 fraction_bits; > union { > u64 step; > u64 menu_skip_mask; > @@ -426,6 +429,7 @@ struct v4l2_ctrl_handler { > * @dims:The size of each dimension. > * @elem_size: The size in bytes of the control. > * @flags: The control's flags. > + * @fraction_bits: The number of fraction bits for fixed point values. > * @menu_skip_mask: The control's skip mask for menu controls. This makes it > * easy to skip menu items that are not valid. If bit X is set, > * then menu item X is skipped. Of course, this only works for > @@ -455,6 +459,7 @@ struct v4l2_ctrl_config { > u32 dims[V4L2_CTRL_MAX_DIMS]; > u32 elem_size; > u32 flags; > + u32 fraction_bits; > u64 menu_skip_mask; > const char * const *qmenu; > const s64 *qmenu_int; > diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h > index c3d4e490ce7c..26ecac19722a 100644 > --- a/include/uapi/linux/videodev2.h > +++ b/include/uapi/linux/videodev2.h > @@ -1944,9 +1944,27 @@ struct v4l2_query_ext_ctrl { > __u32elems; > __u32nr_of_dims; > __u32dims[V4L2_CTRL_MAX_DIMS]; > - __u32reserved[32]; > + __u32fraction_bits; Thinking forward, what if the control uses a different type of quantization, for instance if the control is exponential instead of linear ? Is this something we want to plan for already (without necessarily implementing it yet) ? For instance, the CCS spec defines a linear gain model where the gain is expressed as gain = (m0 * x + c0) / (m1 * x + c1) where x is the control value, gain is the real gain, and m0, c0, m1 and c1 are device-specific 16-bit integer constants (with the constraint that one of m0 and m1 has to be zero, but not both). There's also an exponential model in CCS, with gain = linear_gain * 2 ^ exponential_gain where linear_gain and exponential_gain are U8.8 values. > + __u32reserved[31]; > }; > > +static inline __s64 v4l2_fp_compose(__s64 i, __s64 f, unsigned int > fraction_bits) > +{ > + return (i << fraction_bits) + f; > +} > + > +static inline __s64 v4l2_fp_integer(__s64 v, unsigned int fraction_bits) > +{ > + return v / (1LL << fraction_bits); > +} > + > +static inline __s64 v4l2_fp_fraction(__s64 v, unsigned int fraction_bits) > +{ > + __u64 mask = (1ULL << fraction_bits) - 1; > + > + return v < 0 ? -((-v) & mask) : (v & mask); > +} I woudln't add static inline functions to the UAPI. They cause licensing issues, and increase the UAPI surface, thus making UAPI evolutions more difficult. > + > /* Used in the VIDIOC_QUERYMENU ioctl for querying menu items */ > struct v4l2_querymenu { > __u32 id; -- Regards, Laurent Pinchart
Re: [PATCH 6/9] fs_enet: Be an of_platform device when CONFIG_PPC_CPM_NEW_BINDING is set.
Hi Scott, I apologize for bringing this after so much time. Hope the "better late than never" motto applies. On Friday 21 September 2007 00:01, Scott Wood wrote: > The existing OF glue code was crufty and broken. Rather than fix it, it > will be removed, and the ethernet driver now talks to the device tree > directly. > > The old, non-CONFIG_PPC_CPM_NEW_BINDING code can go away once CPM > platforms are dropped from arch/ppc (which will hopefully be soon), and > existing arch/powerpc boards that I wasn't able to test on for this > patchset get converted (which should be even sooner). > > Signed-off-by: Scott Wood <[EMAIL PROTECTED]> [snip] > diff --git a/drivers/net/fs_enet/fs_enet.h b/drivers/net/fs_enet/fs_enet.h > index f8c7ee8..14ebba8 100644 > --- a/drivers/net/fs_enet/fs_enet.h > +++ b/drivers/net/fs_enet/fs_enet.h > @@ -24,19 +24,6 @@ struct fec_info { > #include > #endif > > -/* This is used to operate with pins. > - Note that the actual port size may > -be different; cpm(s) handle it OK */ > -struct bb_info { > - u8 mdio_dat_msk; > - u8 mdio_dir_msk; > - u8 *mdio_dir; > - u8 *mdio_dat; > - u8 mdc_msk; > - u8 *mdc_dat; > - int delay; > -}; > - [snip] > diff --git a/drivers/net/fs_enet/mii-bitbang.c > b/drivers/net/fs_enet/mii-bitbang.c index 8f766a5..2b9c44c 100644 > --- a/drivers/net/fs_enet/mii-bitbang.c > +++ b/drivers/net/fs_enet/mii-bitbang.c [snip] > +struct bb_info { > + __be32 __iomem *dir; > + __be32 __iomem *dat; > + u32 mdio_msk; > + u32 mdc_msk; > + int delay; > +}; We're loosing the possibility of having MDC and MDIO on different ports. This is quite easy to fix for the non-CONFIG_PPC_CPM_NEW_BINDING case but I'm not familiar with OF bindings (yet) to fix the CONFIG_PPC_CPM_NEW_BINDING case. -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpjs8I2SJNNv.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] fs_enet: Remove unused fields in the fs_mii_bb_platform_info structure.
The mdio_port, mdio_bit, mdc_port and mdc_bit fields in the fs_mii_bb_platform_info structure are left-overs from the move to the Phy Abstraction Layer subsystem. They can be safely removed. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- arch/powerpc/sysdev/fsl_soc.c |4 include/linux/fs_enet_pd.h|4 2 files changed, 0 insertions(+), 8 deletions(-) diff --git a/arch/powerpc/sysdev/fsl_soc.c b/arch/powerpc/sysdev/fsl_soc.c index 3ace747..cefc8b4 100644 --- a/arch/powerpc/sysdev/fsl_soc.c +++ b/arch/powerpc/sysdev/fsl_soc.c @@ -804,10 +804,6 @@ static int __init fs_enet_of_init(void) mdio_bb_prop[1]; fs_enet_mdio_bb_data.mdc_dat.bit = mdio_bb_prop[2]; - fs_enet_mdio_bb_data.mdio_port = - mdio_bb_prop[3]; - fs_enet_mdio_bb_data.mdc_port = - mdio_bb_prop[4]; fs_enet_mdio_bb_data.delay = mdio_bb_prop[5]; diff --git a/include/linux/fs_enet_pd.h b/include/linux/fs_enet_pd.h index 9bc045b..decc4b5 100644 --- a/include/linux/fs_enet_pd.h +++ b/include/linux/fs_enet_pd.h @@ -103,10 +103,6 @@ struct fs_mii_bb_platform_info { struct fs_mii_bit mdio_dir; struct fs_mii_bit mdio_dat; struct fs_mii_bit mdc_dat; - int mdio_port; /* port & bit for MDIO */ - int mdio_bit; - int mdc_port; /* port & bit for MDC */ - int mdc_bit; int delay; /* delay in us */ int irq[32];/* irqs per phy's */ }; -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpFrBpzfoZsN.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
How to describe FPGA-based devices in the device tree ?
Hi everybody, I'm (at last) trying to move from ARCH=ppc to ARCH=powerpc. After reading Documentation/powerpc/booting-without-of.txt and various device trees in arch/powerpc/boot/dts, I still don't know how to express some devices in the device tree. The target board has several devices on the processor local bus, as described in the following device tree fragment. [EMAIL PROTECTED] { compatible = "fsl,mpc8260-localbus", "fsl,pq2-localbus"; #address-cells = <2>; #size-cells = <1>; reg = ; ranges = <0 0 4000 0100 2 0 f200 0010 3 0 f300 0010 4 0 f400 0010>; [EMAIL PROTECTED],0 { compatible = "cfi-flash"; reg = <0 0 0100>; bank-width = <2>; }; [EMAIL PROTECTED],0 { compatible = "mtd-ram"; reg = <2 0 0010>; bank-width = <2>; }; [EMAIL PROTECTED],0 { device_type = "board-control"; reg = <3 0 0020>; }; [EMAIL PROTECTED],0 { reg = <4 0 0001>; }; }; The fourth device is a FPGA that contains several IP cores such as an interrupt controller and a SD/MMC host controller. If I understand things correctly, each IP core should have its own node in the device tree to allow proper binding with device drivers. As booting-without-of.txt describes the localbus node ranges as corresponding to a single chipselect and covering the entire chipselect access window, I can't have nodes for each IP core as children of the localbus node. Should I put IP core nodes as children of the FPGA node ? If so, how do I map addresses at the FPGA level ? A ranges property in the FPGA node would let me map addresses in the FPGA scope to the localbus scope. However, as the localbus scope use the chipselect number as its first address cell and 0 as its second address cell, I don't see how I could translate offsets in the FPGA into an address at the localbus scope. Could anyone advice me regarding how to properly describe my hardware in the device tree ? Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpFjgc9ybEq2.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
How to notify userspace of custom bus master/slave access mode changes ?
Hi everybody, I'm a bit unsure about how to notify userspace of changes to access mode to a custom bus. This is more an architecture issue than an implementation issue. My system has an ISA-like cold-pluggable extension bus. The bus controller sits on the CPU board which is plugged along with slave devices into a passive backplane. To add CPU redundancy support to the system, I implemented master and slave modes for the CPU board. The slave monitors the master and can disconnect it from the bus when a failure is detected. The bus drivers haven't been developed with hotplug in mind, but I eventually managed to fix them. The kernel is now able to scan the bus when a CPU board switches from slave to master mode, adding peripherals as they are detected, and to remove all peripherals when the CPU board switches from master to slave. I now have to notify userspace applications of master <-> slave mode changes. I thought about using kobject netlink notifications, but the kernel only generates events for individual peripherals (when they are added or removed). The bus driver registers a peripheral for the bus controller at boot time. I thought about registering the peripheral only when the system switches from slave to master, and deregistering it when it switches back to slave mode, but that's not very clean as the bus controller is always attached to the CPU, regardless of its bus access mode. Beside, I need to access the bus controller to detect master <-> slave transitions, so this would be a bit hackish. Is there a kobject netlink event I could use that I haven't thought about ? Am I missing a pseudo-peripheral in my implementation ? Is there a preferred way to notify userspace of such events ? Thanks for any help you can provide (and thanks for having read this mail throughout :-)). Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpP35yPlaXJH.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: CPM2 USB host driver
On Friday 30 November 2007 13:48, Anton Vorontsov wrote: > On Fri, Nov 30, 2007 at 01:30:18PM +0100, Laurent Pinchart wrote: > > On Friday 30 November 2007 12:16, Vitaly Bordug wrote: > > > On Fri, 30 Nov 2007 11:45:49 +0100 > > > > > > Laurent Pinchart wrote: > > > > Hi everybody, > > > > > > > > Linux USB host support for the CPM, CPM2 and CPM2 pro is far from > > > > complete. Many people showed interest on this list (and on > > > > linuxppc-embedded) in the past, but nobody managed to complete a > > > > driver and get it merged. > > > > > > that is mainly because of its semi-software nature. However, any > > > approach would be helpful I beleive. > > > > The CPM/CPM2 USB host controller does indeed put some pressure on the > > CPU. The PowerQuick III family is much better in that respect as its USB > > host controller is EHCI compliant. > > I didn't yet compare with CPM/CPM2, but I wonder if PQIIPro (MPC8360E) > USB controller is similar to cpms?.. If I'm not mistaken it is very similar to the CPM2 USB host controller, with the added ability to generate SOF packets automatically. The CPM has a packet-level USB host controller. CPM2 introduced a transaction-level mode (it still supports packet-level mode). CPM2 pro fixes the SOF packets generation issue that wakes the CPU once per millisecond with CPM and CPM2. Some members of the PQ2 and PQ2 pro families don't have a CPM but include a USB EHCI controller. I suppose those processors are less versatile (as the communication controllers are hardware-based instead of software-based) but offer more communication performance. If I'm not mistaken, the only processor to support USB in the PQ3 family is the MPC8555E. Its USB host controller is similar to the CPM2 as it supports both packet-level and transaction-level modes, but doesn't generate SOF packets automatically. I assume the PQ3 integrates a CPM2 and not a CPM2 pro. To summarize the issue (and I'd appreciate if someone could confirm this), a packet-level FHCI driver would work with CPM, CPM2 and CPM2 pro based processors. A transaction-level FHCI driver would work with the CPM2 and CPM2 pro based processors. For CPM2 pro based processors, SOF packet generation can be handled by the CPM. > I tried to forward-port FHCI from Freescale 2.6.11 kernels. Twice. > But these efforts always stumbled over more important tasks. Do you think I start from the FHCI driver provided by Freescale for 2.6.11, from the cpm2usb driver or from scratch ? Both the cpm2usb and FHCI drivers seem to use the packet-level interface. Is there any reason not to use the transaction-level interface ? Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpBZIv5FcGUi.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: CPM2 USB host driver
On Friday 30 November 2007 12:16, Vitaly Bordug wrote: > On Fri, 30 Nov 2007 11:45:49 +0100 > > Laurent Pinchart wrote: > > Hi everybody, > > > > Linux USB host support for the CPM, CPM2 and CPM2 pro is far from > > complete. Many people showed interest on this list (and on > > linuxppc-embedded) in the past, but nobody managed to complete a > > driver and get it merged. > > that is mainly because of its semi-software nature. However, any approach > would be helpful I beleive. The CPM/CPM2 USB host controller does indeed put some pressure on the CPU. The PowerQuick III family is much better in that respect as its USB host controller is EHCI compliant. We plan to use an external USB host controller when we will redesign the hardware. My goal in writting a CPM2 USB host driver is mainly to enable the hardware team to perform EMC testing on the USB interface. I don't expect it to be shipped to any client, but I'd still like to get it merged (if I complete the project) as I don't like throwing away useful code. > > As I need USB host support on my MPC8248 (CPM2), I decided to scratch > > the itch and try to get a working driver that could be merged > > upstream. This mail describes my plans to make sure the code I write > > will be useful for other people. > > > > The driver will be based on the cpm2usb project > > (http://cpm2usb.sf.net/). As I don't own any CPM1-based platform, I > > will convert the driver to use the CPM2 transaction-level interface, > > thus making it incompatible with the CPM1. CPM1 support could be > > added back later. There is no planned release date, and more urgent > > projects could put this development on hold. > > > > Comments are welcome (and contributions too, especially in the form > > of a working driver :-)). > > I may have some WIP material left, will try to dig it out... Should I wait ? Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgp4lyufMcDLS.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
CPM2 USB host driver
Hi everybody, Linux USB host support for the CPM, CPM2 and CPM2 pro is far from complete. Many people showed interest on this list (and on linuxppc-embedded) in the past, but nobody managed to complete a driver and get it merged. As I need USB host support on my MPC8248 (CPM2), I decided to scratch the itch and try to get a working driver that could be merged upstream. This mail describes my plans to make sure the code I write will be useful for other people. The driver will be based on the cpm2usb project (http://cpm2usb.sf.net/). As I don't own any CPM1-based platform, I will convert the driver to use the CPM2 transaction-level interface, thus making it incompatible with the CPM1. CPM1 support could be added back later. There is no planned release date, and more urgent projects could put this development on hold. Comments are welcome (and contributions too, especially in the form of a working driver :-)). Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpCVFj8EFahn.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
What's the preferred way to export board information to userspace ?
Hi everybody, it seems linuxppc-embedded is going away. I should have posted this here in the first place, so sorry for the cross-post. I need to export some read-only board-specific information (serial number, boot mode jumper configuration, ...) that are collected from various locations (CPLD, flash, U-Boot, ...) to userspace applications. Could anyone advice me on the preferred way to do that ? I can easily add a quick&dirty sysfs/procfs based implementation, but I was wondering if there was some kind of clean and generic way. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: What's the preferred way to export board information to userspace ?
Hi Ben, On Tuesday 02 October 2007 10:39, Benjamin Herrenschmidt wrote: > On Tue, 2007-10-02 at 10:10 +0200, Laurent Pinchart wrote: > > Hi everybody, > > > > it seems linuxppc-embedded is going away. I should have posted this here > > in the first place, so sorry for the cross-post. > > > > I need to export some read-only board-specific information (serial > > number, boot mode jumper configuration, ...) that are collected from > > various locations (CPLD, flash, U-Boot, ...) to userspace applications. > > > > Could anyone advice me on the preferred way to do that ? I can easily add > > a quick&dirty sysfs/procfs based implementation, but I was wondering if > > there was some kind of clean and generic way. > > Userspace can read /proc/device-tree no ? :-) Except I'm still using ARCH=ppc. I know I shouldn't :-) The MPC8272 isn't well supported in ARCH=powerpc. Scott Wood submitted some patches I need before switching. > Appart from that, it's common to stick that sort of thing > in /proc/cpuinfo... /sys/firmware may be an option if you have shitloads > of stuff .. It's not really CPU information, but I guess I can stick that in there. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [RFC] GPIO-based flow control in the cpm_uart driver
Hi David, On Tuesday 20 May 2008 02:25, David Brownell wrote: > On Tuesday 15 April 2008, Laurent Pinchart wrote: > > I'm implementing flow control and modem control lines support in the > > cpm_uart driver. > > > > The implementation is based on the GPIO lib. Modem control lines are > > described in the device tree as GPIO resources and accessed through the OF > > GPIO bindings. The I/O ports have to be initialized as GPIOs in the > > platform-specific code. > > I don't follow the "have to be" ... why couldn't the platform > setup code know that if a given UART is being used with hardware > handshaking, that means a particular pin mux config should be used? Flow control and modem control lines are two different beasts (although flow control uses a subset of the modem control lines). The UART driver needs to get and set the modem control lines state. This is easily handled by the GPIO library without requiring any change to GPIO lib. The only requirement, as stated above, is that platform-specific code initializes the corresponding pins as GPIOs. I'm not sure to understand what you don't get, as my point is that the platform setup code must configure the pins appropriately, exactly as you mention in your comment. > Are there maybe some on-chip routing options, so that for example > RTS2 could come out on any of three different balls? (In which > case that setup code could just be told *which* config to use...) That's possible yes, but that's not really an issue. Platform setup code is by definition platform-specific and has knowledge of the hardware, so it can setup pins correctly. > In this case I'm asking specifically about the normal all-works-ok > situation ... no errata or similar complications I mention below. > > > An option would be to export gpio_to_chip from drivers/gpio/gpiolib.c > > and use cpm1/2_set_pin in the cpm_uart driver. > > Help me to understand this a bit. UARTs are a fair example of places > where I've seen such pin reconfiguration be useful, but it's never > seemed to be generalizable except possibly as callbacks to SOC-specific > code (which don't imply updating generic programming interfaces). Callbacks worked fine on ppc but are now more difficult to implement in powerpc. This is why I'm looking for a generic solution based on the device tree. > I've seen examples where the hardware flow control normally works, so > that board setup sets up those pins in "hardware flow control" mode > when it's told the board has them wired up that way ... but then, some > chip revisions have errata forcing the driver to use a particular > port's handshake pins as GPIOs in some cases. (Obviously troublesome > except at low data rates, so one hopes the board designers just avoid > using those UARTs in that mode.) The issue is that flow control must be configurable. Userspace software can turn it on or off. To handle that on CPM2-based platforms, the pins connected to CTS and RTS must be switched between GPIO mode (no flow control) and dedicated function mode (hard flow control) at runtime in the UART driver. > I've also seen examples where the UART clock needs to be disabled in > deeper sleep states, but the system still wants the UART to be a wake > event source ... by having either the START bit, or maybe BREAK (it > gives a longer latch period), kick in a gpio-based pin change IRQ on > the UARTn.RX line. > > Now in *both* of those examples I've seen before, the solution could > not be generic. When the pin was used as GPIO, a particular pinmux > configuration was needed. (Bitfield X of register Y has value Z ... > or maybe a couple registers needed to change.) And when used as a > UART signal, a different one was used. (Same setup. Maybe value Z > became value W. Or again, maybe a few registers needed changing.) > > And for different chips in the family, sometimes even different > revisions of one chip, the W/X/Y/Z/etc values differed even for UART1. > For other UARTs, the same was true. > > So given that ... how would knowing the GPIO chip help, when I'd > still need to find out the W/X/Y/Z/etc values? And if I've got a > way to convey W/X/Y/Z/etc -- canonical example being a callback to > the relevant SOC-specific code -- why shouldn't that obviate the > need for any scheme to look up the gpio chip? On Freescale PowerPC hardware (specifically CPM2-based, but CPM1 and QE are quite similar), pins can either have a dedicated functionality or be used as a GPIO. This is handled through a group of registers, which are - the data register, used in GPIO mode only - the direction register, used in both GPIO and dedicated modes - the pin assignment
Re: [RFC] GPIO-based flow control in the cpm_uart driver
On Tuesday 20 May 2008 02:31, David Brownell wrote: > On Tuesday 15 April 2008, Laurent Pinchart wrote: > > Or maybe some kind of gpio_set_option() with flags specific to the > > controller ? This could be used to enable open-drain outputs or internal > > pull-ups for instance. > > That presumes that the pin config is associated with the GPIO > controller, rather than being an independent modules... > > In the cases that a platform has such an association, there > should be no problem just using a platform-specific code to > do the relevant magic. To call GPIO chip specific functions, the driver needs a way to get the GPIO chip associated with the GPIO. -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgp9VNmMUEwT7.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv2 1/2] [POWERPC] CPM2: Implement GPIO LIB API on CPM2 Freescale SoC.
On Friday 18 April 2008 19:16, Jochen Friedrich wrote: > Based on earlier work by Laurent Pinchart. > > This patch implement GPIO LIB support for the CPM2 GPIOs. > > Signed-off-by: Jochen Friedrich <[EMAIL PROTECTED]> > Cc: Laurent Pinchart <[EMAIL PROTECTED]> Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> Is there any showstopper or can this one be applied to powerpc-next ? > --- > arch/powerpc/platforms/Kconfig |2 + > arch/powerpc/sysdev/cpm2.c | 11 > arch/powerpc/sysdev/cpm_common.c | 123 > ++ > include/asm-powerpc/cpm.h|3 + > 4 files changed, 139 insertions(+), 0 deletions(-) > > Changes from version 1: > > - conditionally include #include in cpm_common.c, so it > compiles > when GPIO on 8xx is not selected. > > diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig > index f6eecd1..78911f6 100644 > --- a/arch/powerpc/platforms/Kconfig > +++ b/arch/powerpc/platforms/Kconfig > @@ -283,6 +283,8 @@ config CPM2 > depends on MPC85xx || 8260 > select CPM > select PPC_LIB_RHEAP > + select GENERIC_GPIO > + select HAVE_GPIO_LIB > help > The CPM2 (Communications Processor Module) is a coprocessor on > embedded CPUs made by Freescale. Selecting this option means that > diff --git a/arch/powerpc/sysdev/cpm2.c b/arch/powerpc/sysdev/cpm2.c > index 5a6c5df..9311778 100644 > --- a/arch/powerpc/sysdev/cpm2.c > +++ b/arch/powerpc/sysdev/cpm2.c > @@ -377,3 +377,14 @@ void cpm2_set_pin(int port, int pin, int flags) > else > clrbits32(&iop[port].odr, pin); > } > + > +static int cpm_init_par_io(void) > +{ > + struct device_node *np; > + > + for_each_compatible_node(np, NULL, "fsl,cpm2-pario-bank") > + cpm2_gpiochip_add32(np); > + return 0; > +} > +arch_initcall(cpm_init_par_io); > + > diff --git a/arch/powerpc/sysdev/cpm_common.c > b/arch/powerpc/sysdev/cpm_common.c > index cb7df2d..43bac6e 100644 > --- a/arch/powerpc/sysdev/cpm_common.c > +++ b/arch/powerpc/sysdev/cpm_common.c > @@ -19,6 +19,8 @@ > > #include > #include > +#include > +#include > > #include > #include > @@ -28,6 +30,10 @@ > > #include > > +#if defined(CONFIG_CPM2) || defined(CONFIG_8xx_GPIO) > +#include > +#endif > + > #ifdef CONFIG_PPC_EARLY_DEBUG_CPM > static u32 __iomem *cpm_udbg_txdesc = > (u32 __iomem __force *)CONFIG_PPC_EARLY_DEBUG_CPM_ADDR; > @@ -198,3 +204,120 @@ dma_addr_t cpm_muram_dma(void __iomem *addr) > return muram_pbase + ((u8 __iomem *)addr - muram_vbase); > } > EXPORT_SYMBOL(cpm_muram_dma); > + > +#if defined(CONFIG_CPM2) || defined(CONFIG_8xx_GPIO) > + > +struct cpm2_ioports { > + u32 dir, par, sor, odr, dat; > + u32 res[3]; > +}; > + > +struct cpm2_gpio32_chip { > + struct of_mm_gpio_chip mm_gc; > + spinlock_t lock; > + > + /* shadowed data register to clear/set bits safely */ > + u32 cpdata; > +}; > + > +static inline struct cpm2_gpio32_chip * > +to_cpm2_gpio32_chip(struct of_mm_gpio_chip *mm_gc) > +{ > + return container_of(mm_gc, struct cpm2_gpio32_chip, mm_gc); > +} > + > +static void cpm2_gpio32_save_regs(struct of_mm_gpio_chip *mm_gc) > +{ > + struct cpm2_gpio32_chip *cpm2_gc = to_cpm2_gpio32_chip(mm_gc); > + struct cpm2_ioports __iomem *iop = mm_gc->regs; > + > + cpm2_gc->cpdata = in_be32(&iop->dat); > +} > + > +static int cpm2_gpio32_get(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > + struct cpm2_ioports __iomem *iop = mm_gc->regs; > + u32 pin_mask; > + > + pin_mask = 1 << (31 - gpio); > + > + return !!(in_be32(&iop->dat) & pin_mask); > +} > + > +static void cpm2_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int > value) > +{ > + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > + struct cpm2_gpio32_chip *cpm2_gc = to_cpm2_gpio32_chip(mm_gc); > + struct cpm2_ioports __iomem *iop = mm_gc->regs; > + unsigned long flags; > + u32 pin_mask = 1 << (31 - gpio); > + > + spin_lock_irqsave(&cpm2_gc->lock, flags); > + > + if (value) > + cpm2_gc->cpdata |= pin_mask; > + else > + cpm2_gc->cpdata &= ~pin_mask; > + > + out_be32(&iop->dat, cpm2_gc->cpdata); > + > + spin_unlock_irqrestore(&cpm2_gc->lock, flags); > +} > + > +static int cpm2_gpio32_dir_out(struct gpio_chip
Re: [PATCHv2 1/2] [POWERPC] CPM2: Implement GPIO LIB API on CPM2 Freescale SoC.
On Friday 13 June 2008 16:57, Anton Vorontsov wrote: > On Fri, Jun 13, 2008 at 02:46:20PM +0200, Laurent Pinchart wrote: > > On Friday 18 April 2008 19:16, Jochen Friedrich wrote: > > > Based on earlier work by Laurent Pinchart. > > > > > > This patch implement GPIO LIB support for the CPM2 GPIOs. > > > > > > Signed-off-by: Jochen Friedrich <[EMAIL PROTECTED]> > > > Cc: Laurent Pinchart <[EMAIL PROTECTED]> > > > > Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> > > > > Is there any showstopper or can this one be applied to powerpc-next ? > > One comment below. > > [...] > > > + mm_gc->save_regs = cpm2_gpio32_save_regs; > > > + of_gc->gpio_cells = 1; > > I would strongly suggest to use gpio_cells = 2, otherwise you will not > able to pass GPIO flags (such as active-low etc) without breaking the > compatibility with older trees. Agreed. Jochen, will you resubmit or should I do it ? -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpd8xa3QyK7q.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: CPM2 mii-bitbang: Allowing mdio on port pins other than port C
Hi Mark, On Monday 16 June 2008 08:19, Mark Ware wrote: > Hello, > > I am preparing a board port (from 2.4.18!) for a proprietary board which > has it's mdio on a different port than mdc. The current mii-bitbang > driver in fs_enet assumes both pins are connected to port C. I have > created a fairly simple patch to make this more flexible, but I'm new to > device trees and am unsure how best to describe the situation in the > dts. > > The current mdio node for CPM2 looks something like: > > [EMAIL PROTECTED] { > device_type = "mdio"; > compatible = "fsl,cpm2-mdio-bitbang"; > #address-cells = <1>; > #size-cells = <0> > reg = <0x10d40 0x14>; > fsl,mdio-pin = <12>; > fsl,mdc-pin = <15>; > } > > I have made mdio work on our board by adding a second reg range and > using the first one for mdc and the second one for mdio: > > reg = <0x10d40 0x14 0x10d60 0x14>; // mdc=port D, mdio=port A > fsl,mdio-pin = <12>; // PD12 > fsl,mdc-pin = <15>; // PC15 > > The code remains backwards compatible, in that if only one reg range is > present it is used for both. > > Is this a valid (and acceptable) way to extend the reg property? It is. Sergej Stepanov submitted similar patches some times ago. You can find them at http://www.spinics.net/lists/netdev/msg45778.html http://www.spinics.net/lists/netdev/msg47159.html > Is their a cleaner way I should look at? Using the GPIO lib might be cleaner. Have a look at http://www.nabble.com/-PATCH-0-2--MDIO-on-GPIO-support-for-the-fs_enet-driver-ts17468958.html for a patch. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpyOuhpUGJ9F.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] fs_enet: MDIO on GPIO support
On Monday 26 May 2008 11:53, Laurent Pinchart wrote: > Port the fs_enet driver to support the MDIO on GPIO driver for PHY access > in addition to the mii-bitbang driver. Now that 1/2 has been applied by Jeff, could this one make it to powerpc-next ? > Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> > --- > drivers/net/fs_enet/fs_enet-main.c | 31 --- > 1 files changed, 20 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c > index 67b4b07..b0eb1d2 100644 > --- a/drivers/net/fs_enet/fs_enet-main.c > +++ b/drivers/net/fs_enet/fs_enet-main.c > @@ -43,6 +43,7 @@ > #include > > #ifdef CONFIG_PPC_CPM_NEW_BINDING > +#include > #include > #endif > > @@ -1172,8 +1173,7 @@ static int __devinit find_phy(struct device_node *np, >struct fs_platform_info *fpi) > { > struct device_node *phynode, *mdionode; > - struct resource res; > - int ret = 0, len; > + int ret = 0, len, bus_id; > const u32 *data; > > data = of_get_property(np, "fixed-link", NULL); > @@ -1190,19 +1190,28 @@ static int __devinit find_phy(struct device_node *np, > if (!phynode) > return -EINVAL; > > - mdionode = of_get_parent(phynode); > - if (!mdionode) > + data = of_get_property(phynode, "reg", &len); > + if (!data || len != 4) { > + ret = -EINVAL; > goto out_put_phy; > + } > > - ret = of_address_to_resource(mdionode, 0, &res); > - if (ret) > - goto out_put_mdio; > + mdionode = of_get_parent(phynode); > + if (!mdionode) { > + ret = -EINVAL; > + goto out_put_phy; > + } > > - data = of_get_property(phynode, "reg", &len); > - if (!data || len != 4) > - goto out_put_mdio; > + bus_id = of_get_gpio(mdionode, 0); > + if (bus_id < 0) { > + struct resource res; > + ret = of_address_to_resource(mdionode, 0, &res); > + if (ret) > + goto out_put_mdio; > + bus_id = res.start; > + } > > - snprintf(fpi->bus_id, 16, "%x:%02x", res.start, *data); > + snprintf(fpi->bus_id, 16, "%x:%02x", bus_id, *data); > > out_put_mdio: > of_node_put(mdionode); > -- > 1.5.0 > > -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpUol02feTbo.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: CPM2 mii-bitbang: Allowing mdio on port pins other than port C
Hi Mark, On Wednesday 18 June 2008 14:21, Mark Ware wrote: > Hi Laurent, > > Hi Mark, > > > > On Monday 16 June 2008 08:19, Mark Ware wrote: > > > Hello, > > > > > > I am preparing a board port (from 2.4.18!) for a proprietary board > > > which has it's mdio on a different port than mdc. The current > > > mii-bitbang driver in fs_enet assumes both pins are > > connected to port > > > C. I have created a fairly simple patch to make this more > > flexible, > > > but I'm new to device trees and am unsure how best to describe the > > > situation in the dts. > > > > > > The current mdio node for CPM2 looks something like: > > > > > > [EMAIL PROTECTED] { > > > device_type = "mdio"; > > > compatible = "fsl,cpm2-mdio-bitbang"; > > > #address-cells = <1>; > > > #size-cells = <0> > > > reg = <0x10d40 0x14>; > > > fsl,mdio-pin = <12>; > > > fsl,mdc-pin = <15>; > > > } > > > > > > I have made mdio work on our board by adding a second reg range and > > > using the first one for mdc and the second one for mdio: > > > > > > reg = <0x10d40 0x14 0x10d60 0x14>; // mdc=port D, mdio=port A > > > fsl,mdio-pin = <12>; // PD12 > > > fsl,mdc-pin = <15>; // PC15 > > > > > > The code remains backwards compatible, in that if only one > > reg range > > > is present it is used for both. > > > > > > Is this a valid (and acceptable) way to extend the reg property? > > > > It is. Sergej Stepanov submitted similar patches some times > > ago. You can find them at > > > > http://www.spinics.net/lists/netdev/msg45778.html > > http://www.spinics.net/lists/netdev/msg47159.html > > > > > Is their a cleaner way I should look at? > > > > Using the GPIO lib might be cleaner. Have a look at > > http://www.nabble.com/-PATCH-0-2--MDIO-on-GPIO-support-for-the > > -fs_enet-driver-ts17468958.html > > for a patch. > > > > Thanks for the links. It looks like I should have been searching in > netdev not powerpc for this. > > I will look at Sergej's patch and perhaps submit a merged version, but > with the GPIO version likely to be merged is there much point? The GPIO > lib method appears to be a more generic replacement. I don't see much reason to use the non-GPIO version (except maybe on very memory constrained platforms to avoid pulling the GPIO lib in the kernel, but that would help much). -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpeUaa1ahIs0.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] fs_enet: MDIO on GPIO support
Hi Scott, On Monday 16 June 2008 18:34, Scott Wood wrote: > On Mon, Jun 16, 2008 at 10:57:02AM +0200, Laurent Pinchart wrote: > > On Monday 26 May 2008 11:53, Laurent Pinchart wrote: > > > Port the fs_enet driver to support the MDIO on GPIO driver for PHY > > > access in addition to the mii-bitbang driver. > > > > Now that 1/2 has been applied by Jeff, could this one make it to > > powerpc-next ? > > This patch should probably go through Jeff as well... Jeff, what's your opinion on this ? > Acked-by: Scott Wood <[EMAIL PROTECTED]> > > > > - data = of_get_property(phynode, "reg", &len); > > > - if (!data || len != 4) > > > - goto out_put_mdio; > > > + bus_id = of_get_gpio(mdionode, 0); > > > + if (bus_id < 0) { > > > + struct resource res; > > > + ret = of_address_to_resource(mdionode, 0, &res); > > > + if (ret) > > > + goto out_put_mdio; > > > + bus_id = res.start; > > > + } > > > > > > - snprintf(fpi->bus_id, 16, "%x:%02x", res.start, *data); > > > + snprintf(fpi->bus_id, 16, "%x:%02x", bus_id, *data); > > It'd be nice if this sort of thing could be moved to phylib, so the latter > could simply be passed a device node (in addition to current mechanisms for > the benefit of unfortunate device-tree-less architectures). Adding a phy_connect_of that would move the above logic to phy_device.c shouldn't be too difficult. Would that be enough ? The mdio bus code would still use the bus id to identify phy devices in mdiobus_register. -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpoXpSImGemR.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] fs_enet: restore promiscuous and multicast settings in restart()
The restart() function is called when the link state changes and resets multicast and promiscous settings. This patch restores those settings at the end of restart(). Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- drivers/net/fs_enet/mac-fcc.c |3 +++ 2 files changed, 4 insertions(+), 1 deletions(-) diff --git a/drivers/net/fs_enet/mac-fcc.c b/drivers/net/fs_enet/mac-fcc.c index ce40cf9..1a95cf1 100644 --- a/drivers/net/fs_enet/mac-fcc.c +++ b/drivers/net/fs_enet/mac-fcc.c @@ -464,6 +464,9 @@ static void restart(struct net_device *dev) C32(fccp, fcc_fpsmr, FCC_PSMR_FDE | FCC_PSMR_LPB); S32(fccp, fcc_gfmr, FCC_GFMR_ENR | FCC_GFMR_ENT); + + /* Restore multicast and promiscous settings */ + set_multicast_list(dev); } static void stop(struct net_device *dev) -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpUJWEMOHz7B.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] fs_enet: MDIO on GPIO support
On Wednesday 18 June 2008 17:00, Jeff Garzik wrote: > Laurent Pinchart wrote: > > Hi Scott, > > > > On Monday 16 June 2008 18:34, Scott Wood wrote: > >> On Mon, Jun 16, 2008 at 10:57:02AM +0200, Laurent Pinchart wrote: > >>> On Monday 26 May 2008 11:53, Laurent Pinchart wrote: > >>>> Port the fs_enet driver to support the MDIO on GPIO driver for PHY > >>>> access in addition to the mii-bitbang driver. > >>> Now that 1/2 has been applied by Jeff, could this one make it to > >>> powerpc-next ? > >> This patch should probably go through Jeff as well... > > > > Jeff, what's your opinion on this ? > > > >> Acked-by: Scott Wood <[EMAIL PROTECTED]> > >> > >>>> -data = of_get_property(phynode, "reg", &len); > >>>> -if (!data || len != 4) > >>>> -goto out_put_mdio; > >>>> +bus_id = of_get_gpio(mdionode, 0); > >>>> +if (bus_id < 0) { > >>>> +struct resource res; > >>>> +ret = of_address_to_resource(mdionode, 0, &res); > >>>> +if (ret) > >>>> +goto out_put_mdio; > >>>> +bus_id = res.start; > >>>> +} > >>>> > >>>> -snprintf(fpi->bus_id, 16, "%x:%02x", res.start, *data); > >>>> +snprintf(fpi->bus_id, 16, "%x:%02x", bus_id, *data); > > What are the patch dependencies, if any? > > My general rule is, anytime I see 80%+ of the patch dealing with > arch-specific API functions (such as OF resource stuff), I tend to > prefer that goes via an arch tree. > > If it's a networking change, of course I'd prefer it came in my direction. The patch modifies the way the Freescale SoC fs_enet driver computes the PHY bus_id field when it connects to a PHY. The 'legacy' binding method was to use the MDIO general purpose I/O register address to identify the mii bus. My first patch (OpenFirmware GPIO based MDIO bitbang driver) introduces a new binding using the GPIO library. With this patch the mii bus is now identified by the GPIO lib I/O resource number if available and falls back to the register address when the device tree uses the legacy binding. There should be no dependencies. When the OF GPIO support is not selected linux/of_gpio.h will define of_get_gpio() as a stub, so the fs_enet driver will fall back to the legacy binding. -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpfpapob706J.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[RFC] cpm2: Rework baud rate generators configuration to support external clocks.
Hi everybody, I have to configure a CPM2 baud rate generator to use an external clock in a device driver. To avoid code duplication I'd like to reorganize the CPM2 baud rate configuration functions as proposed in the attached patch. Would this be acceptable ? -- The CPM2 BRG setup functions cpm_setbrg and cpm2_fastbrg don't support external clocks. This patch adds a new exported __cpm2_setbrg function that takes the clock rate and clock source as extra parameters, and moves cpm_setbrg and cpm2_fastbrg to include/asm-powerpc/cpm2.h where they become inline wrappers around __cpm2_setbrg. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- arch/powerpc/sysdev/cpm2.c | 34 +++ include/asm-powerpc/cpm2.h | 46 +++ 2 files changed, 37 insertions(+), 43 deletions(-) diff --git a/arch/powerpc/sysdev/cpm2.c b/arch/powerpc/sysdev/cpm2.c index 9311778..f1c3395 100644 --- a/arch/powerpc/sysdev/cpm2.c +++ b/arch/powerpc/sysdev/cpm2.c @@ -115,16 +115,10 @@ EXPORT_SYMBOL(cpm_command); * Baud rate clocks are zero-based in the driver code (as that maps * to port numbers). Documentation uses 1-based numbering. */ -#define BRG_INT_CLK(get_brgfreq()) -#define BRG_UART_CLK (BRG_INT_CLK/16) - -/* This function is used by UARTS, or anything else that uses a 16x - * oversampled clock. - */ -void -cpm_setbrg(uint brg, uint rate) +void __cpm2_setbrg(uint brg, uint rate, uint clk, int div16, int src) { u32 __iomem *bp; + u32 val; /* This is good enough to get SMCs running. */ @@ -135,34 +129,14 @@ cpm_setbrg(uint brg, uint rate) brg -= 4; } bp += brg; - out_be32(bp, (((BRG_UART_CLK / rate) - 1) << 1) | CPM_BRG_EN); - - cpm2_unmap(bp); -} - -/* This function is used to set high speed synchronous baud rate - * clocks. - */ -void -cpm2_fastbrg(uint brg, uint rate, int div16) -{ - u32 __iomem *bp; - u32 val; - - if (brg < 4) { - bp = cpm2_map_size(im_brgc1, 16); - } else { - bp = cpm2_map_size(im_brgc5, 16); - brg -= 4; - } - bp += brg; - val = ((BRG_INT_CLK / rate) << 1) | CPM_BRG_EN; + val = (((clk / rate) - 1) << 1) | CPM_BRG_EN | src; if (div16) val |= CPM_BRG_DIV16; out_be32(bp, val); cpm2_unmap(bp); } +EXPORT_SYMBOL(__cpm2_setbrg); int cpm2_clk_setup(enum cpm_clk_target target, int clock, int mode) { diff --git a/include/asm-powerpc/cpm2.h b/include/asm-powerpc/cpm2.h index 4c85ed9..dab9949 100644 --- a/include/asm-powerpc/cpm2.h +++ b/include/asm-powerpc/cpm2.h @@ -12,6 +12,7 @@ #include #include +#include #ifdef CONFIG_PPC_85xx #define CPM_MAP_ADDR (get_immrbase() + 0x8) @@ -119,10 +120,40 @@ extern void cpm_dpdump(void); extern void *cpm_dpram_addr(unsigned long offset); #endif -extern void cpm_setbrg(uint brg, uint rate); -extern void cpm2_fastbrg(uint brg, uint rate, int div16); extern void cpm2_reset(void); +/* Baud rate generators. +*/ +#define CPM_BRG_RST((uint)0x0002) +#define CPM_BRG_EN ((uint)0x0001) +#define CPM_BRG_EXTC_INT ((uint)0x) +#define CPM_BRG_EXTC_CLK3_9((uint)0x4000) +#define CPM_BRG_EXTC_CLK5_15 ((uint)0x8000) +#define CPM_BRG_ATB((uint)0x2000) +#define CPM_BRG_CD_MASK((uint)0x1ffe) +#define CPM_BRG_DIV16 ((uint)0x0001) + +#define CPM2_BRG_INT_CLK (get_brgfreq()) +#define CPM2_BRG_UART_CLK (CPM2_BRG_INT_CLK/16) + +extern void __cpm2_setbrg(uint brg, uint rate, uint clk, int div16, int src); + +/* This function is used by UARTS, or anything else that uses a 16x + * oversampled clock. + */ +static inline void cpm_setbrg(uint brg, uint rate) +{ + __cpm2_setbrg(brg, rate, CPM2_BRG_UART_CLK, 0, CPM_BRG_EXTC_INT); +} + +/* This function is used to set high speed synchronous baud rate + * clocks. + */ +static inline void cpm2_fastbrg(uint brg, uint rate, int div16) +{ + __cpm2_setbrg(brg, rate, CPM2_BRG_INT_CLK, div16, CPM_BRG_EXTC_INT); +} + /* Function code bits, usually generic to devices. */ #define CPMFCR_GBL ((u_char)0x20) /* Set memory snooping */ @@ -221,17 +252,6 @@ typedef struct smc_uart { #define SMCM_TX((unsigned char)0x02) #define SMCM_RX((unsigned char)0x01) -/* Baud rate generators. -*/ -#define CPM_BRG_RST((uint)0x0002) -#define CPM_BRG_EN ((uint)0x0001) -#define CPM_BRG_EXTC_INT ((uint)0x) -#define CPM_BRG_EXTC_CLK3_9((uint)0x4000) -#define CPM_BRG_EXTC_CLK5_15 ((uint)0x8000) -#define CPM_BRG_ATB((uint)0x2000) -#define CPM_BRG_CD_MASK((uint)0x1ffe) -#define CPM_BRG_DIV16 ((uint)0x0001) - /* SCCs. */ #define SCC_GSMRH_IRP ((uint
[PATCHv3 1/2] [POWERPC] CPM2: Implement GPIO LIB API on CPM2 Freescale SoC.
Based on earlier work by Jochen Friedrich. This patch implement GPIO LIB support for the CPM2 GPIOs. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> Cc: Jochen Friedrich <[EMAIL PROTECTED]> --- arch/powerpc/platforms/Kconfig |2 + arch/powerpc/sysdev/cpm2.c | 11 arch/powerpc/sysdev/cpm_common.c | 123 ++ include/asm-powerpc/cpm.h|3 + 4 files changed, 139 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig index 87454c5..7e67e26 100644 --- a/arch/powerpc/platforms/Kconfig +++ b/arch/powerpc/platforms/Kconfig @@ -280,6 +280,8 @@ config CPM2 depends on MPC85xx || 8260 select CPM select PPC_LIB_RHEAP + select GENERIC_GPIO + select HAVE_GPIO_LIB help The CPM2 (Communications Processor Module) is a coprocessor on embedded CPUs made by Freescale. Selecting this option means that diff --git a/arch/powerpc/sysdev/cpm2.c b/arch/powerpc/sysdev/cpm2.c index 5a6c5df..9311778 100644 --- a/arch/powerpc/sysdev/cpm2.c +++ b/arch/powerpc/sysdev/cpm2.c @@ -377,3 +377,14 @@ void cpm2_set_pin(int port, int pin, int flags) else clrbits32(&iop[port].odr, pin); } + +static int cpm_init_par_io(void) +{ + struct device_node *np; + + for_each_compatible_node(np, NULL, "fsl,cpm2-pario-bank") + cpm2_gpiochip_add32(np); + return 0; +} +arch_initcall(cpm_init_par_io); + diff --git a/arch/powerpc/sysdev/cpm_common.c b/arch/powerpc/sysdev/cpm_common.c index cb7df2d..b957a48 100644 --- a/arch/powerpc/sysdev/cpm_common.c +++ b/arch/powerpc/sysdev/cpm_common.c @@ -19,6 +19,8 @@ #include #include +#include +#include #include #include @@ -28,6 +30,10 @@ #include +#if defined(CONFIG_CPM2) || defined(CONFIG_8xx_GPIO) +#include +#endif + #ifdef CONFIG_PPC_EARLY_DEBUG_CPM static u32 __iomem *cpm_udbg_txdesc = (u32 __iomem __force *)CONFIG_PPC_EARLY_DEBUG_CPM_ADDR; @@ -198,3 +204,120 @@ dma_addr_t cpm_muram_dma(void __iomem *addr) return muram_pbase + ((u8 __iomem *)addr - muram_vbase); } EXPORT_SYMBOL(cpm_muram_dma); + +#if defined(CONFIG_CPM2) || defined(CONFIG_8xx_GPIO) + +struct cpm2_ioports { + u32 dir, par, sor, odr, dat; + u32 res[3]; +}; + +struct cpm2_gpio32_chip { + struct of_mm_gpio_chip mm_gc; + spinlock_t lock; + + /* shadowed data register to clear/set bits safely */ + u32 cpdata; +}; + +static inline struct cpm2_gpio32_chip * +to_cpm2_gpio32_chip(struct of_mm_gpio_chip *mm_gc) +{ + return container_of(mm_gc, struct cpm2_gpio32_chip, mm_gc); +} + +static void cpm2_gpio32_save_regs(struct of_mm_gpio_chip *mm_gc) +{ + struct cpm2_gpio32_chip *cpm2_gc = to_cpm2_gpio32_chip(mm_gc); + struct cpm2_ioports __iomem *iop = mm_gc->regs; + + cpm2_gc->cpdata = in_be32(&iop->dat); +} + +static int cpm2_gpio32_get(struct gpio_chip *gc, unsigned int gpio) +{ + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); + struct cpm2_ioports __iomem *iop = mm_gc->regs; + u32 pin_mask; + + pin_mask = 1 << (31 - gpio); + + return !!(in_be32(&iop->dat) & pin_mask); +} + +static void cpm2_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int value) +{ + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); + struct cpm2_gpio32_chip *cpm2_gc = to_cpm2_gpio32_chip(mm_gc); + struct cpm2_ioports __iomem *iop = mm_gc->regs; + unsigned long flags; + u32 pin_mask = 1 << (31 - gpio); + + spin_lock_irqsave(&cpm2_gc->lock, flags); + + if (value) + cpm2_gc->cpdata |= pin_mask; + else + cpm2_gc->cpdata &= ~pin_mask; + + out_be32(&iop->dat, cpm2_gc->cpdata); + + spin_unlock_irqrestore(&cpm2_gc->lock, flags); +} + +static int cpm2_gpio32_dir_out(struct gpio_chip *gc, unsigned int gpio, int val) +{ + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); + struct cpm2_ioports __iomem *iop = mm_gc->regs; + u32 pin_mask; + + pin_mask = 1 << (31 - gpio); + + setbits32(&iop->dir, pin_mask); + + cpm2_gpio32_set(gc, gpio, val); + + return 0; +} + +static int cpm2_gpio32_dir_in(struct gpio_chip *gc, unsigned int gpio) +{ + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); + struct cpm2_ioports __iomem *iop = mm_gc->regs; + u32 pin_mask; + + pin_mask = 1 << (31 - gpio); + + clrbits32(&iop->dir, pin_mask); + + return 0; +} + +int cpm2_gpiochip_add32(struct device_node *np) +{ + struct cpm2_gpio32_chip *cpm2_gc; + struct of_mm_gpio_chip *mm_gc; + struct of_gpio_chip *of_gc; + struct gpio_chip *gc; + + cpm2_gc = kzalloc(sizeof(*cpm2_gc), GFP_KERNEL); + if
Re: [PATCH] fs_enet: restore promiscuous and multicast settings in restart()
On Friday 20 June 2008 05:01, Bill Fink wrote: > On Wed, 18 Jun 2008, Laurent Pinchart wrote: > > > The restart() function is called when the link state changes and resets > > multicast and promiscous settings. This patch restores those settings at the > > end of restart(). > > > > Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> > > --- > > drivers/net/fs_enet/mac-fcc.c |3 +++ > > 2 files changed, 4 insertions(+), 1 deletions(-) > > Is the whole patch here? The above says 2 files changed and 5 lines > changed, but what's included here is only 1 file and 3 line changes. Yes it is. I've hand-removed a completely unrelated change that I had committed to my git tree in the same go (back when I wasn't very familiar with git). Do I need to resubmit ? > > diff --git a/drivers/net/fs_enet/mac-fcc.c b/drivers/net/fs_enet/mac-fcc.c > > index ce40cf9..1a95cf1 100644 > > --- a/drivers/net/fs_enet/mac-fcc.c > > +++ b/drivers/net/fs_enet/mac-fcc.c > > @@ -464,6 +464,9 @@ static void restart(struct net_device *dev) > > C32(fccp, fcc_fpsmr, FCC_PSMR_FDE | FCC_PSMR_LPB); > > > > S32(fccp, fcc_gfmr, FCC_GFMR_ENR | FCC_GFMR_ENT); > > + > > + /* Restore multicast and promiscous settings */ > > + set_multicast_list(dev); > > } > > > > static void stop(struct net_device *dev) -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgp1f94C0lLFr.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] fs_enet: restore promiscuous and multicast settings in restart()
Hi, On Friday 20 June 2008 10:55, Matvejchikov Ilya wrote: > >> > diff --git a/drivers/net/fs_enet/mac-fcc.c b/drivers/net/fs_enet/mac-fcc.c > >> > index ce40cf9..1a95cf1 100644 > >> > --- a/drivers/net/fs_enet/mac-fcc.c > >> > +++ b/drivers/net/fs_enet/mac-fcc.c > >> > @@ -464,6 +464,9 @@ static void restart(struct net_device *dev) > >> > C32(fccp, fcc_fpsmr, FCC_PSMR_FDE | FCC_PSMR_LPB); > >> > > >> > S32(fccp, fcc_gfmr, FCC_GFMR_ENR | FCC_GFMR_ENT); > >> > + > >> > + /* Restore multicast and promiscous settings */ > >> > + set_multicast_list(dev); > >> > } > > Is it right to call set_multicast_list() after turning on transmitter > and receiver? May be swap this lines around? I'm not sure if that will make a difference, but you're right, restoring the multicast and promiscuous settings is better done before turning the receiver and transmitter on. I'll send a new patch. -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpOS8CP0yMhx.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCHv2] fs_enet: restore promiscuous and multicast settings in restart()
The restart() function is called when the link state changes and resets multicast and promiscuous settings. This patch restores those settings at the end of restart(). Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- drivers/net/fs_enet/mac-fcc.c |3 +++ 1 files changed, 3 insertions(+), 0 deletions(-) diff --git a/drivers/net/fs_enet/mac-fcc.c b/drivers/net/fs_enet/mac-fcc.c index e363211..849afbe 100644 --- a/drivers/net/fs_enet/mac-fcc.c +++ b/drivers/net/fs_enet/mac-fcc.c @@ -463,6 +463,9 @@ static void restart(struct net_device *dev) else C32(fccp, fcc_fpsmr, FCC_PSMR_FDE | FCC_PSMR_LPB); + /* Restore multicast and promiscuous settings */ + set_multicast_list(dev); + S32(fccp, fcc_gfmr, FCC_GFMR_ENR | FCC_GFMR_ENT); } -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpDHy8kCNxT5.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv3 1/2] [POWERPC] CPM2: Implement GPIO LIB API on CPM2 Freescale SoC.
On Wednesday 18 June 2008 19:08, Laurent Pinchart wrote: > Based on earlier work by Jochen Friedrich. > > This patch implement GPIO LIB support for the CPM2 GPIOs. Is there any pending issue or can this be applied to powerpc-next ? > Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> > Cc: Jochen Friedrich <[EMAIL PROTECTED]> > --- > arch/powerpc/platforms/Kconfig |2 + > arch/powerpc/sysdev/cpm2.c | 11 > arch/powerpc/sysdev/cpm_common.c | 123 > ++ > include/asm-powerpc/cpm.h|3 + > 4 files changed, 139 insertions(+), 0 deletions(-) > > diff --git a/arch/powerpc/platforms/Kconfig b/arch/powerpc/platforms/Kconfig > index 87454c5..7e67e26 100644 > --- a/arch/powerpc/platforms/Kconfig > +++ b/arch/powerpc/platforms/Kconfig > @@ -280,6 +280,8 @@ config CPM2 > depends on MPC85xx || 8260 > select CPM > select PPC_LIB_RHEAP > + select GENERIC_GPIO > + select HAVE_GPIO_LIB > help > The CPM2 (Communications Processor Module) is a coprocessor on > embedded CPUs made by Freescale. Selecting this option means that > diff --git a/arch/powerpc/sysdev/cpm2.c b/arch/powerpc/sysdev/cpm2.c > index 5a6c5df..9311778 100644 > --- a/arch/powerpc/sysdev/cpm2.c > +++ b/arch/powerpc/sysdev/cpm2.c > @@ -377,3 +377,14 @@ void cpm2_set_pin(int port, int pin, int flags) > else > clrbits32(&iop[port].odr, pin); > } > + > +static int cpm_init_par_io(void) > +{ > + struct device_node *np; > + > + for_each_compatible_node(np, NULL, "fsl,cpm2-pario-bank") > + cpm2_gpiochip_add32(np); > + return 0; > +} > +arch_initcall(cpm_init_par_io); > + > diff --git a/arch/powerpc/sysdev/cpm_common.c > b/arch/powerpc/sysdev/cpm_common.c > index cb7df2d..b957a48 100644 > --- a/arch/powerpc/sysdev/cpm_common.c > +++ b/arch/powerpc/sysdev/cpm_common.c > @@ -19,6 +19,8 @@ > > #include > #include > +#include > +#include > > #include > #include > @@ -28,6 +30,10 @@ > > #include > > +#if defined(CONFIG_CPM2) || defined(CONFIG_8xx_GPIO) > +#include > +#endif > + > #ifdef CONFIG_PPC_EARLY_DEBUG_CPM > static u32 __iomem *cpm_udbg_txdesc = > (u32 __iomem __force *)CONFIG_PPC_EARLY_DEBUG_CPM_ADDR; > @@ -198,3 +204,120 @@ dma_addr_t cpm_muram_dma(void __iomem *addr) > return muram_pbase + ((u8 __iomem *)addr - muram_vbase); > } > EXPORT_SYMBOL(cpm_muram_dma); > + > +#if defined(CONFIG_CPM2) || defined(CONFIG_8xx_GPIO) > + > +struct cpm2_ioports { > + u32 dir, par, sor, odr, dat; > + u32 res[3]; > +}; > + > +struct cpm2_gpio32_chip { > + struct of_mm_gpio_chip mm_gc; > + spinlock_t lock; > + > + /* shadowed data register to clear/set bits safely */ > + u32 cpdata; > +}; > + > +static inline struct cpm2_gpio32_chip * > +to_cpm2_gpio32_chip(struct of_mm_gpio_chip *mm_gc) > +{ > + return container_of(mm_gc, struct cpm2_gpio32_chip, mm_gc); > +} > + > +static void cpm2_gpio32_save_regs(struct of_mm_gpio_chip *mm_gc) > +{ > + struct cpm2_gpio32_chip *cpm2_gc = to_cpm2_gpio32_chip(mm_gc); > + struct cpm2_ioports __iomem *iop = mm_gc->regs; > + > + cpm2_gc->cpdata = in_be32(&iop->dat); > +} > + > +static int cpm2_gpio32_get(struct gpio_chip *gc, unsigned int gpio) > +{ > + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > + struct cpm2_ioports __iomem *iop = mm_gc->regs; > + u32 pin_mask; > + > + pin_mask = 1 << (31 - gpio); > + > + return !!(in_be32(&iop->dat) & pin_mask); > +} > + > +static void cpm2_gpio32_set(struct gpio_chip *gc, unsigned int gpio, int > value) > +{ > + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > + struct cpm2_gpio32_chip *cpm2_gc = to_cpm2_gpio32_chip(mm_gc); > + struct cpm2_ioports __iomem *iop = mm_gc->regs; > + unsigned long flags; > + u32 pin_mask = 1 << (31 - gpio); > + > + spin_lock_irqsave(&cpm2_gc->lock, flags); > + > + if (value) > + cpm2_gc->cpdata |= pin_mask; > + else > + cpm2_gc->cpdata &= ~pin_mask; > + > + out_be32(&iop->dat, cpm2_gc->cpdata); > + > + spin_unlock_irqrestore(&cpm2_gc->lock, flags); > +} > + > +static int cpm2_gpio32_dir_out(struct gpio_chip *gc, unsigned int gpio, int > val) > +{ > + struct of_mm_gpio_chip *mm_gc = to_of_mm_gpio_chip(gc); > + struct cpm2_ioports __iomem *iop = mm_gc->regs; > + u32 pin_mask; > + > +
Re: [PATCH] powerpc: Modem control lines support for the cpm_uart driver
On Monday 26 May 2008 11:58, Laurent Pinchart wrote: > On Wednesday 16 April 2008 11:10, Laurent Pinchart wrote: > > This patch replaces the get_mctrl/set_mctrl stubs with modem control line > > read/write access through the GPIO lib. > > > > Available modem control lines are described in the device tree using GPIO > > bindings. > > Any show stopper on this patch ? Could it get into powerpc-next ? Sorry to bother everybody again, but I'd like this patch to go in 2.6.27. Is there any pending issue ? Platforms without GPIO support shouldn't be affected and get_mctrl()/set_mctrl() behaviour is backward compatible for existing platforms. > > Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> > > --- > > Documentation/powerpc/booting-without-of.txt | 10 ++ > > drivers/serial/cpm_uart/cpm_uart.h | 10 ++ > > drivers/serial/cpm_uart/cpm_uart_core.c | 40 -- > > 3 files changed, 57 insertions(+), 3 deletions(-) > > > > diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt > > index 1e5572a..012f231 100644 > > --- a/Documentation/powerpc/booting-without-of.txt > > +++ b/Documentation/powerpc/booting-without-of.txt > > @@ -1976,6 +1976,14 @@ platforms are moved over to use the flattened-device-tree model. > > - fsl,cpm2-scc-uart > > - fsl,qe-uart > > > > + Modem control lines connected to GPIO controllers are listed in the > > + gpios property as described in section VIII.1 in the following order: > > + > > + CTS, RTS, DCD, DSR, DTR, and RI. > > + > > + The gpios property is optional and can be left out when control lines are > > + not used. > > + > > Example: > > > > [EMAIL PROTECTED] { > > @@ -1987,6 +1995,8 @@ platforms are moved over to use the flattened-device-tree model. > > interrupt-parent = <&PIC>; > > fsl,cpm-brg = <1>; > > fsl,cpm-command = <0080>; > > + gpios = <&gpio_c 15 0 > > +&gpio_d 29 0>; > > }; > > > > iii) Network > > diff --git a/drivers/serial/cpm_uart/cpm_uart.h b/drivers/serial/cpm_uart/cpm_uart.h > > index 0cc39f8..d0c55e2 100644 > > --- a/drivers/serial/cpm_uart/cpm_uart.h > > +++ b/drivers/serial/cpm_uart/cpm_uart.h > > @@ -50,6 +50,15 @@ > > > > #define SCC_WAIT_CLOSING 100 > > > > +#define GPIO_CTS 0 > > +#define GPIO_RTS 1 > > +#define GPIO_DCD 2 > > +#define GPIO_DSR 3 > > +#define GPIO_DTR 4 > > +#define GPIO_RI5 > > + > > +#define NUM_GPIOS (GPIO_RI+1) > > + > > struct uart_cpm_port { > > struct uart_portport; > > u16 rx_nrfifos; > > @@ -82,6 +91,7 @@ struct uart_cpm_port { > > int wait_closing; > > /* value to combine with opcode to form cpm command */ > > u32 command; > > + int gpios[NUM_GPIOS]; > > }; > > > > #ifndef CONFIG_PPC_CPM_NEW_BINDING > > diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c b/drivers/serial/cpm_uart/cpm_uart_core.c > > index 7a704ff..0f08071 100644 > > --- a/drivers/serial/cpm_uart/cpm_uart_core.c > > +++ b/drivers/serial/cpm_uart/cpm_uart_core.c > > @@ -42,6 +42,8 @@ > > #include > > #include > > #include > > +#include > > +#include > > > > #include > > #include > > @@ -152,13 +154,41 @@ static unsigned int cpm_uart_tx_empty(struct uart_port *port) > > > > static void cpm_uart_set_mctrl(struct uart_port *port, unsigned int mctrl) > > { > > - /* Whee. Do nothing. */ > > + struct uart_cpm_port *pinfo = (struct uart_cpm_port *)port; > > + > > + if (pinfo->gpios[GPIO_RTS] >= 0) > > + gpio_set_value(pinfo->gpios[GPIO_RTS], !(mctrl & TIOCM_RTS)); > > + > > + if (pinfo->gpios[GPIO_DTR] >= 0) > > + gpio_set_value(pinfo->gpios[GPIO_DTR], !(mctrl & TIOCM_DTR)); > > } > > > > static unsigned int cpm_uart_get_mctrl(struct uart_port *port) > > { > > - /* Whee. Do nothing. */ > > - return TIOCM_CAR | TIOCM_DSR | TIOCM_CTS; > > + struct uart_cpm_port *pinfo = (struct uart_cpm_port *)port; > > + unsigned int mctrl = TIOCM_CTS | TIOCM_DSR | TIOCM_CAR; > > + > > + if (pinfo->gpios[GPIO_CTS] >= 0) { > > + if (gpio_get_value(pinfo
Re: [PATCH 2/2] fs_enet: MDIO on GPIO support
Jeff, Scott, On Wednesday 18 June 2008 17:16, Laurent Pinchart wrote: > On Wednesday 18 June 2008 17:00, Jeff Garzik wrote: > > Laurent Pinchart wrote: > > > Hi Scott, > > > > > > On Monday 16 June 2008 18:34, Scott Wood wrote: > > >> On Mon, Jun 16, 2008 at 10:57:02AM +0200, Laurent Pinchart wrote: > > >>> On Monday 26 May 2008 11:53, Laurent Pinchart wrote: > > >>>> Port the fs_enet driver to support the MDIO on GPIO driver for PHY > > >>>> access in addition to the mii-bitbang driver. > > >>> Now that 1/2 has been applied by Jeff, could this one make it to > > >>> powerpc-next ? > > >> This patch should probably go through Jeff as well... > > > > > > Jeff, what's your opinion on this ? > > > > > >> Acked-by: Scott Wood <[EMAIL PROTECTED]> > > >> > > >>>> - data = of_get_property(phynode, "reg", &len); > > >>>> - if (!data || len != 4) > > >>>> - goto out_put_mdio; > > >>>> + bus_id = of_get_gpio(mdionode, 0); > > >>>> + if (bus_id < 0) { > > >>>> + struct resource res; > > >>>> + ret = of_address_to_resource(mdionode, 0, &res); > > >>>> + if (ret) > > >>>> + goto out_put_mdio; > > >>>> + bus_id = res.start; > > >>>> + } > > >>>> > > >>>> - snprintf(fpi->bus_id, 16, "%x:%02x", res.start, *data); > > >>>> + snprintf(fpi->bus_id, 16, "%x:%02x", bus_id, *data); > > > > What are the patch dependencies, if any? > > > > My general rule is, anytime I see 80%+ of the patch dealing with > > arch-specific API functions (such as OF resource stuff), I tend to > > prefer that goes via an arch tree. > > > > If it's a networking change, of course I'd prefer it came in my direction. > > The patch modifies the way the Freescale SoC fs_enet driver computes the PHY > bus_id field when it connects to a PHY. > > The 'legacy' binding method was to use the MDIO general purpose I/O register > address to identify the mii bus. My first patch (OpenFirmware GPIO based MDIO > bitbang driver) introduces a new binding using the GPIO library. > > With this patch the mii bus is now identified by the GPIO lib I/O resource > number if available and falls back to the register address when the device > tree uses the legacy binding. > > There should be no dependencies. When the OF GPIO support is not selected > linux/of_gpio.h will define of_get_gpio() as a stub, so the fs_enet driver > will fall back to the legacy binding. Have we reached a consensus on which tree the patch should go to ? -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpLVrvJJ7FCw.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[RFC] cpm_uart: Fix break generation
When generating a break condition on a serial port, the CPM must be told beforehand how long the break should be. Unfortunately, this information is not available through the current serial break handling API. This patch works around the problem by requesting a 32767 characters break followed by a 0 characters break after the requested duration. The CPM will stop the first break when the second one is requested. This might not work with future CPM revisions. An alternative would be to change the serial break handling API to pass the break duration down to the serial drivers. --- drivers/serial/cpm_uart/cpm_uart_core.c |9 + 1 files changed, 9 insertions(+), 0 deletions(-) diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c b/drivers/serial/cpm_uart/cpm_uart_core.c index c29d87d..aa0a284 100644 --- a/drivers/serial/cpm_uart/cpm_uart_core.c +++ b/drivers/serial/cpm_uart/cpm_uart_core.c @@ -268,14 +268,23 @@ static void cpm_uart_break_ctl(struct uart_port *port, int break_state) static void cpm_uart_break_ctl(struct uart_port *port, int break_state) { struct uart_cpm_port *pinfo = (struct uart_cpm_port *)port; + volatile u16 *brkcr = IS_SMC(pinfo) ? &pinfo->smcup->smc_brkcr + : &pinfo->sccup->scc_brkcr; pr_debug("CPM uart[%d]:break ctrl, break_state: %d\n", port->line, break_state); if (break_state) + { + *brkcr = 32767; cpm_line_cr_cmd(pinfo, CPM_CR_STOP_TX); + } else + { + *brkcr = 0; + cpm_line_cr_cmd(pinfo, CPM_CR_STOP_TX); cpm_line_cr_cmd(pinfo, CPM_CR_RESTART_TX); + } } /* -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgp47x6yQB2zG.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] cpm_uart: Support uart_wait_until_sent()
Set port->fifosize to the software FIFO size, and update the port timeout when the baud rate is modified. SCC ports have an optional 32 byte hardware FIFO which is currently not taken into account, as there is no documented way to check when the FIFO becomes empty. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- drivers/serial/cpm_uart/cpm_uart_core.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c b/drivers/serial/cpm_uart/cpm_uart_core.c index a19dc7e..151cad2 100644 --- a/drivers/serial/cpm_uart/cpm_uart_core.c +++ b/drivers/serial/cpm_uart/cpm_uart_core.c @@ -547,6 +547,11 @@ static void cpm_uart_set_termios(struct uart_port *port, } /* +* Update the timeout +*/ + uart_update_timeout(port, termios->c_cflag, baud); + + /* * Set up parity check flag */ #define RELEVANT_IFLAG(iflag) (iflag & (IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK)) @@ -1154,6 +1159,7 @@ int cpm_uart_drv_get_platform_data(struct platform_device *pdev, int is_con) pinfo->port.uartclk = pdata->uart_clk; pinfo->port.mapbase = (unsigned long)mem; pinfo->port.irq = platform_get_irq(pdev, 0); + pinfo->port.fifosize = pinfo->tx_nrfifos * pinfo->tx_fifosize; return 0; } -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpIiAloV7v6o.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Commits added to powerpc.git master and powerpc-next branches
Hi Paul, On Tuesday 01 July 2008, Paul Mackerras wrote: > The following commits have been added to the master and powerpc-next > branches of the powerpc.git repository. This includes patches pulled > from Kumar's and Josh's trees. Any change of getting the following patches into 2.6.27 ? They have either been acked and not picked, or just ignored so far :-/ [PATCHv2] fs_enet: restore promiscuous and multicast settings in restart() http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=19228 [PATCH 2/2] fs_enet: MDIO on GPIO support http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=18693 [PATCHv3 1/2] [POWERPC] CPM2: Implement GPIO LIB API on CPM2 Freescale SoC. http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=19045 [PATCH] powerpc: Modem control lines support for the cpm_uart driver http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=17928 [PATCH] cpm_uart: Support uart_wait_until_sent() http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=19233 Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 signature.asc Description: This is a digitally signed message part. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: patches for 2.6.27...
Hi Kumar, On Wednesday 02 July 2008, Kumar Gala wrote: > Please point out any patches that have been posted but havent made it > into a git tree related to Freescale chips. > > I know there are probably a slew of CPM patches that need to get into > the tree. Here are 5 patches that I'd like to see in 2.6.27. They haven't made into any git tree as far as I know. [PATCHv2] fs_enet: restore promiscuous and multicast settings in restart() http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=19228 [PATCH 2/2] fs_enet: MDIO on GPIO support http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=18693 [PATCHv3 1/2] [POWERPC] CPM2: Implement GPIO LIB API on CPM2 Freescale SoC. http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=19045 [PATCH] powerpc: Modem control lines support for the cpm_uart driver http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=17928 [PATCH] cpm_uart: Support uart_wait_until_sent() http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=19233 -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 signature.asc Description: This is a digitally signed message part. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Commits added to powerpc.git master and powerpc-next branches
On Wednesday 02 July 2008, Kumar Gala wrote: > > On Jul 1, 2008, at 3:49 AM, Laurent Pinchart wrote: > > > Hi Paul, > > > > On Tuesday 01 July 2008, Paul Mackerras wrote: > >> The following commits have been added to the master and powerpc-next > >> branches of the powerpc.git repository. This includes patches pulled > >> from Kumar's and Josh's trees. > > > > Any change of getting the following patches into 2.6.27 ? They have > > either been acked and not picked, or just ignored so far :-/ > > > > [PATCHv2] fs_enet: restore promiscuous and multicast settings in > > restart() > > http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=19228 > > Should go via netdev unless jgarizk says otherwise On 2008-06-18, Jeff wrote: "My general rule is, anytime I see 80%+ of the patch dealing with arch-specific API functions (such as OF resource stuff), I tend to prefer that goes via an arch tree. If it's a networking change, of course I'd prefer it came in my direction." Jeff, could you please tell us if you want to pick this one or if it should go through the powerpc tree ? The patch has been posted to both the linuxppc-dev and netdev lists, and I haven't received any comment from the netdev folks. > > [PATCH 2/2] fs_enet: MDIO on GPIO support > > http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=18693 > > ditto Jeff, same question :-) > > [PATCHv3 1/2] [POWERPC] CPM2: Implement GPIO LIB API on CPM2 > > Freescale SoC. > > http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=19045 > > I'll look at this one Thanks. > > [PATCH] powerpc: Modem control lines support for the cpm_uart driver > > http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=17928 > > this seems to depend on the previous patch. It doesn't strictly depend on the previous one. You will need GPIO LIB API on CPM2 to use the modem control lines, but the cpm_uart driver will fall back to the current behaviour if no GPIO LIB support is present. > > [PATCH] cpm_uart: Support uart_wait_until_sent() > > http://patchwork.ozlabs.org/linuxppc/patch?person=968&id=19233 > > > applied -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 signature.asc Description: This is a digitally signed message part. ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCHv2] cpm_uart: Support uart_wait_until_sent()
Set port->fifosize to the software FIFO size, and update the port timeout when the baud rate is modified. SCC ports have an optional 32 byte hardware FIFO which is currently not taken into account, as there is no documented way to check when the FIFO becomes empty. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- drivers/serial/cpm_uart/cpm_uart_core.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c b/drivers/serial/cpm_uart/cpm_uart_core.c index a19dc7e..151cad2 100644 --- a/drivers/serial/cpm_uart/cpm_uart_core.c +++ b/drivers/serial/cpm_uart/cpm_uart_core.c @@ -608,6 +608,11 @@ static void cpm_uart_set_termios(struct uart_port *port, } /* +* Update the timeout +*/ + uart_update_timeout(port, termios->c_cflag, baud); + + /* * Set up parity check flag */ #define RELEVANT_IFLAG(iflag) (iflag & (IGNBRK|BRKINT|IGNPAR|PARMRK|INPCK)) @@ -1068,6 +1073,7 @@ int cpm_uart_drv_get_platform_data(struct platform_device *pdev, int is_con) pinfo->port.type = PORT_CPM; pinfo->port.ops = &cpm_uart_pops, pinfo->port.iotype = UPIO_MEM; + pinfo->port.fifosize = pinfo->tx_nrfifos * pinfo->tx_fifosize; spin_lock_init(&pinfo->port.lock); pinfo->port.irq = of_irq_to_resource(np, 0, NULL); -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussee de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] fs_enet: Remove unused fields in the fs_mii_bb_platform_info structure.
On Friday 15 February 2008 14:27, Laurent Pinchart wrote: > The mdio_port, mdio_bit, mdc_port and mdc_bit fields in the > fs_mii_bb_platform_info structure are left-overs from the move to the Phy > Abstraction Layer subsystem. They can be safely removed. > > Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> [snip] Is there something wrong with the patch ? -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpU6SQlmwjoo.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] fs_enet: Don't call NAPI functions when NAPI is not used.
fs_enet_close() calls napi_disable() unconditionally. This patch skips the call when use_napi isn't set. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- drivers/net/fs_enet/fs_enet-main.c |3 ++- 1 files changed, 2 insertions(+), 1 deletions(-) diff --git a/drivers/net/fs_enet/fs_enet-main.c b/drivers/net/fs_enet/fs_enet-main.c index c83bd65..1801ce3 100644 --- a/drivers/net/fs_enet/fs_enet-main.c +++ b/drivers/net/fs_enet/fs_enet-main.c @@ -835,7 +835,8 @@ static int fs_enet_close(struct net_device *dev) netif_stop_queue(dev); netif_carrier_off(dev); - napi_disable(&fep->napi); + if (fep->fpi->use_napi) + napi_disable(&fep->napi); phy_stop(fep->phydev); spin_lock_irqsave(&fep->lock, flags); -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpOmGeeqvzf6.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Trouble with SCC UART ports when moving from ppc to powerpc
Hi everybody, I'm trying to move from ARCH=ppc to ARCH=powerpc on an MPC8272 based board. I updated my bootloader (U-Boot) to get FDT support, wrote a device tree and compiled a powerpc kernel with CONFIG_PPC_CPM_NEW_BINDING set. No problem so far (well, no problem I haven't been able to solve). I then tried to get the serial console on SCC1 to work. The serial port was silent, and the kernel hanged in cpm_uart_console_write while waiting for the CPM to clear the ready bit in tx buffer descriptors. After checking the SCC1 configuration registers, parameter RAM and buffer descriptors, I found out that something was overwriting the buffer descriptors were stored in the DPRAM at offset 0. Right after initializing the rx buffer descriptors, dumping the rx bds dpram with the BDI2000 gave me 9088 003518e0 9008 00351900 9000 00351911 b540 2dace564 while I was expecting 9088 003518e0 9008 00351900 9000 00351920 b000 00351940 Some data was clearly being overwritten by something. The CPM dual port ram was defined in the device tree as follows (copied from the MPC8272ADS board device tree). [EMAIL PROTECTED] { #address-cells = <1>; #size-cells = <1>; ranges = <0 0 1>; [EMAIL PROTECTED] { compatible = "fsl,cpm-muram-data"; reg = <0 2000 9800 800>; }; }; Changing the reg property to reg = <80 1f80 9800 800>; fixed my problem. Does anyone have any clue regarding what could write to the dpram ? I thought about some CPM peripheral set up by the boot loader, but my board initialization code calls cpm2_reset() long before initializing SCC1. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpTHo7CzOAd3.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 6/9] fs_enet: Be an of_platform device when CONFIG_PPC_CPM_NEW_BINDING is set.
Hi Sergej, On Friday 15 February 2008 16:38, Sergej Stepanov wrote: > Am Freitag, den 15.02.2008, 13:50 +0100 schrieb Laurent Pinchart: > > We're loosing the possibility of having MDC and MDIO on different ports. > > This is quite easy to fix for the non-CONFIG_PPC_CPM_NEW_BINDING case but > > I'm not familiar with OF bindings (yet) to fix the > > CONFIG_PPC_CPM_NEW_BINDING case. > > for OF issue i had this for a paar month: > http://www.spinics.net/lists/netdev/msg45778.html > http://www.spinics.net/lists/netdev/msg47159.html > i'll be glad if it helps... Could these be applied ? Any showstopper ? Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgp5yA0gAdi94.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 6/9] fs_enet: Be an of_platform device when CONFIG_PPC_CPM_NEW_BINDING is set.
On Monday 10 March 2008 08:50, Sergej Stepanov wrote: > > Acked-by: Scott Wood <[EMAIL PROTECTED]>, if I haven't already. > > > > -Scott > > I thought, you did it. > Thank you. Your documentation patch states that "The first reg resource is the I/O port register block on which MDIO resides. The second reg resource is the I/O port register block on which MDC resides. If there is only one reg resource, it is used for both MDIO and MDC." If I'm not mistaken the code uses the first reg resource for MDC and the second reg resource for MDIO. Which one should be fixed, the doc or the code ? Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpYSU5l95b8u.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Trouble with SCC UART ports when moving from ppc to powerpc
On Friday 07 March 2008 17:21, Scott Wood wrote: > On Fri, Mar 07, 2008 at 03:20:55PM +0100, Laurent Pinchart wrote: > > The CPM dual port ram was defined in the device tree as follows (copied > > from the MPC8272ADS board device tree). > > > > [EMAIL PROTECTED] { > > #address-cells = <1>; > > #size-cells = <1>; > > ranges = <0 0 1>; > > > > [EMAIL PROTECTED] { > > compatible = "fsl,cpm-muram-data"; > > reg = <0 2000 9800 800>; > > }; > > }; > > > > Changing the reg property to > > > > reg = <80 1f80 9800 800>; > > > > fixed my problem. > > Perhaps there's an SMC1 running with its parameter RAM at offset zero? Good guess. U-Boot configured SMC1 with its parameter RAM at offset 0. > > Does anyone have any clue regarding what could write to the dpram ? I > > thought about some CPM peripheral set up by the boot loader, but my board > > initialization code calls cpm2_reset() long before initializing SCC1. > > cpm2_reset() doesn't currently actually reset the CPM, for some reason > (unlike cpm1). This should probably be fixed, though then we'd have to > deal with assigning SMC parameter RAM addresses ourselves. I had overlooked that. Resetting the CPM in cpm2_reset() helps. Is there any reason not to rset the CPM in cpm2_reset() ? How should SMC parameter RAM assignment be handled ? I'm not very familiar with the CPM1, but for CPM2 the cpm_uart driver could easily cpm_dpalloc() parameter RAM. > For now, the above fix should be done on any board with an active SMC > device (assuming SMC parameter RAM at zero, as u-boot sets it; other > bootloaders may need to exempt a different region). Thanks for your help. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpgzAwItLUnQ.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
OF compatible MTD platform RAM driver ?
Hi everybody, as part of a ARCH=ppc to ARCH=powerpc migration process, I'm looking for an OpenFirmware compatible way to handle a RAM-based MTD device. On the platform_device based ppc architecture, the drivers/mtd/maps/plat-ram.c driver handled "mtd-ram" platform devices. There is no such driver for the OF-based powerpc architecture. As a temporary workaround I hacked the physmap_of driver to handle "direct-mapped" OF devices oh type "ram" by adding a corresponding entry in the of_flash_match[] array. This seems to work fine. What would be the preferred way to handle OF-compatible RAM-based MTD devices ? The 3 ways I can think of are 1. porting the plat-ram driver to OF (the driver isn't used in the kernel tree but I suspect it is used by out-of-tree boards) 2. creating a new plat-ram-of driver, much like the physmap_of driver comes from the physmap driver 3. extending the physmap_of driver to handle RAM devices (in which case references to "flash" in the function names should probably be replaced by "mtd") I live option 3 better so far. Has anyone already worked on this ? Is there any defined device tree mapping for MTD RAM devices ? Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpwuFdl1bULg.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: OF compatible MTD platform RAM driver ?
On Tuesday 11 March 2008 01:45, David Gibson wrote: > On Mon, Mar 10, 2008 at 12:00:22PM -0500, Rune Torgersen wrote: > > [EMAIL PROTECTED] wrote: > > > Hi everybody, > > > > > > as part of a ARCH=ppc to ARCH=powerpc migration process, I'm > > > looking for an > > > OpenFirmware compatible way to handle a RAM-based MTD device. > > > > > > On the platform_device based ppc architecture, the > > > drivers/mtd/maps/plat-ram.c > > > driver handled "mtd-ram" platform devices. There is no such > > > driver for the > > > OF-based powerpc architecture. > > > > > > As a temporary workaround I hacked the physmap_of driver to > > > handle "direct-mapped" OF devices oh type "ram" by adding a > > > corresponding entry in the of_flash_match[] array. This seems to work > > > fine. > > > > > > What would be the preferred way to handle OF-compatible RAM-based MTD > > > devices ? The 3 ways I can think of are > > > > > > 1. porting the plat-ram driver to OF (the driver isn't used > > > in the kernel tree > > > but I suspect it is used by out-of-tree boards) > > > > > > 2. creating a new plat-ram-of driver, much like the > > > physmap_of driver comes > > > from the physmap driver > > > > > > 3. extending the physmap_of driver to handle RAM devices (in > > > which case > > > references to "flash" in the function names should probably > > > be replaced > > > by "mtd") > > > > > > I live option 3 better so far. > > > > > > Has anyone already worked on this ? Is there any defined > > > device tree mapping > > > for MTD RAM devices ? > > > > We ran ito the same issue. > > We did option 3, as it was efinetly the easiest, > > I think this is the best option in principle. I'll implement that and post a patch after completing the ppc-to-powerpc migration. > > here is the sram entry in our dts: > > Except that your implementation of it is not good. > > You're relying on the old obsolete flash binding with the "probe-type" > field. The solution should be adapted to the new approach which uses > values in the "compatible" field to indicate various sorts of flash > device. What "compatible" values should I use for ROM and RAM mappings ? Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgp2iwSqncFAX.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Interrupt handling documentation
Hi everybody, is there any documentation describing interrupt handling for the powerpc architecture ? I'm writing a driver for a cascaded interrupt controller and the only source of information I found was the code. I'm particularly interested in information about irq hosts (allocation and initialisation, especially the map and unmap callbacks) and irq chaining. Different drivers seem to implement cascaded irqs differently (for instance arch/powerpc/sysdev/uic.c uses setup_irq to register the cascaded irq handler, while arch/powerpc/platforms/82xx/pq2ads-pci-pic.c uses set_irq_chained_handler) so I'm a bit lost here. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpdyZnZmTDmd.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Help needed to describe a custom bus in the device tree
interrupts = <18 8>; interrupt-parent = <&cpm_pic>; }; [EMAIL PROTECTED] { compatible = "tbox,sdhci"; reg = <5000 1000>; interrupts = <16 8>; interrupt-parent = <&cpm_pic>; }; }; [EMAIL PROTECTED],1 { compatible = "tbox,cp11-msbus"; #address-cells = <1>; #size-cells = <1>; #interrupt-cells = <1>; reg = <4 1 4>; interrupt-parent = <&msbus_pic>; ranges = <5 0 0 0400>; }; }; Is this correct ? Is that the best way to describe my custom bus in the device tree ? How would the relationships between the bus and its PIC and SPI controller be handled in the drivers ? I also don't understand how interrupt mappings are supposed to be handled. PCI busses have two CPM interrupt lines, one for the PCI PIC and one for the PCI bus, with the PCI bus having the CPM PIC as its interrupt controller. My bus PIC uses a single interrupt line. Is there some documentation explaining how PICs and interrupt mappings should be described ? Thanks in advance for any help you can provide. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpu9Zq5pHnNM.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Interrupt handling documentation
Hi Michael, On Wednesday 12 March 2008 01:51, Michael Ellerman wrote: > On Tue, 2008-03-11 at 11:58 +0100, Laurent Pinchart wrote: > > Hi everybody, > > > > is there any documentation describing interrupt handling for the powerpc > > architecture ? I'm writing a driver for a cascaded interrupt controller > > and the only source of information I found was the code. > > I don't think there's much documentation. I feared so :-) > You might want to look at arch/powerpc/platforms/cell/axon_msi.c, it's a > reasonably simple example of how to setup an irq_host and so on - well I > think so :D Thanks for the pointer. > > I'm particularly interested in information about irq hosts (allocation > > and initialisation, especially the map and unmap callbacks) and irq > > chaining. Different drivers seem to implement cascaded irqs differently > > (for instance arch/powerpc/sysdev/uic.c uses setup_irq to register the > > cascaded irq handler, while arch/powerpc/platforms/82xx/pq2ads-pci-pic.c > > uses set_irq_chained_handler) so I'm a bit lost here. > > uic.c uses set_irq_chained_handler() now, so that probably answers that > question. I don't think it makes all that much difference if you set it > up by hand, but set_irq_chained_handler() is the neat way to do it. That pretty much answers my question. It's always a bit disturbing when different drivers use different APIs to accomplish the same task, especially when the lack of documentation doesn't clearly state which API should be used and which API is internal/deprecated. Thanks for your answer. Cheers, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpNUX6ai2xcl.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Help needed to describe a custom bus in the device tree
Hi Dave, On Tuesday 11 March 2008 23:54, David Gibson wrote: > On Tue, Mar 11, 2008 at 03:27:26PM +0100, Laurent Pinchart wrote: > > Hi everybody, > > > > the migration process from ARCH=ppc to ARCH=powerpc is easier than I > > thought in some parts, but a few devices are still giving me > > headaches. This should hopefully be one of my last major requests > > for help (I'm sure most of you will be happy to see traffic on this > > list going down when I'll be done :-)) > > > > I'm having trouble describing a custom bus named MS bus (completely > > unrelated to a well-known software company) in the device tree. The > > hardware is MPC8248-based and has the following hardware topology. > > > > MPC8248 <-- localbus --> FPGA <-- ms bus --> Custom peripherals > > > > The bus interrupt controller, serial access (SPI) controller and > > status registers are accessed through memory-mapped registers in the > > FPGA. Parallel access to the MS bus is handled transparently by the > > FPGA which handles address mapping. > > > > The FPGA is mapped on the locabus at address 0xf400. Bus control > > registers are at 0xf4002000 - 0xf4003000. The parallel bus memory > > window on the localbus is located at 0xf500. > > > > My current dts draft describes that topology as follows (unrelated > > devices on the local bus such as flash memory are removed for > > clarity). > > > > [EMAIL PROTECTED] { > > compatible = "fsl,pq2-localbus"; > > #address-cells = <2>; > > #size-cells = <1>; > > reg = ; > > > > ranges = <0 0 4000 0100 > > 2 0 f200 0010 > > 3 0 f300 0010 > > 4 0 f400 0010 > > 5 0 f500 0010>; > > > > [EMAIL PROTECTED],0 { > > #address-cells = <1>; > > #size-cells = <1>; > > ranges = <4 0 0 0001>; > > > > [EMAIL PROTECTED] { > > compatible = "tbox,cp11-msbus-arbitrer"; > > reg = <2000 4>; > > }; > > > > msbus_pic: [EMAIL PROTECTED] { > > compatible = "tbox,cp11-msbus-pic"; > > reg = <2100 8>; > > interrupts = <17 2>; > > interrupt-parent = <&cpm_pic>; > > #interrupt-cells = <1>; > > interrupt-controller; > > }; > > > > [EMAIL PROTECTED] { > > compatible = "tbox,cp11-msbus-spi"; > > reg = <2200 100>; > > interrupts = <18 8>; > > interrupt-parent = <&cpm_pic>; > > }; > > > > [EMAIL PROTECTED] { > > compatible = "tbox,sdhci"; > > reg = <5000 1000>; > > interrupts = <16 8>; > > interrupt-parent = <&cpm_pic>; > > }; > > }; > > > > [EMAIL PROTECTED],0 { > > compatible = "tbox,cp11-msbus"; > > #address-cells = <1>; > > #size-cells = <1>; > > #interrupt-cells = <1>; > > reg = <5 0 0 0400>; > > interrupt-parent = <&msbus_pic>; > > }; > > }; > > > > The device tree reflects the physical topology but makes driver > > access to the bus quite complex. An OF platform device driver > > matching on compatible = "tbox,cp11-msbus" will not have the bus > > FPGA registers described in its device node. > > > > Having a look at the various device trees included in the kernel > > sources, it seems platforms with a PCI bus experience a similar > > problem. To solve it the PCI bus node address and registers describe > > the configuration registers, and the memory window to access PCI > > devices is described by the ranges property. Applying that to my > > custom bus would lead to the following tree. > > > > [EMAIL PROTECTED] { > > compatible = "fsl,pq2-localbus"; > > #address-cells = <2>; > > #size-cells = <1>; > > reg = ; > > > > ranges = <0 0 4000 0100 > >
Re: Interrupt handling documentation
On Thursday 13 March 2008 14:56, Laurent Pinchart wrote: > Hi Michael, > > On Wednesday 12 March 2008 01:51, Michael Ellerman wrote: > > On Tue, 2008-03-11 at 11:58 +0100, Laurent Pinchart wrote: > > > Hi everybody, > > > > > > is there any documentation describing interrupt handling for the > > > powerpc architecture ? I'm writing a driver for a cascaded interrupt > > > controller and the only source of information I found was the code. > > > > I don't think there's much documentation. > > I feared so :-) > > > You might want to look at arch/powerpc/platforms/cell/axon_msi.c, it's a > > reasonably simple example of how to setup an irq_host and so on - well I > > think so :D > > Thanks for the pointer. > > > > I'm particularly interested in information about irq hosts (allocation > > > and initialisation, especially the map and unmap callbacks) and irq > > > chaining. Different drivers seem to implement cascaded irqs differently > > > (for instance arch/powerpc/sysdev/uic.c uses setup_irq to register the > > > cascaded irq handler, while > > > arch/powerpc/platforms/82xx/pq2ads-pci-pic.c uses > > > set_irq_chained_handler) so I'm a bit lost here. > > > > uic.c uses set_irq_chained_handler() now, so that probably answers that > > question. I don't think it makes all that much difference if you set it > > up by hand, but set_irq_chained_handler() is the neat way to do it. > > That pretty much answers my question. It's always a bit disturbing when > different drivers use different APIs to accomplish the same task, > especially when the lack of documentation doesn't clearly state which API > should be used and which API is internal/deprecated. I've been struggling with spurious interrupts related to my irq host for a day. Now that I've been able to solve the problem I thought I'd share the results here. The PIC I am working with is linked to a falling-edge external irq on the CPM2. When the first PIC interrupt was generated the kernel called the PIC chained irq handler endlessly. After some investigation it turned out the external interrupt bit in the CPM2 interrupt pending register never got cleared. set_irq_chained_handler() registers the chained irq handler at the lowest level in the irq stack, bypassing all the interrupt acknowledgement/masking logic. The fix was easy, all I had to do was to call desc->chip->ack(irq) at the beginning on the chained irq handler and desc->chip->eoi(irq) at the end. However, I'm wondering if this really belongs in the PIC code, or if PICs shouldn't be registered at a higher level (setup_irq or even request_irq) so that they would reuse the handle_*_irq handlers. Any opinion on this ? Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgp2J594AklTf.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Trouble with SCC UART ports when moving from ppc to powerpc
Hi Scott, On Monday 17 March 2008 16:33, Scott Wood wrote: > On Mon, Mar 10, 2008 at 01:17:01PM +0100, Laurent Pinchart wrote: > > On Friday 07 March 2008 17:21, Scott Wood wrote: > > > cpm2_reset() doesn't currently actually reset the CPM, for some reason > > > (unlike cpm1). This should probably be fixed, though then we'd have to > > > deal with assigning SMC parameter RAM addresses ourselves. > > > > I had overlooked that. Resetting the CPM in cpm2_reset() helps. Is there > > any reason not to rset the CPM in cpm2_reset() ? > > The only issue I'm aware of other than the SMC parameter RAM relocation is > that the reset can't happen if the early udbg printk is being used. > > > How should SMC parameter RAM assignment be handled ? I'm not very > > familiar with the CPM1, > > CPM1 has hardcoded SMC parameter RAM addresses, so it's not relevant here. > > > but for CPM2 the cpm_uart driver could easily cpm_dpalloc() parameter > > RAM. > > It can, yes. Patches welcome. :-) I'm working on a patch. I'm not sure how to handle the early udbg printk problem, so the first version will just ignore it. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpJ3u1AGlujw.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: Trouble with SCC UART ports when moving from ppc to powerpc
On Thursday 20 March 2008 16:08, Scott Wood wrote: > On Thu, Mar 20, 2008 at 02:39:56PM +0100, Laurent Pinchart wrote: > > I'm working on a patch. I'm not sure how to handle the early udbg printk > > problem, so the first version will just ignore it. > > udbg printk is extremely useful... I'd not be fond of it breaking. I don't want to kill it either. What I meant is that my first patch will not care about udbg so that I can get feedback about the CPM serial driver part. I'll then see how to make udbg fit into that. > Perhaps we should stick more initialization into the cpm udbg code, so it > isn't so dependent on the bootloader setup... though we'd have push the > CPM reset and SMC allocation earlier in such a case. When SMC1 is used as a serial console, SMC1 is the first CPM dpram allocation user. This means the CPM dpram allocator will allocate a buffer at the beginning of the dpram. As he SMC controller will already be using the 64 first bytes of the dpram, it will not see any difference when being reinitialized. This of course depends on the boot loader allocating parameter ram at the beginning of the dpram for SMC1, but we already depend on that if I'm not mistaken. -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgp33kdhK60Ds.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH] cpm_uart: Allocate DPRAM memory for SMC ports on CPM2-based platforms.
Hi everybody, the following patch fixes SMC DPRAM handling on CPM2-based platforms. It makes the cpm_uart driver independent of any SMC initialisation made by the boot loader. This is required to be able to reset the CPM in cpm2_reset() which should be the next step in making CPM2 initialisation independent of the boot loader. I was concerned this would break udbg, but udbg doesn't seem to be supported on PQ2-based platforms. --- This patch allocates parameter RAM for SMC serial ports without relying on previous initialisation by a boot loader or a wrapper layer. SMC parameter RAM on CPM2-based platforms can be allocated anywhere in the general-purpose areas of the dual-port RAM. The current code relies on the boot loader to allocate a section of general-purpose CPM RAM and gets the section address from the device tree. This patch modifies the device tree address usage to reference the SMC parameter RAM base pointer instead of a pre-allocated RAM section and allocates memory from the CPM dual-port RAM when initialising the SMC port. CPM1-based platforms are not affected. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- drivers/serial/cpm_uart/cpm_uart.h |3 ++ drivers/serial/cpm_uart/cpm_uart_core.c | 19 +++ drivers/serial/cpm_uart/cpm_uart_cpm1.c | 12 ++ drivers/serial/cpm_uart/cpm_uart_cpm2.c | 37 +++ 4 files changed, 61 insertions(+), 10 deletions(-) diff --git a/drivers/serial/cpm_uart/cpm_uart.h b/drivers/serial/cpm_uart/cpm_uart.h index 80a7d60..5334653 100644 --- a/drivers/serial/cpm_uart/cpm_uart.h +++ b/drivers/serial/cpm_uart/cpm_uart.h @@ -93,6 +93,9 @@ extern struct uart_cpm_port cpm_uart_ports[UART_NR]; /* these are located in their respective files */ void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd); +void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port, + struct device_node *np); +void cpm_uart_unmap_pram(struct uart_cpm_port *port, void __iomem *pram); int cpm_uart_init_portdesc(void); int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con); void cpm_uart_freebuf(struct uart_cpm_port *pinfo); diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c b/drivers/serial/cpm_uart/cpm_uart_core.c index af875ad..56f39ca 100644 --- a/drivers/serial/cpm_uart/cpm_uart_core.c +++ b/drivers/serial/cpm_uart/cpm_uart_core.c @@ -997,24 +997,23 @@ static int cpm_uart_init_port(struct device_node *np, if (!mem) return -ENOMEM; - pram = of_iomap(np, 1); - if (!pram) { - ret = -ENOMEM; - goto out_mem; - } - if (of_device_is_compatible(np, "fsl,cpm1-scc-uart") || of_device_is_compatible(np, "fsl,cpm2-scc-uart")) { pinfo->sccp = mem; - pinfo->sccup = pram; + pinfo->sccup = pram = cpm_uart_map_pram(pinfo, np); } else if (of_device_is_compatible(np, "fsl,cpm1-smc-uart") || of_device_is_compatible(np, "fsl,cpm2-smc-uart")) { pinfo->flags |= FLAG_SMC; pinfo->smcp = mem; - pinfo->smcup = pram; + pinfo->smcup = pram = cpm_uart_map_pram(pinfo, np); } else { ret = -ENODEV; - goto out_pram; + goto out_mem; + } + + if (!pram) { + ret = -ENOMEM; + goto out_mem; } pinfo->tx_nrfifos = TX_NUM_FIFO; @@ -1038,7 +1037,7 @@ static int cpm_uart_init_port(struct device_node *np, return cpm_uart_request_port(&pinfo->port); out_pram: - iounmap(pram); + cpm_uart_unmap_pram(pinfo, pram); out_mem: iounmap(mem); return ret; diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.c b/drivers/serial/cpm_uart/cpm_uart_cpm1.c index 52fb044..060f566 100644 --- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c +++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c @@ -58,6 +58,18 @@ void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd) while (in_be16(cpcr) & CPM_CR_FLG) ; } + +void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port, + struct device_node *np) +{ + return of_iomap(np, 1); +} + +void cpm_uart_unmap_pram(struct uart_cpm_port *port, void __iomem *pram) +{ + iounmap(pram); +} + #else void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd) { diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/serial/cpm_uart/cpm_uart_cpm2.c index 88daad1..9391dc6 100644 --- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c +++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c @@ -41,6 +41,9 @@ #include #include #include +#ifdef CONFIG_PPC_CPM_NEW_BINDING +#include +#endif #include #include @@ -60,6 +63,40 @@ void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd)
Re: OF compatible MTD platform RAM driver ?
On Tuesday 11 March 2008 23:40, David Gibson wrote: > On Tue, Mar 11, 2008 at 11:39:08AM +0100, Laurent Pinchart wrote: > > On Tuesday 11 March 2008 01:45, David Gibson wrote: > > > On Mon, Mar 10, 2008 at 12:00:22PM -0500, Rune Torgersen wrote: > > > > [EMAIL PROTECTED] wrote: > [snip] > > > > We ran ito the same issue. > > > > We did option 3, as it was efinetly the easiest, > > > > > > I think this is the best option in principle. > > > > I'll implement that and post a patch after completing the ppc-to-powerpc > > migration. > > > > > > here is the sram entry in our dts: > > > > > > Except that your implementation of it is not good. > > > > > > You're relying on the old obsolete flash binding with the "probe-type" > > > field. The solution should be adapted to the new approach which uses > > > values in the "compatible" field to indicate various sorts of flash > > > device. > > > > What "compatible" values should I use for ROM and RAM mappings ? > > That I'm not so sure of. We'll need to find some consensus. > > There may be existing IEEE1275 bindings for roms, which we should > investigate. Do you (or someone else here) have access to the IEEE1275 specification ? Is there any ROM binding in there ? > Arguably RAM should be represented by a memory node, but > that's going to get messy for this sort of application. We're talking about a very specific type of RAM, used for permanent storage with a battery backup. The RAM is really meant to be used as an MTD device and as such I think it makes sense to describe it as an mtd-compatible device on the local bus. What about the following definition for the RAM node ? [EMAIL PROTECTED], { compatible = "mtd,ram"; reg = <2 0x 0x0010>; bank-width = <2>; }; Or should the node have a device-type property of either 'ram' or 'rom' with the compatible property just referencing MTD ? Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpBeTwQpIHsf.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] cpm_uart: Allocate DPRAM memory for SMC ports on CPM2-based platforms.
On Tuesday 25 March 2008 15:58, Scott Wood wrote: > On Tue, Mar 25, 2008 at 12:24:40PM +0100, Laurent Pinchart wrote: > > I was concerned this would break udbg, but udbg doesn't seem to be supported > > on PQ2-based platforms. > > Yes, it is (see arch/powerpc/sysdev/cpm_common.c). I thought that was for CPM1 only. My bad. > > This patch modifies the device tree address usage to reference the SMC > > parameter RAM base pointer instead of a pre-allocated RAM section and > > allocates memory from the CPM dual-port RAM when initialising the SMC port. > > CPM1-based platforms are not affected. > > Please maintain backward compatibility with older device trees (by > checking the length of the second reg resource). At the very least, > update the device trees that are affected. I haven't seen any CPM2-based board using SMC ports in the device trees available in arch/powerpc/boot/dts. Should I still maintain compatibility with older device trees ? Is there any out-of-tree PQ2 boards using udbg and SMC ? What about printing a warning if the second reg resource has the wrong size ? > > + offset = cpm_dpalloc(PROFF_SMC_SIZE, 64); > > + out_be16(pram, offset); > > Up to this point, if we don't reset the CPM prior to any dpalloc calls > (and if we do, udbg printk breaks), the SMC could be running and > clobbering some other bit of dpram, which could have been allocated to > something else. If udbg uses the parameter RAM allocated by the boot loader, that section of DPRAM should be removed from the muram node in the device tree. Otherwise the SMC DPRAM will eventually be allocated to something else and the system will break. It should thus be safe to reset the CPM if udbg isn't used, and the device tree should explicitely exclude the pre-allocated parameter RAM from the muram node when udbg is used. > After this point, even if you don't reset the CPM, udbg printk is broken > because you moved pram. The udbg disabling will have to be moved before > this. Moving the SMC pram doesn't break udbg printk in itself. What will break it is moving the TX BDs, but the end result is the same, moving pram will result in udbg being broken. The cpm_uart driver disable udbg before allocating the new BDs. What about moving that right before moving the parameter RAM ? cpm_uart_request_port(), which is called in between, already disables RX and TX on the port, so we won't loose any debug message. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgptH6LGTcryW.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: OF compatible MTD platform RAM driver ?
Hi Sergei, On Tuesday 25 March 2008 16:29, Sergei Shtylyov wrote: > Hello. > > Laurent Pinchart wrote: > > >>>>>here is the sram entry in our dts: > > >>>>Except that your implementation of it is not good. > > >>>>You're relying on the old obsolete flash binding with the "probe-type" > >>>>field. The solution should be adapted to the new approach which uses > >>>>values in the "compatible" field to indicate various sorts of flash > >>>>device. > > >>>What "compatible" values should I use for ROM and RAM mappings ? > > >>That I'm not so sure of. We'll need to find some consensus. > > >>There may be existing IEEE1275 bindings for roms, which we should > >>investigate. > > > Do you (or someone else here) have access to the IEEE1275 specification ? > > Is > > Yeah, and I can point you to it -- see the documantation section on > http://www.openbios.org/... Thanks a lot for the pointer. > > there any ROM binding in there ? > > No. We initially called the flash devices that physmap_of driver > controlled "rom" (I mean the "device_type" property) -- now this is obsoleted. > > >>Arguably RAM should be represented by a memory node, but > >>that's going to get messy for this sort of application. > > Note that the OF "memory" type nodes do *not* represent RAM devices. > > > We're talking about a very specific type of RAM, used for permanent storage > > with a battery backup. The RAM is really meant to be used as an MTD device > > and as such I think it makes sense to describe it as an mtd-compatible > > device > > on the local bus. > > > What about the following definition for the RAM node ? > > > [EMAIL PROTECTED], { > > Note that there's a OF "device_type" of "nvram", so your (generic) device > name seems to add some mess. (IIRC, that OF device type didn't actually > represent a "real" device, and only served to provide access to NVRAM for OF). Ok. > > compatible = "mtd,ram"; > > The part before comma should be a company name or a stock ticker. What > did > you mean here? I didn't know that. Let's say I meant "mtd-ram" :-) > > reg = <2 0x 0x0010>; > > bank-width = <2>; > > }; > > > Or should the node have a device-type property of either 'ram' or 'rom' > > with > > the compatible property just referencing MTD ? > > The "device_type" properties are not required and their further creation > has been discouraged on liunxppc-dev. What about [EMAIL PROTECTED], { compatible = "mtd-ram"; reg = <2 0x 0x0010>; bank-width = <2>; }; ROMs could use "mtd-rom" for their compatible property. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpMOYAiInRmR.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] cpm_uart: Allocate DPRAM memory for SMC ports on CPM2-based platforms.
On Tuesday 25 March 2008 17:03, Scott Wood wrote: > On Tue, Mar 25, 2008 at 04:34:46PM +0100, Laurent Pinchart wrote: > > On Tuesday 25 March 2008 15:58, Scott Wood wrote: > > > Please maintain backward compatibility with older device trees (by > > > checking the length of the second reg resource). At the very least, > > > update the device trees that are affected. > > > > I haven't seen any CPM2-based board using SMC ports in the device trees > > available in arch/powerpc/boot/dts. > > ep8248e I should have checked git head. My bad. I'll include the ep8248e device tree in the next patch. > > Should I still maintain compatibility with older device trees ? Is there any > > out-of-tree PQ2 boards using udbg and SMC ? > > Yes, I've answered questions on the lists from at least one person using > a custom board with cpm2 smc. Are you sure it wasn't me ? ;-) > > What about printing a warning if the second reg resource has the wrong > > size ? > > The only way you'll see the warning is if udbg is enabled. :-P > > Will a CPM reset blow away the portion of muram that holds the SMC pram > pointer? If not (and I don't think it will), just return the device tree > reg resource as is currently done if the resource is the wrong size. Ok I'll do that. Should I add a warning message to tell people to update the device tree ? > > > After this point, even if you don't reset the CPM, udbg printk is broken > > > because you moved pram. The udbg disabling will have to be moved before > > > this. > > > > Moving the SMC pram doesn't break udbg printk in itself. What will break it is > > moving the TX BDs, but the end result is the same, moving pram will result in > > udbg being broken. > > > > The cpm_uart driver disable udbg before allocating the new BDs. What about > > moving that right before moving the parameter RAM ? cpm_uart_request_port(), > > which is called in between, already disables RX and TX on the port, so we > > won't loose any debug message. > > cpm_uart_request_port() returns without doing that if it's a console > port. I think the current placement of the udbg disable will be fine. Ok. I'll prepare a new patch that maintains compatibility with old device trees. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgp944Q2msmE7.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] cpm_uart: Allocate DPRAM memory for SMC ports onCPM2-based platforms.
On Tuesday 25 March 2008 17:27, Rune Torgersen wrote: > >Laurent Pinchart wrote: > > On Tuesday 25 March 2008 17:03, Scott Wood wrote: > >> Yes, I've answered questions on the lists from at least one person > >> using a custom board with cpm2 smc. > > > > Are you sure it wasn't me ? ;-) > > > > Probably me, actually. We have a 8280 with SMC's in use (SMC1 and 2) Ok. Do you have any opinion about the proposed patch ? Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpFR20lfWu9J.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: OF compatible MTD platform RAM driver ?
On Tuesday 25 March 2008 17:23, Sergei Shtylyov wrote: > Laurent Pinchart wrote: > > >>>We're talking about a very specific type of RAM, used for permanent storage > >>>with a battery backup. The RAM is really meant to be used as an MTD device > >>>and as such I think it makes sense to describe it as an mtd-compatible device > >>>on the local bus. > > >>>What about the following definition for the RAM node ? > > >>>[EMAIL PROTECTED], { > > >>Note that there's a OF "device_type" of "nvram", so your (generic) device > >>name seems to add some mess. (IIRC, that OF device type didn't actually > >>represent a "real" device, and only served to provide access to NVRAM for OF). > > > Ok. > > Well, I might have gone too far here -- it should be a real device > (spec'ed in Device Support Extensions recommended practice). It's just that > the spec didn't mention "reg" property, only "#bytes" (the device capacity). > So, it may be worth considering... The nvram device descrived in the Device Support Extensions is probably meant to describe the kind of nvram found in RTC chips. That memory isn't directly accessible. As the spec doesn't mention this explicitely we could still reuse the nvram device type for direct-mapped battery-backed ram. I have no strong opinion for or against that. > >>>compatible = "mtd,ram"; > > >>The part before comma should be a company name or a stock ticker. What did > >>you mean here? > > > I didn't know that. Let's say I meant "mtd-ram" :-) > > >>>reg = <2 0x 0x0010>; > >>>bank-width = <2>; > >>>}; > > >>>Or should the node have a device-type property of either 'ram' or 'rom' with > >>>the compatible property just referencing MTD ? > > >>The "device_type" properties are not required and their further creation > >>has been discouraged on liunxppc-dev. > > > What about > > > [EMAIL PROTECTED], { > > compatible = "mtd-ram"; > > reg = <2 0x 0x0010>; > > bank-width = <2>; > > }; > > > ROMs could use "mtd-rom" for their compatible property. > > Heh, there was a whole company against mentioning "mtd" when we started > working on this (of course, the first idea was to call the flash device type > "mtd"). I don't think "mtd" looks good here -- I'd suggest "flash-ram" (if > this is just a linearly mapped NVRAM). I'm fine with "flash-ram" (even thought it looks a bit weird). I'll prepare a patch. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpzz0chgLDkF.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: OF compatible MTD platform RAM driver ?
On Tuesday 25 March 2008 18:02, Sergei Shtylyov wrote: > Laurent Pinchart wrote: > > > > Heh, there was a whole company against mentioning "mtd" when we started > > > working on this (of course, the first idea was to call the flash device > > > type "mtd"). I don't think "mtd" looks good here -- I'd suggest > > > "flash-ram" (if this is just a linearly mapped NVRAM). > > > I'm fine with "flash-ram" (even thought it looks a bit weird). I'll > > prepare a patch. > > Yeah. I forgeot that "flash" means EEPROM. Actually, the main facts about > the NVRAM that I'd want to be stated in the "compatible" property is that > it's non-volatile and directly/lineraly mapped... Just "nvram" doesn't seem > enopugh, maybe "linear-nvram" is. Direct mapping is a hard requirement for the nvram if we want to use it with the MTD subsystem. Regarding non-volatility nothing prevents a user from using a volatile RAM as an MTD device, but there's little point in doing so. Would it be acceptable for the "linear-nvram" specification not to include volatile RAM ? ROM chips would be excluded too. Is that an issue ? > And we can specify "device_type" of "nvram" indeed (and #size). I suppose you meant #bytes. What about sub-partitions support ? Nothing prevents RAM-based MTD devices from being partioned. Would it be acceptable to reference the CFI/JEDEC flash section in Documentation/powerpc/booting-without-of.txt in the description of the nvram node ? Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpFY5Ra1Z7Qi.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: OF compatible MTD platform RAM driver ?
On Tuesday 25 March 2008 18:56, Rune Torgersen wrote: > Laurent Pinchart wrote: > > On Tuesday 25 March 2008 18:02, Sergei Shtylyov wrote: > >> Laurent Pinchart wrote: > >> > > Regarding non-volatility nothing prevents a user from using a > > volatile RAM as an MTD device, but there's little point in doing so. > > Would it be acceptable for the "linear-nvram" specification > > not to include > volatile RAM ? ROM chips would be excluded too. Is > that an issue ? > > We actually use a volatile ram (SRAM) as an MTD device. We use it to > store info from bootloader and system specific values between resets. So we're left with two main options. - Reusing the nvram device type from the Device Support Extensions. Volatile devices wouldn't be supported, and we'd need a separate device specification for linear-mapped volatile RAMs. I'm not very happy with that. - Using another device node with a compatible value set to "linear-ram" (or something similar). This would support both volatile and non-volatile devices, and a property could be added to specify if the device is volatile or not. I'd go for the second option, and I'd specify a "linear-rom" compatible value as well while we're at it. Both volatile and non-volatile RAMs can be handled by the physmap_of MTD driver. They both use the same map probe type ("map_ram"). Volatility isn't handled there. ROMs should be handled by the same driver and should use the "mtd_rom" map probe type. As all those devices use the physmap_of MTD driver, what about using "physmap-ram" and "physmap-rom" as compatibility names ? Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpUqXLkuZAbu.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCHv2 0/3] cpm2: Reset the CPM at startup and fix the cpm_uart driver accordingly.
Hi everybody, these 3 patches reset the CPM in cpm2_reset() and fix the cpm_uart driver to initialise SMC ports correctly without relying on any initialisation performed by the boot loader/wrapper. They update the EP8248E device tree to match the new SMC registers description. -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpT05gWeKPZI.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCHv2 1/3] cpm_uart: Allocate DPRAM memory for SMC ports on CPM2-based platforms.
This patch allocates parameter RAM for SMC serial ports without relying on previous initialisation by a boot loader or a wrapper layer. SMC parameter RAM on CPM2-based platforms can be allocated anywhere in the general-purpose areas of the dual-port RAM. The current code relies on the boot loader to allocate a section of general-purpose CPM RAM and gets the section address from the device tree. This patch modifies the device tree address usage to reference the SMC parameter RAM base pointer instead of a pre-allocated RAM section and allocates memory from the CPM dual-port RAM when initialising the SMC port. CPM1-based platforms are not affected. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- drivers/serial/cpm_uart/cpm_uart.h |3 ++ drivers/serial/cpm_uart/cpm_uart_core.c | 19 +-- drivers/serial/cpm_uart/cpm_uart_cpm1.c | 12 +++ drivers/serial/cpm_uart/cpm_uart_cpm2.c | 52 +++ 4 files changed, 76 insertions(+), 10 deletions(-) diff --git a/drivers/serial/cpm_uart/cpm_uart.h b/drivers/serial/cpm_uart/cpm_uart.h index 80a7d60..5334653 100644 --- a/drivers/serial/cpm_uart/cpm_uart.h +++ b/drivers/serial/cpm_uart/cpm_uart.h @@ -93,6 +93,9 @@ extern struct uart_cpm_port cpm_uart_ports[UART_NR]; /* these are located in their respective files */ void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd); +void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port, + struct device_node *np); +void cpm_uart_unmap_pram(struct uart_cpm_port *port, void __iomem *pram); int cpm_uart_init_portdesc(void); int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con); void cpm_uart_freebuf(struct uart_cpm_port *pinfo); diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c b/drivers/serial/cpm_uart/cpm_uart_core.c index 1ea123c..3a44a3f 100644 --- a/drivers/serial/cpm_uart/cpm_uart_core.c +++ b/drivers/serial/cpm_uart/cpm_uart_core.c @@ -997,24 +997,23 @@ static int cpm_uart_init_port(struct device_node *np, if (!mem) return -ENOMEM; - pram = of_iomap(np, 1); - if (!pram) { - ret = -ENOMEM; - goto out_mem; - } - if (of_device_is_compatible(np, "fsl,cpm1-scc-uart") || of_device_is_compatible(np, "fsl,cpm2-scc-uart")) { pinfo->sccp = mem; - pinfo->sccup = pram; + pinfo->sccup = pram = cpm_uart_map_pram(pinfo, np); } else if (of_device_is_compatible(np, "fsl,cpm1-smc-uart") || of_device_is_compatible(np, "fsl,cpm2-smc-uart")) { pinfo->flags |= FLAG_SMC; pinfo->smcp = mem; - pinfo->smcup = pram; + pinfo->smcup = pram = cpm_uart_map_pram(pinfo, np); } else { ret = -ENODEV; - goto out_pram; + goto out_mem; + } + + if (!pram) { + ret = -ENOMEM; + goto out_mem; } pinfo->tx_nrfifos = TX_NUM_FIFO; @@ -1038,7 +1037,7 @@ static int cpm_uart_init_port(struct device_node *np, return cpm_uart_request_port(&pinfo->port); out_pram: - iounmap(pram); + cpm_uart_unmap_pram(pinfo, pram); out_mem: iounmap(mem); return ret; diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.c b/drivers/serial/cpm_uart/cpm_uart_cpm1.c index 6ea0366..e692593 100644 --- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c +++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c @@ -54,6 +54,18 @@ void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd) { cpm_command(port->command, cmd); } + +void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port, + struct device_node *np) +{ + return of_iomap(np, 1); +} + +void cpm_uart_unmap_pram(struct uart_cpm_port *port, void __iomem *pram) +{ + iounmap(pram); +} + #else void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd) { diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/serial/cpm_uart/cpm_uart_cpm2.c index 6291094..a4cfb0b 100644 --- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c +++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c @@ -41,6 +41,9 @@ #include #include #include +#ifdef CONFIG_PPC_CPM_NEW_BINDING +#include +#endif #include #include @@ -54,6 +57,55 @@ void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd) { cpm_command(port->command, cmd); } + +void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port, + struct device_node *np) +{ + void __iomem *pram; + unsigned long offset; + struct resource res; + unsigned long len; + + /* Don't remap parameter RAM if it has already been initialized +* during console setup. +*/ + if (IS_SMC(port) && port->smcup) + re
[PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.
This patch modifies the Embedded Planet EP8248E device tree to reference the SMC paramater RAM base register instead of the parameter RAM allocated by the boot loader. The cpm_uart driver will allocate parameter RAM itself, making the serial port initialisation independent of the boot loader. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- arch/powerpc/boot/dts/ep8248e.dts |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/boot/dts/ep8248e.dts b/arch/powerpc/boot/dts/ep8248e.dts index 5d2fb76..756758f 100644 --- a/arch/powerpc/boot/dts/ep8248e.dts +++ b/arch/powerpc/boot/dts/ep8248e.dts @@ -121,8 +121,7 @@ [EMAIL PROTECTED] { compatible = "fsl,cpm-muram-data"; - reg = <0 0x1100 0x1140 - 0xec0 0x9800 0x800>; + reg = <0 0x2000 0x9800 0x800>; }; }; @@ -138,7 +137,7 @@ device_type = "serial"; compatible = "fsl,mpc8248-smc-uart", "fsl,cpm2-smc-uart"; - reg = <0x11a80 0x20 0x1100 0x40>; + reg = <0x11a80 0x20 0x87fc 2>; interrupts = <4 8>; interrupt-parent = <&PIC>; fsl,cpm-brg = <7>; -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpfdPlbxgeFx.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCHv2 0/3] cpm2: Reset the CPM when early debugging is not enabled.
Similarly to what is done for PQ1-based platforms, this patch resets the PQ2 Communication Processor Module in cpm2_reset() when early debugging is not enabled. This helps avoiding conflicts when the boot loader configured the CPM in an unexpected way. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- arch/powerpc/sysdev/cpm2.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/sysdev/cpm2.c b/arch/powerpc/sysdev/cpm2.c index 7be7112..57ed1a4 100644 --- a/arch/powerpc/sysdev/cpm2.c +++ b/arch/powerpc/sysdev/cpm2.c @@ -80,6 +80,12 @@ void __init cpm2_reset(void) /* Tell everyone where the comm processor resides. */ cpmp = &cpm2_immr->im_cpm; + +#ifndef CONFIG_PPC_EARLY_DEBUG_CPM + /* Reset the CPM. +*/ + cpm_command(CPM_CR_RST, 0); +#endif } static DEFINE_SPINLOCK(cmd_lock); -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpN9vhUtQFHw.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv2 0/3] cpm2: Reset the CPM when early debugging is not enabled.
This one should of course have been PATCHv2 3/3. -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgplBaVg4MUjK.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH] cpm_uart: Allocate DPRAM memory for SMC ports onCPM2-based platforms.
On Wednesday 26 March 2008 09:31, Sergej Stepanov wrote: > > Am Dienstag, den 25.03.2008, 17:32 +0100 schrieb Laurent Pinchart: > > > Do you have any opinion about the proposed patch ? > > > > I have to say, it could be some off-topic. > But if you would use the cpm_uart-driver for cpm2(or cpm1) as kernel > module, at linking you can get warnings about: > - cpm_setbrg (cpm_set_brg): something with section mismatch > - __alloc_bootmem (cpm_uart_allocbuf): the same > > The symbols are not exported. > Is it ok, about kernel modules? I get the same section-mismatch warning for __alloc_bootmem in cpm_uart_allocbuf. cpm_setbrg doesn't cause any warning here. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpMpqMw7ZVe4.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 0/2] Add support for RAM & ROM mappings to the physmap_of driver
Hi everybody, these two patches add support for memory-mapped RAM & ROM chips to the physmap_of MTD driver. Best regards, -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 1/2] [MTD] Add support for RAM & ROM mappings in the physmap_of MTD driver.
Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- drivers/mtd/maps/physmap_of.c |8 1 files changed, 8 insertions(+), 0 deletions(-) diff --git a/drivers/mtd/maps/physmap_of.c b/drivers/mtd/maps/physmap_of.c index 49acd41..65c30b5 100644 --- a/drivers/mtd/maps/physmap_of.c +++ b/drivers/mtd/maps/physmap_of.c @@ -273,6 +273,14 @@ static struct of_device_id of_flash_match[] = { .data = (void *)"jedec_probe", }, { + .compatible = "physmap-ram", + .data = (void *)"map_ram", + }, + { + .compatible = "physmap-rom", + .data = (void *)"map_rom", + }, + { .type = "rom", .compatible = "direct-mapped" }, -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCH 2/2] [POWERPC] Describe memory-mapped RAM&ROM chips of bindings
Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- Documentation/powerpc/booting-without-of.txt | 31 +- 1 files changed, 30 insertions(+), 1 deletions(-) diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt index 7b4e8a7..53d1cf8 100644 --- a/Documentation/powerpc/booting-without-of.txt +++ b/Documentation/powerpc/booting-without-of.txt @@ -57,7 +57,8 @@ Table of Contents n) 4xx/Axon EMAC ethernet nodes o) Xilinx IP cores p) Freescale Synchronous Serial Interface - q) USB EHCI controllers + q) USB EHCI controllers + r) Memory-mapped RAM & ROM VII - Specifying interrupt information for devices 1) interrupts property @@ -2816,6 +2817,34 @@ platforms are moved over to use the flattened-device-tree model. big-endian; }; + r) Memory-mapped RAM & ROM + +Dedicated RAM and ROM chips are often used as storage for temporary or +permanent data in embedded devices. Possible usage include non-volatile +storage in battery-backed SRAM, semi-permanent storage in dedicated SRAM +to preserve data accross reboots and firmware storage in dedicated ROM. + + - compatible : should contain the specific model of RAM/ROM chip(s) + used, if known, followed by either "physmap-ram" or "physmap-rom" + - reg : Address range of the RAM/ROM chip + - bank-width : Width (in bytes) of the RAM/ROM bank. Equal to the + device width times the number of interleaved chips. + - device-width : (optional) Width of a single RAM/ROM chip. If + omitted, assumed to be equal to 'bank-width'. + +Similarly to memory-mapped NOR flash, memory-mapped RAM & ROM chips +can be partionned. See the "j) CFI and JEDEC memory-mapped NOR flash" +section for information about how to represent partitions in the +device tree. + +Example: + + [EMAIL PROTECTED] { + compatible = "renesas,m5m5w816", "physmap-ram"; + reg = ; + bank-width = <2>; + }; + More devices will be defined as this spec matures. -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.
On Wednesday 26 March 2008 16:57, Scott Wood wrote: > On Wed, Mar 26, 2008 at 12:20:42PM +0100, Laurent Pinchart wrote: > > diff --git a/arch/powerpc/boot/dts/ep8248e.dts b/arch/powerpc/boot/dts/ep8248e.dts > > index 5d2fb76..756758f 100644 > > --- a/arch/powerpc/boot/dts/ep8248e.dts > > +++ b/arch/powerpc/boot/dts/ep8248e.dts > > @@ -121,8 +121,7 @@ > > > > [EMAIL PROTECTED] { > > compatible = "fsl,cpm-muram-data"; > > - reg = <0 0x1100 0x1140 > > - 0xec0 0x9800 0x800>; > > + reg = <0 0x2000 0x9800 0x800>; > > }; > > Can we leave this as reserved, so that udbg still works? Ok. Should I add a comment to the DTS to explain additional muram can be reclaimed if udbg isn't used ? -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpTZO4hqHiwt.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: OF compatible MTD platform RAM driver ?
On Wednesday 26 March 2008 13:53, Sergei Shtylyov wrote: > Laurent Pinchart wrote: > > >>>Regarding non-volatility nothing prevents a user from using a > >>>volatile RAM as an MTD device, but there's little point in doing so. > >>>Would it be acceptable for the "linear-nvram" specification > >>>not to include > volatile RAM ? ROM chips would be excluded too. Is > > >>that an issue ? > > >>We actually use a volatile ram (SRAM) as an MTD device. We use it to > >>store info from bootloader and system specific values between resets. > > > So we're left with two main options. > > > - Reusing the nvram device type from the Device Support Extensions. Volatile > > devices wouldn't be supported, and we'd need a separate device specification > > for linear-mapped volatile RAMs. I'm not very happy with that. > > > - Using another device node with a compatible value set to "linear-ram" (or > > something similar). This would support both volatile and non-volatile > > devices, and a property could be added to specify if the device is volatile > > or not. > > > I'd go for the second option, and I'd specify a "linear-rom" compatible value > > as well while we're at it. > > > Both volatile and non-volatile RAMs can be handled by the physmap_of MTD > > driver. They both use the same map probe type ("map_ram"). Volatility isn't > > handled there. > > > ROMs should be handled by the same driver and should use the "mtd_rom" map > > probe type. > > OK, let's go with it. > > > As all those devices use the physmap_of MTD driver, what about > > using "physmap-ram" and "physmap-rom" as compatibility names ? > > Heh, we've gone thru "physmap" before -- it was labelled Linux-specific > name (well, I'd agree with that). physmap stands for physically mapped. That doesn't sound Linux-specific to me, the fact that the MTD driver has the same name is a pure coincidence. linmap-rom and linmap-rom sound even more Linux-specific :-) Could we agree on a name ? I'd like to submit a new patch. -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: OF compatible MTD platform RAM driver ?
On Wednesday 26 March 2008 16:40, Sergei Shtylyov wrote: > Segher Boessenkool wrote: > > >> - Using another device node with a compatible value set to > >> "linear-ram" (or > >> something similar). This would support both volatile and non-volatile > >> devices, and a property could be added to specify if the device is > >> volatile > >> or not. > > > "memory-mapped-memory" perhaps :-) > > > Or, just "memory". Although that one might play havoc with some > > I'd suggest "ram" and "rom" then. Luckily the device trees don't contain > binding for the real RAM chips yet. :-) And when it will we'll be in trouble. Here are a few names. I like physmap-r[ao]m better. Does anyone have another suggestion ? I'd like to send a revised patch. linear-r[ao]m linear-mapped-r[ao]m mapped-r[ao]m memory-mapped-r[ao]m physmap-r[ao]m > > not-quite-correct main memory probing code. > > You mean the there's parsers that search the "compatible" prop for > "memory" as well as "device_type" prop? -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpS3dubbrOIr.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 1/2] [MTD] Add support for RAM & ROM mappings in the physmap_of MTD driver.
On Wednesday 26 March 2008 16:34, Segher Boessenkool wrote: > >>> { > >>> +.compatible= "physmap-ram", > >>> +.data= (void *)"map_ram", > >>> +}, > >>> +{ > >>> +.compatible= "physmap-rom", > >>> +.data= (void *)"map_rom", > >>> +}, > > > >> Why the cast? It's redundant afaics. > > > > To be in line with the surrounding code... > > I see _that_, but it's not a great argument IMNSHO. Could I trick > you into preceding this patch with a cleanup patch for the existing > casts? Ok. I'll submit a new patch as soon as we agree on a compatible name. -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.
On Wednesday 26 March 2008 17:59, Scott Wood wrote: > Laurent Pinchart wrote: > > @@ -138,7 +137,7 @@ > > device_type = "serial"; > > compatible = "fsl,mpc8248-smc-uart", > > "fsl,cpm2-smc-uart"; > > - reg = <0x11a80 0x20 0x1100 0x40>; > > + reg = <0x11a80 0x20 0x87fc 2>; > > interrupts = <4 8>; > > interrupt-parent = <&PIC>; > > fsl,cpm-brg = <7>; > > This breaks the bootwrapper console. And of course I forgot about that :-) The boot wrapper code doesn't have any dpram allocator. Any objection against using a chunk of dpram at a fixed location ? What about at the beginning of the dpram ? The DTS muram node would then exclude a chunk of dpram at offset 0x instead of 0x1100. -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgp5VU0QpinLz.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCH 2/2] [POWERPC] Describe memory-mapped RAM&ROM chips of bindings
On Wednesday 26 March 2008 15:52, Segher Boessenkool wrote: > > +Dedicated RAM and ROM chips are often used as storage for > > temporary or > > +permanent data in embedded devices. Possible usage include > > non-volatile > > +storage in battery-backed SRAM, semi-permanent storage in > > dedicated SRAM > > +to preserve data accross reboots and firmware storage in > > dedicated ROM. > > + > > + - compatible : should contain the specific model of RAM/ROM > > chip(s) > > + used, if known, followed by either "physmap-ram" or > > "physmap-rom" > > + - reg : Address range of the RAM/ROM chip > > + - bank-width : Width (in bytes) of the RAM/ROM bank. Equal to the > > + device width times the number of interleaved chips. > > + - device-width : (optional) Width of a single RAM/ROM chip. If > > + omitted, assumed to be equal to 'bank-width'. > > Maybe I'm rehashing some old discussion here, if so, sorry; but why > do you have bank-width and device-width here? What useful information > does it provide? If this is about saying what the preferred (or only > possible) access width is, better names are in order. device-width isn't used so we can get rid of it. bank-width is used by the map_ram driver for erase operations (mapram_erase in drivers/mtd/chips/map_ram.c). To be honest I'm not sure why it uses such an inefficient approach instead of memsetting the whole area. -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: OF compatible MTD platform RAM driver ?
On Thursday 27 March 2008 11:03, David Gibson wrote: > On Thu, Mar 27, 2008 at 10:13:32AM +0100, Laurent Pinchart wrote: > > On Wednesday 26 March 2008 13:53, Sergei Shtylyov wrote: > > > Laurent Pinchart wrote: > [snip] > > > Heh, we've gone thru "physmap" before -- it was labelled > > > Linux-specific name (well, I'd agree with that). > > > > physmap stands for physically mapped. That doesn't sound > > Linux-specific to me, the fact that the MTD driver has the same name > > is a pure coincidence. linmap-rom and linmap-rom sound even more > > Linux-specific :-) > > It may not be Linux specific per se, but it's a bad name, because the > fact that the device is physically direct mapped isn't a useful > distinguishing feature of the device. Main memory is also direct > physically mapped, after all, but that's not what you want to cover > with this description. In general how a device is wired is described > by where it sits in the tree, not by its properties. > > It only seems like a usefully distinguishing name because it's the > Linux "physmap_of" driver that uses it. So in this sense it is a > Linux specific name after all. In fact, physmap_of is itself very > badly named - right now it only handles direct mapped mtds, but that's > not inherent; it could be trivially extended to also instantiate a > non-direct-mapped device (as long as the underlying mtd layer > supported it, of course). It bears no relation at all to the > "physmap" driver, except historical accident. > > > Could we agree on a name ? I'd like to submit a new patch. > > For ROMs I think just plain "rom" should be sufficient. For RAMs we > need something to indicate that it's memory but intended for secondary > storage, not as main memory. Unfortunately, I'm finding myself unable > to think of something. What about "storage-ram", "auxiliary-ram", "secondary-ram", "application-ram", "user-ram" or "ramdisk" ? -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.
On Thursday 27 March 2008 16:39, Scott Wood wrote: > On Thu, Mar 27, 2008 at 10:10:33AM +0100, Laurent Pinchart wrote: > > On Wednesday 26 March 2008 17:59, Scott Wood wrote: > > > This breaks the bootwrapper console. > > > > And of course I forgot about that :-) > > > > The boot wrapper code doesn't have any dpram allocator. Any objection > > against > > using a chunk of dpram at a fixed location ? What about at the beginning of > > the dpram ? The DTS muram node would then exclude a chunk of dpram at > > offset > > 0x instead of 0x1100. > > I'm not entirely comfortable with using a chunk outside of what's in the > muram node, and assuming that it's for the SMC pram -- what if there's > microcode or something there? > > Since udbg is only for debugging, and is marked as potentially dangerous, > how about just using the end of muram (as described in the device tree)? > If the muram is fully allocated, it won't happen until after the real > serial console is initialized. Very good idea. I'll prepare a new patch. -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpBDePsDZGE8.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.
On Friday 28 March 2008 14:58, Laurent Pinchart wrote: > On Thursday 27 March 2008 16:39, Scott Wood wrote: > > On Thu, Mar 27, 2008 at 10:10:33AM +0100, Laurent Pinchart wrote: > > > On Wednesday 26 March 2008 17:59, Scott Wood wrote: > > > > This breaks the bootwrapper console. > > > > > > And of course I forgot about that :-) > > > > > > The boot wrapper code doesn't have any dpram allocator. Any objection against > > > using a chunk of dpram at a fixed location ? What about at the beginning of > > > the dpram ? The DTS muram node would then exclude a chunk of dpram at offset > > > 0x instead of 0x1100. > > > > I'm not entirely comfortable with using a chunk outside of what's in the > > muram node, and assuming that it's for the SMC pram -- what if there's > > microcode or something there? > > > > Since udbg is only for debugging, and is marked as potentially dangerous, > > how about just using the end of muram (as described in the device tree)? > > If the muram is fully allocated, it won't happen until after the real > > serial console is initialized. > > Very good idea. I'll prepare a new patch. arch/powerpc/boot/cpm-serial.c stores the udbg buffer descriptors at the beginning of the muram. Should I move them at the end as well ? -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpWJ7Sujdp9M.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.
On Thursday 27 March 2008 16:39, Scott Wood wrote: > On Thu, Mar 27, 2008 at 10:10:33AM +0100, Laurent Pinchart wrote: > > On Wednesday 26 March 2008 17:59, Scott Wood wrote: > > > This breaks the bootwrapper console. > > > > And of course I forgot about that :-) > > > > The boot wrapper code doesn't have any dpram allocator. Any objection > > against using a chunk of dpram at a fixed location ? What about at the > > beginning of the dpram ? The DTS muram node would then exclude a chunk of > > dpram at offset 0x instead of 0x1100. > > I'm not entirely comfortable with using a chunk outside of what's in the > muram node, and assuming that it's for the SMC pram -- what if there's > microcode or something there? > > Since udbg is only for debugging, and is marked as potentially dangerous, > how about just using the end of muram (as described in the device tree)? > If the muram is fully allocated, it won't happen until after the real > serial console is initialized. Locating the end of the muram isn't as straightforward as it could be. As the current code already uses the beginning of the muram to store the BDs and data buffers, should I really bother locating the end or can I store the SMC parameter ram at the beginning as well ? If I'm not mistaken, once the SMC parameter ram gets relocated to the beginning/end of the muram, the boot loader preallocated space can be reclaimed and can be added to the muram in the device tree like I did in my previous patch. Is that correct ? -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpHxgjsPOIfJ.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.
On Friday 28 March 2008 18:11, Scott Wood wrote: > Laurent Pinchart wrote: > > Locating the end of the muram isn't as straightforward as it could be. As > > the current code already uses the beginning of the muram to store the BDs > > and data buffers, should I really bother locating the end or can I store > > the SMC parameter ram at the beginning as well ? > > Maybe, but the end would be safer. What's the problem with finding the > end? That requires manual parsing of all the cells in the reg property. The device-tree API doesn't provide a way to get the length of a property, so I'll have to use a big enough pre-allocated buffer. I'm also not sure if resources are guaranteed to be sorted in increasing order. This doesn't make finding the end of the muram really difficult. I was just wondering if the increased code complexity was worth it, especially seeing how the cpm_serial code in the boot wrapper seem quite unstable. I'm not familiar with the boot wrapper code so I'm sometimes not very confident in my assumptions, but isn't the handling of the virtual-reg property in cpm_console_init broken ? void *reg_virt[2]; ... n = getprop(devp, "virtual-reg", reg_virt, sizeof(reg_virt)); if (n < (int)sizeof(reg_virt)) { for (n = 0; n < 2; n++) { if (!dt_xlate_reg(devp, n, ®_phys, NULL)) return -1; reg_virt[n] = (void *)reg_phys; } } if (is_smc) smc = reg_virt[0]; else scc = reg_virt[0]; param = reg_virt[1]; If I'm not mistaken, getprop will return the address and size of the first resource and not the addresses of the first two resources. What is virtual-reg used for ? To report the virtual address without requiring a device tree walk ? Does it provide any information that dt_xlate_reg can't find ? > Even the end of the first reg resource would be OK. If I use the end of the first resource, can I assume it spans 0x - 0x8000 to set the default tx BD address in Kconfig ? > > If I'm not mistaken, once the SMC parameter ram gets relocated to the > > beginning/end of the muram, the boot loader preallocated space can be > > reclaimed and can be added to the muram in the device tree like I did in > > my previous patch. Is that correct ? > > Yes. -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpHOVwIw7v9s.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: OF compatible MTD platform RAM driver ?
On Monday 31 March 2008 00:39, Segher Boessenkool wrote: > >>> For RAMs we > >>> need something to indicate that it's memory but intended for > >>> secondary > >>> storage, not as main memory. > >> > >> How it is intended to be used is not a property of the hardware, so > >> that information doesn't belong in the device tree at all. The Linux > >> platform code should handle this, I imagine. > > > > There must be some reason why it is not intended to be used as main > > memory. Presumably it has something different about it compared to > > "normal" RAM, and that difference could perfectly well be expressed in > > the device tree. > > Sure, that's a different thing. It might sit on a bus that doesn't > do cache coherency, or maybe it's just slow (or sits on a slow bus). > All these things can be usefully expressed in the device tree (but > typically are not, it is left to the client code to know this stuff > implicitly). > > It's still the (platform) probe code its responsibility to figure > out what (if anything) to do with any device. And "main memory" > is probed differently (via /chosen/memory, for example) anyway. > Well, actually, Linux searches for all nodes with device_type "memory", > which should work fine as well [*]. > > So, all in all, I think we should just give these "auxiliary memory" > devices a name of "ram" c.q. "rom", and some "reg", and that should > be all that is needed: the main memory probe stuff won't consider > these nodes, and the (platform) device probe code can do whatever it > wants (create mtd devices, I guess). Ok, I get your point. I'll prepare a new documentation patch; changes to physmap_of.c will go away. If I understand you correctly, there should be no "compatible" property on the ram and rom devices. Should the "non-volatile", "slow" and "static ram" properties still be expressed in the device tree ? -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
Re: [PATCHv2 2/3] ep8248e: Reference SMC parameter RAM base in the device tree.
On Friday 28 March 2008 19:07, Scott Wood wrote: > Laurent Pinchart wrote: > > On Friday 28 March 2008 18:11, Scott Wood wrote: > >> Laurent Pinchart wrote: > >>> Locating the end of the muram isn't as straightforward as it > >>> could be. As the current code already uses the beginning of the > >>> muram to store the BDs and data buffers, should I really bother > >>> locating the end or can I store the SMC parameter ram at the > >>> beginning as well ? > >> Maybe, but the end would be safer. What's the problem with finding > >> the end? > > > > That requires manual parsing of all the cells in the reg property. > > The device-tree API doesn't provide a way to get the length of a > > property, > > Sure it does. Do a getprop with an insufficiently large buffer, and it > tells you how much you really need. :-) Ok thanks. > > so I'll have to use a big enough pre-allocated buffer. I'm also not > > sure if resources are guaranteed to be sorted in increasing order. > > Ah, good point. > > > This doesn't make finding the end of the muram really difficult. I > > was just wondering if the increased code complexity was worth it, > > especially seeing how the cpm_serial code in the boot wrapper seem > > quite unstable. > > Unstable in what way? I was refering to the virtual-reg (non-)issue I mentionned below. > > I'm not familiar with the boot wrapper code so I'm sometimes not very > > confident in my assumptions, but isn't the handling of the > > virtual-reg property in cpm_console_init broken ? > > Not as far as I can see. > > > If I'm not mistaken, getprop will return the address and size of the > > first resource and not the addresses of the first two resources. > > No, it'll get as much of the virtual-reg property as will fit in the > buffer. There's no size in virtual-reg. Ah right. Sorry about the misunderstanding. > > What is virtual-reg used for ? To report the virtual address without > > requiring a device tree walk ? Does it provide any information that > > dt_xlate_reg can't find ? > > Yes, it tells you the virtual address when it's not an identity mapping. > It's not currently used on CPM platforms, but might be used down the > road with a QE device on 85xx. Will the virtual-reg property on the muram node list the addresses of all muram chunks or the address of the first chunk only ? > >> Even the end of the first reg resource would be OK. > > > > If I use the end of the first resource, can I assume it spans 0x > > - 0x8000 to set the default tx BD address in Kconfig ? > > No, especially seeing as it doesn't on any existing boards. :-) I still need a default value :-) It obviously won't work for all boards. > You could set the default to just before 0x2000 with board-specific > exceptions, though. We're getting a bit lost. I'll try to summarize the discussion. - The muram node has a reg property that lists the offsets and sizes of all muram chunks, and an optional virtual-reg property that lists the virtual address of all chunks/the first chunk only. - From the above information I can locate a section of muram at the end of the first chunk (easy) or at the end of the muram (not really difficult, just a bit more complex, especially if chunks are not sorted by their start address). - Kconfig needs a default address for the tx BD. This depends on the allocation strategy (end of first chunk vs. end of last chunk). Is there some consistent default across QE devices ? -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgpl4GQ0cQTU2.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCHv3 4/4] cpm2: Reset the CPM when early debugging is not enabled.
Similarly to what is done for PQ1-based platforms, this patch resets the PQ2 Communication Processor Module in cpm2_reset() when early debugging is not enabled. This helps avoiding conflicts when the boot loader configured the CPM in an unexpected way. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- arch/powerpc/sysdev/cpm2.c |6 ++ 1 files changed, 6 insertions(+), 0 deletions(-) diff --git a/arch/powerpc/sysdev/cpm2.c b/arch/powerpc/sysdev/cpm2.c index 7be7112..57ed1a4 100644 --- a/arch/powerpc/sysdev/cpm2.c +++ b/arch/powerpc/sysdev/cpm2.c @@ -80,6 +80,12 @@ void __init cpm2_reset(void) /* Tell everyone where the comm processor resides. */ cpmp = &cpm2_immr->im_cpm; + +#ifndef CONFIG_PPC_EARLY_DEBUG_CPM + /* Reset the CPM. +*/ + cpm_command(CPM_CR_RST, 0); +#endif } static DEFINE_SPINLOCK(cmd_lock); -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgp8BChQLwtBd.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCHv3 3/4] ep8248e: Reference SMC parameter RAM base in the device tree.
This patch modifies the Embedded Planet EP8248E device tree to reference the SMC paramater RAM base register instead of the parameter RAM allocated by the boot loader. The cpm_uart driver will allocate parameter RAM itself, making the serial port initialisation independent of the boot loader. The patch adds the parameter RAM allocated by the boot loader in the CPM muram node, making it available to the kernel. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- arch/powerpc/boot/dts/ep8248e.dts |5 ++--- 1 files changed, 2 insertions(+), 3 deletions(-) diff --git a/arch/powerpc/boot/dts/ep8248e.dts b/arch/powerpc/boot/dts/ep8248e.dts index 5d2fb76..756758f 100644 --- a/arch/powerpc/boot/dts/ep8248e.dts +++ b/arch/powerpc/boot/dts/ep8248e.dts @@ -121,8 +121,7 @@ [EMAIL PROTECTED] { compatible = "fsl,cpm-muram-data"; - reg = <0 0x1100 0x1140 - 0xec0 0x9800 0x800>; + reg = <0 0x2000 0x9800 0x800>; }; }; @@ -138,7 +137,7 @@ device_type = "serial"; compatible = "fsl,mpc8248-smc-uart", "fsl,cpm2-smc-uart"; - reg = <0x11a80 0x20 0x1100 0x40>; + reg = <0x11a80 0x20 0x87fc 2>; interrupts = <4 8>; interrupt-parent = <&PIC>; fsl,cpm-brg = <7>; -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgprSrdFgLane.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCHv3 0/4] cpm2: Reset the CPM at startup and fix the cpm_uart driver accordingly.
Hi everybody, these 4 patches reset the CPM in cpm2_reset() and fix the cpm_uart driver to initialise SMC ports correctly without relying on any initialisation performed by the boot loader/wrapper. They update the boot wrapper code and the EP8248E device tree to match the new SMC registers description. -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 pgptoxnUKyRkU.pgp Description: PGP signature ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCHv3 1/4] cpm_uart: Allocate DPRAM memory for SMC ports on CPM2-based platforms.
This patch allocates parameter RAM for SMC serial ports without relying on previous initialisation by a boot loader or a wrapper layer. SMC parameter RAM on CPM2-based platforms can be allocated anywhere in the general-purpose areas of the dual-port RAM. The current code relies on the boot loader to allocate a section of general-purpose CPM RAM and gets the section address from the device tree. This patch modifies the device tree address usage to reference the SMC parameter RAM base pointer instead of a pre-allocated RAM section and allocates memory from the CPM dual-port RAM when initialising the SMC port. CPM1-based platforms are not affected. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- drivers/serial/cpm_uart/cpm_uart.h |3 ++ drivers/serial/cpm_uart/cpm_uart_core.c | 19 +-- drivers/serial/cpm_uart/cpm_uart_cpm1.c | 12 +++ drivers/serial/cpm_uart/cpm_uart_cpm2.c | 52 +++ 4 files changed, 76 insertions(+), 10 deletions(-) diff --git a/drivers/serial/cpm_uart/cpm_uart.h b/drivers/serial/cpm_uart/cpm_uart.h index 80a7d60..5334653 100644 --- a/drivers/serial/cpm_uart/cpm_uart.h +++ b/drivers/serial/cpm_uart/cpm_uart.h @@ -93,6 +93,9 @@ extern struct uart_cpm_port cpm_uart_ports[UART_NR]; /* these are located in their respective files */ void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd); +void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port, + struct device_node *np); +void cpm_uart_unmap_pram(struct uart_cpm_port *port, void __iomem *pram); int cpm_uart_init_portdesc(void); int cpm_uart_allocbuf(struct uart_cpm_port *pinfo, unsigned int is_con); void cpm_uart_freebuf(struct uart_cpm_port *pinfo); diff --git a/drivers/serial/cpm_uart/cpm_uart_core.c b/drivers/serial/cpm_uart/cpm_uart_core.c index 1ea123c..3a44a3f 100644 --- a/drivers/serial/cpm_uart/cpm_uart_core.c +++ b/drivers/serial/cpm_uart/cpm_uart_core.c @@ -997,24 +997,23 @@ static int cpm_uart_init_port(struct device_node *np, if (!mem) return -ENOMEM; - pram = of_iomap(np, 1); - if (!pram) { - ret = -ENOMEM; - goto out_mem; - } - if (of_device_is_compatible(np, "fsl,cpm1-scc-uart") || of_device_is_compatible(np, "fsl,cpm2-scc-uart")) { pinfo->sccp = mem; - pinfo->sccup = pram; + pinfo->sccup = pram = cpm_uart_map_pram(pinfo, np); } else if (of_device_is_compatible(np, "fsl,cpm1-smc-uart") || of_device_is_compatible(np, "fsl,cpm2-smc-uart")) { pinfo->flags |= FLAG_SMC; pinfo->smcp = mem; - pinfo->smcup = pram; + pinfo->smcup = pram = cpm_uart_map_pram(pinfo, np); } else { ret = -ENODEV; - goto out_pram; + goto out_mem; + } + + if (!pram) { + ret = -ENOMEM; + goto out_mem; } pinfo->tx_nrfifos = TX_NUM_FIFO; @@ -1038,7 +1037,7 @@ static int cpm_uart_init_port(struct device_node *np, return cpm_uart_request_port(&pinfo->port); out_pram: - iounmap(pram); + cpm_uart_unmap_pram(pinfo, pram); out_mem: iounmap(mem); return ret; diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm1.c b/drivers/serial/cpm_uart/cpm_uart_cpm1.c index 6ea0366..e692593 100644 --- a/drivers/serial/cpm_uart/cpm_uart_cpm1.c +++ b/drivers/serial/cpm_uart/cpm_uart_cpm1.c @@ -54,6 +54,18 @@ void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd) { cpm_command(port->command, cmd); } + +void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port, + struct device_node *np) +{ + return of_iomap(np, 1); +} + +void cpm_uart_unmap_pram(struct uart_cpm_port *port, void __iomem *pram) +{ + iounmap(pram); +} + #else void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd) { diff --git a/drivers/serial/cpm_uart/cpm_uart_cpm2.c b/drivers/serial/cpm_uart/cpm_uart_cpm2.c index 6291094..a4cfb0b 100644 --- a/drivers/serial/cpm_uart/cpm_uart_cpm2.c +++ b/drivers/serial/cpm_uart/cpm_uart_cpm2.c @@ -41,6 +41,9 @@ #include #include #include +#ifdef CONFIG_PPC_CPM_NEW_BINDING +#include +#endif #include #include @@ -54,6 +57,55 @@ void cpm_line_cr_cmd(struct uart_cpm_port *port, int cmd) { cpm_command(port->command, cmd); } + +void __iomem *cpm_uart_map_pram(struct uart_cpm_port *port, + struct device_node *np) +{ + void __iomem *pram; + unsigned long offset; + struct resource res; + unsigned long len; + + /* Don't remap parameter RAM if it has already been initialized +* during console setup. +*/ + if (IS_SMC(port) && port->smcup) + re
[PATCHv2] powerpc: Describe memory-mapped RAM&ROM chips OF bindings
Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- Documentation/powerpc/booting-without-of.txt | 13 - 1 files changed, 12 insertions(+), 1 deletions(-) diff --git a/Documentation/powerpc/booting-without-of.txt b/Documentation/powerpc/booting-without-of.txt index 7b4e8a7..3e1963b 100644 --- a/Documentation/powerpc/booting-without-of.txt +++ b/Documentation/powerpc/booting-without-of.txt @@ -57,7 +57,8 @@ Table of Contents n) 4xx/Axon EMAC ethernet nodes o) Xilinx IP cores p) Freescale Synchronous Serial Interface - q) USB EHCI controllers + q) USB EHCI controllers + r) Memory-mapped RAM & ROM VII - Specifying interrupt information for devices 1) interrupts property @@ -2816,6 +2817,16 @@ platforms are moved over to use the flattened-device-tree model. big-endian; }; + r) Memory-mapped RAM & ROM + +Dedicated RAM and ROM chips are often used as storage for temporary or +permanent data in embedded devices. Possible usage include non-volatile +storage in battery-backed SRAM, semi-permanent storage in dedicated SRAM +to preserve data accross reboots and firmware storage in dedicated ROM. + + - name : should be either "ram" or "rom" + - reg : Address range of the RAM/ROM chip + More devices will be defined as this spec matures. -- 1.5.0 -- Laurent Pinchart CSE Semaphore Belgium Chaussée de Bruxelles, 732A B-1410 Waterloo Belgium T +32 (2) 387 42 59 F +32 (2) 387 42 75 ___ Linuxppc-dev mailing list Linuxppc-dev@ozlabs.org https://ozlabs.org/mailman/listinfo/linuxppc-dev
[PATCHv3 2/4] cpm-serial: Relocate CPM buffer descriptors and SMC parameter ram.
This patch relocates the buffer descriptors and the SMC parameter RAM at the end of the first CPM muram chunk, as described in the device tree. This allows device trees to stop excluding SMC parameter ram allocated by the boot loader from the CPM muram node. Signed-off-by: Laurent Pinchart <[EMAIL PROTECTED]> --- arch/powerpc/Kconfig.debug |2 +- arch/powerpc/boot/cpm-serial.c | 132 ++-- 2 files changed, 87 insertions(+), 47 deletions(-) diff --git a/arch/powerpc/Kconfig.debug b/arch/powerpc/Kconfig.debug index db7cc34..a86d8d8 100644 --- a/arch/powerpc/Kconfig.debug +++ b/arch/powerpc/Kconfig.debug @@ -269,7 +269,7 @@ config PPC_EARLY_DEBUG_CPM_ADDR hex "CPM UART early debug transmit descriptor address" depends on PPC_EARLY_DEBUG_CPM default "0xfa202008" if PPC_EP88XC - default "0xf008" if CPM2 + default "0xf0001ff8" if CPM2 default "0xff002008" if CPM1 help This specifies the address of the transmit descriptor diff --git a/arch/powerpc/boot/cpm-serial.c b/arch/powerpc/boot/cpm-serial.c index 28296fa..8d02b2d 100644 --- a/arch/powerpc/boot/cpm-serial.c +++ b/arch/powerpc/boot/cpm-serial.c @@ -11,6 +11,7 @@ #include "types.h" #include "io.h" #include "ops.h" +#include "page.h" struct cpm_scc { u32 gsmrl; @@ -42,6 +43,22 @@ struct cpm_param { u16 tbase; u8 rfcr; u8 tfcr; + u16 mrblr; + u32 rstate; + u8 res1[4]; + u16 rbptr; + u8 res2[6]; + u32 tstate; + u8 res3[4]; + u16 tbptr; + u8 res4[6]; + u16 maxidl; + u16 idlc; + u16 brkln; + u16 brkec; + u16 brkcr; + u16 rmask; + u8 res5[4]; }; struct cpm_bd { @@ -54,10 +71,10 @@ static void *cpcr; static struct cpm_param *param; static struct cpm_smc *smc; static struct cpm_scc *scc; -struct cpm_bd *tbdf, *rbdf; +static struct cpm_bd *tbdf, *rbdf; static u32 cpm_cmd; -static u8 *muram_start; -static u32 muram_offset; +static void *cbd_addr; +static u32 cbd_offset; static void (*do_cmd)(int op); static void (*enable_port)(void); @@ -119,20 +136,25 @@ static int cpm_serial_open(void) out_8(¶m->rfcr, 0x10); out_8(¶m->tfcr, 0x10); - - rbdf = (struct cpm_bd *)muram_start; - rbdf->addr = (u8 *)(rbdf + 2); + out_be16(¶m->mrblr, 1); + out_be16(¶m->maxidl, 0); + out_be16(¶m->brkec, 0); + out_be16(¶m->brkln, 0); + out_be16(¶m->brkcr, 0); + + rbdf = cbd_addr; + rbdf->addr = (u8 *)rbdf - 1; rbdf->sc = 0xa000; rbdf->len = 1; tbdf = rbdf + 1; - tbdf->addr = (u8 *)(rbdf + 2) + 1; + tbdf->addr = (u8 *)rbdf - 2; tbdf->sc = 0x2000; tbdf->len = 1; sync(); - out_be16(¶m->rbase, muram_offset); - out_be16(¶m->tbase, muram_offset + sizeof(struct cpm_bd)); + out_be16(¶m->rbase, cbd_offset); + out_be16(¶m->tbase, cbd_offset + sizeof(struct cpm_bd)); do_cmd(CPM_CMD_INIT_RX_TX); @@ -173,12 +195,31 @@ static unsigned char cpm_serial_getc(void) return c; } +static int cpm_get_virtual_address(void *devp, void **addr, int ncells) +{ + unsigned long xaddr; + int n; + + n = getprop(devp, "virtual-reg", addr, ncells * sizeof *addr); + if (n < ncells * sizeof *addr) { + for (n = 0; n < ncells; n++) { + if (!dt_xlate_reg(devp, n, &xaddr, NULL)) + return -1; + + addr[n] = (void*)xaddr; + } + } + + return ncells; +} + int cpm_console_init(void *devp, struct serial_console_data *scdp) { - void *reg_virt[2]; - int is_smc = 0, is_cpm2 = 0, n; - unsigned long reg_phys; + void *reg[2]; void *parent, *muram; + void *muram_addr; + int is_smc = 0, is_cpm2 = 0; + unsigned long muram_offset, muram_size; if (dt_is_compatible(devp, "fsl,cpm1-smc-uart")) { is_smc = 1; @@ -202,63 +243,62 @@ int cpm_console_init(void *devp, struct serial_console_data *scdp) else do_cmd = cpm1_cmd; - n = getprop(devp, "fsl,cpm-command", &cpm_cmd, 4); - if (n < 4) + if (getprop(devp, "fsl,cpm-command", &cpm_cmd, 4) < sizeof cpm_cmd) return -1; - n = getprop(devp, "virtual-reg", reg_virt, sizeof(reg_virt)); - if (n < (int)sizeof(reg_virt)) { - for (n = 0; n < 2; n++) { - if (!dt_xlate_reg(devp, n, ®_phys, NULL)) - return -1; - - reg_virt[n] = (void *)reg_phys; - } - } + if