Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1200650 > unrolled thread

Re: [RFC PATCH 0/3] mailbox: hisilicon: add mailbox driver

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [RFC PATCH 0/3] mailbox: hisilicon: add mailbox driver Jassi Brar <jassisinghbrar@gmail.com> - 2015-08-05 13:00 +0200
    Re: [RFC PATCH 0/3] mailbox: hisilicon: add mailbox driver Leo Yan <leo.yan@linaro.org> - 2015-08-06 01:20 +0200
      Re: [RFC PATCH 0/3] mailbox: hisilicon: add mailbox driver Jassi Brar <jassisinghbrar@gmail.com> - 2015-08-10 09:10 +0200
        Re: [RFC PATCH 0/3] mailbox: hisilicon: add mailbox driver Leo Yan <leo.yan@linaro.org> - 2015-08-10 11:00 +0200

#1200650 — Re: [RFC PATCH 0/3] mailbox: hisilicon: add mailbox driver

On Mon, Aug 3, 2015 at 6:43 AM, Leo Yan <leo.yan@linaro.org> wrote:
> This patch series is to implement Hisilicon's mailbox driver and enable
> the mailbox controller on Hi6220.
>
Cool.

> The Hisilicon mailbox supports up to 32 channels. Each channel is
> unidirectional with a maximum message size of 8 words. I/O is performed
> using register access (there is no DMA) and the cell raises an interrupt
> when messages are received.
>
FYI, none of other platforms has mailbox with DMA. 8-word deep fifo,
RX interrupt
and optional TX interrupt is pretty common. So cool still.

> For easily extending for Hisilicon series SoCs (SoCs may have difference
> for register's definition with each other), so firstly implement common
> mailbox driver; this common mailbox driver provides three mainly
> functionality:
>
>  - help register channels into framework;
>  - hook low level callback functions for register's operations;
>  - Enhance rx channel's message queue, which is based on the code in
>    drivers/mailbox/omap-mailbox.c.
>
Not cool.
 Please don't reinvent the wheel by having platform specific
implementation of the mailbox api. Which vendor doesn't plan to roll
out new SoCs, and hence variations of mailbox controllers?  The OMAP
stack predates the common api, and was actually supposed to be
converted over eventually. Please implement just the
drivers/mailbox/hi6220-mailbox.c (preferably by the name of the
mailbox controller, if any)

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1201335

Hi Jassi,

Thanks for review.

On Wed, Aug 05, 2015 at 04:22:01PM +0530, Jassi Brar wrote:
> On Mon, Aug 3, 2015 at 6:43 AM, Leo Yan <leo.yan@linaro.org> wrote:

[...]

> > For easily extending for Hisilicon series SoCs (SoCs may have difference
> > for register's definition with each other), so firstly implement common
> > mailbox driver; this common mailbox driver provides three mainly
> > functionality:
> >
> >  - help register channels into framework;
> >  - hook low level callback functions for register's operations;
> >  - Enhance rx channel's message queue, which is based on the code in
> >    drivers/mailbox/omap-mailbox.c.
> >
> Not cool.
>  Please don't reinvent the wheel by having platform specific
> implementation of the mailbox api. Which vendor doesn't plan to roll
> out new SoCs, and hence variations of mailbox controllers?  The OMAP
> stack predates the common api, and was actually supposed to be
> converted over eventually. Please implement just the
> drivers/mailbox/hi6220-mailbox.c (preferably by the name of the
> mailbox controller, if any)

Understood. Here i have one question, the rx channel's message queue is
looks like a common mechanism and can be added into framework file
mailbox.c, then Soc driver file can _ONLY_ focus on register level's
operations. If so, the common driver in this patch also is unnecessary.

Do you suggest to use upper method to rework patches? Or just think
it's okay to implement rx channel's message queue in hi6220-mailbox.c?

Thanks,
Leo Yan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1203849

On Thu, Aug 6, 2015 at 4:47 AM, Leo Yan <leo.yan@linaro.org> wrote:
> Hi Jassi,
>
> Thanks for review.
>
> On Wed, Aug 05, 2015 at 04:22:01PM +0530, Jassi Brar wrote:
>> On Mon, Aug 3, 2015 at 6:43 AM, Leo Yan <leo.yan@linaro.org> wrote:
>
> [...]
>
>> > For easily extending for Hisilicon series SoCs (SoCs may have difference
>> > for register's definition with each other), so firstly implement common
>> > mailbox driver; this common mailbox driver provides three mainly
>> > functionality:
>> >
>> >  - help register channels into framework;
>> >  - hook low level callback functions for register's operations;
>> >  - Enhance rx channel's message queue, which is based on the code in
>> >    drivers/mailbox/omap-mailbox.c.
>> >
>> Not cool.
>>  Please don't reinvent the wheel by having platform specific
>> implementation of the mailbox api. Which vendor doesn't plan to roll
>> out new SoCs, and hence variations of mailbox controllers?  The OMAP
>> stack predates the common api, and was actually supposed to be
>> converted over eventually. Please implement just the
>> drivers/mailbox/hi6220-mailbox.c (preferably by the name of the
>> mailbox controller, if any)
>
> Understood. Here i have one question, the rx channel's message queue is
> looks like a common mechanism and can be added into framework file
> mailbox.c, then Soc driver file can _ONLY_ focus on register level's
> operations. If so, the common driver in this patch also is unnecessary.
>
Yes, that's what I say, no 'common' driver for a platform.

> Do you suggest to use upper method to rework patches? Or just think
> it's okay to implement rx channel's message queue in hi6220-mailbox.c?
>
The code in drivers/mailbox/ should only manage the controller
(registers and interrupts). Everything else (queues, shmem etc) should
be in platform specific client driver(s).
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1203902

Hi Jassi,

On Mon, Aug 10, 2015 at 12:32:05PM +0530, Jassi Brar wrote:
> On Thu, Aug 6, 2015 at 4:47 AM, Leo Yan <leo.yan@linaro.org> wrote:
> > Hi Jassi,
> >
> > Thanks for review.
> >
> > On Wed, Aug 05, 2015 at 04:22:01PM +0530, Jassi Brar wrote:
> >> On Mon, Aug 3, 2015 at 6:43 AM, Leo Yan <leo.yan@linaro.org> wrote:
> >
> > [...]
> >
> >> > For easily extending for Hisilicon series SoCs (SoCs may have difference
> >> > for register's definition with each other), so firstly implement common
> >> > mailbox driver; this common mailbox driver provides three mainly
> >> > functionality:
> >> >
> >> >  - help register channels into framework;
> >> >  - hook low level callback functions for register's operations;
> >> >  - Enhance rx channel's message queue, which is based on the code in
> >> >    drivers/mailbox/omap-mailbox.c.
> >> >
> >> Not cool.
> >>  Please don't reinvent the wheel by having platform specific
> >> implementation of the mailbox api. Which vendor doesn't plan to roll
> >> out new SoCs, and hence variations of mailbox controllers?  The OMAP
> >> stack predates the common api, and was actually supposed to be
> >> converted over eventually. Please implement just the
> >> drivers/mailbox/hi6220-mailbox.c (preferably by the name of the
> >> mailbox controller, if any)
> >
> > Understood. Here i have one question, the rx channel's message queue is
> > looks like a common mechanism and can be added into framework file
> > mailbox.c, then Soc driver file can _ONLY_ focus on register level's
> > operations. If so, the common driver in this patch also is unnecessary.
> >
> Yes, that's what I say, no 'common' driver for a platform.
> 
> > Do you suggest to use upper method to rework patches? Or just think
> > it's okay to implement rx channel's message queue in hi6220-mailbox.c?
> >
> The code in drivers/mailbox/ should only manage the controller
> (registers and interrupts). Everything else (queues, shmem etc) should
> be in platform specific client driver(s).

Thanks for suggestion, will send new version patches for reviewing.

Thanks,
Leo Yan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web