Re: [PATCH][next] dmaengine: Use fallthrough pseudo-keyword

[Tyrel Datwyler <tyreld@linux.ibm.com>]

On 8/5/20 6:19 AM, Vinod Koul wrote:
> On 27-07-20, 15:34, Gustavo A. R. Silva wrote:
> 
>> diff --git a/drivers/dma/pl330.c b/drivers/dma/pl330.c
>> index 2c508ee672b9..9b69716172a4 100644
>> --- a/drivers/dma/pl330.c
>> +++ b/drivers/dma/pl330.c
>> @@ -1061,16 +1061,16 @@ static bool _start(struct pl330_thread *thrd)
>>  
>>  		if (_state(thrd) == PL330_STATE_KILLING)
>>  			UNTIL(thrd, PL330_STATE_STOPPED)
>> -		/* fall through */
>> +		fallthrough;
>>  
>>  	case PL330_STATE_FAULTING:
>>  		_stop(thrd);
>> -		/* fall through */
>> +		fallthrough;
>>  
>>  	case PL330_STATE_KILLING:
>>  	case PL330_STATE_COMPLETING:
>>  		UNTIL(thrd, PL330_STATE_STOPPED)
>> -		/* fall through */
>> +		fallthrough;
>>  
>>  	case PL330_STATE_STOPPED:
>>  		return _trigger(thrd);
>> @@ -1121,7 +1121,6 @@ static u32 _emit_load(unsigned int dry_run, u8 buf[],
>>  
>>  	switch (direction) {
>>  	case DMA_MEM_TO_MEM:
>> -		/* fall through */
>>  	case DMA_MEM_TO_DEV:
>>  		off += _emit_LD(dry_run, &buf[off], cond);
>>  		break;
>> @@ -1155,7 +1154,6 @@ static inline u32 _emit_store(unsigned int dry_run, u8 buf[],
>>  
>>  	switch (direction) {
>>  	case DMA_MEM_TO_MEM:
>> -		/* fall through */
>>  	case DMA_DEV_TO_MEM:
>>  		off += _emit_ST(dry_run, &buf[off], cond);
>>  		break;
>> @@ -1216,7 +1214,6 @@ static int _bursts(struct pl330_dmac *pl330, unsigned dry_run, u8 buf[],
>>  
>>  	switch (pxs->desc->rqtype) {
>>  	case DMA_MEM_TO_DEV:
>> -		/* fall through */
>>  	case DMA_DEV_TO_MEM:
>>  		off += _ldst_peripheral(pl330, dry_run, &buf[off], pxs, cyc,
>>  			cond);
>> @@ -1266,7 +1263,6 @@ static int _dregs(struct pl330_dmac *pl330, unsigned int dry_run, u8 buf[],
>>  
>>  	switch (pxs->desc->rqtype) {
>>  	case DMA_MEM_TO_DEV:
>> -		/* fall through */
> 
> replacement missed here and above few
> 

Its not obvious via most of the documentation, but a case label followed
immediately by another case label is the only allowed implicit fall through as
it is obvious to the eye that there is no postceding statement.

For example the following is legal:

    case FOO:
    	/* fallthrough */ (or fallthrough;)
    case BAR:

is converted to:

    case FOO:
    case BAR:

I would assume the justification is that it is common to have a switch statement
where several case statements fall directly through to a single code block and
annotating each case label seems like overkill.

        switch (vhost->state) {
        case IBMVFC_LINK_DEAD:
        case IBMVFC_HOST_OFFLINE:
                result = DID_NO_CONNECT << 16;
                break;
        case IBMVFC_NO_CRQ:
        case IBMVFC_INITIALIZING:
        case IBMVFC_HALTED:
        case IBMVFC_LINK_DOWN:
                result = DID_REQUEUE << 16;
                break;
        case IBMVFC_ACTIVE:
                result = 0;
                break;
        }

Regards,

Tyrel