Re: [PATCH 5/7 v2] dma: sh: use an integer slave ID to improve APIcompatibility

[Vinod Koul]

On Mon, 2012-07-16 at 12:55 +0200, Guennadi Liakhovetski wrote:
> On Mon, 16 Jul 2012, Vinod Koul wrote:
> 
> > On Mon, 2012-07-16 at 12:01 +0200, Guennadi Liakhovetski wrote:
> > > On Mon, 16 Jul 2012, Vinod Koul wrote:
> > > 
> > > > On Mon, 2012-07-16 at 10:47 +0200, Guennadi Liakhovetski wrote:
> > > > > > I want to know what does ccr and mid_rid mean to dmac here?
> > > > > 
> > > > > CHCR contains a few fields, some enable various interrupt sources, some 
> > > > > specify repeat- and renew-modes, others yet specify transfer size, source 
> > > > > and destination address-modes (incrementing, constant, decrementing), 
> > > > > others yet select a DMA client category (slave / memcpy / ...), and a 
> > > > > transfer flag. Some of these fields could be calculated, others are 
> > > > > pre-defined for various slaves, the exact layout of those fields can also 
> > > > > vary between SoCs.
> > > > I do not understand how clients would provide these values. 
> > > > For pre-defined values, they should be dmac property why should client
> > > > like spi or mmc have clue about it?
> > > > 
> > > > For others like you mentioned, i guess they could be easily calculated,
> > > > right?
> > > > 
> > > > > MID_RID is actually a slave-selector, it contains a magic value, that 
> > > > > cannot be calculated. 
> > > > and again, how does client know this?
> > > 
> > > I might be misunderstanding you, but from earlier discussions I got an 
> > > impression, that the DMAC should know nothing about clients, i.e., should 
> > > receive no client-specific information from its platform data. Instead 
> > > clients should provide it when configuring the channel. If this is 
> > > correct, then the preferred way would be to specify these values in client 
> > > platform data and then pass it to the DMAC with slave-config calls? Or 
> > > have I misunderstood you and this per-client information should be kept in 
> > > DMAC platform data?
> > You haven't misunderstood me. dmacs should not know of client data and
> > should be client agnostic. 
> > 
> > BUT, are these parameters ccr and mid_rid client data, i do not think
> > so, they seem to be dmac controller parameters which you need to
> > configure channel or is that not the case. If not why bother passing
> > them to dmac?
> 
> Yes, that's right - these values have to be written to DMAC channel 
> configuration registers, so, we do not have to change anything, those 
> values can remain DMAC parameters and be passed to it directly from 
> platform data.
Can you get that in future fixes.
> 
> > Either way something looks fishy to me.
> 
> You can always tell me what you don't like about the driver, but I don't 
> see why this specific patch should cause any problem - it only changes the 
> type of the slave-id variable from unsigned to signed to be able to pass 
> negative error values in it too.
This question was not specific to this patchset

Btw I am quite happy with this patchset. Good direction for this and
thanks for taking it up.


-- 
~Vinod

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