Re: [PATCH v2 1/9] mm: Introduce new vm_insert_range API

2018-12-02 Thread Mike Rapoport
On Mon, Dec 03, 2018 at 09:51:45AM +0530, Souptick Joarder wrote:
> Hi Mike,
> 
> On Sun, Dec 2, 2018 at 4:43 PM Mike Rapoport  wrote:
> >
> > On Sun, Dec 02, 2018 at 11:49:44AM +0530, Souptick Joarder wrote:
> > > Previouly drivers have their own way of mapping range of
> > > kernel pages/memory into user vma and this was done by
> > > invoking vm_insert_page() within a loop.
> > >
> > > As this pattern is common across different drivers, it can
> > > be generalized by creating a new function and use it across
> > > the drivers.
> > >
> > > vm_insert_range is the new API which will be used to map a
> > > range of kernel memory/pages to user vma.
> > >
> > > This API is tested by Heiko for Rockchip drm driver, on rk3188,
> > > rk3288, rk3328 and rk3399 with graphics.
> > >
> > > Signed-off-by: Souptick Joarder 
> > > Reviewed-by: Matthew Wilcox 
> > > Tested-by: Heiko Stuebner 
> > > ---
> > >  include/linux/mm_types.h |  3 +++
> > >  mm/memory.c  | 38 ++
> > >  mm/nommu.c   |  7 +++
> > >  3 files changed, 48 insertions(+)
> > >
> > > diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
> > > index 5ed8f62..15ae24f 100644
> > > --- a/include/linux/mm_types.h
> > > +++ b/include/linux/mm_types.h
> > > @@ -523,6 +523,9 @@ extern void tlb_gather_mmu(struct mmu_gather *tlb, 
> > > struct mm_struct *mm,
> > >  extern void tlb_finish_mmu(struct mmu_gather *tlb,
> > >   unsigned long start, unsigned long end);
> > >
> > > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > > + struct page **pages, unsigned long page_count);
> > > +
> >
> > This seem to belong to include/linux/mm.h, near vm_insert_page()
> 
> Ok, I will change it. Apart from this change does it looks good ?

With this change you can add

Reviewed-by: Mike Rapoport 
 
> >
> > >  static inline void init_tlb_flush_pending(struct mm_struct *mm)
> > >  {
> > >   atomic_set(>tlb_flush_pending, 0);
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index 15c417e..84ea46c 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -1478,6 +1478,44 @@ static int insert_page(struct vm_area_struct *vma, 
> > > unsigned long addr,
> > >  }
> > >
> > >  /**
> > > + * vm_insert_range - insert range of kernel pages into user vma
> > > + * @vma: user vma to map to
> > > + * @addr: target user address of this page
> > > + * @pages: pointer to array of source kernel pages
> > > + * @page_count: number of pages need to insert into user vma
> > > + *
> > > + * This allows drivers to insert range of kernel pages they've allocated
> > > + * into a user vma. This is a generic function which drivers can use
> > > + * rather than using their own way of mapping range of kernel pages into
> > > + * user vma.
> > > + *
> > > + * If we fail to insert any page into the vma, the function will return
> > > + * immediately leaving any previously-inserted pages present.  Callers
> > > + * from the mmap handler may immediately return the error as their caller
> > > + * will destroy the vma, removing any successfully-inserted pages. Other
> > > + * callers should make their own arrangements for calling unmap_region().
> > > + *
> > > + * Context: Process context. Called by mmap handlers.
> > > + * Return: 0 on success and error code otherwise
> > > + */
> > > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > > + struct page **pages, unsigned long page_count)
> > > +{
> > > + unsigned long uaddr = addr;
> > > + int ret = 0, i;
> > > +
> > > + for (i = 0; i < page_count; i++) {
> > > + ret = vm_insert_page(vma, uaddr, pages[i]);
> > > + if (ret < 0)
> > > + return ret;
> > > + uaddr += PAGE_SIZE;
> > > + }
> > > +
> > > + return ret;
> > > +}
> > > +EXPORT_SYMBOL(vm_insert_range);
> > > +
> > > +/**
> > >   * vm_insert_page - insert single page into user vma
> > >   * @vma: user vma to map to
> > >   * @addr: target user address of this page
> > > diff --git a/mm/nommu.c b/mm/nommu.c
> > > index 749276b..d6ef5c7 100644
> > > --- a/mm/nommu.c
> > > +++ b/mm/nommu.c
> > > @@ -473,6 +473,13 @@ int vm_insert_page(struct vm_area_struct *vma, 
> > > unsigned long addr,
> > >  }
> > >  EXPORT_SYMBOL(vm_insert_page);
> > >
> > > +int vm_insert_range(struct vm_area_struct *vma, unsigned long addr,
> > > + struct page **pages, unsigned long page_count)
> > > +{
> > > + return -EINVAL;
> > > +}
> > > +EXPORT_SYMBOL(vm_insert_range);
> > > +
> > >  /*
> > >   *  sys_brk() for the most part doesn't need the global kernel
> > >   *  lock, except when an application is doing something nasty
> > > --
> > > 1.9.1
> > >
> >
> > --
> > Sincerely yours,
> > Mike.
> >
> 

-- 
Sincerely yours,
Mike.



Re: rt-mutex usage in i2c

2015-03-15 Thread Mike Rapoport
On Sat, Mar 14, 2015 at 1:32 PM, Wolfram Sang w...@the-dreams.de wrote:
 On Sat, Mar 14, 2015 at 12:27:03PM +0100, Wolfram Sang wrote:
 Hi Sebastian,

  - i2c_transfer() has this piece:
2091 if (in_atomic() || irqs_disabled()) {
2092 ret = i2c_trylock_adapter(adap);
 
is this irqs_disabled() is what bothers me and should not be there.
pxa does a spin_lock_irq() which would enable interrupts on return /
too early.
mxs has a wait_for_completion() which needs irqs enabled _and_ makes
in_atomic() problematic, too. I have't checked other drivers but the
commit, that introduced it, does not explain why it is required.

That was some time ago, but as far as I remember, PIO in i2c_pxa was
required to enable communication with PMIC in interrupt context.


 I haven't really looked into it, but a quick search gave me this thread
 explaining the intention of the code in question:

 http://lists.lm-sensors.org/pipermail/i2c/2007-November/002268.html

 Regards,

Wolfram


 And adding a recent mail address from Mike to cc.




-- 
Sincerely yours,
Mike.
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Bug in AMDs V4L2 driver lxv4l2?

2010-05-17 Thread Mike Rapoport

Hi Daniel,

Daniel Borkmann wrote:

Hi everyone,

I am using an AMD Geode webcam with a V4L driver (lxv4l2). For the userspace 
I've implemented a V4L
binding with memory mapping of the frames. After sucessfull receiving frames it 
lasts about two or
three minutes and then either the timestamp of the frame is not changing 
anymore or a kernel oops
happens. I am not quite sure whether this is caused by my userspace test program 
(fw_singapore)
or whether this is a bug within AMDs driver code ...


I while ago I've done some work on the Geode LX V4L driver. It was not 
cleaned up enough to be submitted, but probably you'll find it useful.


http://compulab.co.il/workspace/mediawiki/index.php5?title=CM-iGLX:_Linux:_Video_Input_Port


Thanks for help.
Cheers,
Daniel

Here the causing driver function within AMDs lx.c (and below the dmesg output):

Nevertheless, I noticed that this function is also a potential deadlock 
candidate since
spin_lock_irqsave(io-lock, flags); is called an the else return 0; does 
not release the lock.


/** lx_capt_resume2 - queue capture buffers to vip */
int
lx_capt_resume2(VidDevice *dp,io_queue *io)
{
   io_buf *op, *ep;
   int eidx, oidx, vip_buffers;
   int task = dp-capt_vip_task;
   int task_buffers = vip_task_data[task].task_buffers;
   VIPINPUTBUFFER *vip_inpbfr = dp-vip_inpbfr;

   unsigned long flags;
   struct list_head *lp;
   io_buf *bp1;

   dp-capt_stalled = 1;
   task = dp-capt_vip_task;
   task_buffers = vip_task_data[task].task_buffers;

   if( dp-capt_addr_is_set == 0 ) {
  op = dp-capt_toggle == capt_toggle_odd ? dp-capt_elch : dp-capt_onxt;
  if( op == NULL ) {
 if( list_empty(io-rd_qbuf) != 0 )
 {
   // there are no more buffers into the input list for grabbing images,
   // so requeue first output buffer into input list

   spin_lock_irqsave(io-lock, flags);

   lp = io-rd_dqbuf;
   if( ! list_empty(lp) ) {
  bp1 = list_entry(lp-next,io_buf,bfrq);// get the struct for 
this entry / list_entry ( ptr, type, member) struct list_head pointer/ type of 
the struct this is embedded in/
name of the list_struct within the struct.
  list_del_init(bp1-bfrq); //deletes entry from list and 
reinitialize it
   }
   else return 0;

   lp = io-rd_qbuf;
   list_move_tail(bp1-bfrq,lp);

   bp1-sequence = io-sequence++;
   bp1-flags = ~V4L2_BUF_FLAG_DONE;
   bp1-flags |= V4L2_BUF_FLAG_QUEUED;

   if( dp-capt_stalled != 0 )
   {
  DMSG(3, v4l_qbfr : capt != 0  dp-capt_stalled != 
0\n);
  //v4l_capt_unstall(dp);
   }

   spin_unlock_irqrestore(io-lock,flags);
   //return 0;
 }

 op = list_entry(io-rd_qbuf.next,io_buf,bfrq);
 list_del_init(op-bfrq);
  }
  if( dp-capt_toggle == capt_toggle_both ||
  dp-capt_toggle == capt_toggle_odd ) {
 if( (ep=dp-capt_enxt) == NULL ) {
if( list_empty(io-rd_qbuf) != 0 ) {
   list_add(op-bfrq,io-rd_qbuf);
   return 0;
}
ep = list_entry(io-rd_qbuf.next,io_buf,bfrq);
list_del_init(ep-bfrq);
 }
  }
  else
  {
 ep = op;
  }
  dp-capt_onxt = op;  oidx = op-index;
  dp-capt_enxt = ep;  eidx = ep-index;
   }
   else
   {
  oidx = eidx = 0;
   }

   if( oidx != eidx ) {
  vip_inpbfr-current_buffer = eidx;
  vip_buffers = vip_task_data[task].vip_even_buffers;
  vip_toggle_video_offsets(vip_buffers,vip_inpbfr);
  vip_inpbfr-current_buffer = oidx;
  vip_buffers = vip_task_data[task].vip_odd_buffers;
  vip_toggle_video_offsets(vip_buffers,vip_inpbfr);
   }
   else {
  vip_inpbfr-current_buffer = oidx;
  vip_buffers = vip_task_data[task].vip_buffers;
  vip_toggle_video_offsets(vip_buffers,vip_inpbfr);
   }
   dp-capt_stalled = 0;

   ++dp-capt_sequence;
   ++dp-capt_jiffy_sequence;

   return 1;
}


dmesg and uname:


n...@purzel [1] [~]$ uname -a
Linux purzel 2.6.29.6-rt24-aldebaran-rt #1 PREEMPT RT Fri Feb 12 17:51:46 CET 
2010 i586 GNU/Linux

[   17.877211] AMD Linux LX video2linux/2 driver 3.2.0100
[   17.893218] Found Geode LX VIP at IRQ 11
[   17.910414] OmniVision ov7670 sensor driver, at your service (v 2.00)
[   17.939093] ov7670/1: driver attached: adapter id: 10002
[   17.955545] Trying to detect OmniVision 7670/7672 I2C adapters
[   19.847126] ov7670_init returned 0
[   19.847139] Phase 1
[   19.849429] Phase 2
[   19.849440] Phase 3
[   19.851728] Phase 4
[   19.851739] Phase 5
[   19.854054] Phase 6
[   19.854064] Phase 7
[   19.856357] Phase 8
[   19.856367] Phase 9
[   19.856377] OmniVision 7670/7671 I2C Found
[   19.868799] ov7670/1: driver attached: adapter id: 0
[   20.324975] eth0: link up, 100Mbps, full-duplex, lpa 0x45E1
[   22.963201] spurious 8259A interrupt: IRQ7.
[   74.863542] warning: 

Re: [PATCH 1/3] em-x270: don't use pxa_camera init() callback

2009-11-17 Thread Mike Rapoport


Antonio Ospite wrote:
 pxa_camera init() is going to be removed.
 
 Signed-off-by: Antonio Ospite osp...@studenti.unina.it
 ---
  arch/arm/mach-pxa/em-x270.c |9 +
  1 files changed, 5 insertions(+), 4 deletions(-)

Acked-by: Mike Rapoport m...@compulab.co.il

 diff --git a/arch/arm/mach-pxa/em-x270.c b/arch/arm/mach-pxa/em-x270.c
 index aec7f42..f71f34c 100644
 --- a/arch/arm/mach-pxa/em-x270.c
 +++ b/arch/arm/mach-pxa/em-x270.c
 @@ -967,7 +967,7 @@ static inline void em_x270_init_gpio_keys(void) {}
  #if defined(CONFIG_VIDEO_PXA27x) || defined(CONFIG_VIDEO_PXA27x_MODULE)
  static struct regulator *em_x270_camera_ldo;
  
 -static int em_x270_sensor_init(struct device *dev)
 +static int em_x270_sensor_init(void)
  {
   int ret;
  
 @@ -996,7 +996,6 @@ static int em_x270_sensor_init(struct device *dev)
  }
  
  struct pxacamera_platform_data em_x270_camera_platform_data = {
 - .init   = em_x270_sensor_init,
   .flags  = PXA_CAMERA_MASTER | PXA_CAMERA_DATAWIDTH_8 |
   PXA_CAMERA_PCLK_EN | PXA_CAMERA_MCLK_EN,
   .mclk_10khz = 2600,
 @@ -1049,8 +1048,10 @@ static struct platform_device em_x270_camera = {
  
  static void  __init em_x270_init_camera(void)
  {
 - pxa_set_camera_info(em_x270_camera_platform_data);
 - platform_device_register(em_x270_camera);
 + if (em_x270_sensor_init() == 0) {
 + pxa_set_camera_info(em_x270_camera_platform_data);
 + platform_device_register(em_x270_camera);
 + }
  }
  #else
  static inline void em_x270_init_camera(void) {}

-- 
Sincerely yours,
Mike.

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] Add RGB555X and RGB565X formats to pxa-camera

2009-09-06 Thread Mike Rapoport
Hi Guennadi,

Guennadi Liakhovetski wrote:
 On Mon, 3 Aug 2009, Mike Rapoport wrote:
 
 2. Mike, while reviewing this patch I came across code in 
 pxa_camera_setup_cicr(), introduced by your earlier patch:

 case V4L2_PIX_FMT_RGB555:
 cicr1 |= CICR1_RGB_BPP_VAL(1) | CICR1_RGBT_CONV_VAL(2) |
 CICR1_TBIT | CICR1_COLOR_SP_VAL(1);
 break;

 Why are you enabling the RGB to RGBT conversion here unconditionally? 
 Generally, what are the advantages of configuring CICR1 for a specific RGB 
 format compared to using just a raw capture? Do I understand it right, 
 that ATM we are not using any of those features?
 As far as I remember I've tried to overlay the captured imagery using pxa
 overlay1. Most probably it's left here after those tries.
 
 Mike, could you, please, verify that those bits are indeed unneeded and 
 provide patch to remove them?

Unfortunately, I don't have the sensor handy at the time :( The one I've used
then is now broken (physically) and there's no replacement for it :(

 Thanks
 Guennadi
 ---
 Guennadi Liakhovetski, Ph.D.
 Freelance Open-Source Software Developer
 http://www.open-technology.de/
 --
 To unsubscribe from this list: send the line unsubscribe linux-media in
 the body of a message to majord...@vger.kernel.org
 More majordomo info at  http://vger.kernel.org/majordomo-info.html
 

-- 
Sincerely yours,
Mike.

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html