Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, July 30, 2015 at 02:18:09 PM, Michal Suchanek wrote: On 30 July 2015 at 13:24, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 10:43:05 PM, Michal Suchanek wrote: On 27 July 2015 at 19:43, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. The flash chip can transfer data as long as you keep the clock going, especially when you transfer 64k off a 4M flash. The read command is bound just by stopping the clock. There is always more data to be had. Sure, if you keep clocking the chip, the data will keep going. Is the chip being clocked ? There is always data. Like in whenever you want to read SPI data you sample the input pin and watever you sample is data so far as SPI bus is concerned. Aren't you forgetting that the data are synchronous to the clock which you (your SPI block) generate ? Doesn't the PL330 have some kind of a counter register where you can check how much data were received so far ? That should be sufficient to verify this hypothesis of mine. Could check that, yes. Maybe I could read the counter in a function which dmaengine client uses to tear down the dma transfer. Might be a good idea. Check if the register is stuck for too long and then zap the transfer. On a related note I enabled more prints on the spidev test program. I have code that tests maximum transfer size up to the 4k spidev limit and it can transfer full 4k buffer of NOPs at 80MHz. The recieved data is all ones except a 00 at the start. So it looks like read-only transfers fail but full-duplex sort of work. And it reproduces the 00 prefix that was seen in the dma-only setup in otherwise working setup :-S An attempt to read a page of data using the fast-read command fails with timeout. It would be a good idea to detail what your program exactly does on the bus (like, my program sends 3 bytes of data 0xf0 0x0b 0xar and then receives 60 bytes of data). Maybe I'm lost, but your test is really not clear. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. There is no way to add timeout in the DMA driver since it does not know the SPI clock. The timeout is handled by the SPI driver and it should be possible to augment it to drain the FIFO when DMA fails so long as FIFO level is readable somewhere. If the DMA doesn't complete in certain amount of time, it should time out I'd say. Don't you think ? No. The DMA driver has no idea if the transfer is at 133MHz, 40MHz, 1MHz, 1kHz, whatever. That's not really important, is it ? If the transfer doesn't finish in, say, 1 second, and it is a 4096b transfer, then it is most likely timeout. On the other hand, adding a flush_fifo in the SPI driver after DMA failure allows probing the chip reliably by realoding the driver even just after a failed transfer. OK - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Monday, July 27, 2015 at 10:43:05 PM, Michal Suchanek wrote: On 27 July 2015 at 19:43, Marek Vasut ma...@denx.de wrote: On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. The flash chip can transfer data as long as you keep the clock going, especially when you transfer 64k off a 4M flash. The read command is bound just by stopping the clock. There is always more data to be had. Sure, if you keep clocking the chip, the data will keep going. Is the chip being clocked ? Doesn't the PL330 have some kind of a counter register where you can check how much data were received so far ? That should be sufficient to verify this hypothesis of mine. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. There is no way to add timeout in the DMA driver since it does not know the SPI clock. The timeout is handled by the SPI driver and it should be possible to augment it to drain the FIFO when DMA fails so long as FIFO level is readable somewhere. If the DMA doesn't complete in certain amount of time, it should time out I'd say. Don't you think ? - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed the s3c64xx driver to use DMA for all transfers. This caused identification to fail in spin-nor because the ID was prefixed with extra 00 byte. Testing with spidev confirmed that everything is prefixed with extra 00. The determinism of this is in fact excellent news. Also when the DMA controller locked up no transfers were possible anymore. When DMA was not used for sending commands the controller would recover on next command. I made the option to always use DMA configurable and it turns out that the returned data is prefixed with 00 only when no transfer without DMA was ever made. Loading the spi-nor driver with the dma-only option off and then with dma-only option on results in correct operation. Only loading the dma-only driver first causes the 00 prefix. Can we conclude that the PIO codepath somehow programs a register somewhere which gets rid of this 0x00 prefix ? If so, this should then also be part of the DMA codepath, or even better, this should be set in probe() somewhere. Yes, it looks like it. Did you find what this could be please ? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Monday, July 27, 2015 at 11:46:25 AM, Michal Suchanek wrote: On 24 July 2015 at 10:34, Marek Vasut ma...@denx.de wrote: On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Hi! [...] It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! Hello, this does not quite work. My test with spidev: # ./spinor /dev/spidev1.0 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 I receive correct ID but spi-nor complains it does not know ID 00 c8 60. IIRC garbage should be sent only at the time command is transferred so only one byte of garbage should be received. Also the garbage tends to be the last state of the data output - all 0 or all 1. So it seems using DMA for all transfers including 1-byte commands results in (some?) received data getting an extra 00 prefix. I also managed to lock up the controller completely since there is some error passing the SPI speed somewhere :( [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977540] spidev spi1.0: spi mode 0 [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977582] spidev spi1.0: msb first [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977620] spidev spi1.0: 0 bits per word [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w, 2690588672 Hz max -- 0 [ 1352.977726] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977753] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer bpw 8 speed -1604378624 [ 1352.977760] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977781] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using dma [ 1352.977797] dma-pl330 121b.pdma: setting up request on thread 1 Hmm, on a second thought it probably works as expected more or less. The nonsensical value was passed from application and there is no guard against that. Since I don't do PIO the controller remains locked up indefinitely. I have to admit, I don't quite understand the above. I also don't quite know what your spidev test does. Can you possibly just bind a regular SPI NOR driver and run mtdtests to see if it is stable ? Ok, so here is some summary. I have a NOR flash attached to a s3c64xx SPI controller with 64byte fifo and a pl330 dma controller. Normally DMA controller is used for transfers that do not fit into the FIFO. I tried adding the flash memory ID to the spi-nor driver table and adding a DT node for it. The flash is rated at 120MHz so I used that speed but the ID was bit-shifted and identification failed. There is DT property samsung,spi-feedback-delay for addressing this and at 120MHz it must be 2 or 3 on this board. 40MHz works with default value 0. The next step after identification worked was to try reading the flash content. For this the DMA controller is used because data is transferred in blocks larger than 64 bytes. When reading the whole 4MB flash the transfer failed silently. I got a 4MB file of all ones or all zeroes. It turns out that - the pl330 locks up when transfering large amount of data. Specifically, the maximum power of 2 sized transfer at 120MHz is 128 bytes and 64k at 40MHz. Transferring more than this results in the pl330 locking up and never signalling completion of the transfer. Hypothesis: I think this might just be that the controller didn't catch all the inbound clock ticks and thus counted less inbound data than it was set up to receive. The controller thus waits for more data. Data is left in FIFO which causes subsequent commands to fail since garbage is returned instead of command reply. Also subsequent read may cause I/O error or or return an empty page depending on the garbage around. So the driver for the DMA controller might need to be augmented to handle this case -- add a timeout and in case DMA times out, drain the FIFO to make sure subsequent transfers do not fail. - the I/O errors are not checked in spi-nor at all which leads to silent data corruption. On a suggestion that this may improve reliability I changed the s3c64xx driver to use DMA for all transfers. This caused identification to fail in spin-nor because the ID was prefixed with extra 00 byte. Testing with spidev confirmed that everything is prefixed with extra 00. The determinism of this is in fact
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, July 23, 2015 at 07:03:47 PM, Michal Suchanek wrote: Hi! [...] It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! Hello, this does not quite work. My test with spidev: # ./spinor /dev/spidev1.0 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 Sending 90 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff ff ff ff c8 15 c8 15 c8 15 c8 15 c8 15 c8 Sending 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 Received 00 ff c8 60 16 c8 60 16 c8 60 16 c8 60 16 c8 60 I receive correct ID but spi-nor complains it does not know ID 00 c8 60. IIRC garbage should be sent only at the time command is transferred so only one byte of garbage should be received. Also the garbage tends to be the last state of the data output - all 0 or all 1. So it seems using DMA for all transfers including 1-byte commands results in (some?) received data getting an extra 00 prefix. I also managed to lock up the controller completely since there is some error passing the SPI speed somewhere :( [ 1352.977530] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977540] spidev spi1.0: spi mode 0 [ 1352.977576] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977582] spidev spi1.0: msb first [ 1352.977614] spidev spi1.0: setup mode 0, 8 bits/w, 8000 Hz max -- 0 [ 1352.977620] spidev spi1.0: 0 bits per word [ 1352.977652] spidev spi1.0: setup mode 0, 8 bits/w, 2690588672 Hz max -- 0 [ 1352.977726] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977753] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: xfer bpw 8 speed -1604378624 [ 1352.977760] spi_master spi1: s3c64xx_spi_config: clk_from_cmu 1 src_clk sclk_spi1 mode bpw 8 [ 1352.977781] spi_master spi1: spi1.0 s3c64xx_spi_transfer_one: using dma [ 1352.977797] dma-pl330 121b.pdma: setting up request on thread 1 Hmm, on a second thought it probably works as expected more or less. The nonsensical value was passed from application and there is no guard against that. Since I don't do PIO the controller remains locked up indefinitely. I have to admit, I don't quite understand the above. I also don't quite know what your spidev test does. Can you possibly just bind a regular SPI NOR driver and run mtdtests to see if it is stable ? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. However, if this was not tied to the particular slave setting picked in the current DT a formula would be needed that translates arbitrary client settings to transfer size limit and there would be need to somehow get the client settings to the formula in the dmaengine driver. Thanks Michal Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 10:38:14 AM, Michal Suchanek wrote: On 22 July 2015 at 10:24, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: On 22 July 2015 at 09:58, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? The SPI driver uses PIO for short transfers and DMA for transfers longer than the controller FIFO. This seems to be the standard way to do things.It works flawlessly so long as submitting overly long DMA programs is avoided. Can you try doing JUST DMA, no PIO ? I remember seeing some bus synchronisation issues when I did mixed PIO/DMA on the MXS and it was nasty to track down. Just give pure DMA a go to see if the thing stabilizes somehow. It's probably slower to set up DMA for 2-byte commands but it might work nonetheless. It is, the overhead will be considerable. It might help the stability though. I'm really looking forward to the results! I will give it a go. Do you have the option to connect a bus analyzer? I can probably offer you some tools, I'm in Prague ... The flash chip is accessible when removing the bottom cover. It is VSOP8 package slightly lower than SOP8 so attaching clips to it might be a bit problematic. That's the only accessible part. Everything other than SPI is inside the SoC. That might be doable, though you might want to try the above thing first. Since SPI has no verification whatsoever the chip might be completely dead and you can still read fine all zeroes or all ones when attempting a read from it. I observed this behaviour when I used a flash chip in a socket and it was not firmly seated. It was with a different SPI controller, though. You should run into issues as soon as the SPI NOR framework tries to read status register, no ? Yes, when the DMA transfer fails the next command fails due to garbage lying around. However, you can unload the SPI NOR driver, load spidev driver, and read enough garbage to empty the fifos. Then the flash identifies as normal again and you can access it. Yikes :( When the flash is not seated properly and acts autistic you get all 0xff or all 0 back whatever you send to it, obvously. The identification by the SPI NOR driver fails then. Thanks Michal -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? Do you have the option to connect a bus analyzer? I can probably offer you some tools, I'm in Prague ... Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 22, 2015 at 10:18:04 AM, Michal Suchanek wrote: On 22 July 2015 at 09:58, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:45:27 AM, Michal Suchanek wrote: On 22 July 2015 at 09:33, Marek Vasut ma...@denx.de wrote: On Wednesday, July 22, 2015 at 09:30:54 AM, Michal Suchanek wrote: On 22 July 2015 at 06:49, Vinod Koul vinod.k...@intel.com wrote: On Tue, Jul 21, 2015 at 10:14:11AM +0200, Michal Suchanek wrote: Or alternatively we could publish the limitations of the channel using capabilities so SPI knows I have a dmaengine channel and it can transfer max N length transfers so would be able to break rather than guessing it or coding in DT. Yes it may come from DT but that should be dmaengine driver rather than client driver :) This can be done by dma_get_slave_caps(chan, caps) And we add max_length as one more parameter to existing set Also all this could be handled in generic SPI-dmaengine layer so that individual drivers don't have to code it in Let me know if this idea is okay, I can push the dmaengine bits... It would be ok if there was a fixed limit. However, the limit depends on SPI slave settings. Presumably for other buses using the dmaengine the limit would depend on the bus or slave settings as well. I do not see a sane way of passing this all the way to the dmaengine driver. I don't see why this should be client (SPI) dependent. The max length supported is a dmaengine constraint, typically flowing from max blocks/length it can transfer. Know this limit can allow clients to split transfers. In practice on the board I have the maximum transfer length before it fails depends on SPI bus speed which is set up per slave. I did not try searching the space of possible settings thorougly and settled for a setting that gives reasonable speed and transfer length. This looks more like a signal integrity issue though. It certainly does on the surface. However, when wrong data is delivered over the SPI bus (such as when I use wrong phase setting) the SPI controller happily delivers wrong data over PIO. The failure I am seeing is that the pl330 DMA program which repeatedly waits for data from the SPI controller never finishes the read loop and does not signal the interrupt. It seems it also leaves some data in a FIFO somewhere so next command on the flash returns garbage and fails. I observed something similar on MXS (mx28) SPI block. Do you use mixed PIO/DMA mode perhaps ? The SPI driver uses PIO for short transfers and DMA for transfers longer than the controller FIFO. This seems to be the standard way to do things.It works flawlessly so long as submitting overly long DMA programs is avoided. Can you try doing JUST DMA, no PIO ? I remember seeing some bus synchronisation issues when I did mixed PIO/DMA on the MXS and it was nasty to track down. Just give pure DMA a go to see if the thing stabilizes somehow. Do you have the option to connect a bus analyzer? I can probably offer you some tools, I'm in Prague ... The flash chip is accessible when removing the bottom cover. It is VSOP8 package slightly lower than SOP8 so attaching clips to it might be a bit problematic. That's the only accessible part. Everything other than SPI is inside the SoC. That might be doable, though you might want to try the above thing first. Since SPI has no verification whatsoever the chip might be completely dead and you can still read fine all zeroes or all ones when attempting a read from it. I observed this behaviour when I used a flash chip in a socket and it was not firmly seated. It was with a different SPI controller, though. You should run into issues as soon as the SPI NOR framework tries to read status register, no ? -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, July 16, 2015 at 03:19:35 AM, Brian Norris wrote: On Wed, Jul 15, 2015 at 07:15:50PM +0200, Marek Vasut wrote: On Wednesday, July 15, 2015 at 05:59:46 PM, Brian Norris wrote: 1. Fix up the SPI driver so that it knows how to break large SPI transfers up into smaller segments that its constituent hardware (DMA controllers, fast clocks, etc.) can handle. BTW, Mark Brown already commented on this approach: http://lists.infradead.org/pipermail/linux-mtd/2015-May/059364.html I quote: | With modern drivers using transfer_one() where we have control of the | chip select we do also have the ability to add support to the core for | chopping large transfers up into smaller ones that the hardware can | handle transparently. That won't for everything though since it depends | on us being able to manually control the chip select which not all | hardware can do. | I think this might actually be easier -- just do a transfer where you don't toggle CS and just stops the clock at the last bit, then do another (multiple) transfers which don't toggle CS at all, then finally do a transfer which toggles a CS at the end. Sounds OK to me. And as Mark noted, this could probably be done in the core. I suppose Mark could suggest whether the most expedient path is to hack the buggy driver to do this, or whether reworking the SPI core to have the appropriate spi_master field(s) (and caveats) to support this. Yep, agreed. This should be pretty trivial to do and I think for example spi-mxs.c does this. I don't think spi-mxs.c really does this; it chooses between PIO and DMA based on length, but with either option, it runs through each SPI transfer in one go. You might be confused by the fact that this driver implements -transfer_one_message instead of -transfer_one, so it has to loop through all transfers in the message. It does chop up the DMA transfers between multiple runs of the DMA engine IIRC. But it does so within the driver, which apparently is not the way to go :) (IIUC, spi-mxs.c could easily be rewritten to use -transfer, and some of that not-too-complicated boilerplate could be killed off.) Most likely, yes. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 15, 2015 at 05:59:46 PM, Brian Norris wrote: Hi Michal, Hi all, On Wed, Jul 15, 2015 at 01:52:27PM +0200, Marek Vasut wrote: The problem is, if you add a new DT binding, you'd have to support it forever, no matter how bad idea that binding turned out to be. Agreed, and a solid NAK to this patch. I could have sworn I gave such a response when this was originally being discussed a month ago. AFAICT, you have one of two general approaches available to you: 1. Fix up the SPI driver so that it knows how to break large SPI transfers up into smaller segments that its constituent hardware (DMA controllers, fast clocks, etc.) can handle. I think this might actually be easier -- just do a transfer where you don't toggle CS and just stops the clock at the last bit, then do another (multiple) transfers which don't toggle CS at all, then finally do a transfer which toggles a CS at the end. This should be pretty trivial to do and I think for example spi-mxs.c does this. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, July 15, 2015 at 11:45:07 AM, Michal Suchanek wrote: On 4 June 2015 at 19:15, Richard Cochran richardcoch...@gmail.com wrote: On Thu, Jun 04, 2015 at 10:31:45AM +0200, Michal Suchanek wrote: You might want to try to run the bus at 60MHz or 80MHz and then the values would probably again be different. The first two values are set in DT so the logical place for setting the third is also in DT. Otherwise you would need some in-kernel table of these settings. Or a formula. This formula probably needs to take into account - the unknown reason for the pl330 to fail transfer Shouldn't that be fixed at the PL330 level ? This looks like fixing a problem at the wrong place :) - the device transfer speed and transfer phase as set in DT - possibly device-specific latency and board-specific trace design and assembly tolerances If the design is broken, then cap the speed as for normal SPI device. Seriously, until I have at least a vague idea why the transfer fails I am not comfortable pulling some formula out of thin air and pretending I have a working patch. On the other hand, a parameter you can set in the DT and which comes with a suggested value which can be tuned depending on the system seems more viable. The problem is, if you add a new DT binding, you'd have to support it forever, no matter how bad idea that binding turned out to be. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/11] mtd: ofpart: do not fail probe when no partitions exist
On Thursday, June 04, 2015 at 05:40:58 PM, Michal Suchanek wrote:
On 4 June 2015 at 17:28, Marek Vasut ma...@denx.de wrote:
On Thursday, June 04, 2015 at 06:54:00 AM, Michal Suchanek wrote:
On 4 June 2015 at 00:58, Marek Vasut ma...@denx.de wrote:
On Wednesday, June 03, 2015 at 11:26:40 PM, Michal Suchanek wrote:
On Exynos it is necessary to set SPI controller parameters that apply
to a SPI slave in a DT subnode of the slave device. The ofpart code
returns an error when there are subnodes of the SPI flash but no
partitions are found. Change this condition to a warning so that
flash without partitions can be accessed on Exynos.
I have to admit the rationale for this patch is not very clear to me,
sorry. Can you please explain this a bit more ?
This is how the DT entry for SPI slave looks with s3c64xx:
flash: m25p80@0 {
#address-cells = 1;
#size-cells = 1;
compatible = jedec,spi-nor;
reg = 0;
spi-max-frequency = 4000;
linux,max_tx_len = 65536;
SIDENOTE: I thought this was actually added by your patch #8 in this
series. The underscores in the name of the property are not really
consistent with the rest of the names.
m25p,fast-read;
controller-data {
samsung,spi-feedback-delay = 0;
};
};
this is example of flash partitions:
flash@0 {
#address-cells = 1;
#size-cells = 1;
partition@0 {
label = u-boot;
reg = 0x000 0x10;
read-only;
};
uimage@10 {
reg = 0x010 0x20;
};
};
The parser ignores any flash without subnodes and returns 0 (no
partititon). When there is a subnode it assumes the flash is
partitioned and tries to parse the subnodes as partitions. When there
are subnodes and none parses as partition an error is returned. As
shown above it is valid to have subnodes on unpartitioned flash.
When an error is returned from a partition parser the mtdpart code
passes on this error to the flash probe function and the proble of the
flash fails.
What does /proc/mtd tell you when you have no partitions defined
in the DT ? It should provide you with the entire MTD device and
the code shouldn't even try to parse any OF partitions, since you
don't have any.
mtdinfo shows I have no mtd devices and the log shows that probe
failed unless I patch the kernel.
When there is *support* for of partitions the ofparser is run on mtd
probe. If it fails because it considers
controller-data {
samsung,spi-feedback-delay = 0;
};
an invalid partition and does not find any valid partition the probe fails.
OK, now it has become clearer to me, thanks!
There are two problems here
1) the above is not invalid. It just is not a partition. The parser
should not fail seeing this
The parser should complain verbosely and ignore this I guess ?
2) the mtd probe should not fail when a partition parser fails and
should present the unpartitioned device
OK
Both are addressed in separate patches.
Thanks
Michal
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/11] Enable access to SPI NOR flash on Samsung Snow board
On Thursday, June 04, 2015 at 06:21:32 AM, Michal Suchanek wrote: On 4 June 2015 at 00:53, Marek Vasut ma...@denx.de wrote: On Wednesday, June 03, 2015 at 11:26:39 PM, Michal Suchanek wrote: Hello, Hi, this patch series makes it possible to access the SPI NOR flash in the Samsung XE303 'Snow' Chromebook. Unfortunately not all issues are resolved. To work around an issue with the pl330 dma engine I respun the patch for limiting transfer size in m25p80 driver. Looks like fixing a bug at the wrong place to me ... As the flash does not contain any sane filesystem and is only likely to be accessed with mtd_debug or similar tool the limit of the dma engine is easily reached. Filesystems using shorter data transfers are less likely to be affected. Sounds like the DMA engine driver should be fixed. I looked at the pl330 datasheet and don't see anything obviusly wrong with the pl330 driver in Linux. Ideally it would be fixed but I have no idea what's wrong with it. Ask maybe Catalin from ARM to help you -- he might be able to point you to the right hardware guy. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Thursday, June 04, 2015 at 06:31:45 AM, Michal Suchanek wrote: On 4 June 2015 at 01:03, Marek Vasut ma...@denx.de wrote: On Wednesday, June 03, 2015 at 11:26:41 PM, Michal Suchanek wrote: On sunxi the SPI controller currently does not have DMA support and fails any transfer larger than 63 bytes. On Exynos the pl330 DMA controller fails any transfer larger than 64kb when using slower speed like 40MHz and any transfer larger than 128bytes when running at 133MHz. This smells more like some corruption of the data on the bus or something even more obscure. If the data was corrupted you would get corrupted data and not transfer failure. AFAIK there is no checksum or anything. I actually do get corrupted data when I use wrong feedback delay setting. OK The best thing is that in both cases the controller can just lock up and never finish potentially leaving the hardware in unusable state. [...] --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt @@ -19,6 +19,12 @@ Optional properties: all chips and support for it can not be detected at runtime. Refer to your chips' datasheet to check if this is supported by your chip. +- linux,max_tx_len : With some SPI controller drivers possible transfer size is + limited. This may be hardware or driver bug. + Transfer data in chunks no larger than this value. + Using this option may severely degrade performance and + possibly flash memory life when max_tx_len is smaller than + flash page size (typically 256 bytes) Will we need similar patch for all other SPI slave drivers, like SPI NAND ? Probably. Some SPI slave drivers already do have similar option. So the SPI device drivers are implementing workarounds for buggy SPI hosts, even if those SPI devices themselves don't suffer from such limitations. Yes, this seems like the seriously wrong thing to do. In general it cannot be expected that you can reliably transfer arbitrarily large data over SPI it seems. This is what should be expected because this is how the bus is supposed to work in fact. You can transfer an arbitrary amount of data over SPI bus. If some driver has a limitation, it's that (bus, dma) driver which needs to implement a fix or a workaround. However, if the nand driver transfers data a page at a time it should be fine. ... until someone issues multi-block read, in which case it all falls down like a house of cards. It is impossible to build a reliable system on should and similar assumptions ; the driver must be solid. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/11] mtd: ofpart: do not fail probe when no partitions exist
On Thursday, June 04, 2015 at 06:54:00 AM, Michal Suchanek wrote:
On 4 June 2015 at 00:58, Marek Vasut ma...@denx.de wrote:
On Wednesday, June 03, 2015 at 11:26:40 PM, Michal Suchanek wrote:
On Exynos it is necessary to set SPI controller parameters that apply to
a SPI slave in a DT subnode of the slave device. The ofpart code returns
an error when there are subnodes of the SPI flash but no partitions are
found. Change this condition to a warning so that flash without
partitions can be accessed on Exynos.
I have to admit the rationale for this patch is not very clear to me,
sorry. Can you please explain this a bit more ?
This is how the DT entry for SPI slave looks with s3c64xx:
flash: m25p80@0 {
#address-cells = 1;
#size-cells = 1;
compatible = jedec,spi-nor;
reg = 0;
spi-max-frequency = 4000;
linux,max_tx_len = 65536;
SIDENOTE: I thought this was actually added by your patch #8 in this
series. The underscores in the name of the property are not really
consistent with the rest of the names.
m25p,fast-read;
controller-data {
samsung,spi-feedback-delay = 0;
};
};
this is example of flash partitions:
flash@0 {
#address-cells = 1;
#size-cells = 1;
partition@0 {
label = u-boot;
reg = 0x000 0x10;
read-only;
};
uimage@10 {
reg = 0x010 0x20;
};
};
The parser ignores any flash without subnodes and returns 0 (no
partititon). When there is a subnode it assumes the flash is
partitioned and tries to parse the subnodes as partitions. When there
are subnodes and none parses as partition an error is returned. As
shown above it is valid to have subnodes on unpartitioned flash.
When an error is returned from a partition parser the mtdpart code
passes on this error to the flash probe function and the proble of the
flash fails.
What does /proc/mtd tell you when you have no partitions defined
in the DT ? It should provide you with the entire MTD device and
the code shouldn't even try to parse any OF partitions, since you
don't have any.
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 11/11] dt: Exynos: add Snow SPI NOR node.
On Thursday, June 04, 2015 at 04:04:18 AM, Krzysztof Kozlowski wrote:
On 04.06.2015 06:26, Michal Suchanek wrote:
The Snow has onboard SPI NOR flash which contains the bootloader.
Add DT node for this flash chip. The flash is rated 133MHz but the pl330
controller can transfer only up to 128 bytes at this speed so use more
conservative settings. Even at 40MHz pl330 can transfer at most 64k with
the current driver.
Signed-off-by: Michal Suchanek hramr...@gmail.com
---
arch/arm/boot/dts/exynos5250-snow.dts | 12
1 file changed, 12 insertions(+)
diff --git a/arch/arm/boot/dts/exynos5250-snow.dts
b/arch/arm/boot/dts/exynos5250-snow.dts index 1fa72cf..38e4cda 100644
--- a/arch/arm/boot/dts/exynos5250-snow.dts
+++ b/arch/arm/boot/dts/exynos5250-snow.dts
@@ -691,6 +691,18 @@
num-cs = 1;
cs-gpios = gpa2 5 0;
status = okay;
+ flash: m25p80@0 {
The indentation looks odd. This should be at the same level as status.
+ #address-cells = 1;
+ #size-cells = 1;
+ compatible = jedec,spi-nor;
+ reg = 0;
+ spi-max-frequency = 4000;
So actually you wanted 133 MHz but as a workaround for DMA issue you use
40 MHz, right? Could you add here a small TODO note in comment about it?
I disagree. There is a problem, the problem should actually be analyzed and
fixed, not just postponed with some TODO nonsense.
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 10/11] spi: add more debug prints in s3c64xx
On Wednesday, June 03, 2015 at 11:26:42 PM, Michal Suchanek wrote: The SPI NOR transfers mysteriously fail so add more debug prints about SPI transactions. Signed-off-by: Michal Suchanek hramr...@gmail.com dev_dbg() and friends would certainly be nicer here. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/11] mtd: ofpart: do not fail probe when no partitions exist
On Wednesday, June 03, 2015 at 11:26:40 PM, Michal Suchanek wrote: On Exynos it is necessary to set SPI controller parameters that apply to a SPI slave in a DT subnode of the slave device. The ofpart code returns an error when there are subnodes of the SPI flash but no partitions are found. Change this condition to a warning so that flash without partitions can be accessed on Exynos. I have to admit the rationale for this patch is not very clear to me, sorry. Can you please explain this a bit more ? Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/11] Enable access to SPI NOR flash on Samsung Snow board
On Wednesday, June 03, 2015 at 11:26:39 PM, Michal Suchanek wrote: Hello, Hi, this patch series makes it possible to access the SPI NOR flash in the Samsung XE303 'Snow' Chromebook. Unfortunately not all issues are resolved. To work around an issue with the pl330 dma engine I respun the patch for limiting transfer size in m25p80 driver. Looks like fixing a bug at the wrong place to me ... As the flash does not contain any sane filesystem and is only likely to be accessed with mtd_debug or similar tool the limit of the dma engine is easily reached. Filesystems using shorter data transfers are less likely to be affected. Sounds like the DMA engine driver should be fixed. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 08/11] MTD: m25p80: Add option to limit SPI transfer size.
On Wednesday, June 03, 2015 at 11:26:41 PM, Michal Suchanek wrote: On sunxi the SPI controller currently does not have DMA support and fails any transfer larger than 63 bytes. On Exynos the pl330 DMA controller fails any transfer larger than 64kb when using slower speed like 40MHz and any transfer larger than 128bytes when running at 133MHz. This smells more like some corruption of the data on the bus or something even more obscure. The best thing is that in both cases the controller can just lock up and never finish potentially leaving the hardware in unusable state. So it is required that the m25p80 driver actively prevents doing transfers that are too large for the current driver state on a particular piece of hardware. Signed-off-by: Michal Suchanek hramr...@gmail.com -- Update commit message and documentation text. --- .../devicetree/bindings/mtd/jedec,spi-nor.txt | 6 ++ drivers/mtd/devices/m25p80.c | 24 -- 2 files changed, 28 insertions(+), 2 deletions(-) diff --git a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt index 2bee681..4e63ae8 100644 --- a/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt +++ b/Documentation/devicetree/bindings/mtd/jedec,spi-nor.txt @@ -19,6 +19,12 @@ Optional properties: all chips and support for it can not be detected at runtime. Refer to your chips' datasheet to check if this is supported by your chip. +- linux,max_tx_len : With some SPI controller drivers possible transfer size is + limited. This may be hardware or driver bug. + Transfer data in chunks no larger than this value. + Using this option may severely degrade performance and + possibly flash memory life when max_tx_len is smaller than + flash page size (typically 256 bytes) Will we need similar patch for all other SPI slave drivers, like SPI NAND ? Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] PCI: designware: split Exynos and i.MX bindings
On Monday, March 31, 2014 at 11:28:29 AM, Lucas Stach wrote: Am Sonntag, den 30.03.2014, 19:36 +0200 schrieb Marek Vasut: On Friday, March 28, 2014 at 05:52:53 PM, Lucas Stach wrote: The glue around the core designware IP is significantly different between the Exynos and i.MX implementation, which is reflected in the DT bindings. This changes the i.MX6 binding to reuse as much as possible from the common designware binding and removes old cruft. I removed the optional GPIOs with the following reasoning: - disable-gpio: endpoint specific GPIO, not currently wired up in any code. Should be handled by the PCI device driver, not the host controller driver. - wake-up-gpio: same as above. - power-on-gpio: No user in any upstream DT. This should be handled by a regulator which shouldn't be controlled by the host driver, but rather by the PCI device driver. This power-on-gpio should indeed be handled by the regulator, but the regulator cannot be handled by the PCIe device driver. This power-on-gpio must be operated on per-slot basis if I understand it correctly, so it cannot be controlled by the host controller driver either. The reason why this cannot be controlled by the device driver is that if the device is powered down, it won't be detected on the PCIe bus, thus it cannot enable the regulator which will power up the slot the device is sitting in. So we are on the same page with regard to a GPIO being the wrong abstraction for this, I think. Yes. For the regulator part I would argue that PCI is a bus that is built around the ability to inspect the bus and detect devices on the bus at probe time, so any regulator that's powering a PCI device should be boot-on. This thing about regulator being boot-on should really be documented. Moreover, I think it's a waste of power to keep the devices ON on boot even if the PCIe bus was not started (yet). The bus might not be started at all and the regulators would still be ON, which would be quite a waste. Only after the device driver is loaded it should be able to fetch the regulator to power down/up the device as it wishes. In the x86 world this is AFAIK done using ACPI methods. I think the host controller driver has no business in controlling the device power, more so as there possibly could be a lot of devices on the bus even with a single host lane. The power should be controlled per-slot, but I don't know how to model that. Note that there might be a PCIe device with a switch popped into a single slot, which makes things much more interesting. In such case, you need to power up the slot and neither of the downstream devices should control the power regulator of that slot. btw. am I blind or do I just not see devicetree-discuss on CC ? Hm, there is devicet...@vger.kernel.org on CC, which MAINTAINER says is the right list for this stuff. OK, I was blind, sorry. Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] PCI: designware: split Exynos and i.MX bindings
On Monday, March 31, 2014 at 12:38:01 PM, Lucas Stach wrote: Am Montag, den 31.03.2014, 11:36 +0200 schrieb Marek Vasut: On Monday, March 31, 2014 at 11:28:29 AM, Lucas Stach wrote: Am Sonntag, den 30.03.2014, 19:36 +0200 schrieb Marek Vasut: On Friday, March 28, 2014 at 05:52:53 PM, Lucas Stach wrote: The glue around the core designware IP is significantly different between the Exynos and i.MX implementation, which is reflected in the DT bindings. This changes the i.MX6 binding to reuse as much as possible from the common designware binding and removes old cruft. I removed the optional GPIOs with the following reasoning: - disable-gpio: endpoint specific GPIO, not currently wired up in any code. Should be handled by the PCI device driver, not the host controller driver. - wake-up-gpio: same as above. - power-on-gpio: No user in any upstream DT. This should be handled by a regulator which shouldn't be controlled by the host driver, but rather by the PCI device driver. This power-on-gpio should indeed be handled by the regulator, but the regulator cannot be handled by the PCIe device driver. This power-on-gpio must be operated on per-slot basis if I understand it correctly, so it cannot be controlled by the host controller driver either. The reason why this cannot be controlled by the device driver is that if the device is powered down, it won't be detected on the PCIe bus, thus it cannot enable the regulator which will power up the slot the device is sitting in. So we are on the same page with regard to a GPIO being the wrong abstraction for this, I think. Yes. For the regulator part I would argue that PCI is a bus that is built around the ability to inspect the bus and detect devices on the bus at probe time, so any regulator that's powering a PCI device should be boot-on. This thing about regulator being boot-on should really be documented. Moreover, I think it's a waste of power to keep the devices ON on boot even if the PCIe bus was not started (yet). The bus might not be started at all and the regulators would still be ON, which would be quite a waste. It's the exact same behavior that you get on x86: all devices are powered after boot, once you loaded a device driver it may choose to turn the device off. I don't think it makes sense to deviate here for the sake of being embedded and special. Only after the device driver is loaded it should be able to fetch the regulator to power down/up the device as it wishes. In the x86 world this is AFAIK done using ACPI methods. I think the host controller driver has no business in controlling the device power, more so as there possibly could be a lot of devices on the bus even with a single host lane. The power should be controlled per-slot, but I don't know how to model that. Note that there might be a PCIe device with a switch popped into a single slot, which makes things much more interesting. In such case, you need to power up the slot and neither of the downstream devices should control the power regulator of that slot. We could just add the regulator to the PCI hierarchy in the DT and handle it similar to IRQs where we just walk up the DT starting from the device until we find the matching IRQ/regulator. Still for this to work with a regulator shared between several devices, all the device drivers have to be aware of the regulator. Otherwise a single device may choose to power down and erroneously cut power to a sibling device, where the driver hasn't requested the regulator. In all those scenarios the host controller driver still would not have any business dealing with the regulator. Same situation as with the legacy IRQs, that aren't handled by the host driver, but just routed to the right instance through the DT. OK, it all makes sense. What I find a bit unfortunate is that we loose the Plug- n-Play nature of the PCIe here, but I guess that cannot be helped. -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/8] i.MX6 PCIe binding change and MSI support
On Friday, March 28, 2014 at 05:52:51 PM, Lucas Stach wrote: While working on MSI support for the i.MX6 PCIe host driver it has been discovered that the binding for this host controller is broken in many ways (refer to the patch descriptions for more info) and was introduced without proper discussion about what should/should not be in the binding. This series fixes this and minimizes the difference of the i.MX6 binding to the common designware PCIe binding. I'm aware that this is a quite radical change, but I think it's justified to do this as long as there aren't many user of the old binding (most of the optional properties in the binding aren't even implemented). Looking forward to your feedback. Other but 2/8, which needs further discussion, Reviewed-by: Marek Vasut ma...@denx.de Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/8] ARM: imx6q-clk: parent lvds_gate from lvds_sel
On Friday, March 28, 2014 at 05:52:52 PM, Lucas Stach wrote: Allows fror proper refcounting of the parent clocks when enabling the clock output on CLK1/2 pads. Signed-off-by: Lucas Stach l.st...@pengutronix.de Looks OK: Reviewed-by: Marek Vasut ma...@denx.de Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/8] PCI: designware: split Exynos and i.MX bindings
On Friday, March 28, 2014 at 05:52:53 PM, Lucas Stach wrote: The glue around the core designware IP is significantly different between the Exynos and i.MX implementation, which is reflected in the DT bindings. This changes the i.MX6 binding to reuse as much as possible from the common designware binding and removes old cruft. I removed the optional GPIOs with the following reasoning: - disable-gpio: endpoint specific GPIO, not currently wired up in any code. Should be handled by the PCI device driver, not the host controller driver. - wake-up-gpio: same as above. - power-on-gpio: No user in any upstream DT. This should be handled by a regulator which shouldn't be controlled by the host driver, but rather by the PCI device driver. This power-on-gpio should indeed be handled by the regulator, but the regulator cannot be handled by the PCIe device driver. This power-on-gpio must be operated on per-slot basis if I understand it correctly, so it cannot be controlled by the host controller driver either. The reason why this cannot be controlled by the device driver is that if the device is powered down, it won't be detected on the PCIe bus, thus it cannot enable the regulator which will power up the slot the device is sitting in. [...] btw. am I blind or do I just not see devicetree-discuss on CC ? Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 6/6] PCI: designware: use new OF interrupt mapping when possible
On Thursday, March 06, 2014 at 03:47:03 AM, Jingoo Han wrote: On Wednesday, March 05, 2014 10:26 PM, Lucas Stach wrote: This is the recommended method of doing the IRQ mapping. For old devicetrees we fall back to the previous practice. Signed-off-by: Lucas Stach l.st...@pengutronix.de Acked-by: Arnd Bergmann a...@arndb.de (+cc Mohit KUMAR, Richard Zhu, Pratyush Anand, Marek Vasut, Kishon Vijay Abraham I) Acked-by: Jingoo Han jg1@samsung.com It works properly on Exynos platform. Thank you. Looks reasonable, Reviewed-by: Marek Vasut ma...@denx.de +CC Troy if he wants to test it on SL/N6X . Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] spi: Make core DMA mapping functions generate scatterlists
On Wednesday, February 05, 2014 at 01:00:02 PM, Mark Brown wrote:
On Wed, Feb 05, 2014 at 07:30:57AM +0100, Marek Vasut wrote:
On Sunday, February 02, 2014 at 02:52:52 PM, Mark Brown wrote:
+static int spi_map_buf(struct spi_master *master, struct device *dev,
+struct sg_table *sgt, void *buf, size_t len,
+enum dma_data_direction dir)
+{
+ const bool vmalloced_buf = is_vmalloc_addr(buf);
+ const int desc_len = vmalloced_buf ? PAGE_SIZE : master-max_dma_len;
You might want to rename this to sg_chunk_max_size or something,
desc_len doesn't make much sense here. The variable describes the
maximum size of one single scatterlist element.
A scatterlist entry is pretty much an abstract descriptor though. I
seem to remember looking at the name and thinking it was good that it
was something less easily applicable to the length of the table but it
doesn't make much odds.
It's the length of the entry, but I see your point.
+ const int sgs = DIV_ROUND_UP(len, desc_len);
Looking at this, the variables could generally use a more meaningful
name. I think it'd be clearer to call this num_sg_chunks or so ?
You do know where I lifted most of these variable names from, right? :P
Yeah, I don't like looking at my old code, it's always so frustrating ;-)
Still,
it's a good source for learning how to NOT do things again :)
Looking at the code again everything seems idiomatic with the naming of
the fields inside the sg_table - I probably would apply a patch to
rename but I wouldn't write one.
OK, makes sense.
+ min = min_t(size_t, len, desc_len);
+
+ if (vmalloced_buf) {
+ vm_page = vmalloc_to_page(buf);
Just curious, but shouldn't we check if buf != NULL right at the begining
of this function?
No need, the check is outside the function along with the check that the
controller is OK with DMAing on this transfer and so on.
OK, thank you for checking this.
+static void spi_unmap_buf(struct spi_master *master, struct device
*dev, + struct sg_table *sgt, enum dma_data_direction
dir)
+{
+ if (sgt-orig_nents) {
I don't want to nag, but why not use if (!sgt-...) return; ? This would
cut down one level of indent.
I was looking at some stuff which might add a bit more in here if it's
not just the core doing mappings. Not sure that's sensible though so it
might never materialise.
OK.
Best regards,
Marek Vasut
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] spi: Make core DMA mapping functions generate scatterlists
On Sunday, February 02, 2014 at 02:52:52 PM, Mark Brown wrote:
Hi, thanks for preparing this patch!
I have just a few very minor nitpicks, ignore if you please.
[...]
+static int spi_map_buf(struct spi_master *master, struct device *dev,
+struct sg_table *sgt, void *buf, size_t len,
+enum dma_data_direction dir)
+{
+ const bool vmalloced_buf = is_vmalloc_addr(buf);
+ const int desc_len = vmalloced_buf ? PAGE_SIZE : master-max_dma_len;
You might want to rename this to sg_chunk_max_size or something, desc_len
doesn't make much sense here. The variable describes the maximum size of one
single scatterlist element.
+ const int sgs = DIV_ROUND_UP(len, desc_len);
Looking at this, the variables could generally use a more meaningful name. I
think it'd be clearer to call this num_sg_chunks or so ?
+ struct page *vm_page;
+ void *sg_buf;
+ size_t min;
+ int i, ret;
+
+ ret = sg_alloc_table(sgt, sgs, GFP_KERNEL);
+ if (ret != 0)
+ return ret;
+
+ for (i = 0; i sgs; i++) {
+ min = min_t(size_t, len, desc_len);
+
+ if (vmalloced_buf) {
+ vm_page = vmalloc_to_page(buf);
Just curious, but shouldn't we check if buf != NULL right at the begining of
this function?
[...]
+static void spi_unmap_buf(struct spi_master *master, struct device *dev,
+ struct sg_table *sgt, enum dma_data_direction dir)
+{
+ if (sgt-orig_nents) {
I don't want to nag, but why not use if (!sgt-...) return; ? This would cut
down one level of indent.
+ dma_unmap_sg(dev, sgt-sgl, sgt-orig_nents, dir);
+ sg_free_table(sgt);
+ }
+}
+
[...]
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V5 3/9] ARM: pxa: Provide regulator to pwm-backlight
Dear Andrew Chew, The pwm-backlight driver now takes a mandatory regulator that is gotten during driver probe. Initialize a dummy regulator to satisfy this requirement. Signed-off-by: Andrew Chew ac...@nvidia.com Acked-by: Alexandre Courbot acour...@nvidia.com Looks reasonable. Add my Reviewed-by: Marek Vasut ma...@denx.de Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 6/9] ARM: mxs: Provide regulator to pwm-backlight
Hi Mark, On Tue, Mar 19, 2013 at 04:10:26PM -0600, Stephen Warren wrote: On 03/19/2013 03:27 PM, Marek Vasut wrote: Do we really need a mandatory regulator? Why can't it be optional? IIRC, the previous advice I've seen is that if a device (driver) uses a regulator, it must /require/ a regulator, and if a particular board doesn't actually have a SW-controlled regulator, then a fixed- or dummy- regulator should be provided to satisfy this requirement. CC'ing Mark Brown to make sure I really do Recall Correctly. Yes, and it should be fixed rather than dummy. The issue is partly that it's probably important that the device has power so we don't want to just ignore errors and partly that this is something which applies to essentially all devices so whatever we do for this case ought to be done by the core so all devices can benefit and we don't have to duplicate lots of code in individual drivers. Thanks for clearing this! Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH V4 6/9] ARM: mxs: Provide regulator to pwm-backlight
Dear Andrew Chew, The pwm-backlight driver now takes a mandatory regulator that is gotten during driver probe. Initialize a dummy regulator to satisfy this requirement. Signed-off-by: Andrew Chew ac...@nvidia.com Do we really need a mandatory regulator? Why can't it be optional? Best regards, Marek Vasut -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/4] S3C2443: Move i2s clock definitions to common code
On Sunday, August 21, 2011 07:25:04 PM Heiko Stübner wrote:
Am Sonntag 21 August 2011, 19:13:32 schrieb Russell King - ARM Linux:
On Sat, Aug 20, 2011 at 06:01:29PM +0200, Heiko Stübner wrote:
+/* i2s-ref
+ *
+ * i2s bus reference clock, selectable from external, esysclk or
epllref + *
+ * Note, this used to be two clocks, but was compressed into one.
+*/
+
+struct clk *clk_i2s_srclist[] = {
+ [0] = clk_i2s_eplldiv.clk,
+ [1] = clk_i2s_ext,
+ [2] = clk_epllref.clk,
+ [3] = clk_epllref.clk,
+};
Is there any reason not to make this static (have you run your patch
through checkpatch.pl ?)
Yep I did run all of them through checkpatch (after beeing scolded last
time) and it didn't report anything.
But for this move of code I simply grabbed the code fragments and put them
into their new location (i.e. it was this way in mach-s3c2443/clock.c) and
should have probably taken a closer look at what I'm moving.
So it seems you are right, it should probably be static as everything else
is also static.
And you don't really understand why everything else is static, right ? ;-)
(don't take it as an offense)
Heiko
___
linux-arm-kernel mailing list
linux-arm-ker...@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/4] ARM: S5PC110: add common FIMC setup code
Dne Po 6. září 2010 06:34:28 Marek Szyprowski napsal(a):
Hello,
On 2010-09-06 13:16 Marek Vasut wrote:
Dne Po 6. září 2010 05:50:43 Marek Szyprowski napsal(a):
Add common clocks setup code for FIMC devices on S5PV210 SoCs.
Signed-off-by: Marek Szyprowskim.szyprow...@samsung.com
Signed-off-by: Kyungmin Parkkyungmin.p...@samsung.com
---
arch/arm/mach-s5pv210/Kconfig |6
arch/arm/mach-s5pv210/Makefile|1 +
arch/arm/mach-s5pv210/setup-fimc.c| 46
+ arch/arm/plat-samsung/include/plat/fimc.h
|
24 +++
4 files changed, 77 insertions(+), 0 deletions(-)
create mode 100644 arch/arm/mach-s5pv210/setup-fimc.c
create mode 100644 arch/arm/plat-samsung/include/plat/fimc.h
diff --git a/arch/arm/mach-s5pv210/Kconfig
b/arch/arm/mach-s5pv210/Kconfig index d3a3895..48489bb 100644
--- a/arch/arm/mach-s5pv210/Kconfig
+++ b/arch/arm/mach-s5pv210/Kconfig
@@ -37,6 +37,12 @@ config S5PV210_SETUP_FB_24BPP
help
Common setup code for S5PV210 with an 24bpp RGB display
helper.
+config S5PV210_SETUP_FIMC
+ bool
+ help
+Compile common code for S5PV210 based machines to setup correct
+FIMC clock parameters.
+
config S5PV210_SETUP_KEYPAD
bool
help
diff --git a/arch/arm/mach-s5pv210/Makefile
b/arch/arm/mach-s5pv210/Makefile index 05048c5..c13aef1 100644
--- a/arch/arm/mach-s5pv210/Makefile
+++ b/arch/arm/mach-s5pv210/Makefile
@@ -29,6 +29,7 @@ obj-$(CONFIG_S3C64XX_DEV_SPI)+= dev-spi.o
obj-$(CONFIG_S5PC110_DEV_ONENAND) += dev-onenand.o
obj-$(CONFIG_S5PV210_SETUP_FB_24BPP) += setup-fb-24bpp.o
+obj-$(CONFIG_S5PV210_SETUP_FIMC) += setup-fimc.o
obj-$(CONFIG_S5PV210_SETUP_I2C1) += setup-i2c1.o
obj-$(CONFIG_S5PV210_SETUP_I2C2) += setup-i2c2.o
obj-$(CONFIG_S5PV210_SETUP_IDE) += setup-ide.o
diff --git a/arch/arm/mach-s5pv210/setup-fimc.c
b/arch/arm/mach-s5pv210/setup-fimc.c new file mode 100644
index 000..80c1ffe
--- /dev/null
+++ b/arch/arm/mach-s5pv210/setup-fimc.c
@@ -0,0 +1,46 @@
+/*
+ * Copyright (c) 2010 Samsung Electronics Co., Ltd.
+ *http://www.samsung.com/
+ *
+ * S5PV210 clock setup code for FIMC devices
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+#includelinux/device.h
+#includelinux/clk.h
+#includelinux/err.h
+#includeplat/devs.h
+#includeplat/fimc.h
+
+int __init s5pv210_fimc_setup_clks(void)
+{
+ int err = 0;
+ int i;
+ struct clk *clk_fimc, *parent;
+
+ struct device *fimc_devs[] = {
+ s5p_device_fimc0.dev,
+ s5p_device_fimc1.dev,
+ s5p_device_fimc2.dev
+ };
+
+ parent = clk_get(NULL, mout_epll);
+ if (IS_ERR(parent))
+ return PTR_ERR(parent);
+
+ for (i = 0; err == 0 i ARRAY_SIZE(fimc_devs); i++) {
+ if (fimc_devs[i]) {
+ clk_fimc = clk_get(fimc_devs[i], sclk_fimc);
+ if (IS_ERR(clk_fimc)) {
+ err = PTR_ERR(clk_fimc);
I believe you should clk_put() the clocks that were already clk_get()-ed
here.
Ah, please ignore my stupid comment then, I missed this one.
They are, see clk_put after clk_set_parent.
Cheers
+ break;
btw. can the array fimc_devs[] be sparse ? So you'd replace this break; with
continue; ?
Well ... it's ok :)
Reviewed-by: Marek Vasut marek.va...@gmail.com
+ }
+ clk_set_parent(clk_fimc, parent);
+ clk_put(clk_fimc);
+ }
+ }
+ clk_put(parent);
+ return err;
+}
diff --git a/arch/arm/plat-samsung/include/plat/fimc.h
b/arch/arm/plat-samsung/include/plat/fimc.h new file mode 100644
index 000..2c06f37
--- /dev/null
+++ b/arch/arm/plat-samsung/include/plat/fimc.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2010 Samsung Electronics Co., Ltd.
+ *http://www.samsung.com/
+ *
+ * Common FIMC devices definitions and helper functions
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+*/
+
+#ifndef __PLAT_SAMSUNG_FIMC_H
+#define __PLAT_SAMSUNG_FIMC_H __FILE__
+
+/**
+ * s5pv210_fimc_setup_clks() - S5PV210/S5PC110 fimc clocks setup
function + *
+ * Set correct parent clocks on machines which bootloaded did not
configured + * fimc clocks correctly. FIMC devices works properly only
if sourced from + * certain clock sources. mout_epll clock is the
recommended one. + */
+extern int s5pv210_fimc_setup_clks(void
Re: [PATCH 1/3] h1940: use gpiolib for latch access
Dne Čt 19. srpna 2010 17:00:01 Vasily Khoruzhick napsal(a):
This patch adds gpiolib support for h1940 latch.
With this patch it's possible to use leds-gpio and uda1380
drivers (they require gpiolib support for appropriate pins).
And now it's possible to drop leds-h1940 driver.
Signed-off-by: Vasily Khoruzhick anars...@gmail.com
---
arch/arm/mach-s3c2410/h1940-bluetooth.c | 13 +++-
arch/arm/mach-s3c2410/include/mach/h1940-latch.h | 57 +---
arch/arm/mach-s3c2410/mach-h1940.c | 77
-- arch/arm/plat-s3c24xx/Kconfig|
1 +
4 files changed, 100 insertions(+), 48 deletions(-)
diff --git a/arch/arm/mach-s3c2410/h1940-bluetooth.c
b/arch/arm/mach-s3c2410/h1940-bluetooth.c index 8cdeb14..8aa2f19 100644
--- a/arch/arm/mach-s3c2410/h1940-bluetooth.c
+++ b/arch/arm/mach-s3c2410/h1940-bluetooth.c
@@ -30,7 +30,7 @@ static void h1940bt_enable(int on)
{
if (on) {
/* Power on the chip */
- h1940_latch_control(0, H1940_LATCH_BLUETOOTH_POWER);
+ gpio_set_value(H1940_LATCH_BLUETOOTH_POWER, 1);
/* Reset the chip */
mdelay(10);
@@ -43,7 +43,7 @@ static void h1940bt_enable(int on)
mdelay(10);
gpio_set_value(S3C2410_GPH(1), 0);
mdelay(10);
- h1940_latch_control(H1940_LATCH_BLUETOOTH_POWER, 0);
+ gpio_set_value(H1940_LATCH_BLUETOOTH_POWER, 0);
}
}
@@ -64,7 +64,14 @@ static int __devinit h1940bt_probe(struct
platform_device *pdev)
ret = gpio_request(S3C2410_GPH(1), dev_name(pdev-dev));
if (ret) {
- dev_err(pdev-dev, could not get GPH1\n);\
+ dev_err(pdev-dev, could not get GPH1\n);
+ return ret;
+ }
+
+ ret = gpio_request(H1940_LATCH_BLUETOOTH_POWER, dev_name(pdev-dev));
This should contain the name of the GPIO, not dev_name I assume.
+ if (ret) {
+ gpio_free(S3C2410_GPH(1));
What's this constant (the 1) here ? Maybe some sane #define wont hurt or
comment
around it.
+ dev_err(pdev-dev, could not get BT_POWER\n);
return ret;
}
diff --git a/arch/arm/mach-s3c2410/include/mach/h1940-latch.h
b/arch/arm/mach-s3c2410/include/mach/h1940-latch.h index d8a8327..73586f2
100644
--- a/arch/arm/mach-s3c2410/include/mach/h1940-latch.h
+++ b/arch/arm/mach-s3c2410/include/mach/h1940-latch.h
@@ -14,51 +14,30 @@
#ifndef __ASM_ARCH_H1940_LATCH_H
#define __ASM_ARCH_H1940_LATCH_H
+#include mach/gpio.h
-#ifndef __ASSEMBLY__
-#define H1940_LATCH ((void __force __iomem *)0xF800)
-#else
-#define H1940_LATCH 0xF800
-#endif
-
-#define H1940_PA_LATCH (S3C2410_CS2)
+#define H1940_LATCH_GPIO(x) (S3C_GPIO_END + (x))
/* SD layer latch */
-#define H1940_LATCH_SDQ1 (116)
-#define H1940_LATCH_LCD_P1 (117)
-#define H1940_LATCH_LCD_P2 (118)
-#define H1940_LATCH_LCD_P3 (119)
-#define H1940_LATCH_MAX1698_nSHUTDOWN(120) /* LCD
backlight
*/
-#define H1940_LATCH_LED_RED (121)
-#define H1940_LATCH_SDQ7 (122)
-#define H1940_LATCH_USB_DP (123)
+#define H1940_LATCH_SDQ1 H1940_LATCH_GPIO(0)
+#define H1940_LATCH_LCD_P1 H1940_LATCH_GPIO(1)
+#define H1940_LATCH_LCD_P2 H1940_LATCH_GPIO(2)
+#define H1940_LATCH_LCD_P3 H1940_LATCH_GPIO(3)
+#define H1940_LATCH_MAX1698_nSHUTDOWNH1940_LATCH_GPIO(4)
+#define H1940_LATCH_LED_RED H1940_LATCH_GPIO(5)
+#define H1940_LATCH_SDQ7 H1940_LATCH_GPIO(6)
+#define H1940_LATCH_USB_DP H1940_LATCH_GPIO(7)
/* CPU layer latch */
-#define H1940_LATCH_UDA_POWER(124)
-#define H1940_LATCH_AUDIO_POWER (125)
-#define H1940_LATCH_SM803_ENABLE (126)
-#define H1940_LATCH_LCD_P4 (127)
-#define H1940_LATCH_CPUQ5(128) /* untraced */
-#define H1940_LATCH_BLUETOOTH_POWER (129) /* active high */
-#define H1940_LATCH_LED_GREEN(130)
-#define H1940_LATCH_LED_FLASH(131)
-
-/* default settings */
-
-#define H1940_LATCH_DEFAULT \
- H1940_LATCH_LCD_P4 | \
- H1940_LATCH_SM803_ENABLE| \
- H1940_LATCH_SDQ1| \
- H1940_LATCH_LCD_P1 | \
- H1940_LATCH_LCD_P2 | \
- H1940_LATCH_LCD_P3 | \
- H1940_LATCH_MAX1698_nSHUTDOWN | \
- H1940_LATCH_CPUQ5
-
-/* control functions */
-
-extern void h1940_latch_control(unsigned int clear, unsigned int set);
+#define H1940_LATCH_UDA_POWERH1940_LATCH_GPIO(8)
+#define H1940_LATCH_AUDIO_POWER H1940_LATCH_GPIO(9)
+#define H1940_LATCH_SM803_ENABLE H1940_LATCH_GPIO(10)
+#define H1940_LATCH_LCD_P4
Re: [PATCH 2/3] h1940: fix h1940-bluetooth compilation
Dne Čt 19. srpna 2010 17:00:02 Vasily Khoruzhick napsal(a): Signed-off-by: Vasily Khoruzhick anars...@gmail.com Errr ... fix ... in what way? What was broken? I don't understand, please add more explanatory text. Thanks Cheers --- arch/arm/mach-s3c2410/h1940-bluetooth.c |8 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/arch/arm/mach-s3c2410/h1940-bluetooth.c b/arch/arm/mach-s3c2410/h1940-bluetooth.c index 8aa2f19..6b86a72 100644 --- a/arch/arm/mach-s3c2410/h1940-bluetooth.c +++ b/arch/arm/mach-s3c2410/h1940-bluetooth.c @@ -77,13 +77,13 @@ static int __devinit h1940bt_probe(struct platform_device *pdev) /* Configures BT serial port GPIOs */ s3c_gpio_cfgpin(S3C2410_GPH(0), S3C2410_GPH0_nCTS0); - s3c_gpio_cfgpull(S3C2410_GPH(0), S3C_GPIO_PULL_NONE); + s3c_gpio_setpull(S3C2410_GPH(0), S3C_GPIO_PULL_NONE); s3c_gpio_cfgpin(S3C2410_GPH(1), S3C2410_GPIO_OUTPUT); - s3c_gpio_cfgpull(S3C2410_GPH(1), S3C_GPIO_PULL_NONE); + s3c_gpio_setpull(S3C2410_GPH(1), S3C_GPIO_PULL_NONE); s3c_gpio_cfgpin(S3C2410_GPH(2), S3C2410_GPH2_TXD0); - s3c_gpio_cfgpull(S3C2410_GPH(2), S3C_GPIO_PULL_NONE); + s3c_gpio_setpull(S3C2410_GPH(2), S3C_GPIO_PULL_NONE); s3c_gpio_cfgpin(S3C2410_GPH(3), S3C2410_GPH3_RXD0); - s3c_gpio_cfgpull(S3C2410_GPH(3), S3C_GPIO_PULL_NONE); + s3c_gpio_setpull(S3C2410_GPH(3), S3C_GPIO_PULL_NONE); rfk = rfkill_alloc(DRV_NAME, pdev-dev, RFKILL_TYPE_BLUETOOTH, -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3/3] h1940: implement mmc_power function
Dne Čt 19. srpna 2010 17:00:03 Vasily Khoruzhick napsal(a):
Signed-off-by: Vasily Khoruzhick anars...@gmail.com
---
arch/arm/mach-s3c2410/include/mach/h1940-latch.h |2 +-
arch/arm/mach-s3c2410/mach-h1940.c | 23
+++-- 2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/arch/arm/mach-s3c2410/include/mach/h1940-latch.h
b/arch/arm/mach-s3c2410/include/mach/h1940-latch.h index 73586f2..ef7d8cf
100644
--- a/arch/arm/mach-s3c2410/include/mach/h1940-latch.h
+++ b/arch/arm/mach-s3c2410/include/mach/h1940-latch.h
@@ -35,7 +35,7 @@
#define H1940_LATCH_AUDIO_POWER H1940_LATCH_GPIO(9)
#define H1940_LATCH_SM803_ENABLE H1940_LATCH_GPIO(10)
#define H1940_LATCH_LCD_P4 H1940_LATCH_GPIO(11)
-#define H1940_LATCH_CPUQ5H1940_LATCH_GPIO(12)
+#define H1940_LATCH_SD_POWER H1940_LATCH_GPIO(12)
#define H1940_LATCH_BLUETOOTH_POWER H1940_LATCH_GPIO(13)
#define H1940_LATCH_LED_GREENH1940_LATCH_GPIO(14)
#define H1940_LATCH_LED_FLASHH1940_LATCH_GPIO(15)
diff --git a/arch/arm/mach-s3c2410/mach-h1940.c
b/arch/arm/mach-s3c2410/mach-h1940.c index 9717790..c1ccc8e 100644
--- a/arch/arm/mach-s3c2410/mach-h1940.c
+++ b/arch/arm/mach-s3c2410/mach-h1940.c
@@ -116,8 +116,7 @@ static unsigned int latch_state =
H1940_LATCH_BIT(H1940_LATCH_LCD_P4) |
H1940_LATCH_BIT(H1940_LATCH_LCD_P1) |
H1940_LATCH_BIT(H1940_LATCH_LCD_P2) |
H1940_LATCH_BIT(H1940_LATCH_LCD_P3) |
- H1940_LATCH_BIT(H1940_LATCH_MAX1698_nSHUTDOWN) |
- H1940_LATCH_BIT(H1940_LATCH_CPUQ5);
+ H1940_LATCH_BIT(H1940_LATCH_MAX1698_nSHUTDOWN);
static void h1940_latch_control(unsigned int clear, unsigned int set)
{
@@ -259,10 +258,25 @@ static struct platform_device h1940_device_bluetooth
= { .id = -1,
};
+static void h1940_set_mmc_power(unsigned char power_mode, unsigned short
vdd) +{
+ switch (power_mode) {
+ case MMC_POWER_OFF:
+ gpio_set_value(H1940_LATCH_SD_POWER, 0);
+ break;
+ case MMC_POWER_UP:
+ case MMC_POWER_ON:
+ gpio_set_value(H1940_LATCH_SD_POWER, 1);
+ break;
+ default:
+ break;
+ };
+}
+
static struct s3c24xx_mci_pdata h1940_mmc_cfg __initdata = {
.gpio_detect = S3C2410_GPF(5),
.gpio_wprotect = S3C2410_GPH(8),
- .set_power = NULL,
+ .set_power = h1940_set_mmc_power,
.ocr_avail = MMC_VDD_32_33,
};
Can't you implement gpio_power into the s3c24xx mmc driver ? Then you can fix
mach-n30 too.
@@ -402,6 +416,9 @@ static void __init h1940_init(void)
gpio_request(H1940_LATCH_USB_DP, USB pullup);
gpio_direction_output(H1940_LATCH_USB_DP, 0);
+ gpio_request(H1940_LATCH_SD_POWER, SD power);
+ gpio_direction_output(H1940_LATCH_SD_POWER, 0);
Please handle possible return values here !
+
platform_add_devices(h1940_devices, ARRAY_SIZE(h1940_devices));
}
Cheers
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] h1940: fix h1940-bluetooth compilation
Dne Út 24. srpna 2010 15:57:07 Vasily Khoruzhick napsal(a): В сообщении от 24 августа 2010 16:50:46 автор Marek Vasut написал: Dne Čt 19. srpna 2010 17:00:02 Vasily Khoruzhick napsal(a): Signed-off-by: Vasily Khoruzhick anars...@gmail.com Errr ... fix ... in what way? What was broken? I don't understand, please add more explanatory text. Thanks Cheers It's compile fix, without this patch it just doesn't compile. Because the GPIO api changed or what ? Please stick this text into the next version of the patch. Thanks! -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/3] h1940: use gpiolib for latch access
Dne Út 24. srpna 2010 16:50:36 Vasily Khoruzhick napsal(a): В сообщении от 24 августа 2010 17:40:21 автор Marek Vasut написал: Defining S3C2410_GPH_GPIONAME for each bit might be better then. Well, that requires massive refactoring of h1940 (and rx1950) support. And btw, that's common for s3c24xx-based machines to use generic s3c24xx gpio names, not machine specific. I assume I see how it works now. Ok, please put a comment around it then so others will understand what it does. Using machine-specific GPIO names might help readability btw. -- the approach everyone does it this way so I'll do it this way without thinking is bad :) Well, it's just my suggestion for improvement of the overall code quality. Ben, what's your opinion? Cheers Regards Vasily -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v4] libata: pata_samsung_cf: Add Samsung PATA controller driver
Dne Pá 25. června 2010 05:38:19 Kukjin Kim napsal(a): Jeff Garzik wrote: On 06/14/2010 03:07 AM, Kukjin Kim wrote: From: Abhilash Kesavana.kesa...@samsung.com Adds support for the Samsung PATA controller. This driver is based on the Libata subsystem and references the earlier patches sent for IDE subsystem. Signed-off-by: Abhilash Kesavana.kesa...@samsung.com Signed-off-by: Kukjin Kimkgene@samsung.com Acked-by: Jeff Garzik jgar...@redhat.com Send it through the appropriate subsystem tree for this platform... Hi, You mean in my case, appropriate subsystem tree is ARM tree. But I think, as Ben commented, it's better drivers go to upstream via the driver's tree. And of course, I read your comments about this like below... 'platform drivers do not build and/or work without additional platform glue...' Hmm. I'm not sure which way is better... I pushed pata_pxa through Eric as well ... go for the linux-arm tree. Cheers Russell, How do you think? May I send this to your patch tracking system? Or...how should I handle this? Thanks. Best regards, Kgene. -- Kukjin Kim kgene@samsung.com, Senior Engineer, SW Solution Development Team, Samsung Electronics Co., Ltd. -- To unsubscribe from this list: send the line unsubscribe linux-ide in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] ARM: SAMSUNG: Add keypad device support
Dne Po 31. května 2010 02:06:48 Kukjin Kim napsal(a):
Joonyoung Shim wrote:
This patch adds samsung keypad device definition for samsung cpus.
Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
arch/arm/plat-samsung/Kconfig|7 ++-
arch/arm/plat-samsung/Makefile |1 +
arch/arm/plat-samsung/dev-keypad.c | 50
+++
arch/arm/plat-samsung/include/plat/devs.h|2 +
arch/arm/plat-samsung/include/plat/keypad.h | 57
++
arch/arm/plat-samsung/include/plat/regs-keypad.h | 49
+++
6 files changed, 165 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/plat-samsung/dev-keypad.c
create mode 100644 arch/arm/plat-samsung/include/plat/keypad.h
create mode 100644 arch/arm/plat-samsung/include/plat/regs-keypad.h
diff --git a/arch/arm/plat-samsung/Kconfig
b/arch/arm/plat-samsung/Kconfig index 2753fb3..3ef2df7 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -225,7 +225,12 @@ config S3C64XX_DEV_SPI
config SAMSUNG_DEV_TS
bool
help
- Common in platform device definitions for touchscreen device
+ Common in platform device definitions for touchscreen device
+
Above changing is not for keypad.
+config SAMSUNG_DEV_KEYPAD
+ bool
+ help
+ Compile in platform device definitions for keypad
# DMA
diff --git a/arch/arm/plat-samsung/Makefile
b/arch/arm/plat-samsung/Makefile
index 228c2ad..ef00c47 100644
--- a/arch/arm/plat-samsung/Makefile
+++ b/arch/arm/plat-samsung/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_S3C_DEV_RTC) += dev-rtc.o
obj-$(CONFIG_SAMSUNG_DEV_ADC) += dev-adc.o
obj-$(CONFIG_SAMSUNG_DEV_TS) += dev-ts.o
+obj-$(CONFIG_SAMSUNG_DEV_KEYPAD) += dev-keypad.o
# DMA support
diff --git a/arch/arm/plat-samsung/dev-keypad.c
b/arch/arm/plat-samsung/dev-
keypad.c
new file mode 100644
index 000..70e2e2d
--- /dev/null
+++ b/arch/arm/plat-samsung/dev-keypad.c
@@ -0,0 +1,50 @@
+/*
+ * linux/arch/arm/plat-samsung/dev-keypad.c
+ *
+ * Copyright (C) 2010 Samsung Electronics Co.Ltd
+ * Author: Joonyoung Shim jy0922.s...@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or
modify
it
+ * under the terms of the GNU General Public License as published by
the
+ * Free Software Foundation; either version 2 of the License, or (at
your
+ * option) any later version.
+ *
+ */
+
+#include linux/platform_device.h
+#include mach/irqs.h
+#include mach/map.h
+#include plat/cpu.h
+#include plat/devs.h
+#include plat/keypad.h
+
+static struct resource samsung_kp_resources[] = {
How about samsung_keypad_resources easily to reading?
IMO doesn't matter ... kp is fine and it's well known acronym anyway.
+ [0] = {
+ .start = SAMSUNG_PA_KEYPAD,
+ .end= SAMSUNG_PA_KEYPAD + 0x20 - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ [1] = {
+ .start = IRQ_KEYPAD,
+ .end= IRQ_KEYPAD,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+
+struct platform_device samsung_device_keypad = {
+ .name = samsung-keypad,
+ .id = -1,
+ .num_resources = ARRAY_SIZE(samsung_kp_resources),
+ .resource = samsung_kp_resources,
+};
+
+void __init samsung_kp_set_platdata(struct samsung_kp_platdata *pd)
+{
+ struct samsung_kp_platdata *npd;
+
+ npd = s3c_set_platdata(pd, sizeof(struct samsung_kp_platdata),
+ samsung_device_keypad);
+
+ if (!npd-cfg_gpio)
+ npd-cfg_gpio = samsung_keypad_cfg_gpio;
+}
diff --git a/arch/arm/plat-samsung/include/plat/devs.h b/arch/arm/plat-
samsung/include/plat/devs.h
index 6760999..c06386b 100644
--- a/arch/arm/plat-samsung/include/plat/devs.h
+++ b/arch/arm/plat-samsung/include/plat/devs.h
@@ -100,6 +100,8 @@ extern struct platform_device s5pc100_device_iis0;
extern struct platform_device s5pc100_device_iis1;
extern struct platform_device s5pc100_device_iis2;
+extern struct platform_device samsung_device_keypad;
+
/* s3c2440 specific devices */
#ifdef CONFIG_CPU_S3C2440
diff --git a/arch/arm/plat-samsung/include/plat/keypad.h b/arch/arm/plat-
samsung/include/plat/keypad.h
new file mode 100644
index 000..d144f42
--- /dev/null
+++ b/arch/arm/plat-samsung/include/plat/keypad.h
@@ -0,0 +1,57 @@
+/*
+ * linux/arch/arm/plat-samsung/include/plat/keypad.h
+ *
+ * Copyright (C) 2010 Samsung Electronics Co.Ltd
+ * Author: Joonyoung Shim jy0922.s...@samsung.com
+ *
+ * Samsung Platform - Keypad platform data definitions
+ *
+ * This program is free software; you can
Re: [PATCH v2 5/5] input: samsung-keypad - Add samsung keypad driver
Dne Ne 30. května 2010 05:06:24 Joonyoung Shim napsal(a):
This patch adds support for keypad driver running on Samsung cpus. This
driver is tested on GONI and Aquila board using S5PC110 cpu.
Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
drivers/input/keyboard/Kconfig |9 +
drivers/input/keyboard/Makefile |1 +
drivers/input/keyboard/samsung-keypad.c | 364
+++ 3 files changed, 374 insertions(+), 0
deletions(-)
create mode 100644 drivers/input/keyboard/samsung-keypad.c
diff --git a/drivers/input/keyboard/Kconfig
b/drivers/input/keyboard/Kconfig index d8fa5d7..bf6a50f 100644
--- a/drivers/input/keyboard/Kconfig
+++ b/drivers/input/keyboard/Kconfig
@@ -342,6 +342,15 @@ config KEYBOARD_PXA930_ROTARY
To compile this driver as a module, choose M here: the
module will be called pxa930_rotary.
+config KEYBOARD_SAMSUNG
+ tristate Samsung keypad support
+ depends on SAMSUNG_DEV_KEYPAD
+ help
+ Say Y here if you want to use the Samsung keypad.
+
+ To compile this driver as a module, choose M here: the
+ module will be called samsung-keypad.
+
config KEYBOARD_STOWAWAY
tristate Stowaway keyboard
select SERIO
diff --git a/drivers/input/keyboard/Makefile
b/drivers/input/keyboard/Makefile index 4596d0c..8f973ed 100644
--- a/drivers/input/keyboard/Makefile
+++ b/drivers/input/keyboard/Makefile
@@ -32,6 +32,7 @@ obj-$(CONFIG_KEYBOARD_OPENCORES)+= opencores-kbd.o
obj-$(CONFIG_KEYBOARD_PXA27x)+= pxa27x_keypad.o
obj-$(CONFIG_KEYBOARD_PXA930_ROTARY) += pxa930_rotary.o
obj-$(CONFIG_KEYBOARD_QT2160)+= qt2160.o
+obj-$(CONFIG_KEYBOARD_SAMSUNG) += samsung-keypad.o
obj-$(CONFIG_KEYBOARD_SH_KEYSC) += sh_keysc.o
obj-$(CONFIG_KEYBOARD_STOWAWAY) += stowaway.o
obj-$(CONFIG_KEYBOARD_SUNKBD)+= sunkbd.o
diff --git a/drivers/input/keyboard/samsung-keypad.c
b/drivers/input/keyboard/samsung-keypad.c new file mode 100644
index 000..f4bcf97
--- /dev/null
+++ b/drivers/input/keyboard/samsung-keypad.c
@@ -0,0 +1,364 @@
+/*
+ * samsung-keypad.c -- Samsung keypad driver
+ *
+ * Copyright (C) 2010 Samsung Electronics Co.Ltd
+ * Author: Joonyoung Shim jy0922.s...@samsung.com
+ * Author: Donghwa Lee dh09@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
it + * under the terms of the GNU General Public License as published
by the + * Free Software Foundation; either version 2 of the License,
or (at your + * option) any later version.
+ */
+
+#include linux/clk.h
+#include linux/delay.h
+#include linux/err.h
+#include linux/init.h
+#include linux/input.h
+#include linux/interrupt.h
+#include linux/io.h
+#include linux/module.h
+#include linux/platform_device.h
+#include linux/slab.h
+#include plat/cpu.h
+#include plat/keypad.h
+#include plat/regs-keypad.h
+
+struct samsung_kp {
+ struct input_dev *input_dev;
+ struct timer_list timer;
+ struct clk *clk;
+ struct work_struct work;
+ void __iomem *base;
+ unsigned short *keycodes;
+ unsigned int row_shift;
+ unsigned int rows;
+ unsigned int cols;
+ unsigned int row_state[SAMSUNG_MAX_COLS];
+ int irq;
+};
+
+static void samsung_kp_scan(struct samsung_kp *keypad, unsigned int
*row_state) +{
+ unsigned int col;
+ unsigned int val;
+
+ for (col = 0; col keypad-cols; col++) {
+#if CONFIG_ARCH_S5PV210
+ val = S5PV210_KEYIFCOLEN_MASK;
+ val = ~(1 col) 8;
+#else
+ val = SAMSUNG_KEYIFCOL_MASK;
+ val = ~(1 col);
+#endif
No, what if you want to run this on both S5PV210 and some other samsung soc?
Fix the #if CONFIG_ARCH_S5PV210 please. Maybe like this:
if (cpu_is_s5pv210()) {} else {} ?
The rest looks good.
Cheers!
--
To unsubscribe from this list: send the line unsubscribe linux-samsung-soc in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/5] ARM: SAMSUNG: Add keypad device support
Dne Ne 30. května 2010 05:06:20 Joonyoung Shim napsal(a):
This patch adds samsung keypad device definition for samsung cpus.
Signed-off-by: Joonyoung Shim jy0922.s...@samsung.com
Signed-off-by: Kyungmin Park kyungmin.p...@samsung.com
---
arch/arm/plat-samsung/Kconfig|7 ++-
arch/arm/plat-samsung/Makefile |1 +
arch/arm/plat-samsung/dev-keypad.c | 50
+++ arch/arm/plat-samsung/include/plat/devs.h|
2 +
arch/arm/plat-samsung/include/plat/keypad.h | 57
++ arch/arm/plat-samsung/include/plat/regs-keypad.h |
49 +++ 6 files changed, 165 insertions(+), 1 deletions(-)
create mode 100644 arch/arm/plat-samsung/dev-keypad.c
create mode 100644 arch/arm/plat-samsung/include/plat/keypad.h
create mode 100644 arch/arm/plat-samsung/include/plat/regs-keypad.h
diff --git a/arch/arm/plat-samsung/Kconfig b/arch/arm/plat-samsung/Kconfig
index 2753fb3..3ef2df7 100644
--- a/arch/arm/plat-samsung/Kconfig
+++ b/arch/arm/plat-samsung/Kconfig
@@ -225,7 +225,12 @@ config S3C64XX_DEV_SPI
config SAMSUNG_DEV_TS
bool
help
- Common in platform device definitions for touchscreen device
+ Common in platform device definitions for touchscreen device
+
+config SAMSUNG_DEV_KEYPAD
+ bool
+ help
+ Compile in platform device definitions for keypad
# DMA
diff --git a/arch/arm/plat-samsung/Makefile
b/arch/arm/plat-samsung/Makefile index 228c2ad..ef00c47 100644
--- a/arch/arm/plat-samsung/Makefile
+++ b/arch/arm/plat-samsung/Makefile
@@ -50,6 +50,7 @@ obj-$(CONFIG_S3C_DEV_RTC) += dev-rtc.o
obj-$(CONFIG_SAMSUNG_DEV_ADC)+= dev-adc.o
obj-$(CONFIG_SAMSUNG_DEV_TS) += dev-ts.o
+obj-$(CONFIG_SAMSUNG_DEV_KEYPAD) += dev-keypad.o
# DMA support
diff --git a/arch/arm/plat-samsung/dev-keypad.c
b/arch/arm/plat-samsung/dev-keypad.c new file mode 100644
index 000..70e2e2d
--- /dev/null
+++ b/arch/arm/plat-samsung/dev-keypad.c
@@ -0,0 +1,50 @@
+/*
+ * linux/arch/arm/plat-samsung/dev-keypad.c
+ *
+ * Copyright (C) 2010 Samsung Electronics Co.Ltd
+ * Author: Joonyoung Shim jy0922.s...@samsung.com
+ *
+ * This program is free software; you can redistribute it and/or modify
it + * under the terms of the GNU General Public License as published
by the + * Free Software Foundation; either version 2 of the License,
or (at your + * option) any later version.
+ *
+ */
+
+#include linux/platform_device.h
+#include mach/irqs.h
+#include mach/map.h
+#include plat/cpu.h
+#include plat/devs.h
+#include plat/keypad.h
+
+static struct resource samsung_kp_resources[] = {
+ [0] = {
+ .start = SAMSUNG_PA_KEYPAD,
+ .end= SAMSUNG_PA_KEYPAD + 0x20 - 1,
+ .flags = IORESOURCE_MEM,
+ },
+ [1] = {
+ .start = IRQ_KEYPAD,
+ .end= IRQ_KEYPAD,
+ .flags = IORESOURCE_IRQ,
+ },
+};
+
+struct platform_device samsung_device_keypad = {
+ .name = samsung-keypad,
+ .id = -1,
+ .num_resources = ARRAY_SIZE(samsung_kp_resources),
+ .resource = samsung_kp_resources,
+};
+
+void __init samsung_kp_set_platdata(struct samsung_kp_platdata *pd)
+{
+ struct samsung_kp_platdata *npd;
+
+ npd = s3c_set_platdata(pd, sizeof(struct samsung_kp_platdata),
+ samsung_device_keypad);
+
+ if (!npd-cfg_gpio)
+ npd-cfg_gpio = samsung_keypad_cfg_gpio;
+}
diff --git a/arch/arm/plat-samsung/include/plat/devs.h
b/arch/arm/plat-samsung/include/plat/devs.h index 6760999..c06386b 100644
--- a/arch/arm/plat-samsung/include/plat/devs.h
+++ b/arch/arm/plat-samsung/include/plat/devs.h
@@ -100,6 +100,8 @@ extern struct platform_device s5pc100_device_iis0;
extern struct platform_device s5pc100_device_iis1;
extern struct platform_device s5pc100_device_iis2;
+extern struct platform_device samsung_device_keypad;
+
/* s3c2440 specific devices */
#ifdef CONFIG_CPU_S3C2440
diff --git a/arch/arm/plat-samsung/include/plat/keypad.h
b/arch/arm/plat-samsung/include/plat/keypad.h new file mode 100644
index 000..d144f42
--- /dev/null
+++ b/arch/arm/plat-samsung/include/plat/keypad.h
@@ -0,0 +1,57 @@
+/*
+ * linux/arch/arm/plat-samsung/include/plat/keypad.h
+ *
+ * Copyright (C) 2010 Samsung Electronics Co.Ltd
+ * Author: Joonyoung Shim jy0922.s...@samsung.com
+ *
+ * Samsung Platform - Keypad platform data definitions
+ *
+ * This program is free software; you can redistribute it and/or modify
it + * under the terms of the GNU General Public License as published
by the + * Free Software Foundation; either version 2 of the License,
or (at your + * option) any later version.
+ *
+ */
+
+#ifndef __PLAT_SAMSUNG_KEYPAD_H
+#define __PLAT_SAMSUNG_KEYPAD_H
+
+#include linux/input/matrix_keypad.h
+
